[cfe-dev] Completing CXXRecordDecls when .bases() is called.

[Richard Smith via cfe-dev]

On 28 July 2017 at 18:23, Lang Hames <lhames at gmail.com> wrote:

> Hi Cfe-dev, Richard,
>
> I've run into a test case in LLDB where we crash evaluating an expression
> in the following program:
>
> class Foo {};
> class Bar : public Foo {};
>
> class Base {
> public:
>   virtual Foo* baz() { return nullptr; }
> };
>
> class Derived : public Base {
> public:
>   Bar* baz() override { return nullptr; }
> };
>
> int main() {
>   Derived d;
>   Base *b = *d;
>   b->baz(); // Break here and eval 'd.baz()' to crash.
>   return 0;
> }
>
> LLDB is crashing in IRGen when we call the CXXRecordDecl::bases() method
> for the Bar class. (I think this is being called to determine whether we
> need to fix up the pointer returned from Derived::baz()). Because the
> CXXRecordDecl for Bar is generated by DWARFASTParserClang as a minimal,
> lazily completed decl (and is then imported into a different context for
> good measure), and because no other operations on Bar force it to be
> completed, it's still incomplete when we call bases(). In particular, the
> DerivedData field is still null: it's the access to this field that causes
> the crash.
>
> I don't grok the model for lazy decl completion/import well enough to know
> exactly how to proceed (Is there a good source explaining it?). I know that
> some operations on a Decl will cause that Decl to be completed
> automatically. Should calling 'bases()' also trigger completion? Or should
> that be the responsibility of callers of bases()? (I tested forcing
> completion in CXXBasePaths::lookupInBases(…) and that fixed my issue, but
> it's not clear to me that that's the Right Thing to do here).
>

The intent is that the ExternalASTSource and the AST conspire to create the
illusion that all declarations are already loaded -- calling bases() is
intended to be sufficient to get the bases of the class with no other
action involved.

The way this is supposed to work is:
 * bases() calls data()
 * data() calls dataPtr()
 * dataPtr() calls getMostRecentDecl()
 * getMostRecentDecl() (in Redeclarable.h) calls getNext() on the
RedeclLink of the canonical decl (which is supposed to point to the "most
recent" declaration)
 * RedeclLink queries its LazyGenerationalUpdatePtr, which calls
CompleteRedeclChain on the ExternalASTSource if necessary to complete the
redeclaration chain and obtain the definition

I guess that your ExternalASTSource supplies the type definition in
response ExternalASTSource::CompleteType, not in response to
ExternalASTSource::CompleteRedeclChain? I don't see how that mechanism is
supposed to work -- it's only called when Sema wants a complete type, and
not when any other part of the compiler is assuming that a type is complete.

Try adding the definition in CompleteRedeclChain instead, and see if that
helps. If so, perhaps we should remove ExternalASTSource::CompleteType.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20170728/54b9c449/attachment.html>