[lldb] Add asserts that prevent construction of cycles in the decl origin tracking
LLDB tracks where any imported `clang::Decl` originally came from via a simple map from 'imported decl' to 'original decl'. That information is used to later complete parts of the Decl when more information is requested about a certain Decl (e.g., via the ExternalASTSource interface from Clang). When finding the 'original decl' for a given decl, the ASTImporterDelegate essentially just recursively follows the previously mentioned map from 'imported' to 'original decl' until it can find any further 'original decl'. The final found decl is then the one that will be imported. The recursion is necessary as in LLDB we don't just import decls from one ASTContext to another, but also from one ASTContext to another via a (potentially temporary) ASTContext. For example, the expression parser creates a temporary ASTContext for parsing the current expression. The problem with the recursion is however that if we somehow get a cycle into our mapping, then the ASTImporterDelegate will just infinite recurse. As the infinite recursion usually happens after the cycle was already created in a code path such as completing a type, the crash backtraces we get for these bugs are not very useful. However having the backtrace where the faulty map entry is created usually makes the code trivial to fix (as there should be some rogue CopyType call or something similar nearby. See for example D96366). This patch tries to make these issues easier to track down by putting a bunch of sanity asserts in the code that fills out the map. All the asserts are just checking that there is no direct cycle (ASTContext maps to itself) when updating the origin tracking map. The assert in the ASTImportDelegate constructor is an `lldbassert` (which also is getting checked in release builds with disabled asserts) as the code path there is pretty cold and we can reliably detect a rogue CopyType call from there. I also had to update some code in `ClangASTImporter::ASTImporterDelegate::Imported`. This code already had a safety check for creating a cycle in the origin tracking map, but it still constructed an ASTImporter while checking for the cycle (by requesting a delegate via `GetDelegate` and passing two identical ASTContexts which looks like a rogue CopyType call to the checks). Reviewed By: shafik Differential Revision: https://reviews.llvm.org/D97300
Loading
Please register or sign in to comment