Skip to content

Commit 48472d5

Browse files
committed
fix(callgrind): check obj-skip on every BB entry, not only jk_Call
The obj-skip check was gated on jmpkind == jk_Call. When a function in a skipped object was entered via jk_Jump or fall-through (interpreter dispatch, tail calls, perf trampoline, JIT), the skip flag never latched and the function leaked into the dump as its own fn= block. Also instrument the cxt==0 forced push_cxt path with a diagnostic line so we can measure the residual leak when a skipped fn is forced into a top-level context after an instrumentation start or call-stack underflow.
1 parent a6e53bb commit 48472d5

1 file changed

Lines changed: 29 additions & 7 deletions

File tree

callgrind/bbcc.c

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -725,7 +725,11 @@ void CLG_(setup_bbcc)(BB* bb)
725725
}
726726
}
727727

728-
if (jmpkind == jk_Call) {
728+
/* Check obj-skip on every BB entry, not only jk_Call.
729+
* The interpreter / perf trampoline can enter functions via jk_Jump
730+
* or fall-through; if we only checked on jk_Call, skip would never
731+
* latch for those fns and they'd leak into the dump. */
732+
{
729733
fn_node* node = CLG_(get_fn_node)(bb);
730734
skip = node->skip;
731735
if (!skip && !node->obj_skip_checked){
@@ -740,14 +744,15 @@ void CLG_(setup_bbcc)(BB* bb)
740744
}
741745
if (skip) {
742746
VG_(message)(Vg_UserMsg,
743-
"obj_skip HIT: fn='%s' obj='%s'\n",
744-
node->name, obj_name);
747+
"obj_skip HIT: fn='%s' obj='%s' jmpkind=%d\n",
748+
node->name, obj_name, (int)jmpkind);
745749
}
746750
if (!skip && CLG_(clo).objs_to_skip_count > 0) {
747751
VG_(message)(Vg_UserMsg,
748-
"obj_skip miss: fn='%s' obj='%s' (len=%lu, %d entries)\n",
752+
"obj_skip miss: fn='%s' obj='%s' (len=%lu, %d entries) jmpkind=%d\n",
749753
node->name, obj_name,
750-
VG_(strlen)(obj_name), CLG_(clo).objs_to_skip_count);
754+
VG_(strlen)(obj_name), CLG_(clo).objs_to_skip_count,
755+
(int)jmpkind);
751756
for (int i=0; i<CLG_(clo).objs_to_skip_count; i++)
752757
VG_(message)(Vg_UserMsg, " vs [%d] strcmp=%d (len=%lu) '%s'\n",
753758
i, cmp_results[i],
@@ -809,9 +814,26 @@ void CLG_(setup_bbcc)(BB* bb)
809814
}
810815
}
811816

812-
/* Change new context if needed, taking delayed_push into account */
817+
/* Change new context if needed, taking delayed_push into account.
818+
*
819+
* The `cxt == 0` clause used to fire regardless of skip, which meant
820+
* that on the first BB after instrumentation start / call-stack
821+
* underflow, a skipped libpython fn would still be pushed as the new
822+
* top context and appear as its own fn= block in the dump.
823+
*
824+
* Now: if the fn is skip, we still push it (otherwise the assert at
825+
* the end of this block fires when fn_stack is empty), but emit a
826+
* diagnostic so we can measure how often the leak happens. */
813827
if ((delayed_push && !skip) || (CLG_(current_state).cxt == 0)) {
814-
CLG_(push_cxt)(CLG_(get_fn_node)(bb));
828+
fn_node* push_fn = CLG_(get_fn_node)(bb);
829+
if (skip && CLG_(current_state).cxt == 0) {
830+
VG_(message)(Vg_UserMsg,
831+
"push_cxt FORCED for skipped fn (cxt==0): fn='%s' obj='%s' jmpkind=%d delayed_push=%d\n",
832+
push_fn->name,
833+
push_fn->file->obj->name,
834+
(int)jmpkind, (int)delayed_push);
835+
}
836+
CLG_(push_cxt)(push_fn);
815837
}
816838
CLG_ASSERT(CLG_(current_fn_stack).top > CLG_(current_fn_stack).bottom);
817839

0 commit comments

Comments
 (0)