[PATCH] D113741: [RFC][DwarfDebug][AsmPrinter] Support emitting function-local declaration for a lexical block

Wed May 25 09:43:29 PDT 2022

krisb added a comment.

In D113741#3532850 <https://reviews.llvm.org/D113741#3532850>, @dblaikie wrote:

> In D113741#3516038 <https://reviews.llvm.org/D113741#3516038>, @krisb wrote:
>
>> There is an alternative implementation that relies on addition field of DISubprogram/DILexicalScope that tracks static locals, local imports and types D125693 <https://reviews.llvm.org/D125693>.
>
> Ah, OK - so that 5-part sequence is an alternative to this one?

Mostly. The alternative implementation reuses large part of work of this patch.
Basically, this first implementation has (had) 3 patches in sequence:

- D114705 <https://reviews.llvm.org/D114705> (moving handling of globals, types and imports from beginModule() to endModule() in DwarfDebug.cpp)
- D113741 <https://reviews.llvm.org/D113741> (this one, backend's part)
- D113743 <https://reviews.llvm.org/D113743> (clang's part)

The new one has 5 patches:

- D125691 <https://reviews.llvm.org/D125691> (adding new 'localDecls' field to DILexicalBlock and DISubprogram),
- D114705 <https://reviews.llvm.org/D114705> (taken from previous implementation w/o changes),
- D125693 <https://reviews.llvm.org/D125693> (backend's part, replacement for D113741 <https://reviews.llvm.org/D113741>)
- D125694 <https://reviews.llvm.org/D125694>, D125695 <https://reviews.llvm.org/D125695> (clang's part, the same as D113743 <https://reviews.llvm.org/D113743>, but split on 2 patches and with some minor improvements).

> Might be good to figure out the issue here before debating which direction to go in, etc. (hmm, one of the patches in that series I've already approved? So am I reviewing alternatives here? Given the complexity here that's a bit costly for me/end up feeling like I'm drowning under trying to figure out all these moving parts... - a linear sequence is generally OK/good, I can review the easy parts and focus on the difficult ones, but multiple large alternatives can be a bit daunting)

I'm ready to continue working with this patchset to find and fix the issue with pubnames, but I have no ideas how I can reproduce it, so I decided to move forward and try another approach which looks a bit better to me.
If you have some time to help me finding the reproducer or could give some hints on this, I'll be happy to fix the aforementioned problem first.

>> In D113741#3448685 <https://reviews.llvm.org/D113741#3448685>, @jmorse wrote:
>>
>>> Drive-by comment on the topic of the gnu pubnames error,
>>>
>>> In D113741#3439940 <https://reviews.llvm.org/D113741#3439940>, @krisb wrote:
>>>
>>>> If I remember correctly, this issue with .debug_gnu_pubnames was caused by another patch (https://reviews.llvm.org/rG78d15a112cbd545fbb6e1aa37c221ef5aeffb3f2). 
>>>> Did I understand you right that the same issue is reproduced with w/o that patch?
>>>
>>> I've run into that error myself when trying to alter which unit a subprogram definition goes in -- see the comment at D94976#2543257 <https://reviews.llvm.org/D94976#2543257> . That patch never landed in the end because I couldn't figure out exactly what was wrong. IMHO, there's some latent weirdness that triggers that error, exactly how is unclear though. IMO, it's not specific to rG78d15a112cbd5 <https://reviews.llvm.org/rG78d15a112cbd545fbb6e1aa37c221ef5aeffb3f2> .
>>>
>>> [0] "(.debug_gnu_pubnames): name lookup table at offset (...) has a terminator at offset (...) before the expected end at (...)"
>>
>> I've tried to reproduce the issue using the example from D94976 <https://reviews.llvm.org/D94976> (and with this patch), it compiled fine and didn't show the mentioned problem with pubnames. 
>> Another question is about the combination of FullLTO and split-dwarf which has been already discussed in this thread and which isn't (fully) supported by LLVM mainline (yet, at least in a way that doesn't violate DWARF specification).
>>
>> So, I'm still looking for a reproducer for the issue with pubnames. Any help will be appreciated.
>
> Yeah, I still need to figure out how to do that/get you that. :/
>
> (is this at all/possibly related to https://reviews.llvm.org/D124892 ?)

One of the issues you mentioned here https://reviews.llvm.org/D113741#3207125 seems to relate to D124892 <https://reviews.llvm.org/D124892>, but as I said, it reproduces w/o this patchset and likely has different root cause.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113741/new/

https://reviews.llvm.org/D113741