This is an archive of the discontinued LLVM Phabricator instance.

[DWARF DUMP] Fix infinite recursion in Type Printer.
ClosedPublic

Authored by ayermolo on Aug 29 2022, 5:49 PM.

Details

Summary

There is an implicit circular dependency in a debug information coming from GCC.
This results in a coredump.
It tries to resolve scope for DIE TAGs that are scopeless.

Diff Detail

Event Timeline

ayermolo created this revision.Aug 29 2022, 5:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 29 2022, 5:49 PM
ayermolo requested review of this revision.Aug 29 2022, 5:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 29 2022, 5:49 PM
ayermolo edited the summary of this revision. (Show Details)Aug 29 2022, 5:50 PM
ayermolo added a reviewer: dblaikie.
ayermolo edited the summary of this revision. (Show Details)

Got a small source example and compilation command line (& gcc version etc)? And/or at least full dwarf dump output?

ah sorry.
can repro with gcc 9 and 11, and latest trunk
git clone https://github.com/gcc-mirror/gcc.git
cd gcc/libstdc++-v3/src/filesystem
gcc -g -gdwarf-4 -std=gnu++17 path.cc -o foo.o -c
llvm-dwarfdump --show-form --verbose --debug-info foo.o &> info

I'll try to create a small repro.

Working example:

0x000002a3:   DW_TAG_ptr_to_member_type [15]   (0x0000000b)
                DW_AT_type [DW_FORM_ref4] (cu + 0x0287 => {0x00000287} "ns::inner")
                DW_AT_containing_type [DW_FORM_ref4]  (cu + 0x0287 => {0x00000287} "ns::inner")
0x000002ac:   DW_TAG_ptr_to_member_type [15]   (0x0000000b)
                DW_AT_type [DW_FORM_ref4] (cu + 0x02b5 => {0x000002b5} "ns::inner (ns::inner *, ns::inner)")
                DW_AT_containing_type [DW_FORM_ref4]  (cu + 0x0287 => {0x00000287} "ns::inner")      
0x000002b5:   DW_TAG_subroutine_type [29] * (0x0000000b)
                DW_AT_type [DW_FORM_ref4] (cu + 0x0287 => {0x00000287} "ns::inner")
0x00000287:     DW_TAG_structure_type [16]   (0x00000282)
                  DW_AT_name [DW_FORM_strp] ( .debug_str[0x000000d6] = "inner")
                  DW_AT_declaration [DW_FORM_flag_present]  (true)
0x00000282:   DW_TAG_namespace [30] * (0x0000000b)
                DW_AT_name [DW_FORM_strp] ( .debug_str[0x000000d3] = "ns")
                
0x000000e1:   DW_TAG_template_type_parameter [5]   (0x0000003c)
                DW_AT_type [DW_FORM_ref4] (cu + 0x02a3 => {0x000002a3} "ns::inner ns::inner::*")
0x000000e6:   DW_TAG_template_type_parameter [5]   (0x0000003c)
                DW_AT_type [DW_FORM_ref4] (cu + 0x02ac => {0x000002ac} "ns::inner (ns::inner::*)()")

Couple traces through calls with DIE offsets printed.
0x000000e1: DW_TAG_template_type_parameter [5] (0x0000003c)
DW_AT_type [DW_FORM_ref4] (cu + 0x02a3 => {0x000002a3} “
appendQualifiedName: 2a3
appendUnqualifiedName: 2a3
appendUnqualifiedNameBefore: 2a3
appendQualifiedNameBefore: 287
appendScopes: 282
appendUnqualifiedName: 282
appendUnqualifiedNameBefore: 282
nsappendUnqualifiedNameAfter: 282
::appendUnqualifiedNameBefore: 287
innerappendTemplateParameters: 287

appendQualifiedName: 287
appendScopes: 282
appendUnqualifiedName: 282
appendUnqualifiedNameBefore: 282
nsappendUnqualifiedNameAfter: 282
::appendUnqualifiedName: 287
appendUnqualifiedNameBefore: 287
innerappendTemplateParameters: 287
appendUnqualifiedNameAfter: 287
::*appendUnqualifiedNameAfter: 2a3
appendUnqualifiedNameAfter: 287
")

0x000000e6: DW_TAG_template_type_parameter [5] (0x0000003c)
DW_AT_type [DW_FORM_ref4] (cu + 0x02ac => {0x000002ac} “
appendQualifiedName: 2ac
appendUnqualifiedName: 2ac
appendUnqualifiedNameBefore: 2ac
appendQualifiedNameBefore: 2b5
appendUnqualifiedNameBefore: 2b5
appendQualifiedNameBefore: 287
appendScopes: 282
appendUnqualifiedName: 282
appendUnqualifiedNameBefore: 282
nsappendUnqualifiedNameAfter: 282
::appendUnqualifiedNameBefore: 287
innerappendTemplateParameters: 287
(
appendQualifiedName: 287
appendScopes: 282
appendUnqualifiedName: 282
appendUnqualifiedNameBefore: 282
nsappendUnqualifiedNameAfter: 282
::appendUnqualifiedName: 287
appendUnqualifiedNameBefore: 287
innerappendTemplateParameters: 287
appendUnqualifiedNameAfter: 287
::*appendUnqualifiedNameAfter: 2ac
)appendUnqualifiedNameAfter: 2b5
appendUnqualifiedName: 2b5
(
appendQualifiedName: 287
appendScopes: 282
appendUnqualifiedName: 282
appendUnqualifiedNameBefore: 282
nsappendUnqualifiedNameAfter: 282
::appendUnqualifiedName: 287
appendUnqualifiedNameBefore: 287
innerappendTemplateParameters: 287
appendUnqualifiedNameAfter: 287
)appendUnqualifiedNameAfter: 287
")

Broken case. Stopping recursion after 5 iterations.

0x0000002a:   DW_TAG_namespace [165] * (0x0000000b)
                DW_AT_name [DW_FORM_string] ("std")
                DW_AT_decl_file [DW_FORM_data1] ("/mnt/gvfs/third-party2/libgcc/d1129753c8361ac8e9453c0f4291337a4507ebe6/11.x/platform010/5684a5a/include/c++/11.x/x86_64-facebook-linux/bits/c++config.h")
                DW_AT_decl_line [DW_FORM_data2] (285)
                DW_AT_decl_column [DW_FORM_data1] (0x0b)
                DW_AT_sibling [DW_FORM_ref4]  (cu + 0x10f07 => {0x00010f07})
0x0000d630:     DW_TAG_class_type [46] * (0x0000002a)
                  DW_AT_name [DW_FORM_strp] ( .debug_str[0x0000ab0d] = "codecvt_base")
                  DW_AT_byte_size [DW_FORM_data1] (0x01)
                  DW_AT_decl_file [DW_FORM_data1] ("/mnt/gvfs/third-party2/libgcc/d1129753c8361ac8e9453c0f4291337a4507ebe6/11.x/platform010/5684a5a/include/c++/11.x/bits/codecvt.h")
                  DW_AT_decl_line [DW_FORM_data1] (49)
                  DW_AT_decl_column [DW_FORM_data1] (0x09)
                  DW_AT_sibling [DW_FORM_ref4]  (cu + 0xd667 => {0x0000d667})
0x0000d63d:       DW_TAG_enumeration_type [188] * (0x0000d630)
                    DW_AT_name [DW_FORM_strp] ( .debug_str[0x00006ad2] = "result")
                    DW_AT_encoding [DW_FORM_data1]  (DW_ATE_unsigned)
                    DW_AT_byte_size [DW_FORM_data1] (0x04)
                    DW_AT_type [DW_FORM_ref4] (cu + 0x13950 => {0x00013950} "unsigned int")
                    DW_AT_decl_file [DW_FORM_data1] ("/mnt/gvfs/third-party2/libgcc/d1129753c8361ac8e9453c0f4291337a4507ebe6/11.x/platform010/5684a5a/include/c++/11.x/bits/codecvt.h")
                    DW_AT_decl_line [DW_FORM_data1] (52)
                    DW_AT_decl_column [DW_FORM_data1] (0x0a)
                    DW_AT_accessibility [DW_FORM_data1] (DW_ACCESS_public)
0x0001711e:     DW_TAG_subroutine_type [117] * (0x0001706b)
                  DW_AT_type [DW_FORM_ref4] (cu + 0xd63d => {0x0000d63d} "std::codecvt_base::result")
                  DW_AT_object_pointer [DW_FORM_ref4] (cu + 0x1712b => {0x0001712b})
                  DW_AT_sibling [DW_FORM_ref4]  (cu + 0x1715f => {0x0001715f})
0x0001712b:       DW_TAG_formal_parameter [2]   (0x0001711e)
                    DW_AT_type [DW_FORM_ref4] (cu + 0x17130 => {0x00017130} "std::codecvt_base::result (std::codecvt_base::result (std::codecvt_base::result (std::codecvt_base::result (std::codecvt_base::result (::std::codecvt_base::result (::const _Codecvt ::std::codecvt_base::result (::const _Codecvt ::std::codecvt_base::result (::const _Codecvt ::std::codecvt_base::result (::const _Codecvt ::std::codecvt_base::result (::const _Codecvt *")
0x00017130:       DW_TAG_pointer_type [10]   (0x0001711e)
                    DW_AT_byte_size [DW_FORM_data1] (0x08)
                    DW_AT_type [DW_FORM_ref4] (cu + 0x17136 => {0x00017136} "std::codecvt_base::result (std::codecvt_base::result (std::codecvt_base::result (std::codecvt_base::result (std::codecvt_base::result (std::codecvt_base::result (::std::codecvt_base::result (::const _Codecvt *::std::codecvt_base::result (::const _Codecvt *::std::codecvt_base::result (::const _Codecvt *::std::codecvt_base::result (::const _Codecvt *::std::codecvt_base::result (::const _Codecvt *::const _Codecvt")
0x00017136:       DW_TAG_const_type [9]   (0x0001711e)
                    DW_AT_type [DW_FORM_ref4] (cu + 0x17106 => {0x00017106} "_Codecvt")

0x0001712b: DW_TAG_formal_parameter [2] (0x0001711e)
DW_AT_type [DW_FORM_ref4] (cu + 0x17130 => {0x00017130} “
appendQualifiedName: 17130
appendScopes: 1711e
appendUnqualifiedName: 1711e
appendUnqualifiedNameBefore: 1711e
appendQualifiedNameBefore: d63d
appendScopes: d630
appendScopes: 2a
appendUnqualifiedName: 2a
appendUnqualifiedNameBefore: 2a
stdappendUnqualifiedNameAfter: 2a
::appendUnqualifiedName: d630
appendUnqualifiedNameBefore: d630
codecvt_baseappendTemplateParameters: d630
appendUnqualifiedNameAfter: d630
::appendUnqualifiedNameBefore: d63d
resultappendTemplateParameters: d63d
appendUnqualifiedNameAfter: 1711e
appendUnqualifiedName: 1711e
(
appendQualifiedName: 17130
appendScopes: 1711e
appendUnqualifiedName: 1711e
appendUnqualifiedNameBefore: 1711e
appendQualifiedNameBefore: d63d
appendScopes: d630
appendScopes: 2a
appendUnqualifiedName: 2a
appendUnqualifiedNameBefore: 2a
stdappendUnqualifiedNameAfter: 2a
::appendUnqualifiedName: d630
appendUnqualifiedNameBefore: d630
codecvt_baseappendTemplateParameters: d630
appendUnqualifiedNameAfter: d630
::appendUnqualifiedNameBefore: d63d
resultappendTemplateParameters: d63d
appendUnqualifiedNameAfter: 1711e
appendUnqualifiedName: 1711e
(
appendQualifiedName: 17130
....

0x00017130: DW_TAG_pointer_type [10] (0x0001711e)
DW_AT_byte_size [DW_FORM_data1] (0x08)
DW_AT_type [DW_FORM_ref4] (cu + 0x17136 => {0x00017136} “
appendQualifiedName: 17136
appendScopes: 1711e
appendUnqualifiedName: 1711e
appendUnqualifiedNameBefore: 1711e
appendQualifiedNameBefore: d63d
appendScopes: d630
appendScopes: 2a
appendUnqualifiedName: 2a
appendUnqualifiedNameBefore: 2a
stdappendUnqualifiedNameAfter: 2a
::appendUnqualifiedName: d630
appendUnqualifiedNameBefore: d630
codecvt_baseappendTemplateParameters: d630
appendUnqualifiedNameAfter: d630
::appendUnqualifiedNameBefore: d63d
resultappendTemplateParameters: d63d
appendUnqualifiedNameAfter: 1711e
appendUnqualifiedName: 1711e
(
appendQualifiedName: 17130
appendScopes: 1711e
appendUnqualifiedName: 1711e
appendUnqualifiedNameBefore: 1711e
appendQualifiedNameBefore: d63d
appendScopes: d630
appendScopes: 2a
appendUnqualifiedName: 2a
appendUnqualifiedNameBefore: 2a
stdappendUnqualifiedNameAfter: 2a
::appendUnqualifiedName: d630
appendUnqualifiedNameBefore: d630
codecvt_baseappendTemplateParameters: d630
appendUnqualifiedNameAfter: d630
::appendUnqualifiedNameBefore: d63d
resultappendTemplateParameters: d63d
appendUnqualifiedNameAfter: 1711e
appendUnqualifiedName: 1711e
(
appendQualifiedName: 17130
....

ayermolo added a comment.EditedAug 31 2022, 4:17 PM

"Minimal" repro. Still relies on some std stuff. :(

#include <streambuf>
#include <bits/codecvt.h>
namespace std
{
    inline bool
    __str_codecvt_out()
    {
      using _State = std::mbstate_t;
      using _CharT = wchar_t;
      using _Codecvt = codecvt<_CharT, char, _State>;
      using _ConvFn
  = codecvt_base::result
    (_Codecvt::*)(_State&, const _CharT*, const _CharT*, const _CharT*&,
      char*, char*, char*&) const;
      _ConvFn __fn = &codecvt<_CharT, char, _State>::out;
      return true;
    }

bool test() {
    return __str_codecvt_out();
}
}

If you could provide preprocessed source (ideally reduced further with cvise or similar) that'd be good. (I'm still out for a week, or I'd look at it further/sooner.

If you could provide preprocessed source (ideally reduced further with cvise or similar) that'd be good. (I'm still out for a week, or I'd look at it further/sooner.

Yeah, will try to reduce it more later this week.

ayermolo added a comment.EditedSep 1 2022, 6:40 PM

OK. I think this is a minimal self contained repro:

class codecvt_base
{
  public:
    enum result {
      ok
    };
};

class __codecvt_abstract_base
    : public codecvt_base
{
    public:
      result out(const wchar_t* __from) const {
	    return result::ok;
      }
    };

    class codecvt
    : public __codecvt_abstract_base{};

    inline bool
    __str_codecvt_out()
    {
      using _Codecvt = codecvt;
      using _ConvFn = codecvt_base::result
    (_Codecvt::*)(const wchar_t*) const;
      _ConvFn __fn = &codecvt::out;
      return true;
    }

bool test() {
    return __str_codecvt_out();
}

Replacing (_Codecvt::*)(const wchar_t*) const; with (codecvt::*)(const wchar_t*) const;
Produces debug info that can be parsed by dwarfdumper.

Reduced down to:

struct t1;

void f1() {
  using t2 = t1;
  void (t2::* __fn)();
}

Still investigating.

Reduced down to:

struct t1;

void f1() {
  using t2 = t1;
  void (t2::* __fn)();
}

Still investigating.

Ah! Thanks.

dblaikie added inline comments.Sep 9 2022, 12:12 PM
llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp
301–303

Avoid the extra lookup.

Though more generally this solution is probably incorrect - presumably it would cause problems for any type that isn't circular, but references the same type twice? eg: "void(x::y, x::y)" would fail/bail out unnecessarily?

To catch the recursion correctly, we'd probably have to use scope chains (walking back up the list of "these are the types we're printing" and see if the type is in the list) or use scoped insertion/removal into the set (each time we go down into a type we add to the set, each time we back out, we remove from the set)?

Reduced down to:

struct t1;

void f1() {
  using t2 = t1;
  void (t2::* __fn)();
}

Still investigating.

Ah, so the critical difference between GCC and Clang's DWARF on this is that GCC creates the pointer type (when it's pointer-to-local-typedef) inside the subroutine_type (it's probably trying to create it inside the function, since the pointer type, while not strictly scoped, would only be usable inside the function type anyway) - this creates the infinite recursion when trying to print the scope of the pointer type (really a pointer type is unscoped, since it's unnamed - maybe we could fix it so we don't print out any scope info for unnameable types (anonymous structs/classes/etc are still logically scoped even though they don't have a name, for instance - but pointer types never really have a scope/name, just a representation))

ayermolo added inline comments.Sep 9 2022, 2:49 PM
llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp
301–303

oh yeah I don't think this is a right way. It breaks at least one unit test.

This comment was removed by ayermolo.
ayermolo updated this revision to Diff 459586.EditedSep 12 2022, 4:17 PM

Something like this? With other types to add that do not have scope, and test.

ayermolo updated this revision to Diff 459587.Sep 12 2022, 4:25 PM

some cleanup

dblaikie added inline comments.Sep 12 2022, 5:32 PM
llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp
300–301

Might be inclined to use a switch here, rather than a set lookup - and it's probably easier to make a positive list of the things that are scoped than those that aren't.

ayermolo added inline comments.Sep 14 2022, 2:17 PM
llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp
300–301

Unfortunately this doesn't work for:

class codecvt_base
{
  public:
    enum result {
      ok
    };
};

class __codecvt_abstract_base
    : public codecvt_base
{
    public:
      result out(const wchar_t* __from) const {
      return result::ok;
      }
    };

    class codecvt
    : public __codecvt_abstract_base{};

    inline bool
    __str_codecvt_out()
    {
      using _Codecvt = codecvt;
      using _ConvFn = codecvt_base::result
    (_Codecvt::*)(const wchar_t*) const;
      _ConvFn __fn = &codecvt::out;
      return true;
    }

bool test() {
    return __str_codecvt_out();
}

It prints out this before eventually crashing.
0x00000133: DW_TAG_formal_parameter [8] (0x00000126)

DW_AT_type [DW_FORM_ref4] (cu + 0x0138 => {0x00000138} "codecvt_base::result (codecvt_base::result (codecvt_base::result (codecvt_base::result (codecvt_base::result (codecvt_base::result (codecvt_base::result (codecvt_base::result (codecvt_base::result (codecvt_base::result (::codecvt_base::result (::const _Codecvt *::codecvt_base::result (::const _Codecvt *::codecvt_base::result (::const _Codecvt *::codecvt_base::result (::const _Codecvt *::codecvt_base::result (::const _Codecvt *::codecvt_base::result (::const _Codecvt *::codecvt_base::result (::const _Codecvt *::codecvt_base::result (::const _Codecvt *::codecvt_base::result (::const _Codecvt *::codecvt_base::result (::const _Codecvt *")

Maybe better approach is to apply this allow list approach to DWARFTypePrinter::appendScopes? So it will return unless it's one of those TAGs.

dblaikie added inline comments.Sep 15 2022, 1:27 PM
llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp
300–301

I don't think it'd work to do this in appendScopes because appendScopes gets passed the scope, not the thing that's being scoped - and the determination on whether to skip scopes should be on the type being scoped, not the scoping.

Could you reduce the test down a bit - help understand what's interesting about this? I guess there's some other place that appends scopes that needs handling? Looks like the only other uses is appendQualifiedNameBefore - so I guess the handling needs to be in both of those, which sounds plausible/reasonable.

But yeah, maybe sinking a wrapper. appendQualification or something, which just does the if (D) / check the type, then appendScopes(D.getParent())

ayermolo added inline comments.Sep 15 2022, 4:36 PM
llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp
300–301

k, will try next week. Thx for helping with this.

ayermolo added a comment.EditedSep 19 2022, 4:08 PM
class codecvt
{
  public:
    void out() const {}
};


void test() {
  using _Codecvt = codecvt;
  using _ConvFn = void(_Codecvt::*)() const;
  _ConvFn __fn = &_Codecvt::out;
}

So in both cases key to pathological case seems to be it's going through:
appendSubroutineNameAfter: DW_TAG_subroutine_type
appendQualifiedName: DW_TAG_pointer_type

ayermolo added inline comments.Sep 19 2022, 5:34 PM
llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp
544

Maybe here is a better check? Something like D.getTag() == DW_TAG_subroutine_type, and also check T tag?

class codecvt
{
  public:
    void out() const {}
};


void test() {
  using _Codecvt = codecvt;
  using _ConvFn = void(_Codecvt::*)() const;
  _ConvFn __fn = &_Codecvt::out;
}

So in both cases key to pathological case seems to be it's going through:
appendSubroutineNameAfter: DW_TAG_subroutine_type
appendQualifiedName: DW_TAG_pointer_type

So in this test case, with the patch I proposed (to have an explicit allowed list of names that get scopes, and so everything else doesn't get scopes) you got infinite recursion? Could you paste the stack trace in more detail - I'm a bit confused how we'd get infinite recursion on a pointer type if we're not printing scopes for pointer types anymore.

llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp
544

I'm not sure what you're referring to here, unfortunately - you mean adding a condition here in appendSubroutineNameAfter, around the appendQualifiedName call? I don't think so, but I'm not sure.

Circled back with fresh eyes. So this example passes with trunk. So looks like current allow list (below), regresses it.

Stack dump:
0.  Program arguments: /home/ayermolo/local/llvm-build-debug/bin/llvm-dwarfdump --show-form --verbose --debug-info --choose-strategy=1 --enable-verbose-mode=false repro.o
  #0 0x000055cc332c360a llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/Support/Unix/Signals.inc:569:11
  #1 0x000055cc332c37bb PrintStackTraceSignalHandler(void*) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/Support/Unix/Signals.inc:636:1
  #2 0x000055cc332c1e36 llvm::sys::RunSignalHandlers() /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/Support/Signals.cpp:103:5
  #3 0x000055cc332c3ea5 SignalHandler(int) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/Support/Unix/Signals.inc:407:1
  #4 0x00007f54e2edbce0 __restore_rt (/lib64/libpthread.so.0+0x12ce0)
  #5 0x000055cc32d645a4 llvm::StringRef::size() const /home/ayermolo/local/server-llvm/llvm-project/llvm/include/llvm/ADT/StringRef.h:137:0
  #6 0x000055cc32dbafe5 llvm::DataExtractor::size() const /home/ayermolo/local/server-llvm/llvm-project/llvm/include/llvm/Support/DataExtractor.h:688:25
  #7 0x000055cc32db7ee9 llvm::DataExtractor::isValidOffset(unsigned long) const /home/ayermolo/local/server-llvm/llvm-project/llvm/include/llvm/Support/DataExtractor.h:665:61
  #8 0x000055cc32db8348 llvm::DataExtractor::isValidOffsetForDataOfSize(unsigned long, unsigned long) const /home/ayermolo/local/server-llvm/llvm-project/llvm/include/llvm/Support/DataExtractor.h:673:41
  #9 0x000055cc331d9500 llvm::DataExtractor::prepareRead(unsigned long, unsigned long, llvm::Error*) const /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/Support/DataExtractor.cpp:19:7
 #10 0x000055cc331da839 unsigned int llvm::DataExtractor::getU<unsigned int>(unsigned long*, llvm::Error*) const /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/Support/DataExtractor.cpp:47:7
 #11 0x000055cc331d97c5 llvm::DataExtractor::getU32(unsigned long*, llvm::Error*) const /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/Support/DataExtractor.cpp:108:3
 #12 0x000055cc331d9912 llvm::DataExtractor::getUnsigned(unsigned long*, unsigned int, llvm::Error*) const /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/Support/DataExtractor.cpp:133:12
 #13 0x000055cc32dfac8b llvm::DWARFDataExtractor::getRelocatedValue(unsigned int, unsigned long*, unsigned long*, llvm::Error*) const /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFDataExtractor.cpp:58:12
 #14 0x000055cc32e52caa llvm::DWARFFormValue::extractValue(llvm::DWARFDataExtractor const&, unsigned long*, llvm::dwarf::FormParams, llvm::DWARFContext const*, llvm::DWARFUnit const*) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFFormValue.cpp:311:25
 #15 0x000055cc32e005f9 llvm::DWARFFormValue::extractValue(llvm::DWARFDataExtractor const&, unsigned long*, llvm::dwarf::FormParams, llvm::DWARFUnit const*) /home/ayermolo/local/server-llvm/llvm-project/llvm/include/llvm/DebugInfo/DWARF/DWARFFormValue.h:104:5
 #16 0x000055cc32dffd93 llvm::DWARFAbbreviationDeclaration::getAttributeValueFromOffset(unsigned int, unsigned long, llvm::DWARFUnit const&) const /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp:181:7
 #17 0x000055cc32dffe7c llvm::DWARFAbbreviationDeclaration::getAttributeValue(unsigned long, llvm::dwarf::Attribute, llvm::DWARFUnit const&) const /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp:0:10
 #18 0x000055cc32e3dd9a llvm::DWARFDie::find(llvm::dwarf::Attribute) const /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp:257:5
 #19 0x000055cc32e3e1e8 llvm::DWARFDie::getAttributeValueAsReferencedDie(llvm::dwarf::Attribute) const /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFDie.cpp:310:32
 #20 0x000055cc32e462b4 llvm::resolveReferencedType(llvm::DWARFDie, llvm::dwarf::Attribute) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:94:12
 #21 0x000055cc32e47051 llvm::DWARFTypePrinter::appendUnqualifiedNameBefore(llvm::DWARFDie, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*)::$_0::operator()() const /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:135:40
 #22 0x000055cc32e466ef llvm::DWARFTypePrinter::appendUnqualifiedNameBefore(llvm::DWARFDie, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:139:36
 #23 0x000055cc32e498e7 llvm::DWARFTypePrinter::appendUnqualifiedName(llvm::DWARFDie, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:572:22
 #24 0x000055cc32e472d3 llvm::DWARFTypePrinter::appendQualifiedName(llvm::DWARFDie) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:351:1
 #25 0x000055cc32e4905b llvm::DWARFTypePrinter::appendSubroutineNameAfter(llvm::DWARFDie, llvm::DWARFDie, bool, bool, bool) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:584:5
 #26 0x000055cc32e4898d llvm::DWARFTypePrinter::appendUnqualifiedNameAfter(llvm::DWARFDie, llvm::DWARFDie, bool) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:253:5
 #27 0x000055cc32e4992b llvm::DWARFTypePrinter::appendUnqualifiedName(llvm::DWARFDie, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:574:3
 #28 0x000055cc32e4986e llvm::DWARFTypePrinter::appendScopes(llvm::DWARFDie) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:707:5
 #29 0x000055cc32e46506 llvm::DWARFTypePrinter::appendQualifiedNameBefore(llvm::DWARFDie) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:0:7
 #30 0x000055cc32e463f2 llvm::DWARFTypePrinter::appendPointerLikeTypeBefore(llvm::DWARFDie, llvm::DWARFDie, llvm::StringRef) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:115:3
 #31 0x000055cc32e4673d llvm::DWARFTypePrinter::appendUnqualifiedNameBefore(llvm::DWARFDie, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:140:5
 #32 0x000055cc32e498e7 llvm::DWARFTypePrinter::appendUnqualifiedName(llvm::DWARFDie, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:572:22
 #33 0x000055cc32e472d3 llvm::DWARFTypePrinter::appendQualifiedName(llvm::DWARFDie) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:351:1
 #34 0x000055cc32e4905b llvm::DWARFTypePrinter::appendSubroutineNameAfter(llvm::DWARFDie, llvm::DWARFDie, bool, bool, bool) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:584:5
 #35 0x000055cc32e4898d llvm::DWARFTypePrinter::appendUnqualifiedNameAfter(llvm::DWARFDie, llvm::DWARFDie, bool) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:253:5
 #36 0x000055cc32e4992b llvm::DWARFTypePrinter::appendUnqualifiedName(llvm::DWARFDie, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:574:3
 #37 0x000055cc32e4986e llvm::DWARFTypePrinter::appendScopes(llvm::DWARFDie) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:707:5
 #38 0x000055cc32e46506 llvm::DWARFTypePrinter::appendQualifiedNameBefore(llvm::DWARFDie) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:0:7
 #39 0x000055cc32e463f2 llvm::DWARFTypePrinter::appendPointerLikeTypeBefore(llvm::DWARFDie, llvm::DWARFDie, llvm::StringRef) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:115:3
 #40 0x000055cc32e4673d llvm::DWARFTypePrinter::appendUnqualifiedNameBefore(llvm::DWARFDie, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:140:5
 #41 0x000055cc32e498e7 llvm::DWARFTypePrinter::appendUnqualifiedName(llvm::DWARFDie, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:572:22
 #42 0x000055cc32e472d3 llvm::DWARFTypePrinter::appendQualifiedName(llvm::DWARFDie) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:351:1
 #43 0x000055cc32e4905b llvm::DWARFTypePrinter::appendSubroutineNameAfter(llvm::DWARFDie, llvm::DWARFDie, bool, bool, bool) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:584:5
 #44 0x000055cc32e4898d llvm::DWARFTypePrinter::appendUnqualifiedNameAfter(llvm::DWARFDie, llvm::DWARFDie, bool) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:253:5
 #45 0x000055cc32e4992b llvm::DWARFTypePrinter::appendUnqualifiedName(llvm::DWARFDie, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*) /home/ayermolo/local/server-llvm/llvm-project/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp:574:3

Print out with TAGS:

0x000000a1:       DW_TAG_formal_parameter [4]   (0x00000098)
                    DW_AT_type [DW_FORM_ref4] (cu + 0x00a6 => {0x000000a6} "
appendQualifiedName:  a6 : DW_TAG_pointer_type
appendUnqualifiedName:  a6 : DW_TAG_pointer_type
appendUnqualifiedNameBefore:  a6 : DW_TAG_pointer_type
appendPointerLikeTypeBefore:  a6 : DW_TAG_pointer_type
appendQualifiedNameBefore:  ac : DW_TAG_const_type
appendScopes:   98 : DW_TAG_subroutine_type
appendScopes:   5e : DW_TAG_subprogram
appendUnqualifiedName:  98 : DW_TAG_subroutine_type
appendUnqualifiedNameBefore:  98 : DW_TAG_subroutine_type
void appendUnqualifiedNameAfter:  98 : DW_TAG_subroutine_type
appendSubroutineNameAfter:  98 : DW_TAG_subroutine_type
(
appendQualifiedName:  a6 : DW_TAG_pointer_type
appendUnqualifiedName:  a6 : DW_TAG_pointer_type
appendUnqualifiedNameBefore:  a6 : DW_TAG_pointer_type
appendPointerLikeTypeBefore:  a6 : DW_TAG_pointer_type
appendQualifiedNameBefore:  ac : DW_TAG_const_type
appendScopes:   98 : DW_TAG_subroutine_type
appendScopes:   5e : DW_TAG_subprogram
appendUnqualifiedName:  98 : DW_TAG_subroutine_type
appendUnqualifiedNameBefore:  98 : DW_TAG_subroutine_type
void appendUnqualifiedNameAfter:  98 : DW_TAG_subroutine_type
appendSubroutineNameAfter:  98 : DW_TAG_subroutine_type
(
appendQualifiedName:  a6 : DW_TAG_pointer_type
appendUnqualifiedName:  a6 : DW_TAG_pointer_type
appendUnqualifiedNameBefore:  a6 : DW_TAG_pointer_type
appendPointerLikeTypeBefore:  a6 : DW_TAG_pointer_type
appendQualifiedNameBefore:  ac : DW_TAG_const_type
appendScopes:   98 : DW_TAG_subroutine_type
appendScopes:   5e : DW_TAG_subprogram
appendUnqualifiedName:  98 : DW_TAG_subroutine_type
appendUnqualifiedNameBefore:  98 : DW_TAG_subroutine_type
void appendUnqualifiedNameAfter:  98 : DW_TAG_subroutine_type
appendSubroutineNameAfter:  98 : DW_TAG_subroutine_type
switch (D.getTag()) {
      case dwarf::DW_TAG_structure_type:
      case dwarf::DW_TAG_class_type:
      case dwarf::DW_TAG_union_type:
      case dwarf::DW_TAG_namespace:
      case DW_TAG_enumeration_type:
        appendScopes(D.getParent());
        break;
      default:
        break;
      }
ayermolo added inline comments.Sep 22 2022, 3:56 PM
llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp
544

Yep exactly.

So, I'm not sure what failures you're seeing, but I tried this locally:

$ git diff
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp b/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp
index 2d8add64537c..969a6525d821 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp
@@ -284,12 +284,32 @@ void DWARFTypePrinter::appendUnqualifiedNameAfter(
 
 void DWARFTypePrinter::appendQualifiedName(DWARFDie D) {
   if (D)
-    appendScopes(D.getParent());
+    switch (D.getTag()) {
+    case dwarf::DW_TAG_structure_type:
+    case dwarf::DW_TAG_class_type:
+    case dwarf::DW_TAG_union_type:
+    case dwarf::DW_TAG_namespace:
+    case dwarf::DW_TAG_enumeration_type:
+      appendScopes(D.getParent());
+      break;
+    default:
+      break;
+    }
   appendUnqualifiedName(D);
 }
 DWARFDie DWARFTypePrinter::appendQualifiedNameBefore(DWARFDie D) {
   if (D)
-    appendScopes(D.getParent());
+    switch (D.getTag()) {
+    case dwarf::DW_TAG_structure_type:
+    case dwarf::DW_TAG_class_type:
+    case dwarf::DW_TAG_union_type:
+    case dwarf::DW_TAG_namespace:
+    case dwarf::DW_TAG_enumeration_type:
+      appendScopes(D.getParent());
+      break;
+    default:
+      break;
+    }
   return appendUnqualifiedNameBefore(D);
 }
 bool DWARFTypePrinter::appendTemplateParameters(DWARFDie D,
@@ -516,7 +536,7 @@ void DWARFTypePrinter::appendSubroutineNameAfter(
   for (DWARFDie P : D) {
     if (P.getTag() != DW_TAG_formal_parameter &&
         P.getTag() != DW_TAG_unspecified_parameters)
-      return;
+      continue;
     DWARFDie T = resolveReferencedType(P);
     if (SkipFirstParamIfArtificial && RealFirst && P.find(DW_AT_artificial)) {
       FirstParamIfArtificial = T;

and I tried the initial test case and the follow-up one and they seem to work correctly without assertions/crashes/infinite recursion for me.

Can you help me understand what you're hitting up against?

(this ^ patch isn't quite ready - would be good to refactor out the two switches into a single switch to avoid code duplication & possibly adding extra test coverage for that return -> continue change that lets the function type be fully printed despite non-parameter DIEs in the subroutine type)

So, I'm not sure what failures you're seeing, but I tried this locally:

$ git diff
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp b/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp
index 2d8add64537c..969a6525d821 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp
@@ -284,12 +284,32 @@ void DWARFTypePrinter::appendUnqualifiedNameAfter(
 
 void DWARFTypePrinter::appendQualifiedName(DWARFDie D) {
   if (D)
-    appendScopes(D.getParent());
+    switch (D.getTag()) {
+    case dwarf::DW_TAG_structure_type:
+    case dwarf::DW_TAG_class_type:
+    case dwarf::DW_TAG_union_type:
+    case dwarf::DW_TAG_namespace:
+    case dwarf::DW_TAG_enumeration_type:
+      appendScopes(D.getParent());
+      break;
+    default:
+      break;
+    }
   appendUnqualifiedName(D);
 }
 DWARFDie DWARFTypePrinter::appendQualifiedNameBefore(DWARFDie D) {
   if (D)
-    appendScopes(D.getParent());
+    switch (D.getTag()) {
+    case dwarf::DW_TAG_structure_type:
+    case dwarf::DW_TAG_class_type:
+    case dwarf::DW_TAG_union_type:
+    case dwarf::DW_TAG_namespace:
+    case dwarf::DW_TAG_enumeration_type:
+      appendScopes(D.getParent());
+      break;
+    default:
+      break;
+    }
   return appendUnqualifiedNameBefore(D);
 }
 bool DWARFTypePrinter::appendTemplateParameters(DWARFDie D,
@@ -516,7 +536,7 @@ void DWARFTypePrinter::appendSubroutineNameAfter(
   for (DWARFDie P : D) {
     if (P.getTag() != DW_TAG_formal_parameter &&
         P.getTag() != DW_TAG_unspecified_parameters)
-      return;
+      continue;
     DWARFDie T = resolveReferencedType(P);
     if (SkipFirstParamIfArtificial && RealFirst && P.find(DW_AT_artificial)) {
       FirstParamIfArtificial = T;

and I tried the initial test case and the follow-up one and they seem to work correctly without assertions/crashes/infinite recursion for me.

Can you help me understand what you're hitting up against?

(this ^ patch isn't quite ready - would be good to refactor out the two switches into a single switch to avoid code duplication & possibly adding extra test coverage for that return -> continue change that lets the function type be fully printed despite non-parameter DIEs in the subroutine type)

Sorry was on PTO.
Ah, so I only had the check in appendQualifiedName. I might have miss understood your original comment. Thanks for pointing this out!

ayermolo updated this revision to Diff 463025.Sep 26 2022, 2:37 PM

new version

ayermolo marked 3 inline comments as done.Sep 26 2022, 2:37 PM
ayermolo retitled this revision from [DWARF] Fix infinite recursion in Type Printer. to [DWARF DUMP] Fix infinite recursion in Type Printer..
ayermolo edited the summary of this revision. (Show Details)
dblaikie accepted this revision.Sep 26 2022, 3:08 PM

Sounds good, thanks!

llvm/lib/DebugInfo/DWARF/DWARFTypePrinter.cpp
294

no need for the break after return

This revision is now accepted and ready to land.Sep 26 2022, 3:08 PM
ayermolo updated this revision to Diff 463038.Sep 26 2022, 3:22 PM
ayermolo edited the summary of this revision. (Show Details)

address comment

ayermolo marked 2 inline comments as done.Sep 26 2022, 3:22 PM

Thanks for all the help!