[PATCH] D103131: support debug info for alias variable

Tue Jun 1 15:57:18 PDT 2021

dblaikie added a comment.

In D103131#2792159 <https://reviews.llvm.org/D103131#2792159>, @probinson wrote:

> In D103131#2791881 <https://reviews.llvm.org/D103131#2791881>, @dblaikie wrote:
>
>> In D103131#2789493 <https://reviews.llvm.org/D103131#2789493>, @probinson wrote:
>>
>>>> Mixed feelings - somewhat in favor of "do the thing that's probably already fairly tested/known to work" (GCC's thing). But open to the idea that that approach has problems, for sure.
>>>
>>> "Known to work" for whom?  gdb, sure, because gcc/gdb cooperate on many things.
>>
>> Yep. Given the diversity of ways of expressing things in DWARF, no consumer will handle everything that could be produced in the way a producer might intend - so we do sort of have a defacto standard of "what would GCC do/what does GDB understand".
>>
>> But also GCC/GDB has had more implementation experience with DWARF in general, and with any feature we haven't implemented yet in particular - that I wouldn't throw out their representational choice too quickly - there may be important tradeoffs that were considered, implemented, and discarded due to limitations.
>
> Well, they aren't the only ones with a decent amount of implementation experience, hrmph, my first contribution to DWARF was in 2002 (my name isn't on the proposal, but I co-developed it).

Sure enough - didn't mean to disparage your work. Mostly given the desire for Clang to be drop-in compatible (more or less) with GCC, I'm generally interested in whatever choice GCC makes for DWARF to check if there's anything we should be learning from it having done it that way for a while most likely.

>>> I'll have to check with my debugger guys whether this would work for us; and I'll just reiterate that it's certainly not what the DWARF spec suggests should be done.
>>
>> I don't agree that it's "certainly not what the DWARF spec suggests should be done" - the spec's pretty generous and exactly how a C or ELF level alias should be rendered in DWARF seems pretty unclear to me. For instance an alias is a symbol in the ELF file, arguably different from a source level alias like a typedef or using declaration that DW_TAG_imported_declaration seems to be promoted for.
>
> The spec is generous and doesn't mandate (much), but what I'm referring to is DWARF 5 section 3.2.3 p.73 lines 1-3, "may be used as a general means to rename or provide an alias for an entity, regardless of the context" which seems like a pretty clear suggestion to me.
> And I certainly would have thought `__attribute__((alias("oldname")))` counted as a source representation?

Re: Source representation: I was trying to draw a distinction between things that are purely in the source representation/AST (like typedefs/using declarations - though, admittedly, neither of those apply to variables - I guess maybe you can't make a pure source level variable alias? oh, only if you keep it the same name and only move it between namespaces with `using ns::name` - not if you want to give it a new name) compared to those that have some manifestation in the object file.

For instance, arguably "extern int i; const int &j = i;` - `j` is an alias, of sorts - perhaps it could be represented with DW_TAG_imported_declaration? Though I'd tend not to think of it that way/use that feature in that way & it's pretty similar to the effect of the alias attribute (in that you get a separate symbol, though admittedly in this case the symbol does store the address of the other thing, rather than being a synonym - but has a similar effect)

>>> The Sony debugger will not be able to figure out the address of "newname" given the DWARF as emitted by gcc (because it looks like an external declaration with no address).  We'd much rather have the DW_TAG_imported_declaration that the original patch had.
>>
>> Good to know - any interest in the debugger supporting declarations without addresses? I guess there's no need for GCC compatibility?
>
> No and no.  In general we seem to think that if you want to debug a CU, you should produce debug info for that CU.
>
> I experimented with `extern int externalname;` and Clang doesn't emit a DIE for that at all, even if it's used; so I think the philosophy is not unsound, and is not inconsistent with how Clang behaves.  If you want to see data from another CU, you generate debug info for that CU.

Ish - we do have -fstandalone-debug for when you haven't built everything with debug info, but you want this one thing you did build with debug info to be fairly debuggable & that could include access to such variables (we do something like this for constants and enums since they might not be "pinned" anywhere else - but also for base classes that could be defined in another file but you want them in this file if you're going to debug a derived class that is defined here).

The ability to drop the DWARF if the IR definition is dropped in LTO (& consequently can drop other type information that it references, etc) could be a nice side effect of a more IR-driven representation. (though that might tend more towards a definition, with a location, rather than a declaration)

>> @kamleshbhalui  - perhaps you could run the gdb and lldb test suites without either change, then with both the variable declaration and imported declaration versions to see how the results vary? (Well, I guess the lldb test suite probably won't show any changes - but maybe worth running anyway) - along with the manual test you've described in the patch description, to demonstrate that both solutions address that?
>
> No objection to running experiments, but Sony will want the imported_declaration construct regardless.

I will say, as the person who implemented DW_TAG_imported_declaration support in Clang - it's a bit not great/unfortunate (in part because we currently have to emit them for all using declarations (because clang doesn't track some of the usedness we would need to more selectively emit them), and then we can't reap them in LTO or other optimizations if they are effectively unused) - but such is the way of things.

================
Comment at: clang/test/CodeGen/debug-info-alias.c:3
+
+// CHECK: !DIGlobalVariable(name: "newname"
+
----------------
If we go this way, might be good to have more detail on this - to check how this DIGlobalVariable is defined and how it is referenced (what's keeping this DI node alive)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103131