[PATCH] D16914: [CodeGen] Fix an assert in CodeGenFunction::EmitFunctionEpilog

[Duncan P. N. Exon Smith via cfe-commits]

> On 2016-Feb-04, at 18:54, Akira Hatanaka via cfe-commits <cfe-commits at lists.llvm.org> wrote:
> 
> ahatanak created this revision.
> ahatanak added a subscriber: cfe-commits.
> 
> The assert near CGCall.cpp:2465 is triggered because QualType::isObjCRetainableType() is called on different types. In CodeGenFunction::StartFunction, it returns true because it is called on the typedef marked with __attribute__((NSObject)), whereas in CodeGenFunction::EmitFunctionEpilog it returns false because it's called on the canonical type.
> 
> To fix the assert, this patch changes the code in CodeGenFunction::EmitFunctionEpilog to get the function's return type from CodeGenFunction::CurCodeDecl or BlockInfo instead of from CGFunctionInfo.
> 
> http://reviews.llvm.org/D16914

I have a nitpick below.  (I think you should find someone that knows
this code better than I do to sign off on this though.)

> Files:
>  lib/CodeGen/CGCall.cpp
>  test/CodeGenObjCXX/auto-release-result-assert.mm
> 
> Index: test/CodeGenObjCXX/auto-release-result-assert.mm
> ===================================================================
> --- /dev/null
> +++ test/CodeGenObjCXX/auto-release-result-assert.mm
> @@ -0,0 +1,35 @@
> +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fblocks -fobjc-arc -o - %s | FileCheck %s
> +
> +// CHECK-LABEL: define %struct.S1* @_Z4foo1i(
> +// CHECK: %[[CALL:[a-z0-9]+]] = call %struct.S1* @_Z4foo0i
> +// CHECK: ret %struct.S1* %[[CALL]]
> +
> +// CHECK-LABEL: define %struct.S1* @_ZN2S22m1Ev(
> +// CHECK: %[[CALL:[a-z0-9]+]] = call %struct.S1* @_Z4foo0i
> +// CHECK: ret %struct.S1* %[[CALL]]
> +
> +// CHECK-LABEL: define internal %struct.S1* @Block1_block_invoke(
> +// CHECK: %[[CALL:[a-z0-9]+]] = call %struct.S1* @_Z4foo0i
> +// CHECK: ret %struct.S1* %[[CALL]]
> +
> +struct S1;
> +
> +typedef __attribute__((NSObject)) struct __attribute__((objc_bridge(id))) S1 * S1Ref;
> +
> +S1Ref foo0(int);
> +
> +struct S2 {
> +  S1Ref m1();
> +};
> +
> +S1Ref foo1(int a) {
> +  return foo0(a);
> +}
> +
> +S1Ref S2::m1() {
> +  return foo0(0);
> +}
> +
> +S1Ref (^Block1)(void) = ^{
> +  return foo0(0);
> +};
> Index: lib/CodeGen/CGCall.cpp
> ===================================================================
> --- lib/CodeGen/CGCall.cpp
> +++ lib/CodeGen/CGCall.cpp
> @@ -14,6 +14,7 @@
> 
> #include "CGCall.h"
> #include "ABIInfo.h"
> +#include "CGBlocks.h"
> #include "CGCXXABI.h"
> #include "CGCleanup.h"
> #include "CodeGenFunction.h"
> @@ -2462,9 +2463,21 @@
>     // In ARC, end functions that return a retainable type with a call
>     // to objc_autoreleaseReturnValue.
>     if (AutoreleaseResult) {
> +      QualType RT;
> +
> +      if (auto *FD = dyn_cast<FunctionDecl>(CurCodeDecl))
> +        RT = FD->getReturnType();
> +      else if (auto *MD = dyn_cast<ObjCMethodDecl>(CurCodeDecl))
> +        RT = MD->getReturnType();
> +      else if (isa<BlockDecl>(CurCodeDecl))
> +        RT = BlockInfo->BlockExpression->getFunctionType()->getReturnType();
> +      else
> +        llvm_unreachable("Unexpected function/method type");

There's no reason for this code to run when NDEBUG, is there?

> +
> +      (void)RT;
>       assert(getLangOpts().ObjCAutoRefCount &&
>              !FI.isReturnsRetained() &&
> -             RetTy->isObjCRetainableType());
> +             RT->isObjCRetainableType());

I think you should change this to:
```
#ifndef NDEBUG
QualType RT;
if (...)
   RT = ...
else if (...)
   RT = ...
else if (...)
   RT = ...
else
   llvm_unreachable(...);
assert(...);
#endif
```

>       RV = emitAutoreleaseOfResult(*this, RV);
>     }
> 
> 
> 
> <D16914.46990.patch>_______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits