Implementation of -fextend-lifetimes and -fextend-this-ptr to aid with debugging of optimized code

[wolfgangp]

Summary

The patch implements the clang support for generating artificial uses of local variables and parameters to aid with debugging of optimized code. I presented this concept in my lightning talk "Debugging of optimized code: Extending the lifetime of local variables" at the October 2017 LLVM conference.

Since the concept is not universally supported, the purpose of this review is to make the patch available to interested parties and perhaps to restart the discussion.

This patch requires the patch posted in https://reviews.llvm.org/D41043 (the llvm portion).

The implementation generates calls to the llvm intrinsic fake.use in similar fashion as end of lifetime markers.

[wolfgangp]

A quick note on usage: Compile your source with "clang -O2 -g -c -fextend-lifetimes foo.cpp" to see the effects (or use -fextend-this-ptr if you just want to extend the lifetime of 'this').

[efriedma]

This might be something we want to turn on automatically for -Og.

[vsk]

Thanks for your patches @wolfgangp! I agree with Eli that we should evaluate enabling this automatically with -Og. I'll test this out on a few internal projects and report back.

[probinson]

I should note we've had at least one request to make this specifiable per-function, which would mean defining an attribute to control the emission of the fake-use intrinsics. Doing the feature that way would be consistent with 'optnone'.

[vsk]

I have some results from the development build of our kernel ('-O2 -g -flto'). According to dwarfdump -statistics, when compiled with -fextend-lifetimes, the percentage of covered scope bytes increases from 62% to 69%. The number of inlined scopes decreases by 4%, and (I think relatedly) the size of the binary increases by 14%. There is a small increase in the number of unique source variables (under 1%). I'd be happy to report back on any other suggested quantitative measures.

My qualitative feedback based on spot-checking a few frames is that -fextend-lifetimes noticeably improves the overall debugging experience. More argument values and local variables tend to be available. I'm not sure how best to put a number to this.

[wolfgangp]

Rebased on r347577 with some test cases.

include/clang/Driver/Options.td

  Flags<[CC1Option]>, HelpText<"Place each data in its own section (ELF Only)">;
 def fno_data_sections : Flag <["-"], "fno-data-sections">, Group<f_Group>,
   Flags<[CC1Option]>;
+def fextend_this_ptr : Flag <["-"], "fextend-this-ptr">, Group<f_Group>,
+  HelpText<"Extend the lifetime of the 'this' pointer to improve visibility "
+           "in optimized debugging">, Flags<[CC1Option]>;
+def fextend_lifetimes : Flag <["-"], "fextend-lifetimes">, Group<f_Group>,
+  HelpText<"Extend the lifetimes of local variables and parameters to improve "
+           "visibility in optimized debugging">, Flags<[CC1Option]>;
 def fstack_size_section : Flag<["-"], "fstack-size-section">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Emit section containing metadata on function stack sizes">;
 def fno_stack_size_section : Flag<["-"], "fno-stack-size-section">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Don't emit section containing metadata on function stack sizes">;
-
 def funique_section_names : Flag <["-"], "funique-section-names">,
   Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Use unique names for text and data sections (ELF Only)">;

include/clang/Frontend/CodeGenOptions.def

 /// The default TLS model to use.
 ENUM_CODEGENOPT(DefaultTLSModel, TLSModel, 2, GeneralDynamicTLSModel)
 
+/// Whether to extend the live range of the this ptr.
+CODEGENOPT(ExtendThisPtr, 1, 0)
+
+/// Whether to extend the live range of all local variables.
+CODEGENOPT(ExtendLifetimes, 1, 0)
+
 /// Number of path components to strip when emitting checks. (0 == full
 /// filename)
 VALUE_CODEGENOPT(EmitCheckPathComponentsToStrip, 32, 0)

lib/CodeGen/CGCall.cpp

          II != IE; ++II) {
       if (llvm::IntrinsicInst *Intrinsic =
               dyn_cast<llvm::IntrinsicInst>(&*II)) {
+        // Ignore fake uses and the instructions that load their
+        // operands.
+        if (Intrinsic->getIntrinsicID() == llvm::Intrinsic::fake_use) {
+          const llvm::Value *FakeUseArg = Intrinsic->getArgOperand(0);
+          if (++II == IE || &*II != FakeUseArg)
+            break;
+          continue;
+        }
         if (Intrinsic->getIntrinsicID() == llvm::Intrinsic::lifetime_end) {
           const llvm::Value *CastAddr = Intrinsic->getArgOperand(1);
           ++II;

lib/CodeGen/CGCleanup.h

     /// Whether this cleanup is a lifetime marker
     unsigned IsLifetimeMarker : 1;
 
+    /// Whether this cleanup is a fake use
+    unsigned IsFakeUse : 1;
+
     /// Whether the normal cleanup should test the activation flag.
     unsigned TestFlagInNormalCleanup : 1;
 

   bool isLifetimeMarker() const { return CleanupBits.IsLifetimeMarker; }
   void setLifetimeMarker() { CleanupBits.IsLifetimeMarker = true; }
 
+  bool isFakeUse() const { return CleanupBits.IsFakeUse; }
+  void setFakeUse() { CleanupBits.IsFakeUse = true; }
+
   bool hasActiveFlag() const { return ActiveFlag != nullptr; }
   Address getActiveFlag() const {
     return Address(ActiveFlag, CharUnits::One());

lib/CodeGen/CGCleanup.cpp

     EHScopeStack::stable_iterator Old) const {
   for (EHScopeStack::iterator it = begin(); stabilize(it) != Old; it++) {
     EHCleanupScope *cleanup = dyn_cast<EHCleanupScope>(&*it);
-    if (!cleanup || !cleanup->isLifetimeMarker())
+    // Augment this test to also ignore fake use "cleanups".
+    if (!cleanup || !(cleanup->isLifetimeMarker() || cleanup->isFakeUse()))
       return false;
   }
 

   bool IsEHCleanup = Kind & EHCleanup;
   bool IsActive = !(Kind & InactiveCleanup);
   bool IsLifetimeMarker = Kind & LifetimeMarker;
+  bool IsFakeUse = Kind & FakeUse;
   EHCleanupScope *Scope =
     new (Buffer) EHCleanupScope(IsNormalCleanup,
                                 IsEHCleanup,

     InnermostEHScope = stable_begin();
   if (IsLifetimeMarker)
     Scope->setLifetimeMarker();
+  if (IsFakeUse)
+    Scope->setFakeUse();
 
   return Scope->getCleanupBuffer();
 }

lib/CodeGen/CGDecl.cpp

   C->setDoesNotThrow();
 }
 
+void CodeGenFunction::EmitFakeUse(Address Addr) {
+  auto NL = ApplyDebugLocation::CreateEmpty(*this);
+  llvm::Value *V = Builder.CreateLoad(Addr, "fake.use");
+  llvm::CallInst *C =
+      Builder.CreateCall(CGM.getLLVMFakeUseFn(), { V });
+  C->setDoesNotThrow();
+  C->setTailCallKind(llvm::CallInst::TCK_NoTail);
+}
+
 void CodeGenFunction::EmitAndRegisterVariableArrayDimensions(
     CGDebugInfo *DI, const VarDecl &D, bool EmitDebugInfo) {
   // For each dimension stores its QualType and corresponding

     EHStack.pushCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker,
                                          emission.getOriginalAllocatedAddress(),
                                          emission.getSizeForLifetimeMarkers());
+  // Analogous to lifetime markers, we use a 'cleanup' to emit fake.use
+  // calls for local variables.
+  if (CGM.getCodeGenOpts().ExtendLifetimes &&
+    hasScalarEvaluationKind(D.getType())) {
+    EHStack.pushCleanup<FakeUse>(NormalFakeUse, emission.getAllocatedAddress());
+  }
 
   return emission;
 }

   return LifetimeEndFn;
 }
 
+/// Lazily declare the @llvm.fake.use intrinsic.
+llvm::Constant *CodeGenModule::getLLVMFakeUseFn() {
+  if (FakeUseFn)
+    return FakeUseFn;
+  FakeUseFn =
+      llvm::Intrinsic::getDeclaration(&getModule(), llvm::Intrinsic::fake_use);
+  return FakeUseFn;
+}
+
 namespace {
   /// A cleanup to perform a release of an object at the end of a
   /// function.  This is used to balance out the incoming +1 of a

 
   setAddrOfLocalVar(&D, DeclPtr);
 
+  // Push a FakeUse 'cleanup' object onto the EHStack for the parameter,
+  // which may be the 'this' pointer. This causes the emission of a fake.use
+  // call with the parameter as argument at the end of the function.
+  if (CurFuncDecl && !CurFuncDecl->isImplicit() &&
+      (&D == CXXABIThisDecl || !D.isImplicit())) {
+    if (CGM.getCodeGenOpts().ExtendLifetimes) {
+      if (IsScalar)
+        EHStack.pushCleanup<FakeUse>(NormalFakeUse, DeclPtr);
+    } else if (CGM.getCodeGenOpts().ExtendThisPtr) {
+      // Enter only the 'this' pointer for -fextend-this-ptr.
+      if (&D == CXXABIThisDecl)
+        EHStack.pushCleanup<FakeUse>(NormalFakeUse, DeclPtr);
+    }
+  }
+
   // Emit debug info for param declaration.
   if (CGDebugInfo *DI = getDebugInfo()) {
     if (CGM.getCodeGenOpts().getDebugInfo() >=

lib/CodeGen/CodeGenFunction.h

     }
   };
 
+  // We are using objects of this 'cleanup' class to emit fake.use calls
+  // for -fextend-lifetimes. They are placed at the end of a variable's 
+  // scope analogous to lifetime markers.
+  class FakeUse final : public EHScopeStack::Cleanup {
+      Address Addr;
+
+  public:
+      FakeUse(Address addr)
+          : Addr(addr) {}
+
+      void Emit(CodeGenFunction &CGF, Flags flags) override {
+          CGF.EmitFakeUse(Addr);
+      }
+  };
+
   /// Header for data within LifetimeExtendedCleanupStack.
   struct LifetimeExtendedCleanupHeader {
     /// The size of the following cleanup object.

 
   RValue EmitAtomicExpr(AtomicExpr *E);
 
+  void EmitFakeUse(Address Addr);
+
   //===--------------------------------------------------------------------===//
   //                         Annotations Emission
   //===--------------------------------------------------------------------===//

lib/CodeGen/CodeGenModule.h

   /// void @llvm.lifetime.end(i64 %size, i8* nocapture <ptr>)
   llvm::Constant *LifetimeEndFn = nullptr;
 
+  /// void @llvm.fake.use(i8* nocapture <ptr>)
+  llvm::Constant *FakeUseFn = nullptr;
+
   GlobalDecl initializedGlobalDecl;
 
   std::unique_ptr<SanitizerMetadata> SanitizerMD;

 
   llvm::Constant *getLLVMLifetimeStartFn();
   llvm::Constant *getLLVMLifetimeEndFn();
+  llvm::Constant *getLLVMFakeUseFn();
 
   // Make sure that this type is translated.
   void UpdateCompletedType(const TagDecl *TD);

lib/CodeGen/EHScopeStack.h

 
   LifetimeMarker = 0x8,
   NormalEHLifetimeMarker = LifetimeMarker | NormalAndEHCleanup,
+
+  // FakeUse needs to be recognized as a special cleanup similar to lifetime
+  // markers chiefly to be ignored in most contexts.
+  FakeUse = 0x10,
+  NormalFakeUse = FakeUse | NormalCleanup,
 };
 
 /// A stack of scopes which respond to exceptions, including cleanups

lib/Driver/ToolChains/Clang.cpp

   if (Args.hasArg(options::OPT_fretain_comments_from_system_headers))
     CmdArgs.push_back("-fretain-comments-from-system-headers");
 
+  if (Args.hasArg(options::OPT_fextend_this_ptr))
+    CmdArgs.push_back("-fextend-this-ptr");
+  if (Args.hasArg(options::OPT_fextend_lifetimes))
+    CmdArgs.push_back("-fextend-lifetimes");
+
   // Forward -fcomment-block-commands to -cc1.
   Args.AddAllArgs(CmdArgs, options::OPT_fcomment_block_commands);
   // Forward -fparse-all-comments to -cc1.

lib/Frontend/CompilerInvocation.cpp

   Opts.CudaGpuBinaryFileName =
       Args.getLastArgValue(OPT_fcuda_include_gpubinary);
 
+  Opts.ExtendThisPtr = Opts.OptimizationLevel > 0 &&
+                       Args.hasArg(OPT_fextend_this_ptr);
+  Opts.ExtendLifetimes = Opts.OptimizationLevel > 0 &&
+                         Args.hasArg(OPT_fextend_lifetimes);
+
   Opts.Backchain = Args.hasArg(OPT_mbackchain);
 
   Opts.EmitCheckPathComponentsToStrip = getLastArgIntValue(

test/CodeGen/fake-use-determinism.c

+// RUN: %clang -S -O2 -emit-llvm -fextend-lifetimes %s -o - | FileCheck %s
+// REQUIRES: asserts
+// 
+// Non-deterministic order of fake.use instructions.
+//
+// We are checking that the fake.use calls for i, j and k appear
+// in a particular order. It is not the order itself that is important
+// but that it remains the same between different test runs.
+//
+// Different targets may have different forms.  For example, on X86, we get a
+// line like:
+//      tail call void (...) @llvm.fake.use(i32 %i)
+// but on ARM, we get a line like:
+//      tail call arm_aapcs_vfpcc void (...) @llvm.fake.use(i32 %i)
+
+// CHECK:       call {{.*}}void (...) @llvm.fake.use(i32 %k)
+// CHECK-NEXT:  call {{.*}}void (...) @llvm.fake.use(i32 %j)
+// CHECK-NEXT:  call {{.*}}void (...) @llvm.fake.use(i32 %i)
+
+extern void bar();
+void foo(int i, int j, int k)
+{
+   for (int l = 0; l < i; l++) {
+      bar();
+   }
+}

test/CodeGen/fake-use-landingpad.c

+// RUN: %clang_cc1 %s -O3 -emit-llvm -fextend-lifetimes -fexceptions -o - | FileCheck %s
+// REQUIRES: asserts
+
+// Check that fake uses do not mistakenly cause a landing pad to be generated when
+// exceptions are enabled.
+
+extern void bar(int);
+void foo(int p) {
+  int a = 17;
+  bar(a);
+}
+
+// CHECK:      define {{.*}} @foo
+// CHECK-NOT:  personality
+// CHECK:      entry:
+// CHECK-NOT:  landingpad

test/CodeGen/fake-use-noreturn.c

+// RUN: %clang_cc1 %s -S -emit-llvm -fextend-lifetimes -o %t.ll
+//
+// Check we don't assert when we have a return in a nested conditional and
+// there is no code at the end of the function.
+
+// CHECK: define{{.*}}main
+// CHECK: call{{.*}}llvm.fake.use
+
+void foo(int i) {
+   while (0)
+     if (1)
+       return;
+}

test/CodeGen/fake-use-return-line.c

+// RUN: %clang_cc1 -S -emit-llvm -O2 -debug-info-kind=limited -fextend-lifetimes -o - %s | FileCheck %s
+int main()
+{
+  volatile int a = 1;
+  int b = a + 2;
+  return b;
+}
+// CHECK: define{{.*}}@main
+// CHECK: ret i32{{.*}}!dbg ![[MDINDEX:[0-9]*]]
+// CHECK: ![[MDINDEX]] = !DILocation(line: 6

test/CodeGen/fake-use-sanitizer.cpp

+// RUN: %clang_cc1 %s -O1 -emit-llvm -fextend-lifetimes -fsanitize=null -fsanitize-trap=null -o - | FileCheck --check-prefix TRAP %s
+// RUN: %clang_cc1 %s -O1 -emit-llvm -fextend-lifetimes -o - | FileCheck --check-prefix FAKEUSE %s
+// REQUIRES: asserts
+
+// With -fextend-lifetimes the compiler generated a fake.use of a
+// reference variable at the end of the function, in the cleanup block. This prompted the 
+// address sanitizer to verify that the variable has been allocated properly, even when 
+// the functions returns early.
+// We check that sanitizers are disabled for code generated for the benefit of fake.use 
+// intrinsics, as well as that the fake.use is no longer generated in the cleanup block.
+// It should be generated in the block preceding the cleanup block instead.
+
+struct A { short s1, s2; };
+extern A& getA();
+
+void foo()
+{
+  auto& va = getA();
+  short s = va.s1 & ~va.s2;
+  if (s == 0)
+    return;
+
+  auto& vb = getA();
+}
+
+// TRAP:         define{{.*}}foo
+// TRAP:         [[COMPARE:%[^\s]*]] = icmp eq
+// TRAP-NOT:     br i1 [[COMPARE]]{{.*}}trap
+// TRAP:         br i1 [[COMPARE]]{{.*}}%if.end
+// TRAP-NOT:     trap:
+// TRAP:         if.end:
+// TRAP-NOT:     call{{.*}}llvm.trap
+
+// FAKEUSE:      if.end:
+// FAKEUSE-NEXT: [[CALL:%[^\s]*]] = {{.*}}call{{.*}}getA
+// FAKEUSE-NOT:  br{{.*}}cleanup
+// FAKEUSE:      call{{.*}}fake.use({{.*}}[[CALL]])
+// FAKEUSE:      cleanup:

test/CodeGen/fake-use-scalar.c

+// RUN: %clang_cc1 %s -O2 -emit-llvm -fextend-lifetimes -o - | FileCheck %s
+// Make sure we don't generate fake.use for non-scalar 
+// variables.
+
+struct A {
+  unsigned long t;
+  char c[1024];
+  unsigned char r[32];
+};
+
+
+int foo()
+{
+  struct A s;
+  struct A v[128];
+  char c[32];
+  return 0;
+}
+
+// CHECK-NOT:  fake.use

test/CodeGen/fake-use-while.c

+// RUN: %clang_cc1 %s -S -emit-llvm -fextend-lifetimes -o %t.ll
+//
+// Check we don't assert when there is no more code after a while statement
+// and the body of the while statement ends in a return, i.e. no insertion point
+// is available.
+
+// CHECK: define{{.*}}main
+// CHECK: call{{.*}}llvm.fake.use
+
+void foo() {
+  {
+    while (1) {
+      int ret;
+      if (1)
+        return;
+    }
+  }
+}

test/CodeGen/fake-use.cpp

+// RUN: %clang_cc1 %s -O2 -emit-llvm -fextend-this-ptr -o - | FileCheck %s
+// Check that we generate a fake_use call with the 'this' pointer as argument.
+// The call should appear after the call to bar().
+
+extern void bar();
+
+class v
+{
+public:
+    int x;
+    int y;
+    int z;
+    int w;
+
+    v(int a, int b, int c, int d) : x(a), y(b), z(c), w(d) {}
+};
+
+class w
+{
+public:
+    v test(int, int, int, int, int, int, int, int, int, int);
+    w(int in): a(in), b(1234) {}
+
+private:
+    int a;
+    int b;
+};
+
+v w::test(int q, int w, int e, int r, int t, int y, int u, int i, int o, int p)
+{
+// CHECK: define{{.*}}test
+    int res = q*w + e - r*t + y*u*i*o*p;
+    int res2 = (w + e + r + t + y + o)*(p*q);
+    int res3 = res + res2;
+    int res4 = q*e + t*y*i + p*e*w * 6 * 4 * 3;
+
+    v V(res, res2, res3, res4);
+
+    bar();
+// CHECK: call{{.*}}bar
+// CHECK: call void (...) @llvm.fake.use(%class.w* %this)
+    return V;
+// CHECK: ret
+}

test/CodeGen/no-fake-use-O0.cpp

+// RUN: %clang_cc1 %s -O0 -emit-llvm -fextend-this-ptr -o -  | FileCheck %s
+// RUN: %clang_cc1 %s -O0 -emit-llvm -fextend-lifetimes -o - | FileCheck %s
+// RUN: %clang_cc1 %s -O1 -emit-llvm -fextend-this-ptr -o -  | FileCheck -check-prefix=OPT %s
+// RUN: %clang_cc1 %s -O1 -emit-llvm -fextend-lifetimes -o - | FileCheck -check-prefix=OPT %s
+// RUN: %clang_cc1 %s -Os -emit-llvm -fextend-this-ptr -o -  | FileCheck -check-prefix=OPT %s
+// RUN: %clang_cc1 %s -Os -emit-llvm -fextend-lifetimes -o - | FileCheck -check-prefix=OPT %s
+// RUN: %clang_cc1 %s -Oz -emit-llvm -fextend-this-ptr -o -  | FileCheck -check-prefix=OPT %s
+// RUN: %clang_cc1 %s -Oz -emit-llvm -fextend-lifetimes -o - | FileCheck -check-prefix=OPT %s
+// Check that we do not generate a fake_use call when we are not optimizing. 
+
+extern void bar();
+
+class v
+{
+public:
+    int x;
+    int y;
+    int z;
+    int w;
+
+    v(int a, int b, int c, int d) : x(a), y(b), z(c), w(d) {}
+};
+
+class w
+{
+public:
+    v test(int, int, int, int, int, int, int, int, int, int);
+    w(int in): a(in), b(1234) {}
+
+private:
+    int a;
+    int b;
+};
+
+v w::test(int q, int w, int e, int r, int t, int y, int u, int i, int o, int p)
+{
+// CHECK: define{{.*}}test
+    int res = q*w + e - r*t + y*u*i*o*p;
+    int res2 = (w + e + r + t + y + o)*(p*q);
+    int res3 = res + res2;
+    int res4 = q*e + t*y*i + p*e*w * 6 * 4 * 3;
+
+    v V(res, res2, res3, res4);
+
+    bar();
+// CHECK-NOT: call void (...) @llvm.fake.use
+// OPT:       call void (...) @llvm.fake.use
+    return V;
+}