[cfe-dev] PR13156: IRGen crash: function declared to take a pointer to a function involving incomplete types

[David Blaikie]

Hi John,

Richard's helped me working through/debugging PR13156 & we've managed
to isolate what appears to be the root cause, but I'm unsure as to the
right fix & wanted to get your (& anyone else's) thoughts on the
matter.

The following C (reproducible in C++ too) code:

  struct A;
  void fA(struct A (*)(void));
  void cA() {
    fA(0);
  }
  struct A {};
  void fA(struct A (*t)(void)) {
    t();
  }

crashes in Clang IRGen. The story goes something like this:

* the LLVM IR declaration of fA is emitted with a {}* parameter since
we don't have enough information to emit the correct void()* type
* the type of the parameter is cached in the TypeCache
* the TypeCache is not invalidated when struct A becomes defined
because struct A is not in the RecordDeclTypes map
* since the TypeCache is not invalidated before the definition of fA,
IRGen'ing the t() call attempts to make a call to a {}* IR type and
asserts (CGCall.cpp:1926 casts to a FunctionType and fails)

For normal (non-function) pointer types we correctly walk down through
the type and add the types to the RecordDeclTypes map (this can be
seen if you add a struct A* parameter to fA and the crash goes away).

We could do the same thing for function pointers - finding all the
types in parameters and return types and adding those to the
RecordDeclTypes map. Is that reasonable? Should we do something more
coarse-grained?

[This approach is at least viable - with the following simple patch:

diff --git lib/CodeGen/CodeGenTypes.cpp lib/CodeGen/CodeGenTypes.cpp
index 9a78dae..602c40d 100644
--- lib/CodeGen/CodeGenTypes.cpp
+++ lib/CodeGen/CodeGenTypes.cpp
@@ -452,6 +452,8 @@ llvm::Type *CodeGenTypes::ConvertType(QualType T) {
     // function type depends on an incomplete type (e.g. a struct or enum), we
     // cannot lower the function type.
     if (!isFuncTypeConvertible(FT)) {
+      if (const RecordType *RT =
dyn_cast<RecordType>(FT->getResultType().getTypePtr()))
+        RecordDeclTypes[RT];
       // This function's type depends on an incomplete tag type.
       // Return a placeholder type.
       ResultType = llvm::StructType::get(getLLVMContext());

This fixes the return type case, but still crashes if the struct is a
parameter to the function pointer instead of its return type - so we'd
need to do a bit more work. This work could be done in
"isFuncTypeConvertible" (perhaps it would be renamed if it were to
take on this purpose), potentially]

Thoughts?

Thanks,
- David