From a05aa9fc8fff30b2fad4565a04ce08d14b08f719 Mon Sep 17 00:00:00 2001 From: "John F. Carr" Date: Tue, 20 Aug 2024 13:39:09 -0400 Subject: [PATCH] Improve messages for pointer assertions --- runtime/closure.h | 36 ++++++++++++++++++------------------ runtime/debug.h | 7 +++++++ runtime/init.c | 4 ++-- runtime/local-reducer-api.c | 2 +- runtime/personality.c | 4 ++-- runtime/readydeque.h | 12 ++++++------ runtime/scheduler.c | 28 ++++++++++++++-------------- 7 files changed, 50 insertions(+), 43 deletions(-) diff --git a/runtime/closure.h b/runtime/closure.h index 33d28fb3..45c6416c 100644 --- a/runtime/closure.h +++ b/runtime/closure.h @@ -196,12 +196,12 @@ static inline void Closure_set_frame(Closure *cl, __cilkrts_stack_frame *sf) { static inline void double_link_children(Closure *left, Closure *right) { if (left) { - CILK_ASSERT(left->right_sib == (Closure *)NULL); + CILK_ASSERT_NULL(left->right_sib); left->right_sib = right; } if (right) { - CILK_ASSERT(right->left_sib == (Closure *)NULL); + CILK_ASSERT_NULL(right->left_sib); right->left_sib = left; } } @@ -211,11 +211,11 @@ static inline void double_link_children(Closure *left, Closure *right) { static inline void unlink_child(Closure *cl) { if (cl->left_sib) { - CILK_ASSERT(cl->left_sib->right_sib == cl); + CILK_ASSERT_POINTER_EQUAL(cl->left_sib->right_sib, cl); cl->left_sib->right_sib = cl->right_sib; } if (cl->right_sib) { - CILK_ASSERT(cl->right_sib->left_sib == cl); + CILK_ASSERT_POINTER_EQUAL(cl->right_sib->left_sib, cl); cl->right_sib->left_sib = cl->left_sib; } // used only for error checking @@ -270,17 +270,17 @@ void Closure_remove_child(worker_id self, Closure *parent, Closure *child) { (void)self; // unused if assertions disabled CILK_ASSERT(child); - CILK_ASSERT(parent == child->spawn_parent); + CILK_ASSERT_POINTER_EQUAL(parent, child->spawn_parent); Closure_assert_ownership(self, parent); Closure_assert_ownership(self, child); if (child == parent->right_most_child) { - CILK_ASSERT(child->right_sib == (Closure *)NULL); + CILK_ASSERT_NULL(child->right_sib); parent->right_most_child = child->left_sib; } - CILK_ASSERT(child->right_ht == (hyper_table *)NULL); + CILK_ASSERT_NULL(child->right_ht); unlink_child(child); } @@ -298,7 +298,7 @@ void Closure_remove_child(worker_id self, Closure *parent, Closure *child) { static inline void Closure_add_temp_callee(Closure *caller, Closure *callee) { CILK_ASSERT(!(caller->has_cilk_callee)); - CILK_ASSERT(callee->spawn_parent == NULL); + CILK_ASSERT_NULL(callee->spawn_parent); callee->call_parent = caller; caller->has_cilk_callee = true; @@ -309,8 +309,8 @@ void Closure_add_callee(Closure *caller, Closure *callee) { // ANGE: instead of checking has_cilk_callee, we just check if callee is // NULL, because we might have set the has_cilk_callee in // Closure_add_tmp_callee to prevent the closure from being resumed. - CILK_ASSERT(caller->callee == NULL); - CILK_ASSERT(callee->spawn_parent == NULL); + CILK_ASSERT_NULL(caller->callee); + CILK_ASSERT_NULL(callee->spawn_parent); CILK_ASSERT((callee->frame->flags & CILK_FRAME_DETACHED) == 0); callee->call_parent = caller; @@ -345,7 +345,7 @@ static inline void Closure_suspend_victim(struct ReadyDeque *deques, Closure_change_status(cl, CLOSURE_RUNNING, CLOSURE_SUSPENDED); cl1 = deque_xtract_bottom(deques, thief_id, victim_id); - CILK_ASSERT(cl == cl1); + CILK_ASSERT_POINTER_EQUAL(cl, cl1); USE_UNUSED(cl1); } @@ -367,7 +367,7 @@ static inline void Closure_suspend(struct ReadyDeque *deques, worker_id self, cl1 = deque_xtract_bottom(deques, self, self); - CILK_ASSERT(cl == cl1); + CILK_ASSERT_POINTER_EQUAL(cl, cl1); USE_UNUSED(cl1); } @@ -377,13 +377,13 @@ static inline void Closure_clean(Closure *t) { (void)t; // unused if assertions disabled // sanity checks - CILK_ASSERT(t->left_sib == (Closure *)NULL); - CILK_ASSERT(t->right_sib == (Closure *)NULL); - CILK_ASSERT(t->right_most_child == (Closure *)NULL); + CILK_ASSERT_NULL(t->left_sib); + CILK_ASSERT_NULL(t->right_sib); + CILK_ASSERT_NULL(t->right_most_child); - CILK_ASSERT(t->user_ht == (hyper_table *)NULL); - CILK_ASSERT(t->child_ht == (hyper_table *)NULL); - CILK_ASSERT(t->right_ht == (hyper_table *)NULL); + CILK_ASSERT_NULL(t->user_ht); + CILK_ASSERT_NULL(t->child_ht); + CILK_ASSERT_NULL(t->right_ht); } /* ANGE: destroy the closure and internally free it (put back to global diff --git a/runtime/debug.h b/runtime/debug.h index e5cadbf0..2666de75 100644 --- a/runtime/debug.h +++ b/runtime/debug.h @@ -87,6 +87,12 @@ CHEETAH_INTERNAL extern const char *const __cilkrts_assertion_failed; ? (void)0 \ : cilkrts_bug(__cilkrts_assertion_failed, __FILE__, __LINE__, #ex)) +#define CILK_ASSERT_NULL(P) \ + ({ void *_t = (P); __builtin_expect(!_t, 1) \ + ? (void)0 \ + : cilkrts_bug("%s: %d: cilk_assertion failed: %s (%p) == NULL", \ + __FILE__, __LINE__, #P, _t);}) + #define CILK_ASSERT_POINTER_EQUAL(P1, P2) \ ({ void *_t1 = (P1), *_t2 = (P2); __builtin_expect(_t1 == _t2, 1) \ ? (void)0 \ @@ -115,6 +121,7 @@ CHEETAH_INTERNAL extern const char *const __cilkrts_assertion_failed; #else #define CILK_ASSERT(ex) +#define CILK_ASSERT_NULL(ex) #define CILK_ASSERT_POINTER_EQUAL(P1, P2) #define CILK_ASSERT_INDEX_ZERO(LEFT, I, RIGHT, FMT) #define CILK_ASSERT_LE(A, B, FMT) diff --git a/runtime/init.c b/runtime/init.c index 5c7538ce..c2fb865d 100644 --- a/runtime/init.c +++ b/runtime/init.c @@ -524,7 +524,7 @@ void __cilkrts_internal_invoke_cilkified_root(__cilkrts_stack_frame *sf) { void *new_rsp = (void *)sysdep_reset_stack_for_resume(root_closure->fiber, sf); USE_UNUSED(new_rsp); - CILK_ASSERT(SP(sf) == new_rsp); + CILK_ASSERT_POINTER_EQUAL(SP(sf), new_rsp); // Mark that this root frame is last (meaning, at the top of the stack) sf->flags |= CILK_FRAME_LAST; @@ -750,7 +750,7 @@ static void workers_deinit(global_state *g) { } CHEETAH_INTERNAL void __cilkrts_shutdown(global_state *g) { - CILK_ASSERT(NULL == exception_reducer.exn); + CILK_ASSERT_NULL(exception_reducer.exn); // If the workers are still running, stop them now. if (g->workers_started) __cilkrts_stop_workers(g); diff --git a/runtime/local-reducer-api.c b/runtime/local-reducer-api.c index 98c72141..1d2f5b34 100644 --- a/runtime/local-reducer-api.c +++ b/runtime/local-reducer-api.c @@ -49,7 +49,7 @@ void *internal_reducer_lookup(__cilkrts_worker *w, void *key, size_t size, struct local_hyper_table *table = get_local_hyper_table(w); struct bucket *b = find_hyperobject(table, (uintptr_t)key); if (__builtin_expect(!!b, true)) { - CILK_ASSERT(key == (void *)b->key); + CILK_ASSERT_POINTER_EQUAL(key, (void *)b->key); // Return the existing view. return b->value.view; } diff --git a/runtime/personality.c b/runtime/personality.c index d361c264..372818e2 100644 --- a/runtime/personality.c +++ b/runtime/personality.c @@ -98,7 +98,7 @@ get_exception_reducer_or_null(__cilkrts_worker *w) { struct bucket *b = find_hyperobject(table, (uintptr_t)key); if (b) { - CILK_ASSERT(key == (void *)b->key); + CILK_ASSERT_POINTER_EQUAL(key, (void *)b->key); // Return the existing view. return (struct closure_exception *)(b->value.view); } @@ -109,7 +109,7 @@ get_exception_reducer_or_null(__cilkrts_worker *w) { // Destroy the current view of the exception-reducer state. void clear_exception_reducer(__cilkrts_worker *w, struct closure_exception *exn_r) { - CILK_ASSERT(exn_r->throwing_fiber == NULL); + CILK_ASSERT_NULL(exn_r->throwing_fiber); free(exn_r); internal_reducer_remove(w, &exception_reducer); } diff --git a/runtime/readydeque.h b/runtime/readydeque.h index 3d2b6d73..e0218442 100644 --- a/runtime/readydeque.h +++ b/runtime/readydeque.h @@ -112,7 +112,7 @@ static inline Closure *deque_xtract_top(ReadyDeque *deques, worker_id self, deques[pn].top = cl->next_ready; /* ANGE: if there is only one entry in the deque ... */ if (cl == deques[pn].bottom) { - CILK_ASSERT(cl->next_ready == (Closure *)NULL); + CILK_ASSERT_NULL(cl->next_ready); deques[pn].bottom = (Closure *)NULL; } else { CILK_ASSERT(cl->next_ready); @@ -120,7 +120,7 @@ static inline Closure *deque_xtract_top(ReadyDeque *deques, worker_id self, } WHEN_CILK_DEBUG(cl->owner_ready_deque = NO_WORKER); } else { - CILK_ASSERT(deques[pn].bottom == (Closure *)NULL); + CILK_ASSERT_NULL(deques[pn].bottom); } return cl; @@ -147,7 +147,7 @@ static inline Closure *deque_peek_top(ReadyDeque *deques, CILK_ASSERT(cl->owner_ready_deque == pn || (self != pn && cl == w->g->root_closure)); } else { - CILK_ASSERT(deques[pn].bottom == (Closure *)NULL); + CILK_ASSERT_NULL(deques[pn].bottom); } return cl; @@ -166,7 +166,7 @@ static inline Closure *deque_xtract_bottom(ReadyDeque *deques, worker_id self, CILK_ASSERT(cl->owner_ready_deque == pn); deques[pn].bottom = cl->prev_ready; if (cl == deques[pn].top) { - CILK_ASSERT(cl->prev_ready == (Closure *)NULL); + CILK_ASSERT_NULL(cl->prev_ready); deques[pn].top = (Closure *)NULL; } else { CILK_ASSERT(cl->prev_ready); @@ -175,7 +175,7 @@ static inline Closure *deque_xtract_bottom(ReadyDeque *deques, worker_id self, WHEN_CILK_DEBUG(cl->owner_ready_deque = NO_WORKER); } else { - CILK_ASSERT(deques[pn].top == (Closure *)NULL); + CILK_ASSERT_NULL(deques[pn].top); } return cl; @@ -193,7 +193,7 @@ deque_peek_bottom(ReadyDeque *deques, worker_id self, worker_id pn) { if (cl) { CILK_ASSERT(cl->owner_ready_deque == pn); } else { - CILK_ASSERT(deques[pn].top == (Closure *)NULL); + CILK_ASSERT_NULL(deques[pn].top); } return cl; diff --git a/runtime/scheduler.c b/runtime/scheduler.c index b7291d25..2ed42bab 100644 --- a/runtime/scheduler.c +++ b/runtime/scheduler.c @@ -258,14 +258,14 @@ void Cilk_set_return(__cilkrts_worker *const w) { CILK_ASSERT(t->child_ht == (hyper_table *)NULL && t->right_ht == (hyper_table *)NULL); CILK_ASSERT(t->call_parent); - CILK_ASSERT(t->spawn_parent == NULL); + CILK_ASSERT_NULL(t->spawn_parent); CILK_ASSERT((t->frame->flags & CILK_FRAME_DETACHED) == 0); Closure *call_parent = t->call_parent; Closure *t1 = deque_xtract_bottom(deques, self, self); USE_UNUSED(t1); - CILK_ASSERT(t == t1); + CILK_ASSERT_POINTER_EQUAL(t, t1); CILK_ASSERT(__cilkrts_stolen(t->frame)); deque_add_bottom(deques, call_parent, self, self); @@ -274,10 +274,10 @@ void Cilk_set_return(__cilkrts_worker *const w) { Closure_unlock(self, t); Closure_lock(self, call_parent); - CILK_ASSERT(call_parent->fiber == t->fiber); + CILK_ASSERT_POINTER_EQUAL(call_parent->fiber, t->fiber); t->fiber = NULL; if (USE_EXTENSION) { - CILK_ASSERT(call_parent->ext_fiber == t->ext_fiber); + CILK_ASSERT_POINTER_EQUAL(call_parent->ext_fiber, t->ext_fiber); t->ext_fiber = NULL; } @@ -363,7 +363,7 @@ static Closure *Closure_return(__cilkrts_worker *const w, worker_id self, Closure_assert_alienation(self, child); CILK_ASSERT(child->has_cilk_callee == 0); - CILK_ASSERT(child->call_parent == NULL); + CILK_ASSERT_NULL(child->call_parent); CILK_ASSERT(parent != NULL); cilkrts_alert(RETURN, "(Closure_return) child %p, parent %p", @@ -516,7 +516,7 @@ static Closure *return_value(__cilkrts_worker *const w, worker_id self, Closure *res = NULL; CILK_ASSERT(t->status == CLOSURE_RETURNING); - CILK_ASSERT(t->call_parent == NULL); + CILK_ASSERT_NULL(t->call_parent); if (t->call_parent == NULL) { res = Closure_return(w, self, t); @@ -674,7 +674,7 @@ static void setup_closures_in_stacklet(__cilkrts_worker *const w, void *extension = USE_EXTENSION ? youngest->extension : NULL; oldest = oldest_non_stolen_frame_in_stacklet(youngest); - CILK_ASSERT(youngest == youngest_cl->frame); + CILK_ASSERT_POINTER_EQUAL(youngest, youngest_cl->frame); CILK_ASSERT(__cilkrts_stolen(youngest)); CILK_ASSERT((oldest_cl->frame == NULL && oldest != youngest) || @@ -761,7 +761,7 @@ static Closure *promote_child(__cilkrts_stack_frame **head, ReadyDeque *deques, CILK_ASSERT(cl->status == CLOSURE_RUNNING); CILK_ASSERT(cl->owner_ready_deque == pn); - CILK_ASSERT(cl->next_ready == NULL); + CILK_ASSERT_NULL(cl->next_ready); /* cl may have a call parent: it might be promoted as its containing * stacklet is stolen, and it's call parent is promoted into full and @@ -966,7 +966,7 @@ static Closure *extract_top_spawning_closure(__cilkrts_stack_frame **head, // is simply cl (i.e., there is only one frame in the stacklet), // so we didn't set res in promote_child. res = deque_xtract_top(deques, self, victim_id); - CILK_ASSERT(cl == res); + CILK_ASSERT_POINTER_EQUAL(cl, res); } res->fiber = cilk_fiber_allocate_from_pool(w); @@ -1121,7 +1121,7 @@ void promote_own_deque(__cilkrts_worker *w) { // unfortunately this function releases both locks Closure *res = extract_top_spawning_closure(head, deques, w, w, cl, self, self); CILK_ASSERT(res); - CILK_ASSERT(res->fiber == NULL); + CILK_ASSERT_NULL(res->fiber); // ANGE: if cl is not the spawning parent, then // there is more frames in the stacklet to promote @@ -1162,7 +1162,7 @@ void longjmp_to_user_code(__cilkrts_worker *w, Closure *t) { // the personality function. __cilkrts_throwing(sf) is true only when // the personality function is syncing sf. if (!__cilkrts_throwing(sf)) { - CILK_ASSERT(t->orig_rsp == NULL); + CILK_ASSERT_NULL(t->orig_rsp); CILK_ASSERT((sf->flags & CILK_FRAME_LAST) || in_fiber(fiber, (char *)FP(sf))); CILK_ASSERT(in_fiber(fiber, (char *)SP(sf))); @@ -1180,7 +1180,7 @@ void longjmp_to_user_code(__cilkrts_worker *w, Closure *t) { } else { void *new_rsp = sysdep_reset_stack_for_resume(fiber, sf); USE_UNUSED(new_rsp); - CILK_ASSERT(SP(sf) == new_rsp); + CILK_ASSERT_POINTER_EQUAL(SP(sf), new_rsp); if (USE_EXTENSION) { w->extension = sf->extension; w->ext_stack = sysdep_get_stack_start(t->ext_fiber); @@ -1241,7 +1241,7 @@ int Cilk_sync(__cilkrts_worker *const w, __cilkrts_stack_frame *frame) { // each sync is executed only once; since we occupy user_ht only // when sync fails, the user_ht should remain NULL at this point. - CILK_ASSERT(t->user_ht == (hyper_table *)NULL); + CILK_ASSERT_NULL(t->user_ht); if (Closure_has_children(t)) { cilkrts_alert(SYNC, "(Cilk_sync) Closure %p has outstanding children", @@ -1400,7 +1400,7 @@ void do_what_it_says_boss(__cilkrts_worker *w, Closure *t) { void worker_scheduler(__cilkrts_worker *w) { Closure *t = NULL; - CILK_ASSERT(w == __cilkrts_get_tls_worker()); + CILK_ASSERT_POINTER_EQUAL(w, __cilkrts_get_tls_worker()); CILK_START_TIMING(w, INTERVAL_SCHED); worker_change_state(w, WORKER_SCHED);