[PATCH] D115325: [DWARF] Fix PR51087 Extraneous enum record in DWARF with type units

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando updated this revision to Diff 394522.
Orlando added a comment.

Thanks for that info on retained types.

In D115325#3190794 <https://reviews.llvm.org/D115325#3190794>, @dblaikie wrote:

> We probably should only put user defined types in the retained types list - but that would require decomposing non-user-defined types in the frontend (eg: if you have a "void (*)(T1, T2)" type retained - we only need to retain T1 and T2, not the function pointer type wrapped around it (it's reasonable to expect a DWARF consumer to be able to build any non-user-defined type - even ones that that never appeared in the source program (so maybe a user ends up needing "T1 (*)(T2)" in their debugging context, the DWARF consumer would be able to create that - there's no need for us to encode that type or any other, so long as we provide T1 and T2)

Ok great, I'll file a bug for this shortly.

> But I guess for now we could apply the same logic to enums and to retained types, with one minor change to use dyn_cast instead of cast, in the retained types version - so if it is a top level user defined type in the retained types list, it'd skip the skeleton type DIE in that case. In other cases it'd still end up with the unnecessary poinetr type, function type, whatever.

I made this change (does this still look okay to land?) and I wanted to add a test for the retained types too so I wrote this:

  $ cat test.cpp
  struct Int {
    int i;
    Int() : i(0) {}
    operator int() const { return i; }
  };
  int f() { return Int(); }

Struct `Int` is indeed a retained type but even with this latest patch `Int` gets a CU DIE:

  0x0000003b:   DW_TAG_structure_type
                  DW_AT_declaration	(true)
                  DW_AT_signature	(0x9366bdce481c7390)

Which is only referenced from within its children, e.g. for the overloaded int conversion:

  0x00000087:   DW_TAG_const_type
                  DW_AT_type	(0x0000003b "Int")
  <snip>
  0x0000003b:   DW_TAG_structure_type
                  DW_AT_declaration	(true)
                  DW_AT_signature	(0x9366bdce481c7390)
  <snip>
  0x00000063:   DW_TAG_subprogram
                  DW_AT_low_pc	(0x0000000000000000)
                  DW_AT_high_pc	(0x0000000000000014)
                  DW_AT_frame_base	(DW_OP_reg6 RBP)
                  DW_AT_object_pointer	(0x00000074)
                  DW_AT_linkage_name	("_ZN3IntC2Ev")
                  DW_AT_specification	(0x00000044 "Int")
  
  0x00000074:     DW_TAG_formal_parameter
                    DW_AT_location	(DW_OP_fbreg -8)
                    DW_AT_name	("this")
                    DW_AT_type	(0x000000a6 "Int *")          # <-- References Int through DW_TAG_pointer_type(DW_TAG_const_type(Int))
                    DW_AT_artificial	(true)
  
  <snip>
  0x000000ab:   DW_TAG_pointer_type
                  DW_AT_type	(0x00000087 "const Int")

These children that are included in the CU (above) are also duplicated in the TU (i.e. we get a full `DW_TAG_subprogram` DIE in both places). This sounds like a bug, or at least that my implementation needs further work to cover all cases for retained types.

I couldn't generate a test for retained types from source by hand or reduction (from CTMark and clang-3.4) that showed this patch working in action.

I'm happy to land either version of this patch - if we go with it in its current state I can file a(nother) bug for improving the retained types situation.


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

https://reviews.llvm.org/D115325

Files:
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h
  llvm/test/DebugInfo/X86/type-units-unused-nested-enums.mir
  llvm/test/DebugInfo/X86/type-units-used-enum.mir

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D115325.394522.patch
Type: text/x-patch
Size: 12766 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211215/11831831/attachment.bin>