[llvm] r231544 - [dsymutil] Apply relocations to DIE data before cloning.
Eric Christopher
echristo at gmail.com
Fri Mar 13 16:14:06 PDT 2015
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 :)
Does this change the rest of the answers?
-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/20150313/d998811d/attachment.html>
More information about the llvm-commits
mailing list