[PATCH] D49454: [DebugInfo] LowerDbgDeclare: Add derefs when handling CallInst users

[Vedant Kumar via Phabricator via llvm-commits]

vsk planned changes to this revision.
vsk added a comment.

In https://reviews.llvm.org/D49454#1166103, @aprantl wrote:

> This looks like the right change as it makes the IR more regular.
>  Note that in the past this worked because allocas were implicitly indirect since they caused AsmPrinter to emit a what DWARF calls a "memory location". It's possible that that code path no longer exists or isn't triggered any more.
>
> Have you tested the clang debuginfo-tests with this change, particularly the asan and array testcases?


Thanks for suggesting this.

As-is, this patch breaks debuginfo-tests/stack-var.c. Without this patch we would emit a dbg.value with a concrete location:

    call void @llvm.dbg.value(metadata i32* %test, metadata !25, metadata !DIExpression()), !dbg !27
  ...
  !12 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
  !25 = !DILocalVariable(name: "test", scope: !21, file: !1, line: 6, type: !12)

When the backend sees this, it knows to emit a DBG_VALUE which refines the concrete location:

  DBG_VALUE %rbp, 0, !"test", !DIExpression(DW_OP_constu, 4, DW_OP_minus); line no:6 indirect

With this patch applied, we would instead emit:

  call void @llvm.dbg.value(metadata i32* %test, metadata !25, metadata !DIExpression(DW_OP_deref)), !dbg !27

The backend sees the extra DW_OP_deref and emits a DBG_VALUE which tells the debugger to dereference a value it pulls out of the stack:

  DBG_VALUE debug-use $rbp, 0, !"test", !DIExpression(DW_OP_constu, 4, DW_OP_minus, DW_OP_deref), debug-location !27; line no:6 indirect

The problem with this patch is that it adds a DW_OP_deref without creating an implicit value to prevent that extra, incorrect dereference. The fix to this seemed simple: just add DW_OP_stack_value. That should tell the debugger not to dereference what it finds in the stack. But, that's just a more expensive way to encode the exact same behavior as a {pointer, empty DIExpression} dbg.value. Critically, it also doesn't fix my ASan test case.

Using a single DW_OP_deref (without appending DW_OP_stack_value) gives the correct location for a ASan-ified program appending to a std::deque:

  (lldb) p test
  (log_t) $0 = size=1 {
    [0] = (a = "toto")
  }
  ...
  Variable: id = {0x00009f25}, name = "test", type = "log_t", location =  [r15+0], decl = r376.cc:12

Note that we get the correct result (1 dereference) even though this is the *wrong* DIExpression to use, because (as an indirect value) it says to do 2 dereferences.

Adding a DW_OP_stack_value -- which fixed the stack-var.c test case -- does not fix the ASan test case. I'd expect it to cause the debugger to do 1 dereference, but instead it does 2:

  (lldb) p test
  error: Couldn't materialize: couldn't get the value of variable test: extracting data from value failed
  ...
       Variable: id = {0x00009f25}, name = "test", type = "log_t", location =  [r15+0] DW_OP_deref , decl = r376.cc:12

I think I know why adding DW_OP_stack_value fixes one test case but breaks another. My theory is that we're treating a pointer to a std::deque as a direct value (aka "implicit location"?) instead of as an indirect value (aka "concrete location"?). Here's some confirmation:

  DBG_VALUE debug-use $r15, debug-use $noreg, !"test"

IIUC the fact that the second DBG_VALUE operand is $noreg means that the value is direct (implicit). The LangRef is vague about what should happen in this case, but it doesn't seem like we should get here to begin with.


https://reviews.llvm.org/D49454