HomePhabricator

[lldb] Disable minimal import mode for RecordDecls that back FieldDecls

Authored by teemperor on Tue, May 25, 2:53 AM.

Description

[lldb] Disable minimal import mode for RecordDecls that back FieldDecls

Clang adds a Decl in two phases to a DeclContext. First it adds it invisible and
then it makes it visible (which will add it to the lookup data structures). It's
important that we can't do lookups into the DeclContext we are currently adding
the Decl to during this process as once the Decl has been added, any lookup will
automatically build a new lookup map and add the added Decl to it. The second
step would then add the Decl a second time to the lookup which will lead to
weird errors later one. I made adding a Decl twice to a lookup an assertion
error in D84827.

In the first step Clang also does some computations on the added Decl if it's
for example a FieldDecl that is added to a RecordDecl.

One of these computations is checking if the FieldDecl is of a record type
and the record type has a deleted constexpr destructor which will delete
the constexpr destructor of the record that got the FieldDecl.

This can lead to a bug with the way we implement MinimalImport in LLDB
and the following code:

struct Outer {
  typedef int HookToOuter;
  struct NestedClass {
    HookToOuter RefToOuter;
  } NestedClassMember; // We are adding this.
};
  1. We just imported Outer minimally so far.
  2. We are now asked to add NestedClassMember as a FieldDecl.
  3. We import NestedClass minimally.
  4. We add NestedClassMember and clang does a lookup for a constexpr dtor in NestedClass. NestedClassMember hasn't been added to the lookup.
  5. The lookup into NestedClass will now load the members of NestedClass.
  6. We try to import the type of RefToOuter which will try to import the HookToOuter typedef.
  7. We import the typedef and while importing we check for conflicts in Outer via a lookup.
  8. The lookup into Outer will cause the invisible NestedClassMember to be added to the lookup.
  9. We continue normally until we get back to the addDecl call in step 2.
  10. We now add NestedClassMember to the lookup even though we already did that in step 8.

The fix here is disabling the minimal import for RecordTypes from FieldDecls. We
actually already did this, but so far we only force the definition of the type
to be imported *after* we imported the FieldDecl. This just moves that code
*before* we import the FieldDecl so prevent the issue above.

Reviewed By: shafik, aprantl

Differential Revision: https://reviews.llvm.org/D102993

Details