Add DW_AT_linkage_name for DW_TAG_labels

[dimitry]

Summary

Since DW_AT_name is not a symbol name for DW_TAG_label DIEs
add DW_AT_linkage_name with original symbol name.

Bug: https://bugs.llvm.org/show_bug.cgi?id=37470

[davide]

This needs a testcase.

[aprantl]

I'm not sure that stripping the leading _ unconditionally is correct either. This code exists to emit DWARF for labels in assembler source code and I'm assuming that this patch will break that use-case.

[aprantl]

It would be good to add both a testcase for your intended use-case and one with an assembler label (if that doesn't exist already).

[dimitry]

Interestingly the test specifically checks if underscore is removed. Looks like this was very much intentional. But I still do not understand why.. It makes the DW_AT_name of the label differ from elf symbol name (in .dynsym section).

[dblaikie]

The goal would be for the name to match what the user wrote - not
necessarily what the symbol name is.

[dimitry]

It looks like in our case it ended up being not what the user wrote.. and this is especially confusing because libc and exit symbol too (and __exit, which gets converted to _exit...

[probinson]

If there's a difference between the in-source name and the exposed-to-linker name, the in-source name should be provided to DW_AT_name and the exposed-to-linker name should be in DW_AT_linkage_name. Don't try to overload DW_AT_name with any kind of decoration imposed for linking purposes.

[dimitry]

Added DW_AT_linkage_name instead of keeping underscore in DW_AT_name.

[dimitry]

git clang-format --style=file

[dblaikie]

Is this correct? What if the user has a label called "_foo"?

I would've thought this '_' handling thing was specifically a OSX/iOS thing (from what I've seen in its handling in LLVM in other places) - so I'm surprised it isn't conditionalized in some way?

[dimitry]

In D46878#1099526, @aprantl wrote:

It would be good to add both a testcase for your intended use-case and one with an assembler label (if that doesn't exist already).

I am not sure if I understand - my intended use-case here is in fact an assembler label (a .glob label). Can you please give me a pointer to another use-case? I am not very familiar with llvm code base.

[dimitry]

ping

[aprantl]

I think this looks good now.

[dimitry]

There is still a question about which fix is the correct one. This one or https://reviews.llvm.org/D47104. To reiterate which use case requires clang to remove leading underscores in DW_AT_name?

[dimitry]

ping?

[JDevlieghere]

Based on @echristo's comment in D47104 it feels like this is the preferable way to move forward with this.

[echristo]

LGTM. Thanks!

-eric

[JohnReagan]

lib/MC/MCDwarf.cpp

562

Uh, DW_AT_linkage_name isn't intended to be used with DW_TAG_labels. It is really meant to provide the mangled names with entries, etc. From DWARF4 spec

"Debugging information entries to which DW_AT_linkage_name may apply include: DW_TAG_common_block, DW_TAG_constant, DW_TAG_entry_point, DW_TAG_subprogram and DW_TAG_variable."

DWARF5 dropped that wording, but you can still infer that list from Appendix A, etc.

I can see that we already emit DW_AT_prototyped for DW_TAG_labels (also extra to the standard).

There are other debuggers who consume this stuff. We're about to start work on the OpenVMS debugger to bring it up past DWARF3 and I would have assumed (incorrectly) that nobody would emit these DW_AT_ attributes for labels.

Is there a document somewhere that tracks these extra-standard extensions?

[aprantl]

lib/MC/MCDwarf.cpp

562

DWARF5 dropped that wording, but you can still infer that list from Appendix A, etc.

I wonder if the information in Appendix A (which is non-normative) isn't an oversight. Might be worth bringing this up on the dwarf-discuss mailing list.

Is there a document somewhere that tracks these extra-standard extensions?

Not that I'm aware of. Generally LLVM tends to be fairly permissive about emitting extra dwarf attributes based on that consumers can always ignore extra attributes they don't understand.

[probinson]

lib/MC/MCDwarf.cpp

562

Appendix A is really a best-effort list, not definitive (which is explicitly stated in the opening text of the appendix). DWARF rarely mandates anything, and permissively allows extra attributes not specified in the normative text to appear in a DIE. Any reader needs to be ready for that sort of thing.
In the case at hand, it seems there are situations where the label's name in the source text is not the same as the name exposed to the linker, and that's pretty much exactly what DW_AT_linkage_name is for. I might prefer that the extra attribute appear only when needed, but we don't make a huge effort to support the best possible debug info for assembler source (which is the case here).

[JohnReagan]

lib/MC/MCDwarf.cpp

562

Got it. Thanks.

I do agree about omitting it if the linkage_name is the same as the name since that reduces the number of references to the string table. Given your lightning talk and the reduction of string references that led to the changes for DWARF5, no sense making a DWARF4 (or earlier) user pay that extra cost. You do end up with needing two Abbrevs however.

I still have a question about the use of DW_AT_prototyped (the normative description is in the "Miscellaneous Subprogram Properties" section) and what it means on a label but I'll go dig for when that was included and the rationale behind it.

lib/MC/MCDwarf.cpp

     // The DW_TAG_label DIE abbrev (2).
     MCOS->EmitULEB128IntValue(2);
 
-    // AT_name, of the label without any leading underbar.
+    // AT_name, of the label.
     MCOS->EmitBytes(Entry.getName());
     MCOS->EmitIntValue(0, 1); // NULL byte to terminate the string.
 

   if (!context.getGenDwarfSectionSyms().count(MCOS->getCurrentSectionOnly()))
     return;
 
-  // The dwarf label's name does not have the symbol name's leading
-  // underbar if any.
   StringRef Name = Symbol->getName();
-  if (Name.startswith("_"))
-    Name = Name.substr(1, Name.size()-1);
 
   // Get the dwarf file number to be used for the dwarf label.
   unsigned FileNumber = context.getGenDwarfFileNumber();