[llvm] r231544 - [dsymutil] Apply relocations to DIE data before cloning.

Eric Christopher echristo at gmail.com
Sun Mar 22 20:51:26 PDT 2015


On Fri, Mar 13, 2015 at 4:37 PM Frédéric Riss <friss at apple.com> wrote:

> On Mar 13, 2015, at 4:14 PM, Eric Christopher <echristo at gmail.com> wrote:
>
>
>
> On Fri, Mar 6, 2015 at 6:29 PM Frédéric Riss <friss at apple.com> wrote:
>
>> [ adding David as he might have an opinion on the question I’m asking
>> back. ]
>>
>> On Mar 6, 2015, at 5:36 PM, Eric Christopher <echristo at gmail.com> wrote:
>>
>> Possible to adapt the existing RelocVisitor code to do this?
>>
>>
>> It’s great that you bring that up, because it is something I wanted to
>> discuss — not really for that use case, but more generally in the mach-o
>> case.
>>
>> To answer your question, I don’t think it fits what I’m doing at all. I’m
>> calling this 'applying relocations', but it’s just patching some of the
>> relocations in debuginfo data with the appropriate value. And the
>> appropriate value is most of the time extracted directly from the debug map
>> and isn’t at all computed using sym+addend like you’d do when processing a
>> relocation (and which is the computation a potential mach-o RelocVisitor
>> would do).
>>
>> Now the part I wanted to discuss. RelocVisitor has no support for mach-o
>> at all. You can see that when running llvm-dwarfdump on an
>>
>
> So there is a patch out for this :)
>
>
> I’ve seen that :-) (And I’m pretty sure it’s not a coincidence that it got
> updated a few minutes after this message!)
>
> Does this change the rest of the answers?
>
>
> Not the ‘could you reuse RelocVisitor in dsymutil’ part, what I’m doing
> there is still not applying relocations in the traditional sense. Then I’ve
> got nothing against implementing the RelocVisitor on mach-o (especially as
> in the review thread you’re referring to the goal is to support debug info
> for JITed code where the processing the relics seems to be necessary). All
> I want is to get rid of the warnings that are displayed when DWARFContext
> parses a relocatable mach-o file.
>
>
Not quite sure what you mean here, it's probably my fault, but could you
rephrase this in a different way?

-eric


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


More information about the llvm-commits mailing list