Still More Slackspot Issues - Patch Included

Stephen D. Cohen steve at uscohens.com
Fri Mar 22 00:31:16 CET 2019


Folks,

     Some time back I submitted a patch for the slackspot issue
regarding when to remove the vm_start offset from the program counter
in the relax handling mechanism in debug.c. My solution had the
unfortunate flaw that it peeked into the actual ELF section headers in
the running system (ewww...) and was deemed unworthy. The solution
that got implemented, using VM_DENYWRITE to detect dynamic objects,
has the unfortunate flaw of not working with position independent
executable programs. The very problem that got me started in the first
place.

     As luck would have it, I recently needed slackspot to figure out
some issues with our application. So I set out to make it work again.
In the process, I also found another issue - when the target
application uses inline routines, all the spots in the same file past
the inlined routine are reported incorrectly. This is due to slackspot
assuming a 1:1 mapping between addresses and reported results. The
inlined routines actually report two sets of results for each address.

     So I propose the attached solution. Move the trouble of
identifying the ELF object type out of the kernel and put it in
slackspot instead. Just store both the pc and vm_start for every spot,
then figure it all out in slackspot. Slackspot then uses elfread to
determine if the file is a dynamic object or not (if it is EXEC, it is
not dynamic, otherwise it needs to have vm_start taken off). I also
fixed the inline issue.

Warmest Regards,

Steve

P.S. I will also attach the patch in case the cut-and-paste below gets
mangled beyond recognition.

/\/\/\/\/\ Begin Patch /\/\/\/\/\
diff -aur a/kernel/cobalt/debug.c b/kernel/cobalt/debug.c
--- a/kernel/cobalt/debug.c 2019-01-14 10:11:19.000000000 -0500
+++ b/kernel/cobalt/debug.c 2019-03-21 17:07:23.466631608 -0400
@@ -123,6 +123,7 @@
  int depth;
  struct backtrace {
  unsigned long pc;
+ unsigned long vm_start;
  const char *mapname;
  } backtrace[SIGSHADOW_BACKTRACE_DEPTH];
  /* Program hash value of the caller. */
@@ -248,17 +249,8 @@
  if (vma == NULL)
  continue;

- /*
- * Hack. Unlike DSOs, executables and interpreters
- * (e.g. dynamic linkers) are protected against write
- * attempts. Use this to determine when $pc should be
- * fixed up by subtracting the mapping base address in
- * the DSO case.
- */
- if (!(vma->vm_flags & VM_DENYWRITE))
- pc -= vma->vm_start;
-
  spot.backtrace[depth].pc = pc;
+ spot.backtrace[depth].vm_start = vma->vm_start;

  /*
  * Even in case we can't fetch the map name, we still
@@ -448,8 +440,9 @@
         reason_str[p->spot.reason], p->spot.thread);

  for (n = 0; n < p->spot.depth; n++)
- xnvfile_printf(it, "0x%lx %s\n",
+ xnvfile_printf(it, "0x%lx 0x%lx %s\n",
         p->spot.backtrace[n].pc,
+        p->spot.backtrace[n].vm_start,
         p->spot.backtrace[n].mapname ?: "?");

  xnvfile_printf(it, ".\n");
diff -aur a/utils/slackspot/slackspot.c b/utils/slackspot/slackspot.c
--- a/utils/slackspot/slackspot.c 2019-01-11 08:40:38.000000000 -0500
+++ b/utils/slackspot/slackspot.c 2019-03-21 17:05:23.666130908 -0400
@@ -87,6 +87,7 @@

 struct location {
  unsigned long pc;
+ unsigned long vm_start;
  char *function;
  char *file;
  int lineno;
@@ -110,6 +111,7 @@
  int depth;
  struct backtrace {
  unsigned long pc;
+ unsigned long vm_start;
  struct mapping *mapping;
  const struct location *where;
  } backtrace[SIGSHADOW_BACKTRACE_DEPTH];
@@ -125,7 +127,7 @@
  return fnmatch(f->exp, p->thread_name, 0);
 }

-static int filter_pid(struct filter *f,  struct relax_spot *p)
+static int filter_pid(struct filter *f, struct relax_spot *p)
 {
  char pid[16];

@@ -336,7 +338,7 @@
 {
  struct relax_spot *p;
  struct mapping *m;
- unsigned long pc;
+ unsigned long pc, vm_start;
  char *mapping, c;
  ENTRY e, *ep;
  int ret;
@@ -375,8 +377,9 @@
  if (c == '.' && getc(fp) == '\n')
  break;
  ungetc(c, fp);
- ret = fscanf(fp, "%lx %m[^\n]\n", &pc, &mapping);
- if (ret != 2)
+ ret = fscanf(fp, "%lx %lx %m[^\n]\n",
+      &pc, &vm_start, &mapping);
+ if (ret != 3)
  goto bad_input;

  mapping = resolve_path(mapping);
@@ -405,6 +408,7 @@
  * usually works fine...
  */
  p->backtrace[p->depth].pc = pc - 1;
+ p->backtrace[p->depth].vm_start = vm_start;
  p->backtrace[p->depth].mapping = m;
  p->backtrace[p->depth].where = &undefined_location;
  p->depth++;
@@ -439,13 +443,15 @@

 static void resolve_spots(void)
 {
- char *a2l, *a2lcmd, *s, buf[BUFSIZ];
+ char *a2l, *a2lcmd, *relfcmd, *s, buf[BUFSIZ];
+ int ch;
  struct relax_spot *p;
  struct backtrace *b;
  struct location *l;
  struct mapping *m;
  struct stat sbuf;
- int ret, depth;
+ int ret, depth, use_offset;
+ unsigned long pc;
  FILE *fp;

  /*
@@ -472,6 +478,7 @@
  goto no_mem;

  l->pc = b->pc;
+ l->vm_start = b->vm_start;
  l->function = NULL;
  l->file = NULL;
  l->lineno = 0;
@@ -493,13 +500,25 @@
  if (ret || !S_ISREG(sbuf.st_mode))
  continue;

+ ret = asprintf(&relfcmd,
+        "%sreadelf -h %s | grep -q \"Type:\\s*DYN\"",
+        toolchain_prefix, m->name);
+ fp = popen(relfcmd, "r");
+ if (fp == NULL)
+ error(1, errno, "cannot run %s", relfcmd);
+
+ use_offset =  !pclose(fp);
+ free(relfcmd);
+
  ret = asprintf(&a2l,
-        "%saddr2line --demangle --inlines --functions --exe=%s",
+        "%saddr2line --addresses --demangle --inlines --functions --exe=%s",
         toolchain_prefix, m->name);
  if (ret < 0)
  goto no_mem;

  for (l = m->locs, s = a2l, a2lcmd = NULL; l; l = l->next) {
+ if (use_offset)
+ l->pc -= l->vm_start;
  ret = asprintf(&a2lcmd, "%s 0x%lx", s, l->pc);
  if (ret < 0)
  goto no_mem;
@@ -512,24 +531,36 @@
  error(1, errno, "cannot run %s", a2lcmd);

  for (l = m->locs; l; l = l->next) {
- ret = fscanf(fp, "%ms\n", &l->function);
+ ret = fscanf(fp, "%lx\n", &pc);
  if (ret != 1)
  goto bad_output;
  /*
- * Don't trust fscanf range specifier, we may
- * have colons in the pathname.
+ * Loop until we get a new address.
  */
- s = fgets(buf, sizeof(buf), fp);
- if (s == NULL)
- goto bad_output;
- s = strrchr(s, ':');
- if (s == NULL)
- continue;
- *s++ = '\0';
- if (strcmp(buf, "??")) {
- l->lineno = atoi(s);
- l->file = strdup(buf);
+ while ((ch = fgetc(fp)) != '0' && ch != EOF) {
+ ungetc(ch, fp);
+ ret = fscanf(fp, "%ms\n", &l->function);
+ if (ret != 1) {
+ goto bad_output;
+ }
+ /*
+ * Don't trust fscanf range specifier, we may
+ * have colons in the pathname.
+ */
+ s = fgets(buf, sizeof(buf), fp);
+ if (s == NULL)
+ goto bad_output;
+ s = strrchr(s, ':');
+ if (s == NULL)
+ continue;
+ *s++ = '\0';
+ if (strcmp(buf, "??")) {
+ l->lineno = atoi(s);
+ l->file = strdup(buf);
+ }
  }
+ if (ch == '0')
+ ungetc(ch, fp);
  }

  pclose(fp);

/\/\/\/\/\ End Patch /\/\/\/\/\
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix_slackspot.patch
Type: application/octet-stream
Size: 5293 bytes
Desc: not available
URL: <http://xenomai.org/pipermail/xenomai/attachments/20190321/541bd5d0/attachment.obj>


More information about the Xenomai mailing list