[lldb] Fix a crash when the ASTImporter is giving us two Imported callbacks...
[lldb] Fix a crash when the ASTImporter is giving us two Imported callbacks for the same target decl The ASTImporter has an `Imported(From, To)` callback that notifies subclasses that a declaration has been imported in some way. LLDB uses this in the `CompleteTagDeclsScope` to see which records have been imported into the scratch context. If the record was declared inside the expression, then the `CompleteTagDeclsScope` will forcibly import the full definition of that record to the scratch context so that the expression AST can safely be disposed later (otherwise we might end up going back to the deleted AST to complete the minimally imported record). The way this is implemented is that there is a list of decls that need to be imported (`m_decls_to_complete`) and we keep completing the declarations inside that list until the list is empty. Every `To` Decl we get via the `Imported` callback will be added to the list of Decls to be completed. There are some situations where the ASTImporter will actually give us two `Imported` calls with the same `To` Decl. One way where this happens is if the ASTImporter decides to merge an imported definition into an already imported one. Another way is that the ASTImporter just happens to get two calls to `ASTImporter::Import` for the same Decl. This for example happens when importing the DeclContext of a Decl requires importing the Decl itself, such as when importing a RecordDecl that was declared inside a function. The bug addressed in this patch is that when we end up getting two `Imported` calls for the same `To` Decl, then we would crash in the `CompleteTagDeclsScope`. That's because the first time we complete the Decl we remove the Origin tracking information (that maps the Decl back to from where it came from). The next time we try to complete the same `To` Decl the Origin tracking information is gone and we hit the `to_context_md->getOrigin(decl).ctx == m_src_ctx` assert (`getOrigin(decl).ctx` is a nullptr the second time as the Origin was deleted). This is actually a regression coming from D72495. Before D72495 `m_decls_to_complete` was actually a set so every declaration in there could only be queued once to be completed. The set was changed to a vector to make the iteration over it deterministic, but that also causes that we now potentially end up trying to complete a Decl twice. This patch essentially just reverts D72495 and makes the `CompleteTagDeclsScope` use a SetVector for the list of declarations to be completed. The SetVector should filter out the duplicates (as the original `set` did) and also ensure that the completion order is deterministic. I actually couldn't find any way to cause LLDB to reproduce this bug by merging declarations (this would require that we for example declare two namespaces in a non-top-level expression which isn't possible). But the bug reproduces very easily by just declaring a class in an expression, so that's what the test is doing. Reviewed By: shafik Differential Revision: https://reviews.llvm.org/D85648
Loading
Please sign in to comment