[PATCH] D37791: [XRay][CodeGen] Use the current function symbol as the associated symbol for the instrumentation map

[David Blaikie via llvm-commits]

On Wed, Sep 13, 2017 at 11:09 PM Dean Michael Berris <dberris at google.com>
wrote:

> On 13 Sep 2017, at 23:07, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Tue, Sep 12, 2017 at 10:42 PM Dean Michael Berris via Phabricator via
> llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
>> dberris created this revision.
>> Herald added a subscriber: aprantl.
>>
>> XRay had been assuming that the previous section is the "text" section
>> of the function. Unfortunately for in-lined functions where we're also
>> generating the comdat definition for debugging purposes, we may be
>> coming from a section that isn't "text" (could be .zdebug_types).
>>
>
> This statement "unfortunately for in-lined functions where we're also
> generating the comdat definition for debugging purposes" is still confusing
> to me - I don't think this has anything to do with comdats.
>
>
> Yes, I've updated the description. PTAL.
>
>
> Indeed I think the test case you mentioned having had no inline functions
> at all - just that if type units are enabled and any debug types are
> emitted during endFunction, then that causes a switch to the debug_types
> section, rather than the text section. (most of the DWARF emission is
> delayed - we mostly don't switch to DWARF sections and produce DWARF bytes
> until the end of the module - except for type units, since they're
> whole/complete by themselves the moment we emit the type, so for those we
> do it immediately - causing section changes)
>
>
>
> That's correct. Please see the updated description?
>

Ah, sure.

Though the description and test centers around the linker failure caused by
this bug + split-dwarf - it looks like split-dwarf exposed a bug that'd be
there even without split-dwarf. That the instrmap was referring to the
wrong section/data. So I'd probably test that directly (objdump -r to test
the relocations refer to the right sections, test without split-dwarf
because that's not integral to the underlying bug, no need to worry
about/mention fission or objcopy except perhaps by way of explaining how
the issue was discovered if you think that's particularly helpful for
future readers).


>
>
>> This fixes an issue with combining -gsplit-dwarf and -fxray-instrument
>> for functions inlined, where the original definition may be lowered
>> into the debug sections. When the debug section is stripped, we're
>> left with references from the xray_instr_map to the debug section. The
>> change now uses the function's symbol instead of the previous
>> section's start symbol.
>>
>>
>> https://reviews.llvm.org/D37791
>>
>> Files:
>>   lib/CodeGen/AsmPrinter/AsmPrinter.cpp
>>
>>
>> Index: lib/CodeGen/AsmPrinter/AsmPrinter.cpp
>> ===================================================================
>> --- lib/CodeGen/AsmPrinter/AsmPrinter.cpp
>> +++ lib/CodeGen/AsmPrinter/AsmPrinter.cpp
>> @@ -2785,7 +2785,7 @@
>>    MCSection *InstMap = nullptr;
>>    MCSection *FnSledIndex = nullptr;
>>    if (MF->getSubtarget().getTargetTriple().isOSBinFormatELF()) {
>> -    auto Associated =
>> dyn_cast<MCSymbolELF>(PrevSection->getBeginSymbol());
>> +    auto Associated = dyn_cast<MCSymbolELF>(CurrentFnSym);
>>      assert(Associated != nullptr);
>>      auto Flags = ELF::SHF_WRITE | ELF::SHF_ALLOC | ELF::SHF_LINK_ORDER;
>>      std::string GroupName;
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170914/d53b79f8/attachment.html>