[COD-3196] fix(callgrind): represent os threads under --separate-threads=yes

callgrind/bbcc.c

@@ -165,7 +165,7 @@ BBCC* lookup_bbcc(BB* bb, Context* cxt)
 	   /* if we don't dump threads separate, tid doesn't have to match */
 	   return bbcc;
        }
-       if (bbcc->tid == CLG_(current_tid)) return bbcc;
+       if (bbcc->tid == CLG_(current_thread_serial)) return bbcc;
    }
 
    CLG_(stat).bbcc_lru_misses++;
@@ -179,7 +179,7 @@ BBCC* lookup_bbcc(BB* bb, Context* cxt)
    }
 
    CLG_DEBUG(2,"  lookup_bbcc(BB %#lx, Cxt %u, fn '%s'): %p (tid %u)\n",
-	    bb_addr(bb), cxt->base_number, cxt->fn[0]->name, 
+	    bb_addr(bb), cxt->base_number, cxt->fn[0]->name,
 	    bbcc, bbcc ? bbcc->tid : 0);
 
    CLG_DEBUGIF(2)
@@ -274,7 +274,7 @@ BBCC* new_bbcc(BB* bb)
 			    sizeof(BBCC) +
 			    (bb->cjmp_count+1) * sizeof(JmpData));
    bbcc->bb  = bb;
-   bbcc->tid = CLG_(current_tid);
+   bbcc->tid = CLG_(current_thread_serial);
 
    bbcc->ret_counter = 0;
    bbcc->skipped = 0;
@@ -381,8 +381,8 @@ static BBCC* clone_bbcc(BBCC* orig, Context* cxt, Int rec_index)
 
     if (rec_index == 0) {
 
-      /* hash insertion is only allowed if tid or cxt is different */
-      CLG_ASSERT((orig->tid != CLG_(current_tid)) ||
+      /* hash insertion is only allowed if thread or cxt is different */
+      CLG_ASSERT((orig->tid != CLG_(current_thread_serial)) ||
 		(orig->cxt != cxt));
 
       bbcc->rec_index = 0;
@@ -394,7 +394,7 @@ static BBCC* clone_bbcc(BBCC* orig, Context* cxt, Int rec_index)
     }
     else {
       if (CLG_(clo).separate_threads)
-	CLG_ASSERT(orig->tid == CLG_(current_tid));
+	CLG_ASSERT(orig->tid == CLG_(current_thread_serial));
 
       CLG_ASSERT(orig->cxt == cxt);
       CLG_ASSERT(orig->rec_array);

callgrind/callstack.c

@@ -26,6 +26,7 @@
 
 #include "global.h"
 #include "pub_tool_stacktrace.h"
+#include "pub_tool_threadstate.h"
 #if defined(VGA_arm64)
 #include "pub_tool_guest.h"     /* VexGuestArchState, for guest_X30 */
 #endif
@@ -500,9 +501,10 @@ Int CLG_(unwind_call_stack)(Addr sp, Int minpops)
  *
  * Called on the OFF->ON instrumentation transition: the client (e.g.
  * pytest_codspeed) typically reaches CALLGRIND_START_INSTRUMENTATION several
- * libpython frames deep. Without seeding, csp stays at 0 while the real
- * stack is non-empty, and every subsequent ret trips handleUnderflow and
- * leaks the returned-into fn as a top-level fn= block.
+ * libpython frames deep, and other threads sit parked mid-stack. Without
+ * seeding, csp stays at 0 while the real stack is non-empty, and every
+ * subsequent ret trips handleUnderflow and leaks the returned-into fn as a
+ * top-level fn= block.
  *
  * We push a (jcc=0, skip-style) call_entry for every native frame so
  * SP-based unwind works. For frames that should appear in the output
@@ -512,7 +514,9 @@ Int CLG_(unwind_call_stack)(Addr sp, Int minpops)
  * deliberately excluded from the cxt chain — they get SP-only entries. */
 #define CLG_RECON_MAX_FRAMES 256
 
-void CLG_(reconstruct_call_stack_from_native)(ThreadId tid)
+/* Seeds the call stack currently installed in the globals, which must be the
+ * one belonging to tid. */
+static void seed_call_stack_from_native(ThreadId tid)
 {
     Addr ips[CLG_RECON_MAX_FRAMES];
     Addr sps[CLG_RECON_MAX_FRAMES];
@@ -523,6 +527,18 @@ void CLG_(reconstruct_call_stack_from_native)(ThreadId tid)
     UInt n = VG_(get_StackTrace)(tid, ips, CLG_RECON_MAX_FRAMES, sps, NULL, 0);
     if (n == 0) return;
 
+    /* A thread other than the running one is unwound from the register state
+     * saved when it was descheduled, usually inside a syscall. A single frame
+     * means the unwinder got no further than that point: the one entry would
+     * carry a made-up entry SP (see the ce->sp comment below), and everything
+     * the thread runs next would be re-parented under it. Starting empty is
+     * the lesser evil. */
+    if (n < 2 && tid != VG_(get_running_tid)()) {
+        CLG_DEBUG(1, "  seed: thread %u unwound to a single frame, skipping\n",
+                  tid);
+        return;
+    }
+
     /* Push bottom-up: oldest caller first, current frame last. */
     for (Int frame = n - 1; frame >= 0; frame--) {
         fn_node* fn = CLG_(get_fn_node_for_addr)(ips[frame]);
@@ -581,4 +597,30 @@ void CLG_(reconstruct_call_stack_from_native)(ThreadId tid)
         ensure_stack_size(cs->sp + 1);
         cs->entry[cs->sp].cxt = 0;
     }
+
+    CLG_DEBUG(1, "  seed: thread %u seeded with %u frame(s), csp=%d\n",
+              tid, n, cs->sp);
+}
+
+void CLG_(reconstruct_call_stack_from_native)(ThreadId requesting_tid)
+{
+    /* Every live thread is seeded, not only the requesting one. A thread parked
+     * in a syscall at the transition has a non-empty native stack and an empty
+     * shadow stack just the same, and its first ret underflows: handleUnderflow
+     * pushes the returned-into fn as a fresh top-level context without
+     * consulting fn->skip, so an obj-skipped frame becomes a visible root and
+     * everything the thread does afterwards is recorded under it.
+     *
+     * The call stack, context chain and fn stack live in globals belonging to
+     * whichever thread is switched in, so each thread is seeded under its own
+     * switch_thread. */
+    for (ThreadId tid = 1; tid < VG_N_THREADS; tid++) {
+        /* Zero for any tid that is not a live thread. */
+        if (VG_(get_thread_lwpid)(tid) == 0) continue;
+
+        CLG_(switch_thread)(tid);
+        seed_call_stack_from_native(tid);
+    }
+
+    CLG_(switch_thread)(requesting_tid);
 }

callgrind/dump.c

@@ -37,6 +37,12 @@ static Int out_counter = 0;
 static HChar* out_file = 0;
 static Bool dumps_initialized = False;
 
+/* With --combine-dumps, whether the single output file already carries its
+ * format header. A part can be skipped entirely (no thread had costs), so the
+ * first actual write may happen at out_counter > 1 yet still needs the header;
+ * hence tracked as file state, not derived from out_counter. */
+static Bool combined_header_written = False;
+
 /* Command */
 static HChar *cmdbuf;
 
@@ -1290,12 +1296,16 @@ void file_err(void)
  * Create a new dump file and write header.
  *
  * Naming: <CLG_(clo).filename_base>.<pid>[.<part>][-<tid>]
- *         <part> is skipped for final dump (trigger==0)
- *         <tid>  is skipped for thread 1 with CLG_(clo).separate_threads=no
+ *         <part>   is skipped for final dump (trigger==0)
+ *         <tid>    is the thread's kernel id, skipped with
+ *                  CLG_(clo).separate_threads=no
+ *
+ * ti is the thread being dumped (used for its tid and name); its costs
+ * must already be installed as the current state.
  *
  * Returns the file descriptor, and -1 on error (no write permission)
  */
-static VgFile *new_dumpfile(int tid, const HChar* trigger)
+static VgFile *new_dumpfile(thread_info* ti, const HChar* trigger)
 {
     Bool appending = False;
     int i;
@@ -1312,14 +1322,14 @@ static VgFile *new_dumpfile(int tid, const HChar* trigger)
 	    i += VG_(sprintf)(filename+i, ".%d", out_counter);
 
 	if (CLG_(clo).separate_threads)
-	    VG_(sprintf)(filename+i, "-%02d", tid);
+	    VG_(sprintf)(filename+i, "-%02d", CLG_(thread_lwpid)(ti));
 
 	fp = VG_(fopen)(filename, VKI_O_WRONLY|VKI_O_TRUNC, 0);
     }
     else {
 	VG_(sprintf)(filename, "%s", out_file);
         fp = VG_(fopen)(filename, VKI_O_WRONLY|VKI_O_APPEND, 0);
-	if (fp && out_counter>1)
+	if (fp && combined_header_written)
 	    appending = True;
     }
 
@@ -1354,11 +1364,17 @@ static VgFile *new_dumpfile(int tid, const HChar* trigger)
 
 	/* "cmd:" line */
 	VG_(fprintf)(fp, "cmd: %s", cmdbuf);
+
+	combined_header_written = True;
     }
 
     VG_(fprintf)(fp, "\npart: %d\n", out_counter);
     if (CLG_(clo).separate_threads) {
-	VG_(fprintf)(fp, "thread: %d\n", tid);
+	const HChar* tname = CLG_(thread_name)(ti);
+
+	VG_(fprintf)(fp, "thread: %d\n", CLG_(thread_lwpid)(ti));
+	if (tname && tname[0])
+	    VG_(fprintf)(fp, "desc: Thread name: %s\n", tname);
     }
 
     /* "desc:" lines */
@@ -1456,7 +1472,6 @@ static VgFile *new_dumpfile(int tid, const HChar* trigger)
    sum = CLG_(get_eventset_cost)( CLG_(sets).full );
    CLG_(zero_cost)(CLG_(sets).full, sum);
    if (CLG_(clo).separate_threads) {
-     thread_info* ti = CLG_(get_current_thread)();
      CLG_(add_diff_cost)(CLG_(sets).full, sum, ti->lastdump_cost,
 			   ti->states.entry[0]->cost);
    }
@@ -1514,21 +1529,55 @@ static void close_dumpfile(VgFile *fp)
 
 static const HChar* print_trigger;
 
+/* Whether any thread section was written for the part being dumped, and
+ * whether to bypass the zero-delta skip (used to force an empty section so
+ * the part still appears in the output). */
+static Bool part_section_written;
+static Bool force_empty_section;
+
 static void print_bbccs_of_thread(thread_info* ti)
 {
   BBCC **p, **array;
   FnPos lastFnPos;
   AddrPos lastAPos;
+  VgFile *print_fp;
 
   CLG_DEBUG(1, "+ print_bbccs(tid %u)\n", CLG_(current_tid));
 
-  VgFile *print_fp = new_dumpfile(CLG_(current_tid), print_trigger);
+  p = array = prepare_dump();
+
+  /* With per-thread dumps, skip a thread that did nothing since the last dump
+   * (no BBCCs and a zero exec-state delta) so parts don't grow empty sections.
+   * A thread with never-dumped cost still has a nonzero delta (its
+   * lastdump_cost baseline is zero), so this never drops real data. */
+  if (CLG_(clo).separate_threads && !force_empty_section && array[0] == 0) {
+    /* Compared element-wise rather than via a scratch cost buffer: those are
+     * pool-allocated and must not be individually freed, and add_diff_cost
+     * would clobber lastdump_cost, which the real dump updates below. */
+    EventSet* es = CLG_(sets).full;
+    ULong* last = ti->lastdump_cost;
+    ULong* cur  = ti->states.entry[0]->cost;
+    Bool empty = True;
+    Int i;
+    for(i = 0; i < es->size; i++)
+      if (cur[i] != last[i]) { empty = False; break; }
+    if (empty) {
+      CLG_DEBUG(1, "- print_bbccs(tid %u): empty delta, skipped\n",
+                CLG_(current_tid));
+      VG_(free)(array);
+      return;
+    }
+  }
+
+  print_fp = new_dumpfile(ti, print_trigger);
   if (print_fp == NULL) {
     CLG_DEBUG(1, "- print_bbccs(tid %u): No output...\n", CLG_(current_tid));
+    VG_(free)(array);
     return;
   }
+  part_section_written = True;
 
-  p = array = prepare_dump();
+  p = array;
   init_fpos(&lastFnPos);
   init_apos(&lastAPos, 0, 0, 0);
 
@@ -1596,6 +1645,16 @@ static void print_bbccs_of_thread(thread_info* ti)
 }
 
 
+/* Write an empty section for the first thread that can be written at all. */
+static void print_one_empty_section(thread_info* ti)
+{
+  if (part_section_written) return;
+
+  force_empty_section = True;
+  print_bbccs_of_thread(ti);
+  force_empty_section = False;
+}
+
 static void print_bbccs(const HChar* trigger, Bool only_current_thread)
 {
   init_dump_array();
@@ -1611,10 +1670,19 @@ static void print_bbccs(const HChar* trigger, Bool only_current_thread)
     print_bbccs_of_thread( CLG_(get_current_thread)() );
     CLG_(switch_thread)(orig_tid);
   }
-  else if (only_current_thread)
-    print_bbccs_of_thread( CLG_(get_current_thread)() );
-  else
-    CLG_(forall_threads)(print_bbccs_of_thread);
+  else {
+    /* Per-thread dumps flush every thread, exited ones included, so each
+     * thread's delta lands under the part being dumped -- otherwise threads
+     * other than the caller only ever get written by the termination dump. The
+     * only_current_thread hint is therefore ignored in this mode. */
+    part_section_written = False;
+    CLG_(forall_threads_incl_exited)(print_bbccs_of_thread);
+    if (!part_section_written)
+      /* Every thread was skipped as zero-delta, but the part header itself
+       * carries metadata (trigger, timerange) that consumers rely on: force
+       * one empty section so the part still appears. */
+      CLG_(forall_threads_incl_exited)(print_one_empty_section);
+  }
 
   free_dump_array();
 }
@@ -1755,6 +1823,7 @@ void CLG_(init_dumps)(void)
 	}
     }
     if (!sr_isError(res)) VG_(close)( (Int)sr_Res(res) );
+    combined_header_written = False;
 
     if (!dumps_initialized)
 	init_cmdbuf();

callgrind/global.h

@@ -380,7 +380,9 @@ struct _BBCC {
     BB*      bb;           /* BB for this cost center */
 
     Context* cxt;          /* execution context of this BBCC */
-    ThreadId tid;          /* only for assertion check purpose */
+    UInt     tid;          /* owning thread's serial; identifies the thread
+                            * across recycled valgrind ThreadId slots.
+                            * Only for lookup/assertion purposes. */
     UInt     rec_index;    /* Recursion index in rec->bbcc for this bbcc */
     BBCC**   rec_array;    /* Variable sized array of pointers to 
 			    * recursion BBCCs. Shared. */
@@ -588,6 +590,19 @@ struct _exec_stack {
  */
 struct _thread_info {
 
+  /* Monotonic identity, never recycled unlike the valgrind ThreadId slot, so
+   * a new OS thread taking over a slot is never conflated with the dead one. */
+  UInt serial;
+  /* Kernel thread id (LWP id), snapshotted when the thread exits: from then on
+   * the core no longer knows it. Live threads are read from the core instead
+   * (see CLG_(thread_lwpid)). */
+  Int tid;
+  ThreadId slot;      /* valgrind ThreadId this thread runs in */
+  /* Name the thread gave itself, snapshotted when it exits (NULL if none);
+   * for live threads the current name is read from the core at dump time. */
+  HChar* name;
+  struct _thread_info* next_created;
+
   /* state */
   fn_stack fns;       /* function stack */
   call_stack calls;   /* context call arc stack */
@@ -710,6 +725,10 @@ void CLG_(collectBlockInfo)(IRSB* bbIn, UInt*, UInt*, Bool*);
 void CLG_(set_instrument_state)(const HChar*,Bool);
 void CLG_(dump_profile)(const HChar* trigger,Bool only_current_thread);
 void CLG_(zero_all_cost)(Bool only_current_thread);
+/* Pop everything left on the current thread's call stack, as done at program
+ * termination, so its pending costs settle into its cost containers. Safe on
+ * an already-unwound or empty state. */
+void CLG_(unwind_thread)(thread_info* t);
 Int CLG_(get_dump_counter)(void);
 void CLG_(fini)(Int exitcode);
 
@@ -785,7 +804,15 @@ thread_info** CLG_(get_threads)(void);
 thread_info* CLG_(get_current_thread)(void);
 void CLG_(switch_thread)(ThreadId tid);
 void CLG_(forall_threads)(void (*func)(thread_info*));
+/* Like forall_threads, but also visits threads that have exited, in creation
+ * order. Their costs stay attributed for the rest of the run, so anything
+ * walking per-thread cost has to see them. */
+void CLG_(forall_threads_incl_exited)(void (*func)(thread_info*));
+/* Kernel thread id (LWP id) of a thread, for the dump only. */
+Int CLG_(thread_lwpid)(thread_info* t);
+const HChar* CLG_(thread_name)(thread_info* t);
 void CLG_(run_thread)(ThreadId tid);
+void CLG_(pre_thread_ll_exit)(ThreadId tid);
 
 void CLG_(init_exec_state)(exec_state* es);
 void CLG_(init_exec_stack)(exec_stack*);
@@ -815,6 +842,8 @@ extern call_stack CLG_(current_call_stack);
 extern fn_stack   CLG_(current_fn_stack);
 extern exec_state CLG_(current_state);
 extern ThreadId   CLG_(current_tid);
+/* Serial of the current thread, 0 if none is loaded. */
+extern UInt       CLG_(current_thread_serial);
 extern FullCost   CLG_(total_cost);
 extern struct cachesim_if CLG_(cachesim);
 extern struct event_sets  CLG_(sets);