[dsymutil] Fix double relocation of DW_AT_call_return_pc
authorJonas Devlieghere <jonas@devlieghere.com>
Fri, 14 Feb 2020 01:39:29 +0000 (17:39 -0800)
committerJonas Devlieghere <jonas@devlieghere.com>
Fri, 14 Feb 2020 01:42:48 +0000 (17:42 -0800)
When the DW_AT_call_return_pc matches a relocation, the call return pc
would get relocated twice, once because of the relocation in the object
file and once because of dsymutil. The same problem exists for the low
and high PC and the fix is the same. We remember the low, high and
return pc of the original DIE and relocate that, rather than the
potentially already relocated value.

Reviewed offline by Fred Riss.

llvm/include/llvm/DWARFLinker/DWARFLinker.h
llvm/lib/DWARFLinker/DWARFLinker.cpp
llvm/test/tools/dsymutil/Inputs/private/tmp/call_return_pc/call [new file with mode: 0755]
llvm/test/tools/dsymutil/Inputs/private/tmp/call_return_pc/call.o [new file with mode: 0644]
llvm/test/tools/dsymutil/X86/call-site-entry-reloc.test [new file with mode: 0644]

index 5a8d081..31e8bcc 100644 (file)
@@ -582,6 +582,9 @@ private:
       /// Value of AT_high_pc in the input DIE
       uint64_t OrigHighPc = 0;
 
+      /// Value of DW_AT_call_return_pc in the input DIE
+      uint64_t OrigCallReturnPc = 0;
+
       /// Offset to apply to PC addresses inside a function.
       int64_t PCOffset = 0;
 
index adcbebf..03919c8 100644 (file)
@@ -1017,7 +1017,8 @@ unsigned DWARFLinker::DIECloner::cloneAddressAttribute(
   } else if (AttrSpec.Attr == dwarf::DW_AT_call_return_pc) {
     // Relocate a return PC address within a call site entry.
     if (Die.getTag() == dwarf::DW_TAG_call_site)
-      Addr += Info.PCOffset;
+      Addr = (Info.OrigCallReturnPc ? Info.OrigCallReturnPc : Addr) +
+             Info.PCOffset;
   }
 
   Die.addValue(DIEAlloc, static_cast<dwarf::Attribute>(AttrSpec.Attr),
@@ -1280,6 +1281,8 @@ DIE *DWARFLinker::DIECloner::cloneDIE(
     // inlining function.
     AttrInfo.OrigLowPc = dwarf::toAddress(InputDIE.find(dwarf::DW_AT_low_pc),
                                           std::numeric_limits<uint64_t>::max());
+    AttrInfo.OrigCallReturnPc =
+        dwarf::toAddress(InputDIE.find(dwarf::DW_AT_call_return_pc), 0);
   }
 
   // Reset the Offset to 0 as we will be working on the local copy of
diff --git a/llvm/test/tools/dsymutil/Inputs/private/tmp/call_return_pc/call b/llvm/test/tools/dsymutil/Inputs/private/tmp/call_return_pc/call
new file mode 100755 (executable)
index 0000000..d7587f9
Binary files /dev/null and b/llvm/test/tools/dsymutil/Inputs/private/tmp/call_return_pc/call differ
diff --git a/llvm/test/tools/dsymutil/Inputs/private/tmp/call_return_pc/call.o b/llvm/test/tools/dsymutil/Inputs/private/tmp/call_return_pc/call.o
new file mode 100644 (file)
index 0000000..df698e8
Binary files /dev/null and b/llvm/test/tools/dsymutil/Inputs/private/tmp/call_return_pc/call.o differ
diff --git a/llvm/test/tools/dsymutil/X86/call-site-entry-reloc.test b/llvm/test/tools/dsymutil/X86/call-site-entry-reloc.test
new file mode 100644 (file)
index 0000000..46e9470
--- /dev/null
@@ -0,0 +1,26 @@
+Test binaries created with the following commands:
+
+$ cat call.c
+__attribute__((noinline, noreturn)) void foo() {
+    asm volatile("" ::: "memory");
+      __builtin_unreachable();
+}
+__attribute__((noinline)) void bar() {
+    asm volatile("nop" :::);
+      foo();
+}
+
+int main() { bar(); }
+
+$ clang -g call.c -fomit-frame-pointer -c -Os -o call.o
+$ clang -g call.o -o call
+
+The test requires the return PC to match a relocation (in this case the
+DW_AT_high_pc of main). Without this change the value would get relocated
+twice.
+
+RUN: dsymutil -oso-prepend-path %p/../Inputs %p/../Inputs/private/tmp/call_return_pc/call -o %t.dSYM
+RUN: llvm-dwarfdump %t.dSYM | FileCheck %s -implicit-check-not=DW_AT_call_return_pc
+
+CHECK: DW_AT_call_return_pc  (0x0000000100000f72)
+CHECK: DW_AT_call_return_pc  (0x0000000100000f78)