[Lldb-commits] [clang] [lldb] [ASTImporter][lldb] Avoid implicit imports in VisitFieldDecl (PR #107828)

[Michael Buch via lldb-commits]

Michael137 wrote:

Thanks for taking a stab at this and for the analysis. You rightly point out that this is the root problem: 
 
> If an implicit import happens between (2) and (3), it may indirectly bring the same field into the context. When (3) happens we add it again, duplicating the field and breaking the record. This is not detected until we crash later during layout computation:

And the LLDB test that was XFAILed, is a special case of exactly this problem (summarized well in https://reviews.llvm.org/D102993).

The more complete solution here is to make LLDB stop relying on this sort of import-pattern. We've been told in the past that the ASTImporter just wasn't designed for this use-case. Ideally, we'd remove the need for minimally importing record types (which is actually something I've been working on, though I can't promise a timeline of when this is supposed to land). In my opinion, adding complexity like this into the ASTImporter just to support LLDB is going to make it a maintenance burden on both. So I'd vote for trying to solve this properly.

I do wonder whether there's something about the build configuration that makes you hit this crash more likely in UE. Are you building with precompiled headers by any chance? Or with `-flimit-debug-info`?

I'm actually looking at a similar crash at the moment for which I have a reproducer. I'm trying to extract it into a smaller test-case.

https://github.com/llvm/llvm-project/pull/107828