[DebugInfo] Ensure fallback artificial location is available for cleanups

[aprantl]

Summary

While finishing a function without a valid end location, fall back to an
artificial debug location when generating IR for cleanups. This fixes a
verifier failure ("inlinable function call without debug info").

rdar://57630879

[aprantl]

This will definitely work; Once you got the testcase, it might be good to check whether there is a more targeted root cause that we could fix and assert on EndLoc here.

[vsk]

Added a test.

[vsk]

In D71042#1771040, @aprantl wrote:

This will definitely work; Once you got the testcase, it might be good to check whether there is a more targeted root cause that we could fix and assert on EndLoc here.

I believe the root cause is that GenerateObjCSetter does not pass a valid EndLoc, and does not have a 'simple' return statement. That seems reasonable to me, after all FinishFunction() defaults EndLoc to an invalid location, and there is no explicit return statement for an ObjC setter in source.

[aprantl]

Do we know why EndLoc is invalid?

clang/test/CodeGenObjCXX/property-object-cleanup.mm

3 ↗

(On Diff #232373)

Add a comment

// Test that call in the cleanup block have a valid debug location.

4 ↗

(On Diff #232373)

FYI there is __attribute__((objc_root_class)) to silence the warning this causes.

[aprantl]

Let me guess.. because setData: is synthesized?

[aprantl]

We looked at this together and it seems the forward declaration for the synthesized setter in the ObjCImplDecl doesn't have a valid source location. I'm going to investigate a more targeted fix.

[aprantl]

Principled fix in D71084.

[aprantl]

Updated patch to assert a valid EndLoc and actually pass it in.

[vsk]

Thanks, lgtm with a minor change.

clang/lib/CodeGen/CodeGenFunction.cpp

384

The braces around the else block are probably needed in the NDEBUG build.

[vsk]

clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm

17

Heads-up: this location should be different once https://reviews.llvm.org/D71084 lands, which I assume this depends on.

[aprantl]

clang/lib/CodeGen/CodeGenFunction.cpp

384

No, this should be fine. The ; is not part of the macro.

[vsk]

This is causing a failure in debuginfo-tests:

http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/4450/testReport/junit/debuginfo-tests/llgdb-tests/block_var_m/

Assertion failed: (EndLoc.isValid() && "no location for inlineable cleanup calls")

clang/lib/CodeGen/CGObjC.cpp

void CodeGenFunction::GenerateObjCGetter(ObjCImplementationDecl *IMP,
   llvm::Constant *AtomicHelperFn =
       CodeGenFunction(CGM).GenerateObjCAtomicGetterCopyHelperFunction(PID);
   ObjCMethodDecl *OMD = PID->getGetterMethodDecl();
   assert(OMD && "Invalid call to generate getter (empty method)");
   StartObjCMethod(OMD, IMP->getClassInterface());
 
   generateObjCGetterBody(IMP, PID, OMD, AtomicHelperFn);
 
-  FinishFunction();
+  FinishFunction(OMD->getEndLoc());
 }
 
 static bool hasTrivialGetExpr(const ObjCPropertyImplDecl *propImpl) {
   const Expr *getter = propImpl->getGetterCXXConstructor();
   if (!getter) return true;
 
   // Sema only makes only of these when the ivar has a C++ class type,
   // so the form is pretty constrained.
void CodeGenFunction::GenerateObjCSetter(ObjCImplementationDecl *IMP,
   llvm::Constant *AtomicHelperFn =
       CodeGenFunction(CGM).GenerateObjCAtomicSetterCopyHelperFunction(PID);
   ObjCMethodDecl *OMD = PID->getSetterMethodDecl();
   assert(OMD && "Invalid call to generate setter (empty method)");
   StartObjCMethod(OMD, IMP->getClassInterface());
 
   generateObjCSetterBody(IMP, PID, AtomicHelperFn);
 
-  FinishFunction();
+  FinishFunction(OMD->getEndLoc());
 }
 
 namespace {
   struct DestroyIvar final : EHScopeStack::Cleanup {
   private:
     llvm::Value *addr;
     const ObjCIvarDecl *ivar;
     CodeGenFunction::Destroyer *destroyer;

clang/lib/CodeGen/CGOpenMPRuntime.cpp

Address SizeAddr = CGM.getOpenMPRuntime().getAddrOfArtificialThreadPrivate(
         generateUniqueName(CGM, "reduction_size", RCG.getRefExpr(N)));
     Size = CGF.EmitLoadOfScalar(SizeAddr, /*Volatile=*/false,
                                 CGM.getContext().getSizeType(), Loc);
   }
   RCG.emitAggregateType(CGF, N, Size);
   // Emit the finalizer body:
   // <destroy>(<type>* %0)
   RCG.emitCleanups(CGF, N, PrivateAddr);
-  CGF.FinishFunction();
+  CGF.FinishFunction(Loc);
   return Fn;
 }
 
 llvm::Value *CGOpenMPRuntime::emitTaskReductionInit(
     CodeGenFunction &CGF, SourceLocation Loc, ArrayRef<const Expr *> LHSExprs,
     ArrayRef<const Expr *> RHSExprs, const OMPTaskDataTy &Data) {
   if (!CGF.HaveInsertPoint() || Data.ReductionVars.empty())
     return nullptr;

clang/lib/CodeGen/CodeGenFunction.cpp

void CodeGenFunction::FinishFunction(SourceLocation EndLoc) {
   // edges will be *really* confused.
   bool HasCleanups = EHStack.stable_begin() != PrologueCleanupDepth;
   bool HasOnlyLifetimeMarkers =
       HasCleanups && EHStack.containsOnlyLifetimeMarkers(PrologueCleanupDepth);
   bool EmitRetDbgLoc = !HasCleanups || HasOnlyLifetimeMarkers;
   if (HasCleanups) {
     // Make sure the line table doesn't jump back into the body for
     // the ret after it's been at EndLoc.
-    if (CGDebugInfo *DI = getDebugInfo())
+    if (CGDebugInfo *DI = getDebugInfo()) {
       if (OnlySimpleReturnStmts)
         DI->EmitLocation(Builder, EndLoc);
+      else
+        assert(EndLoc.isValid() && "no location for inlineable cleanup calls");
+    }
 
     PopCleanupBlocks(PrologueCleanupDepth);
   }
 
   // Emit function epilog (to return).
   llvm::DebugLoc Loc = EmitReturnBlock();
 
   if (ShouldInstrumentFunction()) {

clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm

+// RUN: %clang_cc1 -triple arm64e-apple-ios13.0 -debug-info-kind=standalone -fobjc-arc \
+// RUN:   %s -emit-llvm -o - | FileCheck %s
+
+@interface NSObject
++ (id)alloc;
+@end
+
+@interface NSString : NSObject
+@end
+
+// CHECK: define {{.*}}@"\01-[MyData setData:]"
+// CHECK: [[DATA:%.*]] = alloca %struct.Data
+// CHECK: call %struct.Data* @_ZN4DataD1Ev(%struct.Data* [[DATA]]){{.*}}, !dbg [[LOC:![0-9]+]]
+// CHECK-NEXT: ret void
+
+// [[LOC]] = !DILocation(line: 0
+
+@interface MyData : NSObject
+struct Data {
+    NSString *name;
+};
+@property struct Data data;
+@end
+@implementation MyData
+@end