[CodeGen] Don't create a fake FunctionDecl when generating block/block_byref copy/dispose helper functions

[ahatanak]

Summary

These fake functions were causing clang to crash when https://reviews.llvm.org/D98799 made some changes.

The line number information is no longer emitted for the __Block_byref_object helper functions (see clang/test/CodeGenCXX/debug-info-blocks.cpp), but I think that's okay since the line numbers were incorrect when the helper functions were shared among multiple __block variables. For example, the line number for __Block_byref_object_copy_ and __Block_byref_object_dispose_ was the line number for __block id b in func0, which was incorrect if the helper functions were called when func1 was called.

void (^gb)();

int func0() {
  __block id b;
  gb = ^{ (void)b; };
  return 0;
}

int func1() {
  __block id b;
  gb = ^{ (void)b; };
  return 0;
}

[ahatanak]

Also, is it okay to emit the linkageName, but not the name for the helper functions? It seems okay to me, but I'm not sure.

[dblaikie]

In D104082#2812088, @ahatanak wrote:

Also, is it okay to emit the linkageName, but not the name for the helper functions? It seems okay to me, but I'm not sure.

Not 100% sure - perhaps @aprantl can weigh in on this. If you can find some precedent/existing cases where we do that, might help provide some confidence (one way I'd do that would be to put an assertion in the CGDebugInfo code that makes the choice about which names to add - asserting if only a linkage name is added and no non-linkage name - then run check-clang and see what, if any, tests fail - that'll identify cases that have this property)

[ahatanak]

C++ non-virtual thunks and global initialization functions (_GLOBAL__sub_I_*) have only linkage names.

[dblaikie]

In D104082#2818281, @ahatanak wrote:

C++ non-virtual thunks and global initialization functions (_GLOBAL__sub_I_*) have only linkage names.

Works for me - and the debug info codegen looks like it does handle this situation intentionally: https://github.com/llvm/llvm-project/blob/a11880468e556d20ad9b0d43a1ff43daf62d49b2/clang/lib/CodeGen/CGDebugInfo.cpp#L3896.

Maybe give it a few days (or directly ask) to see if @aprantl has further thoughts, since I'm not especially familiar with Objective C things, but it looks pretty right/good enough for me.

[aprantl]

Block copy helpers don't have any source code associated with them, so as long as they are described by a DISubprogram, the exact format of it most likely does not matter.

clang/lib/CodeGen/CGBlocks.cpp

1956–1957

This comment refers to the FD->setImplicit(); that was removed.

2142–2143

Same here — the comment should be removed.

[ahatanak]

Delete comments.

[zequanwu]

Hi, this caused compiler crash with error "Assertion `cast<DISubprogram>(Scope)->describes(&MF->getFunction())' failed." on iOS platforms. So, I reverted it.
I'm working on a reduced repro.

[fhahn]

In D104082#2835080, @zequanwu wrote:

Hi, this caused compiler crash with error "Assertion `cast<DISubprogram>(Scope)->describes(&MF->getFunction())' failed." on iOS platforms. So, I reverted it.
I'm working on a reduced repro.

FYI this also caused a failure on GreenDragon, with -verify-machineinstrs: https://green.lab.llvm.org/green/job/test-suite-verify-machineinstrs-aarch64-O0-g/9663/consoleFull#-134330334249ba4694-19c4-4d7e-bec5-911270d8a58c

The failure should be re-producible by building the following C++ file from llvm-test-suite:

bin/clang++  -DNDEBUG  -B /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin    -Wno-unused-command-line-argument -mllvm -verify-machineinstrs -O0 -g -arch arm64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS13.5.sdk   -w -Werror=date-time -MD -MT SingleSource/UnitTests/CMakeFiles/block-byref-cxxobj-test.dir/block-byref-cxxobj-test.cpp.o -MF SingleSource/UnitTests/CMakeFiles/block-byref-cxxobj-test.dir/block-byref-cxxobj-test.cpp.o.d -o SingleSource/UnitTests/CMakeFiles/block-byref-cxxobj-test.dir/block-byref-cxxobj-test.cpp.o -c /Users/buildslave/jenkins/workspace/test-suite-verify-machineinstrs-aarch64-O0-g/test-suite/SingleSource/UnitTests/block-byref-cxxobj-test.cpp

[zequanwu]

In D104082#2835496, @fhahn wrote:

In D104082#2835080, @zequanwu wrote:

Hi, this caused compiler crash with error "Assertion `cast<DISubprogram>(Scope)->describes(&MF->getFunction())' failed." on iOS platforms. So, I reverted it.
I'm working on a reduced repro.

FYI this also caused a failure on GreenDragon, with -verify-machineinstrs: https://green.lab.llvm.org/green/job/test-suite-verify-machineinstrs-aarch64-O0-g/9663/consoleFull#-134330334249ba4694-19c4-4d7e-bec5-911270d8a58c

The failure should be re-producible by building the following C++ file from llvm-test-suite:

bin/clang++  -DNDEBUG  -B /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin    -Wno-unused-command-line-argument -mllvm -verify-machineinstrs -O0 -g -arch arm64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS13.5.sdk   -w -Werror=date-time -MD -MT SingleSource/UnitTests/CMakeFiles/block-byref-cxxobj-test.dir/block-byref-cxxobj-test.cpp.o -MF SingleSource/UnitTests/CMakeFiles/block-byref-cxxobj-test.dir/block-byref-cxxobj-test.cpp.o.d -o SingleSource/UnitTests/CMakeFiles/block-byref-cxxobj-test.dir/block-byref-cxxobj-test.cpp.o -c /Users/buildslave/jenkins/workspace/test-suite-verify-machineinstrs-aarch64-O0-g/test-suite/SingleSource/UnitTests/block-byref-cxxobj-test.cpp

Thanks.

[ahatanak]

Thank you for the repro. I added calls to ApplyDebugLocation::CreateArtificial and reapplied the patch in https://github.com/llvm/llvm-project/commit/8db0dbbe2c0544c38f33cf64b4cdd5135d524b23.

clang/lib/CodeGen/CGBlocks.cpp

CodeGenFunction::GenerateCopyHelperFunction(const CGBlockInfo &blockInfo) {
   llvm::FunctionType *LTy = CGM.getTypes().GetFunctionType(FI);
 
   llvm::Function *Fn =
     llvm::Function::Create(LTy, llvm::GlobalValue::LinkOnceODRLinkage,
                            FuncName, &CGM.getModule());
   if (CGM.supportsCOMDAT())
     Fn->setComdat(CGM.getModule().getOrInsertComdat(FuncName));
 
-  IdentifierInfo *II = &C.Idents.get(FuncName);
-
   SmallVector<QualType, 2> ArgTys;
   ArgTys.push_back(C.VoidPtrTy);
   ArgTys.push_back(C.VoidPtrTy);
-  QualType FunctionTy = C.getFunctionType(ReturnTy, ArgTys, {});
 
-  FunctionDecl *FD = FunctionDecl::Create(
-      C, C.getTranslationUnitDecl(), SourceLocation(), SourceLocation(), II,
-      FunctionTy, nullptr, SC_Static, false, false);
   setBlockHelperAttributesVisibility(blockInfo.CapturesNonExternalType, Fn, FI,
                                      CGM);
-  // This is necessary to avoid inheriting the previous line number.
-  FD->setImplicit();
-  StartFunction(FD, ReturnTy, Fn, FI, args);
+  StartFunction(GlobalDecl(), ReturnTy, Fn, FI, args);
   auto AL = ApplyDebugLocation::CreateArtificial(*this);
 
   llvm::Type *structPtrTy = blockInfo.StructureType->getPointerTo();
 
   Address src = GetAddrOfLocalVar(&SrcDecl);
   src = Address(Builder.CreateLoad(src), blockInfo.BlockAlign);
   src = Builder.CreateBitCast(src, structPtrTy, "block.source");
 
CodeGenFunction::GenerateDestroyHelperFunction(const CGBlockInfo &blockInfo) {
   llvm::FunctionType *LTy = CGM.getTypes().GetFunctionType(FI);
 
   llvm::Function *Fn =
     llvm::Function::Create(LTy, llvm::GlobalValue::LinkOnceODRLinkage,
                            FuncName, &CGM.getModule());
   if (CGM.supportsCOMDAT())
     Fn->setComdat(CGM.getModule().getOrInsertComdat(FuncName));
 
-  IdentifierInfo *II = &C.Idents.get(FuncName);
-
   SmallVector<QualType, 1> ArgTys;
   ArgTys.push_back(C.VoidPtrTy);
-  QualType FunctionTy = C.getFunctionType(ReturnTy, ArgTys, {});
-
-  FunctionDecl *FD = FunctionDecl::Create(
-      C, C.getTranslationUnitDecl(), SourceLocation(), SourceLocation(), II,
-      FunctionTy, nullptr, SC_Static, false, false);
 
   setBlockHelperAttributesVisibility(blockInfo.CapturesNonExternalType, Fn, FI,
                                      CGM);
-  // This is necessary to avoid inheriting the previous line number.
-  FD->setImplicit();
-  StartFunction(FD, ReturnTy, Fn, FI, args);
+  StartFunction(GlobalDecl(), ReturnTy, Fn, FI, args);
   markAsIgnoreThreadCheckingAtRuntime(Fn);
 
   auto AL = ApplyDebugLocation::CreateArtificial(*this);
 
   llvm::Type *structPtrTy = blockInfo.StructureType->getPointerTo();
 
   Address src = GetAddrOfLocalVar(&SrcDecl);
   src = Address(Builder.CreateLoad(src), blockInfo.BlockAlign);
generateByrefCopyHelper(CodeGenFunction &CGF, const BlockByrefInfo &byrefInfo,
   llvm::FunctionType *LTy = CGF.CGM.getTypes().GetFunctionType(FI);
 
   // FIXME: We'd like to put these into a mergable by content, with
   // internal linkage.
   llvm::Function *Fn =
     llvm::Function::Create(LTy, llvm::GlobalValue::InternalLinkage,
                            "__Block_byref_object_copy_", &CGF.CGM.getModule());
 
-  IdentifierInfo *II
-    = &Context.Idents.get("__Block_byref_object_copy_");
-
   SmallVector<QualType, 2> ArgTys;
   ArgTys.push_back(Context.VoidPtrTy);
   ArgTys.push_back(Context.VoidPtrTy);
-  QualType FunctionTy = Context.getFunctionType(ReturnTy, ArgTys, {});
-
-  FunctionDecl *FD = FunctionDecl::Create(
-      Context, Context.getTranslationUnitDecl(), SourceLocation(),
-      SourceLocation(), II, FunctionTy, nullptr, SC_Static, false, false);
 
   CGF.CGM.SetInternalFunctionAttributes(GlobalDecl(), Fn, FI);
 
-  CGF.StartFunction(FD, ReturnTy, Fn, FI, args);
+  CGF.StartFunction(GlobalDecl(), ReturnTy, Fn, FI, args);
 
   if (generator.needsCopy()) {
     llvm::Type *byrefPtrType = byrefInfo.Type->getPointerTo(0);
 
     // dst->x
     Address destField = CGF.GetAddrOfLocalVar(&Dst);
     destField = Address(CGF.Builder.CreateLoad(destField),
                         byrefInfo.ByrefAlignment);
generateByrefDisposeHelper(CodeGenFunction &CGF,
 
   // FIXME: We'd like to put these into a mergable by content, with
   // internal linkage.
   llvm::Function *Fn =
     llvm::Function::Create(LTy, llvm::GlobalValue::InternalLinkage,
                            "__Block_byref_object_dispose_",
                            &CGF.CGM.getModule());
 
-  IdentifierInfo *II
-    = &Context.Idents.get("__Block_byref_object_dispose_");
-
   SmallVector<QualType, 1> ArgTys;
   ArgTys.push_back(Context.VoidPtrTy);
-  QualType FunctionTy = Context.getFunctionType(R, ArgTys, {});
-
-  FunctionDecl *FD = FunctionDecl::Create(
-      Context, Context.getTranslationUnitDecl(), SourceLocation(),
-      SourceLocation(), II, FunctionTy, nullptr, SC_Static, false, false);
 
   CGF.CGM.SetInternalFunctionAttributes(GlobalDecl(), Fn, FI);
 
-  CGF.StartFunction(FD, R, Fn, FI, args);
+  CGF.StartFunction(GlobalDecl(), R, Fn, FI, args);
 
   if (generator.needsDispose()) {
     Address addr = CGF.GetAddrOfLocalVar(&Src);
     addr = Address(CGF.Builder.CreateLoad(addr), byrefInfo.ByrefAlignment);
     auto byrefPtrType = byrefInfo.Type->getPointerTo(0);
     addr = CGF.Builder.CreateBitCast(addr, byrefPtrType);
     addr = CGF.emitBlockByrefAddress(addr, byrefInfo, false, "object");
 

clang/test/CodeGenCXX/debug-info-blocks.cpp

 // RUN: %clang_cc1 %s -debug-info-kind=line-tables-only -fblocks -S -emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 %s -debug-info-kind=line-directives-only -fblocks -S -emit-llvm -o - | FileCheck %s
 
 struct A {
   A();
   A(const A &);
   ~A();
 };
 
 void test() {
   __block A a;
   ^{ (void)a; };
 }
 
-// CHECK: !DISubprogram(name: "__Block_byref_object_copy_",
-// CHECK-SAME:          line: 11,
+// CHECK: !DISubprogram(linkageName: "__Block_byref_object_copy_",
 // CHECK-SAME:          DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK: !DISubprogram(name: "__Block_byref_object_dispose_",
-// CHECK-SAME:          line: 11,
+// CHECK: !DISubprogram(linkageName: "__Block_byref_object_dispose_",
 // CHECK-SAME:          DISPFlagLocalToUnit | DISPFlagDefinition

clang/test/CodeGenObjC/debug-info-block-helper.m

 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -emit-llvm -fblocks -debug-info-kind=limited -triple x86_64-apple-darwin10 -fobjc-runtime=macosx-fragile-10.5 %s -o - | FileCheck %s
 extern void foo(void(^)(void));
 
-// CHECK: !DISubprogram(name: "__destroy_helper_block_8_32o40r48r"
+// CHECK: !DISubprogram(linkageName: "__destroy_helper_block_8_32o40r48r"
 
 @interface NSObject {
   struct objc_object *isa;
 }
 @end
 
 @interface A:NSObject @end
 @implementation A

clang/test/CodeGenObjC/debug-info-blocks.m

 // CHECK: load {{.*}}, !dbg ![[DBG_LINE]]
 // CHECK: ret {{.*}}, !dbg ![[DBG_LINE]]
 // CHECK: define {{.*}} @__destroy_helper_block_{{.*}}(i8*
 // CHECK-NOT: ret
 // CHECK: load {{.*}}, !dbg ![[DESTROY_LINE:[0-9]+]]
 // CHECK: ret {{.*}}, !dbg ![[DESTROY_LINE]]
 
 // CHECK-DAG: [[DBG_LINE]] = !DILocation(line: 0, scope: ![[COPY_SP:[0-9]+]])
-// CHECK-DAG: [[COPY_SP]] = distinct !DISubprogram(name: "__copy_helper_block_
+// CHECK-DAG: [[COPY_SP]] = distinct !DISubprogram(linkageName: "__copy_helper_block_
 // CHECK-DAG: [[DESTROY_LINE]] = !DILocation(line: 0, scope: ![[DESTROY_SP:[0-9]+]])
-// CHECK-DAG: [[DESTROY_SP]] = distinct !DISubprogram(name: "__destroy_helper_block_
+// CHECK-DAG: [[DESTROY_SP]] = distinct !DISubprogram(linkageName: "__destroy_helper_block_
 typedef unsigned int NSUInteger;
 
 @protocol NSObject
 @end  
    
 @interface NSObject <NSObject>
 - (id)init;
 + (id)alloc;