Issue 90938
Summary LLDB "forcefully completed" types cause an assert in clang
Labels lldb
Assignees
Reporter asavonic
    When C++ record types are completed in `DWARFASTParserClang::CompleteRecordType` [DWARFASTParserClang.cpp:2239](https://github.com/llvm/llvm-project/blob/6b948705a05261a2ff31cd7e6ea8319d1852ddfc/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L2239), declarations of base types are "forced" to be completed if no definition is found yet. 

This is done to get through the clang interface and not crash immediately, because incomplete base types are not allowed. Since we don't have real definition data for them, they are completed with default properties.

However, in some cases these properties are not valid. For example, clang requires the PrimaryBase class to be polymorphic ([RecordLayoutBuilder.cpp:886](https://github.com/llvm/llvm-project/blob/37f6ba4fb2db2c78cda7d0a69cd0a2eff2b924e3/clang/lib/AST/RecordLayoutBuilder.cpp#L886)) and have zero offset ([RecordLayout.cpp:86](https://github.com/llvm/llvm-project/blob/37f6ba4fb2db2c78cda7d0a69cd0a2eff2b924e3/clang/lib/AST/RecordLayout.cpp#L86)).

If the expected primary base class is forcefully completed with default properties, then it is not polymorphic. Therefore it is never selected as the primary base, and the compiler picks the next polymorphic base class in the list, which is not at zero offset, so we get a crash in ASTRecordLayout ctor in [RecordLayout.cpp:86](https://github.com/llvm/llvm-project/blob/37f6ba4fb2db2c78cda7d0a69cd0a2eff2b924e3/clang/lib/AST/RecordLayout.cpp#L86).

This issue appears in Unreal Engine 5.3.2 build (downloaded from the Epic website) on Linux with LLDB 18.1. I reduced it into a short LIT test based on DWARF from UE, and it triggers a crash ASTRecordLayout ctor that originate from [TypeSystemClang::GetChildCompilerTypeAtIndex](https://github.com/llvm/llvm-project/blob/37f6ba4fb2db2c78cda7d0a69cd0a2eff2b924e3/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp#L6184). Unreal Engine also triggers the same issue in [ClangASTImporter::importRecordLayoutFromOrigin](https://github.com/llvm/llvm-project/blob/37f6ba4fb2db2c78cda7d0a69cd0a2eff2b924e3/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp#L664), but I don't have a short reproducer for it yet.

I've tried to workaround the issue by adding an early-exit to TypeSystemClang::GetChildCompilerTypeAtIndex and ClangASTImporter::importRecordLayoutFromOrigin before we attempt to compute a record layout with a forcefully completed base class.

While it does prevent a crash, clang (or LLDB) now treat this forcefully complete base class as empty, and compute the layout
incorrectly. This affects cases that managed to avoid the crash before. In [TestLimitDebugInfo.py](https://github.com/llvm/llvm-project/blob/37f6ba4fb2db2c78cda7d0a69cd0a2eff2b924e3/lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py#L252) this results in an incorrect offset for `inherits_from_two.two`: instead of `two`, it gets the value of `one`. It also breaks the diagnostic added in d941fcec "Add the ability to see when a type in incomplete".

I think the same issue is described in https://discourse.llvm.org/t/rfc-lldb-more-reliable-completion-of-record-types/77442,
but I haven't checked if the proposed patch fixes it. This approach may regress performance, so timeline for it is unclear.
However, if a workaround is not possible, perhaps it is the way to go.

I've uploaded the reproducer (force-complete.test), and the current workaround to https://github.com/asavonic/llvm-project/commit/02c174b76ee4e9e3f052d2a5c51f057d19eb116a.
_______________________________________________
llvm-bugs mailing list
llvm-bugs@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs

Reply via email to