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

Wed Mar 30 02:43:12 PDT 2022

krisb added a comment.

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

> In D113741#3348795 <https://reviews.llvm.org/D113741#3348795>, @krisb wrote:
>
>> Statement 1. The problem with the example you provided above was because this patch made subprograms and their local "global" entities emitted in different CUs in the case of split DWARF. This is a bug, for sure.
>
> Is that bug now addressed in the proposed patch & with tests to demonstrate it's fixed?

I believe, it is. I created a test from the example you provided above, see llvm/test/DebugInfo/Generic/split-dwarf-local-import.ll.
It checks that the second CU doesn't have `DW_AT_GNU_dwo_name` and `DW_AT_GNU_dwo_id` and the imported entity has a proper parent.
But, if you have a chance to check the updated patch in the same conditions this bug originally reproduced, I'll highly appreciate it.

>> Statement 2. Currently, there is nothing in the code that restricts emitting multiple CUs in a single .dwo (whether for FullLTO or for manually linked modules (as in the tests) we will generate more than one CU per .dwo).
>
> I believe `DwarfDebug.cpp:SplitDwarfCrossCuReferences` is intended to disallow this functionality, so far as I can tell/remember?

Hmm, I'm not sure fully I understood your point. `SplitDwarfCrossCuReferences` seems to _allow_ references between CUs, but it isn't enabled by default.
For example, if you enable split-dwarf with FullLTO, clang would emit a single .dwo per complied&linked binary with all the CUs in this .dwo. Neither clang nor backend would emit any errors/warnings.
I'm not insisting this is correct, or this may be useful for someone (I'm simply not aware), just want to mention that this is possible.

>> All my assumptions and conclusions are based on the code and existing regression tests. Unfortunately, I'm not aware of the real situation of split DWARF or split DWARF inlining support (and I couldn't find anything related in the docs either).
>>
>> Having this, I think we cannot assume that a subprogram is guaranteed to be emitted only in one CU, unless we explicitly disallow the case with multiple CUs per .dwo. DWARF standard seems to assume only one CU per .dwo (but I couldn't find any explicit wording about restrictions on the number of CUs in a .dwo). However, it looks like GDB is okay and works fine with more than one CU in a .dwo. It'd be good to have this situation clarified. But it seems to be out of the scope of this patch. Do you have any thoughs on how to proceed with this?
>
> One of the issues is that the dwp file format describes an index by CU hash (DWO ID) - it's unclear what it would mean to have multiple entries that overlap (because a single region contains multiple CUs) and where the region doesn't start with the CU for the lookup (because it's some region with multiple units).

So the question is: should we explicitly disallow emitting multiple CUs in .dwo (meaning, for example, split-dwarf with FullLTO w/o split-dwarf-cross-cu-references)? By reporting an error or at least specifying somewhere in docs that this case it not correct thus not supported?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113741