rfc patch for buildid < shlib base address

rfc patch for buildid < shlib base address

[Frank Ch. Eigler]

Hi -

On my i686 F10 box, elfutils 0.140 probing /lib/libc-2.9.so, the
buildid data logic results in an address that is smaller than the dwfl
relocation base address for the module.  readelf indicates the
build-id .note section well before .text.  This causes a negative
offset, which in turn causes a pass-4 compile error.

The following patch papers over the issue by making this particular
offset a signed quantity.  I'd appreciate mjw/roland sanity checking,
and someone confirming that we actually check buildids of userspace
modules.

- FChE

diff --git a/runtime/sym.h b/runtime/sym.h
index e642cab..169f9d3 100644
--- a/runtime/sym.h
+++ b/runtime/sym.h
@@ -47,7 +47,7 @@ struct _stp_module {
 	uint32_t unwind_is_ehframe; /* unwind data comes from .eh_frame */
 	/* build-id information */
 	unsigned char *build_id_bits;
-	unsigned long  build_id_offset;
+	long     long  build_id_offset;
 	unsigned long  notes_sect;
 	int build_id_len;
 };
diff --git a/translate.cxx b/translate.cxx
index 135830d..6946758 100644
--- a/translate.cxx
+++ b/translate.cxx
@@ -4713,12 +4713,11 @@ dump_unwindsyms (Dwfl_Module *m,
        correct either.  We may instead need a relocation basis different
        from _stext, such as __start_notes.  */
     if (modname == "kernel")
-      c->output << ".build_id_offset = 0x" << hex << build_id_vaddr
-                << dec << ",\n";
+      c->output << ".build_id_offset = " << build_id_vaddr << ",\n";
     else
-      c->output << ".build_id_offset = 0x" << hex
-                << build_id_vaddr - base
-                << dec << ",\n";
+      c->output << ".build_id_offset = "
+                << (signed long long)(build_id_vaddr - base)
+                << ",\n";
   } else
     c->output << ".build_id_len = 0,\n";
   

Re: rfc patch for buildid < shlib base address

[Roland McGrath]

> On my i686 F10 box, elfutils 0.140 probing /lib/libc-2.9.so, the
> buildid data logic results in an address that is smaller than the dwfl
> relocation base address for the module.  readelf indicates the
> build-id .note section well before .text.  This causes a negative
> offset, which in turn causes a pass-4 compile error.

It is normal that the build ID note appear earlier than the .text section.
What is not normal is that you should care about the .text section's
location at all.  For any non-ET_REL module, the sole relocation base
should correspond to the beginning of its earliest PT_LOAD.  (In fact,
libdwfl doesn't really care if no section info was retained at all in the
stripped file; canonical tools don't strip that way, but they could.)  
This is by definition an address lower than where the note (or anything
else) appears.


Thanks,
Roland

Re: rfc patch for buildid < shlib base address

[Frank Ch. Eigler]

Hi -

On Tue, Feb 24, 2009 at 05:55:25PM -0800, Roland McGrath wrote:
> > On my i686 F10 box, elfutils 0.140 probing /lib/libc-2.9.so, the
> > buildid data logic results in an address that is smaller than the dwfl
> > relocation base address for the module.  readelf indicates the
> > build-id .note section well before .text.  [...]
> 
> It is normal that the build ID note appear earlier than the .text section.
> What is not normal is that you should care about the .text section's
> location at all.  For any non-ET_REL module, the sole relocation base
> should correspond to the beginning of its earliest PT_LOAD.

Yes, but it seems like elfutils 0.140 on this machine does not do that.

./stap -p4 --vp 033 -e 'probe process("/lib/libc-2.9.so").function("atoi").call {}' -w | grep 0x
parsed 'atoi' -> func 'atoi'
focused on module '/lib/libc-2.9.so = [0x7e73000-0x7fe6650, bias 0x0] file /usr/lib/debug/lib/libc-2.9.so.debug ELF machine i?86 (code 3)
[...]
dump_unwindsyms /lib/libc-2.9.so index=0 base=0x7e73000
Found build-id in /lib/libc-2.9.so, length 20, end at 0x7e63198


- FChE

Re: rfc patch for buildid < shlib base address

[Roland McGrath]

> Yes, but it seems like elfutils 0.140 on this machine does not do that.
> 
> ./stap -p4 --vp 033 -e 'probe process("/lib/libc-2.9.so").function("atoi").call {}' -w | grep 0x
> parsed 'atoi' -> func 'atoi'
> focused on module '/lib/libc-2.9.so = [0x7e73000-0x7fe6650, bias 0x0] file /usr/lib/debug/lib/libc-2.9.so.debug ELF machine i?86 (code 3)
> [...]
> dump_unwindsyms /lib/libc-2.9.so index=0 base=0x7e73000
> Found build-id in /lib/libc-2.9.so, length 20, end at 0x7e63198

I am looking at glibc-2.9-3.i686.  Mine is prelinked to a different spot
than the spot yours uses:

eu-unstrip -n -e /lib/libc-2.9.so 
0x13e000+0x173650 08d986453ccaf35f5cb4e94b5fa5507c32232e2e@0x13e184 /lib/libc-2.9.so - 
eu-readelf -lS /lib/libc-2.9.so 
[...]
[ 1] .note.gnu.build-id   NOTE         0013e174 000174 000024  0 A      0   0  4
[...]
  LOAD           0x000000 0x0013e000 0x0013e000 0x16db78 0x16db78 R E 0x1000
[...]
eu-unstrip -n -p $!
0x119000+0x23000 62a3ad35b38bb89ea69c019877e962042525ad9e@0x119124 /lib/ld-2.9.so - /lib/ld-2.9.so
0x13e000+0x171000 08d986453ccaf35f5cb4e94b5fa5507c32232e2e@0x13e184 /lib/libc-2.9.so - /lib/libc-2.9.so
[...]

(That $! being the PID of some 32-bit process, e.g $$ will do if the shell
binary is actually i386.)

If those eu-* tools make all those numbers come out right given your binary
version and its prelinking state (as shown by eu-readelf), then something
funny is going on that only stap sees.


Thanks,
Roland

Re: rfc patch for buildid < shlib base address

[Mark Wielaard]

Hi,

On Tue, 2009-02-24 at 18:38 -0800, Roland McGrath wrote:
> > Yes, but it seems like elfutils 0.140 on this machine does not do that.
> [...]
> If those eu-* tools make all those numbers come out right given your binary
> version and its prelinking state (as shown by eu-readelf), then something
> funny is going on that only stap sees.

It seems that at least on my system the numbers come out right, and stap
sees the same thing. As far as I can see translate.cxx (dump_unwindsyms)
does the right thing (although it should really be using
dwfl_module_relocate_address() instead of adjusting the absolute address
directly, except that would not work with elfutils < 0.138).

Cheers,

Mark

Re: rfc patch for buildid < shlib base address

[Mark Wielaard]

Hi,

On Wed, 2009-02-25 at 13:52 +0100, Mark Wielaard wrote: 
> On Tue, 2009-02-24 at 18:38 -0800, Roland McGrath wrote:
> > > Yes, but it seems like elfutils 0.140 on this machine does not do that.
> > [...]
> > If those eu-* tools make all those numbers come out right given your binary
> > version and its prelinking state (as shown by eu-readelf), then something
> > funny is going on that only stap sees.
> 
> It seems that at least on my system the numbers come out right, and stap
> sees the same thing. As far as I can see translate.cxx (dump_unwindsyms)
> does the right thing (although it should really be using
> dwfl_module_relocate_address() instead of adjusting the absolute address
> directly, except that would not work with elfutils < 0.138).

I am now also seeing this. Double checking libc seems everything is OK,
prelinked at 0x126000.

$ eu-unstrip -n -e /lib/libc-2.9.so 
0x126000+0x173650 08d986453ccaf35f5cb4e94b5fa5507c32232e2e@0x126184 /lib/libc-2.9.so /usr/lib/debug/lib/libc-2.9.so.debug 

$ eu-readelf -lS /lib/libc-2.9.so 
[ 1] .note.gnu.build-id   NOTE         00126174 000174 000024  0 A      0   0  4
[...]
  LOAD           0x000000 0x00126000 0x00126000 0x16db78 0x16db78 R E 0x1000

$ eu-unstrip -n -p $$
0x126000+0x171000 08d986453ccaf35f5cb4e94b5fa5507c32232e2e@0x126184 /lib/libc-2.9.so /usr/lib/debug/lib/libc-2.9.so.debug /lib/libc-2.9.so

But in our dwfl_getmodules() callback for /lib/libc-2.9.so we get
base=0x136000. While dwfl_module_build_id() gives a addr=0x126184.
So, when we then either call dwfl_module_relocate_address() or manually
substract base we end up with a negative address.

So, the address returned by dwfl_module_build_id() looks correct if
taken at the original prelinked address. But mod->main.bias is not equal
to that address, so there is no easy way to turn this into an relative
address that we need.

This is with elfutils 0.140.

Cheers,

Mark

Re: rfc patch for buildid < shlib base address

[Mark Wielaard]

On Sun, 2009-03-15 at 23:31 +0100, Mark Wielaard wrote:
> So, the address returned by dwfl_module_build_id() looks correct if
> taken at the original prelinked address. But mod->main.bias is not equal
> to that address, so there is no easy way to turn this into an relative
> address that we need.

I meant mod->low_addr here, which is also equal to the base passed to
our dwfl_getmodules() callback.

Cheers,

Mark

Re: rfc patch for buildid < shlib base address

[Mark Wielaard]

On Sun, 2009-03-15 at 23:45 +0100, Mark Wielaard wrote:
> On Sun, 2009-03-15 at 23:31 +0100, Mark Wielaard wrote:
> > So, the address returned by dwfl_module_build_id() looks correct if
> > taken at the original prelinked address. But mod->main.bias is not equal
> > to that address, so there is no easy way to turn this into an relative
> > address that we need.
> 
> I meant mod->low_addr here, which is also equal to the base passed to
> our dwfl_getmodules() callback.

And digging deeper I see roland already fixed this issue a couple of
days ago: https://bugzilla.redhat.com/show_bug.cgi?id=489439

I'll look into adding the workaround described in that bug to our code
tomorrow.

Good night,

Mark

Re: rfc patch for buildid < shlib base address

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]

On Sun, 2009-03-15 at 23:54 +0100, Mark Wielaard wrote:
> And digging deeper I see roland already fixed this issue a couple of
> days ago: https://bugzilla.redhat.com/show_bug.cgi?id=489439
> 
> I'll look into adding the workaround described in that bug to our code
> tomorrow.

Added as:

commit 1bb61ae1fd02086560f5cd019db320b5a217ba05
Author: Mark Wielaard <mjw@redhat.com>
Date:   Mon Mar 16 14:19:20 2009 +0100

Add workaround for dwfl_module_build_id bug with elfutils < 0.140.
    
* translate.cxx (dump_unwindsyms): Check elfutils version and whether
  build_id_vaddr < base, and if so add main_bias to address.

Note that it checks both for !_ELFUTILS_PREREQ(0,141) and for
build_id_vaddr < base because it looks like the buggy behavior doesn't
occur always with older elfutils versions and if it doesn't occur, then
adding main.bias is wrong. I assume that if the bug occurs then
build_id_vaddr will always be smaller than main.bias since it is
supposed to go as close as possible to the start of the elf file.

Cheers,

Mark

[-- Attachment #2: translate-build_id.patch --]
[-- Type: text/x-patch, Size: 1608 bytes --]

diff --git a/translate.cxx b/translate.cxx
index 17c37dc..f4c2853 100644
--- a/translate.cxx
+++ b/translate.cxx
@@ -4497,17 +4497,35 @@ dump_unwindsyms (Dwfl_Module *m,
     // see https://bugzilla.redhat.com/show_bug.cgi?id=465872
     // and http://sourceware.org/ml/systemtap/2008-q4/msg00579.html
 #ifdef _ELFUTILS_PREREQ
-#if _ELFUTILS_PREREQ(0,138)
+  #if _ELFUTILS_PREREQ(0,138)
     // Let's standardize to the buggy "end of build-id bits" behavior. 
     build_id_vaddr += build_id_len;
+  #endif
+  #if !_ELFUTILS_PREREQ(0,141)
+    #define NEED_ELFUTILS_BUILDID_WORKAROUND
+  #endif
+#else
+  #define NEED_ELFUTILS_BUILDID_WORKAROUND
 #endif
+
+    // And check for another workaround needed.
+    // see https://bugzilla.redhat.com/show_bug.cgi?id=489439
+    // and http://sourceware.org/ml/systemtap/2009-q1/msg00513.html
+#ifdef NEED_ELFUTILS_BUILDID_WORKAROUND
+    if (build_id_vaddr < base && dwfl_module_relocations (m) == 1)
+      {
+        GElf_Addr main_bias;
+        dwfl_module_getelf (m, &main_bias);
+        build_id_vaddr += main_bias;
+      }
 #endif
-        if (c->session.verbose > 1) {
-           clog << "Found build-id in " << name
-                << ", length " << build_id_len;
-           clog << ", end at 0x" << hex << build_id_vaddr
-                << dec << endl;
-        }
+    if (c->session.verbose > 1)
+      {
+        clog << "Found build-id in " << name
+             << ", length " << build_id_len;
+        clog << ", end at 0x" << hex << build_id_vaddr
+             << dec << endl;
+      }
   }
 
   // Look up the relocation basis for symbols