src/stackprof.cxx: WIP revisions for commit1 comments

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 800ed85b05f822e66d21687c5fac7ff7705e2ab0..43474e9efc7f4e990cfd56d0dec883091146d0b9 100644 (file)
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -44,7 +44,7 @@ src/ranlib.c
 src/readelf.c
 src/size.c
 src/stack.c
-src/stacktrace.c
+src/stackprof.c
 src/strings.c
 src/strip.c
 src/threadlib.c

diff --git a/src/Makefile.am b/src/Makefile.am
index a6c34d15bacc3485ca33499d50a75c91daff7228..08f500303a8293e46bf86b0092c5c5cf62c882c4 100644 (file)
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -127,7 +127,8 @@ srcfiles_SOURCES = srcfiles.cxx
 srcfiles_LDADD = $(libdw) $(libelf) $(libeu) $(argp_LDADD) $(libarchive_LIBS) $(libdebuginfod)
 if ENABLE_STACKPROF
 stackprof_SOURCES = stackprof.cxx
-stackprof_CPPFLAGS = $(AM_CPPFLAGS) $(jsonc_CXXFLAGS)
+# TODO(REVIEW) do we need the CPPFLAGS?
+stackprof_CPPFLAGS = $(AM_CPPFLAGS)
 stackprof_CXXFLAGS = -Wall
 stackprof_LDADD = $(libebl) $(libdw) $(libelf) $(libeu) $(argp_LDADD) $(libpfm_LIBS) $(jsonc_LIBS)
 endif

diff --git a/src/stackprof.cxx b/src/stackprof.cxx
index 6f7d3a42f0edc0c6d94d99309716ef4d52d4433e..0d001b1474374af59e4d80a0573a485b3176c3f7 100644 (file)
--- a/src/stackprof.cxx
+++ b/src/stackprof.cxx
@@ -65,9 +65,12 @@
 #include <gelf.h>
 #include <dwarf.h>
 #include <libdwfl.h>
+#include <libdwfl_stacktrace.h>
 #include <libdw.h>
 #include "../libebl/libebl.h"
-#include "../libdwfl_stacktrace/libdwfl_stacktrace.h"
+
+// optional debug code
+//#define STACKPROF_STATS_DEBUG
 
 using namespace std;
 
@@ -398,7 +401,7 @@ static const struct argp_option options[] =
   { "verbose", 'v', NULL, 0, N_("Increase verbosity of logging messages (modules/samples/frames/more)."), 0 },
   /* TODO: Add "quiet" option suppressing summary table. */
   { "gmon", 'g', NULL, 0, N_("Generate gmon.BUILDID.out files for each binary."), 0 },
-  { "hist-split", 'G', HIST_SPLIT_OPTS, 0, N_("Histogram splitting method for gmon, default 'even'."), 0 },
+  { "hist-split", 'G', HIST_SPLIT_OPTS, 0, N_("Split gmon histogram output into even or flexible chunks, default 'even'."), 0 },
   { "maxframes", 'n', "MAXFRAMES", 0, N_("Maximum number of frames to unwind, default 1 with --gmon, 256 otherwise."), 0 },
   { "output", 'o', "DIR", 0, N_("Output directory for gmon.BUILDID.out files."), 0 },
   { "force", 'f', NULL, 0, N_("Unlink output files to force writing as new."), 0 },
@@ -479,6 +482,8 @@ parse_opt (int key, char *arg, struct argp_state *state)
 
     case 'p':
       pid = atoi(arg);
+      if (pid == 0)
+       argp_error (state, N_("-p PID should be a positive process id."));
       break;
 
     case 'n':
@@ -680,7 +685,7 @@ int main (int argc, char *argv[])
        }
       else
        {
-#if 1
+#ifndef STACKPROF_STATS_DEBUG
          tab = new UnwindStatsTable();
          usc = new UnwindStatsConsumer(tab);
          pcu = new PerfConsumerUnwinder(usc, tab);
@@ -728,6 +733,7 @@ int main (int argc, char *argv[])
   catch (const exception& e)
     {
       cerr << format("{}\n", e.what());
+      return 127;
     }
 
   return 0;
@@ -753,7 +759,8 @@ PerfReader::PerfReader(perf_event_attr* attr, PerfConsumer* consumer, int pid)
   std::string_view machine = u.machine;
   if (machine == "x86_64") em = EM_X86_64;
   else if (machine == "i686" || machine == "i386") em = EM_386;
-  else if (machine == "aarch64" || machine == "armv7l") em = EM_ARM;
+  else if (machine == "aarch64") em = EM_AARCH64;
+  else if (machine == "armv7l") em = EM_ARM;
   else {
     cerr << format("ERROR: Unsupported architecture: {}\n", u.machine);
     exit(1);
@@ -1303,8 +1310,10 @@ uint32_t expected_frame_nregs (Ebl *ebl)
   int m = ebl_get_elfmachine(ebl);
   /* TODO: Generalize the API via libdwflst to allow any architecture.  */
   /* For aarch64, we actually use fewer than ebl->frame_nregs to unwind.  */
-  if (m == EM_ARM) /* TODO also EM_AARCH64 */
-    return 14; /* XXX 16 for 32-bit ARM */
+  if (m == EM_AARCH64)
+    return 14;
+  if (m == EM_ARM)
+    return 16;
   /* On x86, expect everything except FLAGS:  */
   if (m == EM_X86_64 || m == EM_386)
     return ebl_frame_nregs(ebl);
@@ -1332,7 +1341,7 @@ Dwfl *PerfConsumerUnwinder::find_dwfl(pid_t pid, const uint64_t *regs, uint32_t
       goto reuse;
     }
   err = this->find_procfile(dwfl, &pid, &elf, &elf_fd);
-  if (err < 0)
+  if (err != 0) /* TODO check errnos */
     {
       if (verbose)
        cerr << format("WARNING: find_procfile pid {}: {}\n", (long long) pid, dwfl_errmsg(-1));
@@ -1357,7 +1366,7 @@ int PerfConsumerUnwinder::get_sp_reg(bool is_abi32)
   /* TODO: Generalize the API via libdwflst to allow any architecture.  */
   int machine = ebl_get_elfmachine(this->reader->ebl());
   if (machine == EM_X86_64 || machine == EM_386) return is_abi32 ? 4 : 7;
-  else if (machine == EM_ARM) return is_abi32 ? 13 : 31;
+  else if (machine == EM_ARM || machine == EM_AARCH64) return is_abi32 ? 13 : 31;
   else { assert(0); return 7; }
 }
 
@@ -1797,6 +1806,7 @@ void GprofUnwindSampleConsumer::record_gmon_out(const string& buildid, UnwindMod
   if (!of)
     {
       cerr << format(N_("ERROR: buildid {} -- could not open '{}' for writing\n"), buildid, filename);
+      return;
     }
 
   /* Write gmon header.  It and other headers mostly hold
@@ -1845,8 +1855,6 @@ void GprofUnwindSampleConsumer::record_gmon_out(const string& buildid, UnwindMod
          uint64_t next_size = opt_size;
          while (next_size < max_size)
            {
-             if (next_size > max_size)
-               next_size = max_size;
              uint64_t size_inc = sizeof(struct gmon_hdr) + next_size;
              uint64_t size_est = size_inc;
              uint64_t pc = low_pc;
@@ -2008,12 +2016,6 @@ void GprofUnwindSampleConsumer::process(const UnwindSample *sample)
   Dwfl_Module *mod = dwfl_addrmodule(sample->dwfl, pc);
   if (mod == NULL)
     return;
-#if 0
-  // XXX is the bias needed?
-  Dwarf_Addr bias;
-  Elf *elf = dwfl_module_getelf(mod, &bias);
-  (void)elf;
-#endif
 
   Dwfl_Module *mod2 = dwfl_addrmodule(sample->dwfl, pc2);
   // XXX: allowing mod2 == NULL -- callgraph arc will be skipped