From: Frank Ch. Eigler <fche@elastic.org>
Date: Wed, 29 Sep 2010 16:53:37 +0000 (-0400)
Subject: PR10812: undo +20 build-id address offsetting
X-Git-Tag: release-1.4~167
X-Git-Url: https://sourceware.org/git/?a=commitdiff_plain;h=7712327519f2c24e779e42a54315ad50f5db8ba7;p=systemtap.git

PR10812: undo +20 build-id address offsetting

With the new relocation code, this does occasionally cause addresses
to spill over into the next section, leading to unexpected relocation
bases.

* runtime/sym.c (_stp_module_check): Remove the build_id_len
  subtraction.  Provide diagnostics on unexpected addresses/names.
* translate.cxx (dump_unwindsyms): Likewise.
---

diff --git a/runtime/sym.c b/runtime/sym.c
index 895ec8c4d..3471f6320 100644
--- a/runtime/sym.c
+++ b/runtime/sym.c
@@ -267,13 +267,11 @@ static int _stp_module_check(void)
 			  base_addr = m->notes_sect;
 		    }
 
-		    /* build-id note payload start address */
-                    /* XXX: But see https://bugzilla.redhat.com/show_bug.cgi?id=465872;
-                       dwfl_module_build_id was not intended to return the end address. */
-		    notes_addr -= m->build_id_len;
-
-		    if (notes_addr <= base_addr)  /* shouldn't happen */
-			 continue;
+		    if (notes_addr <= base_addr) { /* shouldn't happen */
+                            _stp_warn ("build-id address %lx < base %lx\n", 
+                                       notes_addr, base_addr);
+                            continue;
+                    }
                     for (j=0; j<m->build_id_len; j++) {
                             /* Use set_fs / get_user to access
                              conceivably invalid addresses.  If
diff --git a/translate.cxx b/translate.cxx
index cfdec5bc3..39ea5abd1 100644
--- a/translate.cxx
+++ b/translate.cxx
@@ -1,6 +1,7 @@
 // translation pass
 // Copyright (C) 2005-2010 Red Hat Inc.
 // Copyright (C) 2005-2008 Intel Corporation.
+// Copyright (C) 2010 Novell Corporation.
 //
 // This file is part of systemtap, and is free software.  You can
 // redistribute it and/or modify it under the terms of the GNU General
@@ -4895,9 +4896,9 @@ dump_unwindsyms (Dwfl_Module *m,
     // Enable workaround for elfutils dwfl bug.
     // see https://bugzilla.redhat.com/show_bug.cgi?id=465872
     // and http://sourceware.org/ml/systemtap/2008-q4/msg00579.html
-#if _ELFUTILS_PREREQ(0,138)
-    // Let's standardize to the buggy "end of build-id bits" behavior.
-    build_id_vaddr += build_id_len;
+#if !_ELFUTILS_PREREQ(0,138)
+    // Let's standardize to the new "start of build-id bits" behavior.
+    build_id_vaddr -= build_id_len;
 #endif
 
     // And check for another workaround needed.
@@ -4912,26 +4913,30 @@ dump_unwindsyms (Dwfl_Module *m,
       }
 #endif
 
-    if (modname != "kernel") {
-    	Dwarf_Addr reloc_vaddr = build_id_vaddr;
-    	const char *secname;
-    	int i;
+    if (modname != "kernel")
+      {
+        Dwarf_Addr reloc_vaddr = build_id_vaddr;
+        const char *secname;
+        int i;
 
-      	i = dwfl_module_relocate_address (m, &reloc_vaddr);
-      	dwfl_assert ("dwfl_module_relocate_address", i >= 0);
+        i = dwfl_module_relocate_address (m, &reloc_vaddr);
+        dwfl_assert ("dwfl_module_relocate_address", i >= 0);
 
-      	secname = dwfl_module_relocation_info (m, i, NULL);
-	dwfl_assert ("dwfl_module_relocation_info", 
-		strcmp(secname, ".note.gnu.build-id") == 0);
+        secname = dwfl_module_relocation_info (m, i, NULL);
 
-	build_id_vaddr = reloc_vaddr;
-    }
+        // assert same section name as in runtime/transport/symbols.c
+        if (!secname || strcmp(secname, ".note.gnu.build-id"))
+          throw semantic_error ("unexpected build-id reloc section " +
+                                string(secname ?: "null"));
+
+        build_id_vaddr = reloc_vaddr;
+      }
 
     if (c->session.verbose > 1)
       {
         clog << "Found build-id in " << name
              << ", length " << build_id_len;
-        clog << ", end at 0x" << hex << build_id_vaddr
+        clog << ", start at 0x" << hex << build_id_vaddr
              << dec << endl;
       }
   }