<div dir="ltr">Possible to adapt the existing RelocVisitor code to do this?<br><br>-eric<br></div><br><div class="gmail_quote">On Fri, Mar 6, 2015 at 5:29 PM Frederic Riss <<a href="mailto:friss@apple.com">friss@apple.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: friss<br>
Date: Fri Mar  6 19:25:09 2015<br>
New Revision: 231544<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=231544&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project?rev=231544&view=rev</a><br>
Log:<br>
[dsymutil] Apply relocations to DIE data before cloning.<br>
<br>
Doing this gets function's low_pc and global variable's locations right<br>
in the output debug info. It also could get right other attributes<br>
that need to be relocated (in linker terms), but I don't know of any<br>
other than the address attributes.<br>
<br>
This doesn't fixup low_pc attributes in compile_unit, lexical_block<br>
or inlined subroutine, nor does it get right high_pc attributes<br>
for function. This will come in a subsequent commit.<br>
<br>
Modified:<br>
    llvm/trunk/test/tools/<u></u>dsymutil/X86/basic-linking-<u></u>x86.test<br>
    llvm/trunk/test/tools/<u></u>dsymutil/X86/basic-lto-<u></u>linking-x86.test<br>
    llvm/trunk/tools/dsymutil/<u></u>DwarfLinker.cpp<br>
<br>
Modified: llvm/trunk/test/tools/<u></u>dsymutil/X86/basic-linking-<u></u>x86.test<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/X86/basic-linking-x86.test?rev=231544&r1=231543&r2=231544&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/test/tools/<u></u>dsymutil/X86/basic-linking-<u></u>x86.test?rev=231544&r1=231543&<u></u>r2=231544&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/test/tools/<u></u>dsymutil/X86/basic-linking-<u></u>x86.test (original)<br>
+++ llvm/trunk/test/tools/<u></u>dsymutil/X86/basic-linking-<u></u>x86.test Fri Mar  6 19:25:09 2015<br>
@@ -3,8 +3,8 @@ RUN: llvm-dsymutil -oso-prepend-path=%p/<br>
 RUN: llvm-dwarfdump %t1.dwarf | FileCheck %s<br>
 RUN: llvm-dsymutil -o %t2 -oso-prepend-path=%p/.. %p/../Inputs/basic.macho.x86_<u></u>64<br>
 RUN: llvm-dwarfdump %t2 | FileCheck %s<br>
-RUN: llvm-dsymutil -o - -oso-prepend-path=%p/.. %p/../Inputs/basic.macho.x86_<u></u>64 | llvm-dwarfdump - | FileCheck %s<br>
-RUN: llvm-dsymutil -o - -oso-prepend-path=%p/.. %p/../Inputs/basic-archive.<u></u>macho.x86_64 | llvm-dwarfdump - | FileCheck %s<br>
+RUN: llvm-dsymutil -o - -oso-prepend-path=%p/.. %p/../Inputs/basic.macho.x86_<u></u>64 | llvm-dwarfdump - | FileCheck %s --check-prefix=CHECK --check-prefix=BASIC<br>
+RUN: llvm-dsymutil -o - -oso-prepend-path=%p/.. %p/../Inputs/basic-archive.<u></u>macho.x86_64 | llvm-dwarfdump - | FileCheck %s --check-prefix=CHECK --check-prefix=ARCHIVE<br>
<br>
 CHECK: file format Mach-O 64-bit x86-64<br>
<br>
@@ -24,6 +24,7 @@ CHECK:      DW_AT_prototyped [DW_FORM_fl<br>
 CHECK:      DW_AT_type [DW_FORM_ref4]       (cu + 0x0063 => {0x00000063})<br>
 CHECK:      DW_AT_external [DW_FORM_flag]      (0x01)<br>
 CHECK:      DW_AT_accessibility [DW_FORM_data1]        (DW_ACCESS_public)<br>
+CHECK:      DW_AT_low_pc [DW_FORM_addr]     (0x0000000100000ea0)<br>
 CHECK:      DW_AT_frame_base [DW_FORM_block1]  (<0x01> 56 )<br>
 CHECK:      DW_TAG_formal_parameter [3]<br>
 CHECK:        DW_AT_name [DW_FORM_strp]     ( .debug_str[0x00000056] = "argc")<br>
@@ -64,12 +65,16 @@ CHECK:      DW_AT_name [DW_FORM_strp]<br>
 CHECK:    DW_TAG_variable [7]<br>
 CHECK:      DW_AT_name [DW_FORM_strp]       ( .debug_str[0x00000072] = "private_int")<br>
 CHECK:      DW_AT_type [DW_FORM_ref4]       (cu + 0x0026 => {0x000000a7})<br>
+BASIC:      DW_AT_location [DW_FORM_block1] (<0x09> 03 08 10 00 00 01 00 00 00 )<br>
+ARCHIVE:    DW_AT_location [DW_FORM_block1] (<0x09> 03 04 10 00 00 01 00 00 00 )<br>
 CHECK:    DW_TAG_variable [7]<br>
 CHECK:      DW_AT_name [DW_FORM_strp]       ( .debug_str[0x0000007e] = "baz")<br>
 CHECK:      DW_AT_type [DW_FORM_ref4]       (cu + 0x0026 => {0x000000a7})<br>
+CHECK:      DW_AT_location [DW_FORM_block1] (<0x09> 03 00 10 00 00 01 00 00 00 )<br>
 CHECK:    DW_TAG_subprogram [2] *<br>
 CHECK:      DW_AT_name [DW_FORM_strp]       ( .debug_str[0x00000082] = "foo")<br>
 CHECK:      DW_AT_type [DW_FORM_ref4]       (cu + 0x0026 => {0x000000a7})<br>
+CHECK:      DW_AT_low_pc [DW_FORM_addr]     (0x0000000100000ed0)<br>
 CHECK:      DW_AT_frame_base [DW_FORM_block1]  (<0x01> 56 )<br>
 CHECK:      DW_TAG_formal_parameter [3]<br>
 CHECK:        DW_AT_name [DW_FORM_strp]     ( .debug_str[0x00000086] = "arg")<br>
@@ -79,6 +84,7 @@ CHECK:      NULL<br>
 CHECK:    DW_TAG_subprogram [8]<br>
 CHECK:      DW_AT_name [DW_FORM_strp]       ( .debug_str[0x0000008a] = "inc")<br>
 CHECK:      DW_AT_type [DW_FORM_ref4]       (cu + 0x0026 => {0x000000a7})<br>
+CHECK:      DW_AT_low_pc [DW_FORM_addr]     (0x0000000100000f20)<br>
 CHECK:      DW_AT_frame_base [DW_FORM_block1]  (<0x01> 56 )<br>
 CHECK:    NULL<br>
<br>
@@ -91,6 +97,8 @@ CHECK:    DW_AT_comp_dir [DW_FORM_strp]<br>
 CHECK:    DW_TAG_variable [9]<br>
 CHECK:      DW_AT_name [DW_FORM_strp]       ( .debug_str[0x00000097] = "val")<br>
 CHECK:      DW_AT_type [DW_FORM_ref4]       (cu + 0x003c => {0x00000162})<br>
+BASIC:      DW_AT_location [DW_FORM_block1] (<0x09> 03 04 10 00 00 01 00 00 00 )<br>
+ARCHIVE:    DW_AT_location [DW_FORM_block1] (<0x09> 03 08 10 00 00 01 00 00 00 )<br>
 CHECK:    DW_TAG_volatile_type [10]<br>
 CHECK:      DW_AT_type [DW_FORM_ref4]       (cu + 0x0041 => {0x00000167})<br>
 CHECK:    DW_TAG_base_type [4]<br>
@@ -98,6 +106,7 @@ CHACK:      DW_AT_name [DW_FORM_strp]<br>
 CHECK:    DW_TAG_subprogram [2] *<br>
 CHECK:      DW_AT_name [DW_FORM_strp]       ( .debug_str[0x0000009b] = "bar")<br>
 CHECK:      DW_AT_type [DW_FORM_ref4]       (cu + 0x0041 => {0x00000167})<br>
+CHECK:      DW_AT_low_pc [DW_FORM_addr]     (0x0000000100000f40)<br>
 CHECK:      DW_AT_frame_base [DW_FORM_block1]  (<0x01> 56 )<br>
 CHECK:      DW_TAG_formal_parameter [3]<br>
 CHECK:        DW_AT_name [DW_FORM_strp]     ( .debug_str[0x00000086] = "arg")<br>
@@ -107,6 +116,7 @@ CHECK:      NULL<br>
 CHECK:    DW_TAG_subprogram [8]<br>
 CHECK:      DW_AT_name [DW_FORM_strp]       ( .debug_str[0x0000008a] = "inc")<br>
 CHECK:      DW_AT_type [DW_FORM_ref4]       (cu + 0x0041 => {0x00000167})<br>
+CHECK:      DW_AT_low_pc [DW_FORM_addr]     (0x0000000100000f90)<br>
 CHECK:      DW_AT_frame_base [DW_FORM_block1]  (<0x01> 56 )<br>
<br>
 CHECK:    NULL<br>
<br>
Modified: llvm/trunk/test/tools/<u></u>dsymutil/X86/basic-lto-<u></u>linking-x86.test<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/X86/basic-lto-linking-x86.test?rev=231544&r1=231543&r2=231544&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/test/tools/<u></u>dsymutil/X86/basic-lto-<u></u>linking-x86.test?rev=231544&<u></u>r1=231543&r2=231544&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/test/tools/<u></u>dsymutil/X86/basic-lto-<u></u>linking-x86.test (original)<br>
+++ llvm/trunk/test/tools/<u></u>dsymutil/X86/basic-lto-<u></u>linking-x86.test Fri Mar  6 19:25:09 2015<br>
@@ -18,6 +18,7 @@ CHECK:      DW_AT_prototyped [DW_FORM_fl<br>
 CHECK:      DW_AT_type [DW_FORM_ref4]       (cu + 0x0063 => {0x00000063})<br>
 CHECK:      DW_AT_external [DW_FORM_flag]      (0x01)<br>
 CHECK:      DW_AT_accessibility [DW_FORM_data1]        (DW_ACCESS_public)<br>
+CHECK:      DW_AT_low_pc [DW_FORM_addr]     (0x0000000100000f40)<br>
 CHECK:      DW_AT_frame_base [DW_FORM_block1]  (<0x01> 56 )<br>
 CHECK:      DW_TAG_formal_parameter [3]<br>
 CHECK:        DW_AT_name [DW_FORM_strp]     ( .debug_str[0x00000056] = "argc")<br>
@@ -53,12 +54,15 @@ CHECK:    DW_AT_comp_dir [DW_FORM_strp]<br>
 CHECK:    DW_TAG_variable [7]<br>
 CHECK:      DW_AT_name [DW_FORM_strp]       ( .debug_str[0x00000072] = "private_int")<br>
 CHECK:      DW_AT_type [DW_FORM_ref_addr]   (0x0000000000000063)<br>
+CHECK:      DW_AT_location [DW_FORM_block1] (<0x09> 03 08 10 00 00 01 00 00 00 )<br>
 CHECK:    DW_TAG_variable [7]<br>
 CHECK:      DW_AT_name [DW_FORM_strp]       ( .debug_str[0x0000007e] = "baz")<br>
 CHECK:      DW_AT_type [DW_FORM_ref_addr]   (0x0000000000000063)<br>
+CHECK:      DW_AT_location [DW_FORM_block1] (<0x09> 03 00 10 00 00 01 00 00 00 )<br>
 CHECK:    DW_TAG_subprogram [8] *<br>
 CHECK:      DW_AT_name [DW_FORM_strp]       ( .debug_str[0x00000082] = "foo")<br>
 CHECK:      DW_AT_type [DW_FORM_ref_addr]   (0x0000000000000063)<br>
+CHECK:      DW_AT_low_pc [DW_FORM_addr]     (0x0000000100000f50)<br>
 CHECK:      DW_AT_frame_base [DW_FORM_block1]  (<0x01> 56 )<br>
 CHECK:      DW_TAG_formal_parameter [9]<br>
 CHECK:        DW_AT_name [DW_FORM_strp]     ( .debug_str[0x00000086] = "arg")<br>
@@ -82,11 +86,13 @@ CHECK:    DW_AT_comp_dir [DW_FORM_strp]<br>
 CHECK:    DW_TAG_variable [12]<br>
 CHECK:      DW_AT_name [DW_FORM_strp]       ( .debug_str[0x00000097] = "val")<br>
 CHECK:      DW_AT_type [DW_FORM_ref4]       (cu + 0x003c => {0x00000176})<br>
+CHECK:      DW_AT_location [DW_FORM_block1] (<0x09> 03 04 10 00 00 01 00 00 00 )<br>
 CHECK:    DW_TAG_volatile_type [13]<br>
 CHECK:      DW_AT_type [DW_FORM_ref_addr]   (0x0000000000000063)<br>
 CHECK:    DW_TAG_subprogram [8] *<br>
 CHECK:      DW_AT_name [DW_FORM_strp]       ( .debug_str[0x0000009b] = "bar")<br>
 CHECK:      DW_AT_type [DW_FORM_ref_addr]   (0x0000000000000063)<br>
+CHECK:      DW_AT_low_pc [DW_FORM_addr]     (0x0000000100000f90)<br>
 CHECK:      DW_AT_frame_base [DW_FORM_block1]  (<0x01> 56 )<br>
 CHECK:      DW_TAG_formal_parameter [9]<br>
 CHECK:        DW_AT_name [DW_FORM_strp]     ( .debug_str[0x00000086] = "arg")<br>
<br>
Modified: llvm/trunk/tools/dsymutil/<u></u>DwarfLinker.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/DwarfLinker.cpp?rev=231544&r1=231543&r2=231544&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/tools/<u></u>dsymutil/DwarfLinker.cpp?rev=<u></u>231544&r1=231543&r2=231544&<u></u>view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/tools/dsymutil/<u></u>DwarfLinker.cpp (original)<br>
+++ llvm/trunk/tools/dsymutil/<u></u>DwarfLinker.cpp Fri Mar  6 19:25:09 2015<br>
@@ -463,7 +463,7 @@ private:<br>
   /// consider. As we walk the DIEs in acsending file offset and as<br>
   /// ValidRelocs is sorted by file offset, keeping this index<br>
   /// uptodate is all we have to do to have a cheap lookup during the<br>
-  /// root DIE selection.<br>
+  /// root DIE selection and during DIE cloning.<br>
   unsigned NextValidReloc;<br>
<br>
   bool findValidRelocsInDebugInfo(<u></u>const object::ObjectFile &Obj,<br>
@@ -562,6 +562,10 @@ private:<br>
                                 const DWARFUnit &U, AttributeSpec AttrSpec,<br>
                                 const DWARFFormValue &Val, unsigned AttrSize);<br>
<br>
+  /// \brief Helper for cloneDIE.<br>
+  bool applyValidRelocs(<u></u>MutableArrayRef<char> Data, uint32_t BaseOffset,<br>
+                        bool isLittleEndian);<br>
+<br>
   /// \brief Assign an abbreviation number to \p Abbrev<br>
   void AssignAbbrev(DIEAbbrev &Abbrev);<br>
<br>
@@ -1256,6 +1260,50 @@ unsigned DwarfLinker::cloneAttribute(<u></u>DIE<br>
   return 0;<br>
 }<br>
<br>
+/// \brief Apply the valid relocations found by findValidRelocs() to<br>
+/// the buffer \p Data, taking into account that Data is at \p BaseOffset<br>
+/// in the debug_info section.<br>
+///<br>
+/// Like for findValidRelocs(), this function must be called with<br>
+/// monotonic \p BaseOffset values.<br>
+///<br>
+/// \returns wether any reloc has been applied.<br>
+bool DwarfLinker::applyValidRelocs(<u></u>MutableArrayRef<char> Data,<br>
+                                   uint32_t BaseOffset, bool isLittleEndian) {<br>
+  assert(NextValidReloc == 0 ||<br>
+         BaseOffset > ValidRelocs[NextValidReloc - 1].Offset &&<br>
+             "BaseOffset should only be increasing.");<br>
+  if (NextValidReloc >= ValidRelocs.size())<br>
+    return false;<br>
+<br>
+  // Skip relocs that haven't been applied.<br>
+  while (NextValidReloc < ValidRelocs.size() &&<br>
+         ValidRelocs[NextValidReloc].<u></u>Offset < BaseOffset)<br>
+    ++NextValidReloc;<br>
+<br>
+  bool Applied = false;<br>
+  uint64_t EndOffset = BaseOffset + Data.size();<br>
+  while (NextValidReloc < ValidRelocs.size() &&<br>
+         ValidRelocs[NextValidReloc].<u></u>Offset >= BaseOffset &&<br>
+         ValidRelocs[NextValidReloc].<u></u>Offset < EndOffset) {<br>
+    const auto &ValidReloc = ValidRelocs[NextValidReloc++];<br>
+    assert(ValidReloc.Offset - BaseOffset < Data.size());<br>
+    assert(ValidReloc.Offset - BaseOffset + ValidReloc.Size <= Data.size());<br>
+    char Buf[8];<br>
+    uint64_t Value = ValidReloc.Mapping->getValue()<u></u>.BinaryAddress;<br>
+    Value += ValidReloc.Addend;<br>
+    for (unsigned i = 0; i != ValidReloc.Size; ++i) {<br>
+      unsigned Index = isLittleEndian ? i : (ValidReloc.Size - i - 1);<br>
+      Buf[i] = uint8_t(Value >> (Index * 8));<br>
+    }<br>
+    assert(ValidReloc.Size <= sizeof(Buf));<br>
+    memcpy(&Data[ValidReloc.Offset - BaseOffset], Buf, ValidReloc.Size);<br>
+    Applied = true;<br>
+  }<br>
+<br>
+  return Applied;<br>
+}<br>
+<br>
 /// \brief Recursively clone \p InputDIE's subtrees that have been<br>
 /// selected to appear in the linked output.<br>
 ///<br>
@@ -1284,6 +1332,20 @@ DIE *DwarfLinker::cloneDIE(const DWARFDe<br>
<br>
   // Extract and clone every attribute.<br>
   DataExtractor Data = U.getDebugInfoExtractor();<br>
+  uint32_t NextOffset = U.getDIEAtIndex(Idx + 1)->getOffset();<br>
+<br>
+  // We could copy the data only if we need to aply a relocation to<br>
+  // it. After testing, it seems there is no performance downside to<br>
+  // doing the copy unconditionally, and it makes the code simpler.<br>
+  SmallString<40> DIECopy(Data.getData().substr(<u></u>Offset, NextOffset - Offset));<br>
+  Data = DataExtractor(DIECopy, Data.isLittleEndian(), Data.getAddressSize());<br>
+  // Modify the copy with relocated addresses.<br>
+  applyValidRelocs(DIECopy, Offset, Data.isLittleEndian());<br>
+<br>
+  // Reset the Offset to 0 as we will be working on the local copy of<br>
+  // the data.<br>
+  Offset = 0;<br>
+<br>
   const auto *Abbrev = InputDIE.<u></u>getAbbreviationDeclarationPtr(<u></u>);<br>
   Offset += getULEB128Size(Abbrev-><u></u>getCode());<br>
<br>
@@ -1386,6 +1448,11 @@ bool DwarfLinker::link(const DebugMap &M<br>
       lookForDIEsToKeep(*<u></u>CurrentUnit.getOrigUnit().<u></u>getCompileUnitDIE(), *Obj,<br>
                         CurrentUnit, 0);<br>
<br>
+    // The calls to applyValidRelocs inside cloneDIE will walk the<br>
+    // reloc array again (in the same way findValidRelocsInDebugInfo()<br>
+    // did). We need to reset the NextValidReloc index to the beginning.<br>
+    NextValidReloc = 0;<br>
+<br>
     // Construct the output DIE tree by cloning the DIEs we chose to<br>
     // keep above. If there are no valid relocs, then there's nothing<br>
     // to clone/emit.<br>
<br>
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
</blockquote></div>