[DebugInfo@O2][Dexter] Pointer variable location can be dropped despite being live

[jmorse]

Bugzilla Link	39726
Resolution	FIXED
Resolved on	Jan 28, 2021 09:58
Version	trunk
OS	Linux
Blocks	#38116 #38102
CC	@dwblaikie,@gregbedwell,@MaskRay,@pogo59

Extended Description

Spinning this out of bug 38754, this bug is about an unfortunate dropping of a dbg.value when it's located in the "correct" place. In the code at the bottom of this bug, the "qux" variable in "main" can have its location dropped, preventing the developer seeing anything useful in the function. I've been using llvm/clang r346686 with options "-O2 -g". The problem described below only happens if you disable CodeGenPrepare's placeDbgValues method (which I'm trying to eliminate). Alternately you can replicate this behaviour with other clangs by:

clang++ test.cpp -O2 -g -mllvm -stop-after=codegenprepare -c -o out.mir
[Edit out.mir moving dbg.value for 'qux' to occur after inlined
constructor completes, after the merged store to the 'fgasdf' field]
llc -start-after=codegenprepare out.mir -o out.s
clang++ out.s -o a.out

In plain clang from trunk, in the code below the dbg.value for 'qux' is moved to immediately after the call to 'new': it's emitted as a DBG_VALUE in the first BB, and its lifetime extended across the body of the whole function.

Without placeDbgValues, however, the dbg.value for 'qux' stays in its original location after the constructor call, and due to some curious behaviours the SSA register it's applied to is never used. The DBG_VALUE for qux is never emitted because it appears a dead value if it's left in the "correct" location.

The reason why this happens is easy to explain (see below), there are sane reasons why. This bug however is about the fact that the combined behaviours leads to an (IMHO) unacceptable outcome, where a pointer variable of significant interest to the developer (and which is in a live register the ~whole function) has debug info dropped.

Here's the IR for main (with correctly placed dbg.value) with comments where I've omitted code:
; Call to allocate "qux"
%3 = tail call i8* @​_Znwm(i64 4) #​7
; The constructor for class foo is inlined, including a conditional
; jump between BBs because of the if-condition, finishing with a write
; to fgasdf:
%13 = bitcast i8* %3 to %class.foo*
%14 = bitcast i8* %3 to i32*
store i32 %12, i32* %14, align 4
; The constructor complete, we make the dbg.value call for "qux"
call void @​llvm.dbg.value(metadata i8* %3, metadata !​28, [blah])
; The body for 'spoons' and the printf call are performed using SSA
; registers and no further loads/stores, finally:
tail call void @​ZN3foo9loldeleteEPS(%class.foo* nonnull %13)
ret i32 0

The dbg.value is applied to an SSA variable of type "i8*", but everything else after that instruction either doesn't touch the allocated object, or uses the pointer SSA-reg that's been cast to "%class.foo*", i.e. %13. In combination with the fact that the dbg.value is no longer in the basic block where %3 is defined, SelectionDAG then drops the DBG_VALUE. This scenario is avoided if we were to:

• Not having a jump-forcing conditional in the constructor
• Not inline anything
• Call delete directly rather then calling a static method to delete
  (the delete operation appears to consume an i8*)

To summarise, if we leave the dbg.value for "qux" in the code below in the correct location (after the constructor), then by accident we end up dropping it. It seems obvious that there should be a way around this, either by something looking further through casts, or the dbg.value being applied to the "correctly" (TM) typed SSA-reg-pointer. I think this might be a fairly general problem though, of the form:

• Lots of code inlined into a function
• All the inlined code GEPs or casts pointers to various elements of
  data structures
• After a certain point in the function, all the base pointers to the
  relevant data structures appear to go out of liveness, not because
  they're dead but because subsequent code refers to the memory with a
  different type.

(This is important because placeDbgValues must die IMHO, so this bug will eventually present to an end user). The code is below; I've scattered noinline to emulate function calls that don't get inlined while keeping it single-source.

--------8<--------
#include <stdio.h>

attribute((noinline)) int rand() {
volatile int foo = 4;
return foo;
}

class foo {
public:
int fgasdf;
foo(int i) {
if (rand() == 4)
fgasdf = rand() * i;
else
fgasdf = rand() / i;
}

int spoons(int u) {
return fgasdf * u + rand();
}

attribute((noinline)) static void loldelete(foo *go) {
delete go;
}
};

int main(int argc, char **argv) {
foo *qux = new foo(argc);
int bees = qux->spoons(argc);
printf("%d\n", bees);
foo::loldelete(qux);
return 0;
}
-------->8--------

[MaskRay]

Hey, is this fixed?

With

--- i/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ w/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -607,5 +607,5 @@ bool CodeGenPrepare::runOnFunction(Function &F) {
// Do this last to clean up use-before-def scenarios introduced by other
// preparatory transforms.

• EverMadeChange |= placeDbgValues(F);

• //EverMadeChange |= placeDbgValues(F);

#ifndef NDEBUG

clang++ -O2 -gdwarf-5 -g -fno-inline a.cc -fno-legacy-pass-manager

qux is evaluable in main.

[Edit out.mir moving dbg.value for 'qux' to occur after inlined constructor completes, after the merged store to the 'fgasdf' field]

I have not figured out how to reproduce with this instruction :(

[jmorse]

Yikes, this bug description is hard to read. I must have been in too deep when writing it,

As far as I understand it, my problem was that the pointer value for "qux" was available, but we couldn't find the right SDNode when building a SelectionDAG because it was on the other side of a needless BitCast. And for various reasons at the time, SelectionDAG wouldn't create DBG_VALUEs for arbitary VRegs

Some time later this [0] landed, which refers to this PR. I've just given the "reproducer" a few runs and couldn't make it drop "qux", so I think it's fair to say it's fixed. Thanks for prodding!

[0] https://reviews.llvm.org/D56678