search

Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

[DebugInfo@O2][Dexter] Misleading value for local variable at the return statement. #38312

Open Quuxplusone opened 5 years ago

Quuxplusone commented 5 years ago
Bugzilla Link PR39339
Status REOPENED
Importance P normal
Reported by Carlos Alberto Enciso (international.phantom@gmail.com)
Reported on 2018-10-18 04:39:20 -0700
Last modified on 2020-02-13 09:38:28 -0800
Version trunk
Hardware PC Linux
CC aprantl@apple.com, chackz0x12@gmail.com, david.stenberg@ericsson.com, dblaikie@gmail.com, ftee@flametop.co.uk, greg.bedwell@sony.com, jdevlieghere@apple.com, jeremy.morse.llvm@gmail.com, llvm-bugs@lists.llvm.org, paul.robinson@am.sony.com, stephen.tozer@sony.com, vsk@apple.com, warren_ristow@playstation.sony.com, Wolfgang_Pieb@playstation.sony.com
Fixed by commit(s)
Attachments
Blocks PR38768
Blocked by
See also PR40795 
Spawned from bug 39187:

By modifying the original test case to:

--------8<--------
int main() {
  volatile int foo = 4;

  int beards = 0;
  int birds = 0;

  if (foo == 4) {
    beards = 8;
    birds = 3;
  } else {
    beards = 4;
    birds = 6;
  }

  return beards;
}
-------->8--------

When stopping at the return, the variable 'birds' shows always the value of
zero.
Quuxplusone commented 5 years ago

I'll have a look.

Quuxplusone commented 5 years ago 
For the test in comment 0, when stopping at the return statement:

Original 'return beards;'

'birds' shows 0
'beards' shows 8

Changing the return statement to:

'return beards + birds;'

'birds' shows 0
'beards' shows 0
Quuxplusone commented 5 years ago 
> When stopping at the return, the variable 'birds' shows always the value of
> zero.

For this case, no debug values are generated for 'birds'.

After SROA:

define i32 @main() #0 !dbg !8 {
entry:
[...]

if.then:
  [...]
  br label %if.end, !dbg !27

if.else:
  [...]
  br label %if.end

if.end:
  %beards.0 = phi i32 [ 8, %if.then ], [ 4, %if.else ], !dbg !29
  call void @llvm.dbg.value(... %beards.0, ...), !dbg !22
  [...]
  ret i32 %beards.0, !dbg !31
}

At the return statement, there is no debug_value intrinsic for 'birds'.

All the instructions associated with 'birds' are deleted:

Deleting dead instruction:   store i32 6, i32* %birds, align 4, !dbg !33
Deleting dead instruction:   store i32 3, i32* %birds, align 4, !dbg !29
Deleting dead instruction:   store i32 0, i32* %birds, align 4, !dbg !23
Quuxplusone commented 5 years ago

The following links seems to deal with this situation:

https://reviews.llvm.org/D41044

http://lists.llvm.org/pipermail/llvm-dev/2015-September/090611.html

https://groups.google.com/forum/#!topic/llvm-dev/eNbpOxFYytA

The concept of extending the use (fake) of local variables to improve the debugging experience of optimized code.

Quuxplusone commented 5 years ago

If birds has been completely DCEd, I'd hope to see an "optimized out" message rather than 0 here at least.

What happens in the return beards + birds situation?

Quuxplusone commented 5 years ago 
(In reply to Greg Bedwell from comment #5)
> If birds has been completely DCEd, I'd hope to see an "optimized out"
> message rather than 0 here at least.

Just to confirm that no "optimized out" message is displayed.
Quuxplusone commented 5 years ago 
(In reply to Greg Bedwell from comment #5)
> What happens in the return beards + birds situation?

At the return statement, both variables show the value 0.
Quuxplusone commented 5 years ago

Smells like this might be very similar to bug 40795, the value of "birds" would require a PHI after the conditionals, but SROA/mem2reg doesn't insert one because by that point it's dead code. No dbg.value gets inserted either, and stale variable assignments are left valid until the end of the function.

Quuxplusone commented 5 years ago 
As it happens, this *isn't* a duplicate of bug 40795. Instead, it seems that
what's happening here is SimplifyCFG is folding two now empty blocks together:

--------8<--------
  %4 = icmp eq i32 %3, 4, !dbg !26
  br i1 %4, label %5, label %6, !dbg !27
5:
  call void @llvm.dbg.value(metadata i32 8, metadata !14, [...]
  call void @llvm.dbg.value(metadata i32 3, metadata !15, [...]
  br label %7, !dbg !28
6:
  call void @llvm.dbg.value(metadata i32 4, metadata !14, [...]
  call void @llvm.dbg.value(metadata i32 6, metadata !15, [...]
  br label %7
7:
  %8 = phi i32 [ 8, %5 ], [ 4, %6 ], !dbg !30
-------->8--------

And dropping all the dbg.values, allowing earlier ones to propagate to the
return statement, representing assignments that are now untrue.
Quuxplusone commented 4 years ago

Closed in: https://reviews.llvm.org/D70318

The patch above is currently reverted and pending remerge, but otherwise is the fix for the issue described here.

Quuxplusone commented 4 years ago

A fix has been completed for this issue (see the above comment), but it has twice now been reverted after being merged in due to causing timeouts and build failures, which apparently result from a massive increase in the number of debug intrinsics remaining, described and addressed for one case in[0]. It seems as though it's going to be difficult to implement this change in a way that doesn't result in an excessive number of undef intrinsics being left in the program in order to defend against values falling out of date. There may need to be some broader changes made to how we handle intrinsics (especially undef values) before we can fix this issue permanently.

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