Skip to content

Commit

Permalink
Improve messages for pointer assertions
Browse files Browse the repository at this point in the history
  • Loading branch information
VoxSciurorum authored and neboat committed Sep 3, 2024
1 parent b71274e commit a05aa9f
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 43 deletions.
36 changes: 18 additions & 18 deletions runtime/closure.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand All @@ -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
Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand All @@ -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
Expand Down
7 changes: 7 additions & 0 deletions runtime/debug.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions runtime/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion runtime/local-reducer-api.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions runtime/personality.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down
12 changes: 6 additions & 6 deletions runtime/readydeque.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,15 @@ 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);
(cl->next_ready)->prev_ready = (Closure *)NULL;
}
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;
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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;
Expand Down
28 changes: 14 additions & 14 deletions runtime/scheduler.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) ||
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)));
Expand All @@ -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);
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit a05aa9f

Please sign in to comment.