Change calling convention of dwarf_getmacros to allow opcode 0xff
authorPetr Machata <pmachata@redhat.com>
Tue, 2 Dec 2014 20:22:14 +0000 (21:22 +0100)
committerPetr Machata <pmachata@redhat.com>
Wed, 10 Dec 2014 15:10:34 +0000 (16:10 +0100)
We now require callers to pass DWARF_GETMACROS_START to start the
iteration.  0 is still accepted, but signals to libdw that the
iteration request comes from an old-style caller, and that opcode 0xff
should be rejected when iterating .debug_macro, to avoid confusion.

Signed-off-by: Petr Machata <pmachata@redhat.com>
NEWS
libdw/ChangeLog
libdw/dwarf_getmacros.c
libdw/libdw.h
tests/ChangeLog
tests/dwarf-getmacros.c
tests/run-dwarf-getmacros.sh
tests/testfile-macros-0xff.bz2 [new file with mode: 0755]
tests/testfile-macros-0xff.s [new file with mode: 0644]

diff --git a/NEWS b/NEWS
index 8f0780b..a7ead0a 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -8,8 +8,6 @@ libdw: New function dwarf_peel_type. dwarf_aggregate_size now uses
        dwarf_macro_getparamcnt, and dwarf_macro_param are available
        for more generalized inspection of macros and their parameters.
 
-XXX: Fix serving of 0xff based on what ends up in Dwarf 5.
-
 Version 0.160
 
 libdw: New functions dwarf_cu_getdwarf, dwarf_cu_die.
index bab02e5..909fdbc 100644 (file)
@@ -1,3 +1,10 @@
+2014-12-02  Petr Machata  <pmachata@redhat.com>
+
+       * dwarf_getmacros.c (token_from_offset, offset_from_token): New
+       helper functions.
+       (do_dwarf_getmacros_die): Merge into dwarf_getmacros.
+       * libdw.h (DWARF_GETMACROS_START): New macro.
+
 2014-11-27  Mark Wielaard  <mjw@redhat.com>
 
        * Makefile.am (libdw.so): Use textrel_check.
index 8d9d78b..bd64d60 100644 (file)
@@ -384,13 +384,57 @@ read_macros (Dwarf *dbg, int sec_index,
   return 0;
 }
 
+/* Token layout:
+
+   - The highest bit is used for distinguishing between callers that
+     know that opcode 0xff may have one of two incompatible meanings.
+     The mask that we use for selecting this bit is
+     DWARF_GETMACROS_START.
+
+   - The rest of the token (31 or 63 bits) encodes address inside the
+     macro unit.
+
+   Besides, token value of 0 signals end of iteration and -1 is
+   reserved for signaling errors.  That means it's impossible to
+   represent maximum offset of a .debug_macro unit to new-style
+   callers (which in practice decreases the permissible macro unit
+   size by another 1 byte).  */
+
+static ptrdiff_t
+token_from_offset (ptrdiff_t offset, bool accept_0xff)
+{
+  if (offset == -1 || offset == 0)
+    return offset;
+
+  /* Make sure the offset didn't overflow into the flag bit.  */
+  if ((offset & DWARF_GETMACROS_START) != 0)
+    {
+      __libdw_seterrno (DWARF_E_TOO_BIG);
+      return -1;
+    }
+
+  if (accept_0xff)
+    offset |= DWARF_GETMACROS_START;
+
+  return offset;
+}
+
+static ptrdiff_t
+offset_from_token (ptrdiff_t token, bool *accept_0xffp)
+{
+  *accept_0xffp = (token & DWARF_GETMACROS_START) != 0;
+  token &= ~DWARF_GETMACROS_START;
+
+  return token;
+}
+
 static ptrdiff_t
 gnu_macros_getmacros_off (Dwarf *dbg, Dwarf_Off macoff,
                          int (*callback) (Dwarf_Macro *, void *),
-                         void *arg, ptrdiff_t token, bool accept_0xff,
+                         void *arg, ptrdiff_t offset, bool accept_0xff,
                          Dwarf_Die *cudie)
 {
-  assert (token <= 0);
+  assert (offset >= 0);
 
   if (macoff >= dbg->sectiondata[IDX_debug_macro]->d_size)
     {
@@ -398,23 +442,19 @@ gnu_macros_getmacros_off (Dwarf *dbg, Dwarf_Off macoff,
       return -1;
     }
 
-  ptrdiff_t ret = read_macros (dbg, IDX_debug_macro, macoff,
-                              callback, arg, -token, accept_0xff, cudie);
-  if (ret == -1)
-    return -1;
-  else
-    return -ret;
+  return read_macros (dbg, IDX_debug_macro, macoff,
+                     callback, arg, offset, accept_0xff, cudie);
 }
 
 static ptrdiff_t
 macro_info_getmacros_off (Dwarf *dbg, Dwarf_Off macoff,
                          int (*callback) (Dwarf_Macro *, void *),
-                         void *arg, ptrdiff_t token, Dwarf_Die *cudie)
+                         void *arg, ptrdiff_t offset, Dwarf_Die *cudie)
 {
-  assert (token >= 0);
+  assert (offset >= 0);
 
   return read_macros (dbg, IDX_debug_macinfo, macoff,
-                     callback, arg, token, true, cudie);
+                     callback, arg, offset, true, cudie);
 }
 
 ptrdiff_t
@@ -428,21 +468,22 @@ dwarf_getmacros_off (Dwarf *dbg, Dwarf_Off macoff,
       return -1;
     }
 
-  /* We use token values > 0 for iteration through .debug_macinfo and
-     values < 0 for iteration through .debug_macro.  Return value of
-     -1 also signifies an error, but that's fine, because .debug_macro
-     always contains at least three bytes of headers and after
-     iterating one opcode, we should never see anything above -4.  */
-  assert (token <= 0);
+  bool accept_0xff;
+  ptrdiff_t offset = offset_from_token (token, &accept_0xff);
+  assert (accept_0xff);
 
-  return gnu_macros_getmacros_off (dbg, macoff, callback, arg, token, true,
-                                  NULL);
+  offset = gnu_macros_getmacros_off (dbg, macoff, callback, arg, offset,
+                                    accept_0xff, NULL);
+
+  return token_from_offset (offset, accept_0xff);
 }
 
-static ptrdiff_t
-do_dwarf_getmacros_die (Dwarf_Die *cudie, Dwarf_Off *macoffp,
-                       int (*callback) (Dwarf_Macro *, void *),
-                       void *arg, ptrdiff_t token, bool accept_0xff)
+ptrdiff_t
+dwarf_getmacros (cudie, callback, arg, token)
+     Dwarf_Die *cudie;
+     int (*callback) (Dwarf_Macro *, void *);
+     void *arg;
+     ptrdiff_t token;
 {
   if (cudie == NULL)
     {
@@ -450,50 +491,6 @@ do_dwarf_getmacros_die (Dwarf_Die *cudie, Dwarf_Off *macoffp,
       return -1;
     }
 
-  if (token > 0 && macoffp != NULL)
-    /* A continuation call from DW_AT_macro_info iteration, meaning
-       *MACOFF contains previously-cached offset.  */
-    return macro_info_getmacros_off (cudie->cu->dbg, *macoffp,
-                                    callback, arg, token, cudie);
-
-  /* A fresh start of DW_AT_macro_info iteration, or a continuation
-     thereof without a cache.  */
-  if (token > 0
-      || (token == 0 && dwarf_hasattr (cudie, DW_AT_macro_info)))
-    {
-      Dwarf_Word macoff;
-      if (macoffp == NULL)
-       macoffp = &macoff;
-      if (get_offset_from (cudie, DW_AT_macro_info, macoffp) != 0)
-       return -1;
-      return macro_info_getmacros_off (cudie->cu->dbg, *macoffp,
-                                      callback, arg, token, cudie);
-    }
-
-  if (token < 0 && macoffp != NULL)
-    /* A continuation call from DW_AT_GNU_macros iteration.  */
-    return gnu_macros_getmacros_off (cudie->cu->dbg, *macoffp,
-                                    callback, arg, token, accept_0xff,
-                                    cudie);
-
-  /* Likewise without cache, or iteration start.  */
-  Dwarf_Word macoff;
-  if (macoffp == NULL)
-    macoffp = &macoff;
-  if (get_offset_from (cudie, DW_AT_GNU_macros, macoffp) != 0)
-    return -1;
-  return gnu_macros_getmacros_off (cudie->cu->dbg, *macoffp,
-                                  callback, arg, token, accept_0xff,
-                                  cudie);
-}
-
-ptrdiff_t
-dwarf_getmacros (die, callback, arg, offset)
-     Dwarf_Die *die;
-     int (*callback) (Dwarf_Macro *, void *);
-     void *arg;
-     ptrdiff_t offset;
-{
   /* This function might be called from a code that expects to see
      DW_MACINFO_* opcodes, not DW_MACRO_{GNU_,}* ones.  It is fine to
      serve most DW_MACRO_{GNU_,}* opcodes to such code, because those
@@ -511,7 +508,33 @@ dwarf_getmacros (die, callback, arg, offset)
      some small probability that the two opcodes would look
      superficially similar enough that a client would be confused and
      misbehave as a result.  For this reason, we refuse to serve
-     through this interface 0xff's originating from .debug_macro.  */
+     through this interface 0xff's originating from .debug_macro
+     unless the TOKEN that we obtained indicates the call originates
+     from a new-style caller.  See above for details on what
+     information is encoded into tokens.  */
+
+  bool accept_0xff;
+  ptrdiff_t offset = offset_from_token (token, &accept_0xff);
+
+  /* DW_AT_macro_info */
+  if (dwarf_hasattr (cudie, DW_AT_macro_info))
+    {
+      Dwarf_Word macoff;
+      if (get_offset_from (cudie, DW_AT_macro_info, &macoff) != 0)
+       return -1;
+      offset = macro_info_getmacros_off (cudie->cu->dbg, macoff,
+                                        callback, arg, offset, cudie);
+    }
+  else
+    {
+      /* DW_AT_GNU_macros, DW_AT_macros */
+      Dwarf_Word macoff;
+      if (get_offset_from (cudie, DW_AT_GNU_macros, &macoff) != 0)
+       return -1;
+      offset = gnu_macros_getmacros_off (cudie->cu->dbg, macoff,
+                                        callback, arg, offset, accept_0xff,
+                                        cudie);
+    }
 
-  return do_dwarf_getmacros_die (die, NULL, callback, arg, offset, false);
+  return token_from_offset (offset, accept_0xff);
 }
index c0baa21..b2b2282 100644 (file)
@@ -32,6 +32,7 @@
 #include <gelf.h>
 #include <stdbool.h>
 #include <stddef.h>
+#include <stdint.h>
 
 
 #if __GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ >= 3)
@@ -845,22 +846,24 @@ extern int dwarf_entry_breakpoints (Dwarf_Die *die, Dwarf_Addr **bkpts);
 
 
 /* Iterate through the macro unit referenced by CUDIE and call
-   CALLBACK for each macro information entry.  Keeps iterating while
-   CALLBACK returns DWARF_CB_OK.  If the callback returns
-   DWARF_CB_ABORT, it stops iterating and returns a continuation
-   token, which can be used to restart the iteration at the point
-   where it ended.  A TOKEN of 0 starts the iteration.  Returns -1 for
-   errors or 0 if there are no more macro entries.
+   CALLBACK for each macro information entry.  To start the iteration,
+   one would pass DWARF_GETMACROS_START for TOKEN.
+
+   The iteration continues while CALLBACK returns DWARF_CB_OK.  If the
+   callback returns DWARF_CB_ABORT, the iteration stops and a
+   continuation token is returned, which can be used to restart the
+   iteration at the point where it ended.  Returns -1 for errors or 0
+   if there are no more macro entries.
 
    Note that the Dwarf_Macro pointer passed to the callback is only
    valid for the duration of the callback invocation.
 
-   Note that this interface will refuse to serve opcode 0xff from
-   .debug_macro sections.  Such opcode is considered invalid and will
-   cause dwarf_getmacros to return with error.  Note that this should
-   be no limitation as of now, as DW_MACRO_GNU_* domain doesn't
-   allocate 0xff.  It is however a theoretical possibility with future
-   Dwarf standards.  */
+   For backward compatibility, a token of 0 is accepted for starting
+   the iteration as well, but in that case this interface will refuse
+   to serve opcode 0xff from .debug_macro sections.  Such opcode would
+   be considered invalid and would cause dwarf_getmacros to return
+   with error.  */
+#define DWARF_GETMACROS_START PTRDIFF_MIN
 extern ptrdiff_t dwarf_getmacros (Dwarf_Die *cudie,
                                  int (*callback) (Dwarf_Macro *, void *),
                                  void *arg, ptrdiff_t token)
@@ -871,13 +874,16 @@ extern ptrdiff_t dwarf_getmacros (Dwarf_Die *cudie,
    iterates .debug_macro.  This can be used for handling
    DW_MACRO_GNU_transparent_include's or similar opcodes.
 
+   TOKEN value of DWARF_GETMACROS_START can be used to start the
+   iteration.
+
    It is not appropriate to obtain macro unit offset by hand from a CU
    DIE and then request iteration through this interface.  The reason
    for this is that if a dwarf_macro_getsrcfiles is later called,
    there would be no way to figure out what DW_AT_comp_dir was present
    on the CU DIE, and file names referenced in either the macro unit
    itself, or the .debug_line unit that it references, might be wrong.
-   Use dwarf_getmacro.  */
+   Use dwarf_getmacros.  */
 extern ptrdiff_t dwarf_getmacros_off (Dwarf *dbg, Dwarf_Off macoff,
                                      int (*callback) (Dwarf_Macro *, void *),
                                      void *arg, ptrdiff_t token)
@@ -895,9 +901,11 @@ extern int dwarf_macro_getsrcfiles (Dwarf *dbg, Dwarf_Macro *macro,
 /* Return macro opcode.  That's a constant that can be either from
    DW_MACINFO_* domain or DW_MACRO_GNU_* domain.  The two domains have
    compatible values, so it's OK to use either of them for
-   comparisons.  The only differences is 0xff, which currently is
-   never served from .debug_macro, and can thus be safely assumed to
-   mean DW_MACINFO_vendor_ext.  */
+   comparisons.  The only differences is 0xff, which could be either
+   DW_MACINFO_vendor_ext or a vendor-defined DW_MACRO_* constant.  One
+   would need to look if the CU DIE which the iteration was requested
+   for has attribute DW_AT_macro_info, or either of DW_AT_GNU_macros
+   or DW_AT_macros to differentiate the two interpretations.  */
 extern int dwarf_macro_opcode (Dwarf_Macro *macro, unsigned int *opcodep)
      __nonnull_attribute__ (2);
 
index 87a453d..c22ed8a 100644 (file)
@@ -1,3 +1,13 @@
+2014-12-02  Petr Machata  <pmachata@redhat.com>
+
+       * dwarf-getmacros.c (mac): Skip over DW_MACINFO_undef,
+       DW_MACRO_GNU_undef_indirect opcodes.  Add a default branch.
+       (main): Initialize off to DWARF_GETMACROS_START when an extra
+       command line argument is passed.
+       * testfile-macros-0xff.bz2: New test case.
+       * testfile-macros-0xff.s: New file (source for the above).
+       * run-dwarf-getmacros.sh: Add two tests.
+
 2014-11-27  Mark Wielaard  <mjw@redhat.com>
 
        * vdsosyms.c (main): Call dwfl_linux_proc_attach.
index f203d5b..92e093c 100644 (file)
@@ -34,9 +34,9 @@ mac (Dwarf_Macro *macro, void *dbg)
 {
   static int level = 0;
 
-  switch (({ unsigned int opcode;
-            dwarf_macro_opcode (macro, &opcode);
-            opcode; }))
+  unsigned int opcode;
+  dwarf_macro_opcode (macro, &opcode);
+  switch (opcode)
     {
     case DW_MACRO_GNU_transparent_include:
       {
@@ -50,7 +50,7 @@ mac (Dwarf_Macro *macro, void *dbg)
 
        printf ("%*sinclude %#" PRIx64 "\n", level, "", w);
        ++level;
-       include (dbg, w, 0);
+       include (dbg, w, DWARF_GETMACROS_START);
        --level;
        printf ("%*s/include\n", level, "");
        break;
@@ -88,6 +88,19 @@ mac (Dwarf_Macro *macro, void *dbg)
        printf ("%*s%s\n", level, "", value);
        break;
       }
+
+    case DW_MACINFO_undef:
+    case DW_MACRO_GNU_undef_indirect:
+      break;
+
+    default:
+      {
+       size_t paramcnt;
+       dwarf_macro_getparamcnt (macro, &paramcnt);
+       printf ("%*sopcode %u with %zd arguments\n",
+               level, "", opcode, paramcnt);
+       break;
+      }
     }
 
   return DWARF_CB_ABORT;
@@ -105,17 +118,19 @@ include (Dwarf *dbg, Dwarf_Off macoff, ptrdiff_t token)
 }
 
 int
-main (int argc __attribute__ ((unused)), char *argv[])
+main (int argc, char *argv[])
 {
+  assert (argc >= 3);
   const char *name = argv[1];
   ptrdiff_t cuoff = strtol (argv[2], NULL, 0);
+  bool new_style = argc > 3;
 
   int fd = open (name, O_RDONLY);
   Dwarf *dbg = dwarf_begin (fd, DWARF_C_READ);
 
   Dwarf_Die cudie_mem, *cudie = dwarf_offdie (dbg, cuoff, &cudie_mem);
 
-  for (ptrdiff_t off = 0;
+  for (ptrdiff_t off = new_style ? DWARF_GETMACROS_START : 0;
        (off = dwarf_getmacros (cudie, mac, dbg, off)); )
     if (off == -1)
       {
index 8f7c4a8..0a488fa 100755 (executable)
@@ -696,4 +696,15 @@ file /home/mark/src/tests/macro.c
 /file
 EOF
 
+testfiles testfile-macros-0xff
+testrun_compare ${abs_builddir}/dwarf-getmacros testfile-macros-0xff 0xb <<\EOF
+invalid opcode
+EOF
+testrun_compare ${abs_builddir}/dwarf-getmacros testfile-macros-0xff 0xb '' <<\EOF
+opcode 255 with 0 arguments
+file /home/petr/proj/elfutils/master/elfutils/x.c
+ FOO 0
+/file
+EOF
+
 exit 0
diff --git a/tests/testfile-macros-0xff.bz2 b/tests/testfile-macros-0xff.bz2
new file mode 100755 (executable)
index 0000000..a19662a
Binary files /dev/null and b/tests/testfile-macros-0xff.bz2 differ
diff --git a/tests/testfile-macros-0xff.s b/tests/testfile-macros-0xff.s
new file mode 100644 (file)
index 0000000..7fdd35c
--- /dev/null
@@ -0,0 +1,153 @@
+       .file   "x.c"
+       .text
+.Ltext0:
+       .globl  main
+       .type   main, @function
+main:
+.LFB0:
+       .file 1 "x.c"
+       .loc 1 3 0
+       .cfi_startproc
+       pushq   %rbp
+       .cfi_def_cfa_offset 16
+       .cfi_offset 6, -16
+       movq    %rsp, %rbp
+       .cfi_def_cfa_register 6
+       .loc 1 3 0
+       movl    $0, %eax
+       popq    %rbp
+       .cfi_def_cfa 7, 8
+       ret
+       .cfi_endproc
+.LFE0:
+       .size   main, .-main
+.Letext0:
+       .section        .debug_info,"",@progbits
+.Ldebug_info0:
+       .long   0x52
+       .value  0x4
+       .long   .Ldebug_abbrev0
+       .byte   0x8
+       .uleb128 0x1
+       .long   .LASF244
+       .byte   0x4
+       .string "x.c"
+       .long   .LASF245
+       .quad   .Ltext0
+       .quad   .Letext0-.Ltext0
+       .long   .Ldebug_line0
+       .long   .Ldebug_macro0
+       .uleb128 0x2
+       .long   .LASF246
+       .byte   0x1
+       .byte   0x3
+       .long   0x4e
+       .quad   .LFB0
+       .quad   .LFE0-.LFB0
+       .uleb128 0x1
+       .byte   0x9c
+       .uleb128 0x3
+       .byte   0x4
+       .byte   0x5
+       .string "int"
+       .byte   0
+       .section        .debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+       .uleb128 0x1
+       .uleb128 0x11
+       .byte   0x1
+       .uleb128 0x25
+       .uleb128 0xe
+       .uleb128 0x13
+       .uleb128 0xb
+       .uleb128 0x3
+       .uleb128 0x8
+       .uleb128 0x1b
+       .uleb128 0xe
+       .uleb128 0x11
+       .uleb128 0x1
+       .uleb128 0x12
+       .uleb128 0x7
+       .uleb128 0x10
+       .uleb128 0x17
+       .uleb128 0x2119
+       .uleb128 0x17
+       .byte   0
+       .byte   0
+       .uleb128 0x2
+       .uleb128 0x2e
+       .byte   0
+       .uleb128 0x3f
+       .uleb128 0x19
+       .uleb128 0x3
+       .uleb128 0xe
+       .uleb128 0x3a
+       .uleb128 0xb
+       .uleb128 0x3b
+       .uleb128 0xb
+       .uleb128 0x49
+       .uleb128 0x13
+       .uleb128 0x11
+       .uleb128 0x1
+       .uleb128 0x12
+       .uleb128 0x7
+       .uleb128 0x40
+       .uleb128 0x18
+       .uleb128 0x2117
+       .uleb128 0x19
+       .byte   0
+       .byte   0
+       .uleb128 0x3
+       .uleb128 0x24
+       .byte   0
+       .uleb128 0xb
+       .uleb128 0xb
+       .uleb128 0x3e
+       .uleb128 0xb
+       .uleb128 0x3
+       .uleb128 0x8
+       .byte   0
+       .byte   0
+       .byte   0
+       .section        .debug_aranges,"",@progbits
+       .long   0x2c
+       .value  0x2
+       .long   .Ldebug_info0
+       .byte   0x8
+       .byte   0
+       .value  0
+       .value  0
+       .quad   .Ltext0
+       .quad   .Letext0-.Ltext0
+       .quad   0
+       .quad   0
+       .section        .debug_macro,"",@progbits
+.Ldebug_macro0:
+       .value  0x4
+       .byte   0x6
+       .long   .Ldebug_line0
+       .byte   0x1
+       .byte   0xff
+       .uleb128 0
+       .byte   0xff
+       .byte   0x3
+       .uleb128 0
+       .uleb128 0x1
+       .byte   0x5
+       .uleb128 0x1
+       .long   .LASF243
+       .byte   0x4
+       .byte   0
+       .section        .debug_line,"",@progbits
+.Ldebug_line0:
+       .section        .debug_str,"MS",@progbits,1
+.LASF245:
+       .string "/home/petr/proj/elfutils/master/elfutils"
+.LASF244:
+       .string "GNU C++ 4.9.0 20140422 (Red Hat 4.9.0-1) -mtune=generic -march=x86-64 -g3"
+.LASF243:
+       .string "FOO 0"
+.LASF246:
+       .string "main"
+       .ident  "GCC: (GNU) 4.9.0 20140422 (Red Hat 4.9.0-1)"
+       .section        .note.GNU-stack,"",@progbits