LowerDbgDeclare: look through bitcasts.

[eugenis]

Summary

Emit a value debug intrinsic (with OP_deref) when an alloca address is
passed to a function call after going through a bitcast.

This generates an FP or SP-relative location for the local variable in
the following case:

int x;
use((void *)&x;

[aprantl]

llvm/test/Transforms/Util/dbg-call-bitcast.ll

7

What is %1 going to be after this transformation? Can you add a check for that, too?

[aprantl]

llvm/lib/Transforms/Utils/Local.cpp

1425

I think the worklist is overkill here, since it will contain at most one element. How about turning the loop body into a helper function/lambda and calling it recursively?

[vsk]

llvm/lib/Transforms/Utils/Local.cpp

1425

I think the worklist would contain more than one element in this case:

%a = alloca
%c1 = bitcast %a to ...
call void @use(%c1)
%c2 = bitcast %a to ...
call void @use(%c2)

That is probably worth a test, though.

[aprantl]

llvm/lib/Transforms/Utils/Local.cpp

1425

But that would also work fine with the recursive implementation, right?

1425

Unless we are expecting very deep recursion I would prefer the recursive solution over the worklist since it is much easier to understand.

[vsk]

llvm/lib/Transforms/Utils/Local.cpp

1425

Yes, a recursive approach would work fine. Fwiw, the patch as-written seems idiomatic to me (the alloca use visitation logic in both CaptureTracking & PtrUseVisitor look like this).

[eugenis]

I also find worklist a more common and recognizable pattern, but I don't mind recursion either.

Btw, this function terribly incomplete. This is not the only case when it destroys perfectly good debug info. For example, storing an address of a local variable to memory does not generate a dbg.value. In the following example "y" has no location at -O3:

int *p;
void publish_and_wait();
void f() {
  int y;
  p = &y;
  publish_and_wait();
}

I'm looking at this because HWASan tries to use debug info to recover stack frame layout at run time. This works as long as we get at least one frame pointer based location for a non-promoted variable. I'm considering disabling this optimization altogether in functions with sanitize_hwaddress attribute; or maybe adding a dbg.value with DW_OP_deref to all allocas in such functions.

[vsk]

llvm/lib/Transforms/Utils/Local.cpp

1448

Yeah, this is also giving up on chasing GEPs. PtrUseVisitor has a facility to follow GEPs and figure out the accumulated offset, so we could try to emit:

%a = alloca
%g = gep %a, 0, 4
store i32 %val, %g
; emit: dbg.value(%val, <var>, DIExpression(OP_LLVM_fragment 32 32))

It's certainly a larger change, so I wouldn't be opposed to landing a narrow fix for now, but perhaps that's an interesting future direction.

[aprantl]

In D70752#1768006, @eugenis wrote:

I also find worklist a more common and recognizable pattern, but I don't mind recursion either.

It's not that the worklist is terribly complicated to understand either, but when I see a worklist I'm expecting a tricky algorithm that couldn't be implemented with recursion and then need to stop and spend some time that it's really doing something simple. Not a big deal though. In the grand scheme of things both approaches are fine. The recursion has the advantage that we can give the helper function a descriptive name.

[eugenis]

.

[eugenis]

I don't have a strong opinion either way, but since we seem to have 2-1 in favor of the worklist I'll leave it as is.
Please let me know if you disagree.

I've updated the test case to include two bitcasts of the same alloca.

llvm/lib/Transforms/Utils/Local.cpp

1448

Agreed.

llvm/lib/Transforms/Utils/Local.cpp

if (llvm::any_of(AI->users(), [](User *U) -> bool {
           if (LoadInst *LI = dyn_cast<LoadInst>(U))
             return LI->isVolatile();
           if (StoreInst *SI = dyn_cast<StoreInst>(U))
             return SI->isVolatile();
           return false;
         }))
       continue;
 
-    for (auto &AIUse : AI->uses()) {
+    SmallVector<const Value *, 8> WorkList;
+    WorkList.push_back(AI);
+    while (!WorkList.empty()) {
+      const Value *V = WorkList.pop_back_val();
+      for (auto &AIUse : V->uses()) {
         User *U = AIUse.getUser();
         if (StoreInst *SI = dyn_cast<StoreInst>(U)) {
           if (AIUse.getOperandNo() == 1)
             ConvertDebugDeclareToDebugValue(DDI, SI, DIB);
         } else if (LoadInst *LI = dyn_cast<LoadInst>(U)) {
           ConvertDebugDeclareToDebugValue(DDI, LI, DIB);
         } else if (CallInst *CI = dyn_cast<CallInst>(U)) {
           // This is a call by-value or some other instruction that takes a
           // pointer to the variable. Insert a *value* intrinsic that describes
           // the variable by dereferencing the alloca.
+          if (!CI->isLifetimeStartOrEnd()) {
             DebugLoc NewLoc = getDebugValueLoc(DDI, nullptr);
             auto *DerefExpr =
                 DIExpression::append(DDI->getExpression(), dwarf::DW_OP_deref);
-        DIB.insertDbgValueIntrinsic(AI, DDI->getVariable(), DerefExpr, NewLoc,
-                                    CI);
+            DIB.insertDbgValueIntrinsic(AI, DDI->getVariable(), DerefExpr,
+                                        NewLoc, CI);
+          }
+        } else if (BitCastInst *BI = dyn_cast<BitCastInst>(U)) {
+          if (BI->getType()->isPointerTy())
+            WorkList.push_back(BI);
+        }
       }
     }
     DDI->eraseFromParent();
   }
   return true;
 }
 
 /// Propagate dbg.value intrinsics through the newly inserted PHIs.

llvm/test/Transforms/Util/dbg-call-bitcast.ll

+; RUN: opt -instcombine -S %s | FileCheck %s
+
+define dso_local void @_Z1fv() {
+  %1 = alloca i32, align 4
+  %2 = bitcast i32* %1 to i8*
+  call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %2)
+; CHECK: call void @llvm.dbg.value(metadata i32* %1, {{.*}}, metadata !DIExpression(DW_OP_deref)
+  call void @llvm.dbg.declare(metadata i32* %1, metadata !16, metadata !DIExpression()), !dbg !19
+  call void @_Z1gPv(i8* nonnull %2)
+  call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %2)
+  ret void, !dbg !21
+}
+
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+declare dso_local void @_Z1gPv(i8*)
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!8, !9, !10}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 10.0.0 ([e-mail]:llvm/llvm-project.git cab64b5708f614c71d275ec9d134e68b8c3baedd)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, nameTableKind: None)
+!1 = !DIFile(filename: "dbg.cc", directory: "/tmp")
+!2 = !{}
+!3 = !{!4, !5}
+!4 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: null, size: 64)
+!5 = !DISubprogram(name: "g", linkageName: "_Z1gPv", scope: !1, file: !1, line: 1, type: !6, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
+!6 = !DISubroutineType(types: !7)
+!7 = !{null, !4}
+!8 = !{i32 7, !"Dwarf Version", i32 4}
+!9 = !{i32 2, !"Debug Info Version", i32 3}
+!10 = !{i32 1, !"wchar_size", i32 4}
+!12 = distinct !DISubprogram(name: "f", linkageName: "_Z1fv", scope: !1, file: !1, line: 2, type: !13, scopeLine: 2, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !15)
+!13 = !DISubroutineType(types: !14)
+!14 = !{null}
+!15 = !{!16}
+!16 = !DILocalVariable(name: "x", scope: !12, file: !1, line: 3, type: !17)
+!17 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!18 = !DILocation(line: 3, column: 3, scope: !12)
+!19 = !DILocation(line: 3, column: 7, scope: !12)
+!20 = !DILocation(line: 4, column: 3, scope: !12)
+!21 = !DILocation(line: 5, column: 1, scope: !12)