[llvm] r231544 - [dsymutil] Apply relocations to DIE data before cloning.
Frédéric Riss
friss at apple.com
Fri Mar 6 18:29:29 PST 2015
[ 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 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 <mailto: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 <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 <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 <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 <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 <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <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/20150306/4079129d/attachment.html>
More information about the llvm-commits
mailing list