llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.08k stars 11.99k forks source link

Missing is_stmt flag for 'else if' conditions #104695

Closed nd closed 20 hours ago

nd commented 2 months ago

Originally from https://sourceware.org/bugzilla/show_bug.cgi?id=31896.

Sometimes if conditions don't get is_stmt flag and gdb skips them during stepping.

To reproduce, compile test.cpp:

 1  #include <iostream>
 2
 3  bool predicate(int v) {
 4      return v == 7;
 5  }
 6
 7  void f(int v) {
 8      if (v) { // break here
 9          std::cout << "branch1\n";
10      }
11      else if(predicate(v)) {
12          std::cout << "branch2\n";
13      } else if(predicate(v+1)) {
14          std::cout << "branch3\n";
15      }
16  }
17
18  int main() {
19      f(0);
20      return 0;
21  }

> clang++-18 -g -O0 test.cpp

Try stepping in gdb:

> gdb a.out
(gdb) b test.cpp:8
Breakpoint 1 at 0x116b: file test.cpp, line 8.
(gdb) run
Breakpoint 1, f (v=0) at test.cpp:8
8           if (v) { // break here
(gdb) next
11          else if(predicate(v)) {
(gdb) next
16      }
(gdb) 

Notice that gdb didn't stop at the last branch condition on line 13.

> llvm-dwarfdump-18 --debug-line a.out
...
Address            Line   Column File   ISA Discriminator OpIndex Flags
------------------ ------ ------ ------ --- ------------- ------- -------------
0x0000000000001140      3      0      0   0             0       0  is_stmt
0x0000000000001147      4     14      0   0             0       0  is_stmt prologue_end
0x000000000000114e      4      5      0   0             0       0 
0x0000000000001153      4      5      0   0             0       0  epilogue_begin
0x0000000000001160      7      0      0   0             0       0  is_stmt
0x000000000000116b      8      9      0   0             0       0  is_stmt prologue_end
0x000000000000116f      8      9      0   0             0       0 
0x0000000000001175      9     19      0   0             0       0  is_stmt
0x0000000000001188     10      5      0   0             0       0  is_stmt
0x000000000000118d     11     23      0   0             0       0  is_stmt
0x0000000000001190     11     13      0   0             0       0 
0x0000000000001195     11     13      0   0             0       0 
0x00000000000011a2     12     19      0   0             0       0  is_stmt
0x00000000000011b5     13      5      0   0             0       0  is_stmt
0x00000000000011ba     13     25      0   0             0       0 
0x00000000000011bd     13     26      0   0             0       0 
0x00000000000011c0     13     15      0   0             0       0 
0x00000000000011c5     13     15      0   0             0       0 
0x00000000000011d2     14     19      0   0             0       0  is_stmt
0x00000000000011e5      0     19      0   0             0       0 
0x00000000000011ef     16      1      0   0             0       0  is_stmt epilogue_begin
0x0000000000001200     18      0      0   0             0       0  is_stmt
0x000000000000120f     19      5      0   0             0       0  is_stmt prologue_end
0x0000000000001216     20      5      0   0             0       0  is_stmt
0x0000000000001218     20      5      0   0             0       0  epilogue_begin
0x000000000000121e     20      5      0   0             0       0  end_sequence

For the branch on the line 11 both closing } of the previous block on line 10 and the else if get is_stmt flag.

The branch on the line 13 has only one instruction marked with the is_stmt flag. Gdb treats it as the last instruction of the previous block and doesn't stop there.

llvmbot commented 2 months ago

@llvm/issue-subscribers-debuginfo

Author: None (nd)

Originally from https://sourceware.org/bugzilla/show_bug.cgi?id=31896. Sometimes if conditions don't get is_stmt flag and gdb skips them during stepping. To reproduce, compile test.cpp: ``` 1 #include <iostream> 2 3 bool predicate(int v) { 4 return v == 7; 5 } 6 7 void f(int v) { 8 if (v) { // break here 9 std::cout << "branch1\n"; 10 } 11 else if(predicate(v)) { 12 std::cout << "branch2\n"; 13 } else if(predicate(v+1)) { 14 std::cout << "branch3\n"; 15 } 16 } 17 18 int main() { 19 f(0); 20 return 0; 21 } ``` `> clang++-18 -g -O0 test.cpp` Try stepping in gdb: ``` > gdb a.out (gdb) b test.cpp:8 Breakpoint 1 at 0x116b: file test.cpp, line 8. (gdb) run Breakpoint 1, f (v=0) at test.cpp:8 8 if (v) { // break here (gdb) next 11 else if(predicate(v)) { (gdb) next 16 } (gdb) ``` Notice that gdb didn't stop at the last branch condition on line 13. ``` > llvm-dwarfdump-18 --debug-line a.out ... Address Line Column File ISA Discriminator OpIndex Flags ------------------ ------ ------ ------ --- ------------- ------- ------------- 0x0000000000001140 3 0 0 0 0 0 is_stmt 0x0000000000001147 4 14 0 0 0 0 is_stmt prologue_end 0x000000000000114e 4 5 0 0 0 0 0x0000000000001153 4 5 0 0 0 0 epilogue_begin 0x0000000000001160 7 0 0 0 0 0 is_stmt 0x000000000000116b 8 9 0 0 0 0 is_stmt prologue_end 0x000000000000116f 8 9 0 0 0 0 0x0000000000001175 9 19 0 0 0 0 is_stmt 0x0000000000001188 10 5 0 0 0 0 is_stmt 0x000000000000118d 11 23 0 0 0 0 is_stmt 0x0000000000001190 11 13 0 0 0 0 0x0000000000001195 11 13 0 0 0 0 0x00000000000011a2 12 19 0 0 0 0 is_stmt 0x00000000000011b5 13 5 0 0 0 0 is_stmt 0x00000000000011ba 13 25 0 0 0 0 0x00000000000011bd 13 26 0 0 0 0 0x00000000000011c0 13 15 0 0 0 0 0x00000000000011c5 13 15 0 0 0 0 0x00000000000011d2 14 19 0 0 0 0 is_stmt 0x00000000000011e5 0 19 0 0 0 0 0x00000000000011ef 16 1 0 0 0 0 is_stmt epilogue_begin 0x0000000000001200 18 0 0 0 0 0 is_stmt 0x000000000000120f 19 5 0 0 0 0 is_stmt prologue_end 0x0000000000001216 20 5 0 0 0 0 is_stmt 0x0000000000001218 20 5 0 0 0 0 epilogue_begin 0x000000000000121e 20 5 0 0 0 0 end_sequence ``` For the branch on the line 11 both closing `}` of the previous block on line 10 and the `else if` get is_stmt flag. The branch on the line 13 has only one instruction marked with the is_stmt flag. Gdb treats it as the last instruction of the previous block and doesn't stop there.
dtcxzyw commented 2 months ago

cc @jmorse

SLTozer commented 2 months ago

This looks to be a result of LLVM's fairly primitive handling of is_stmt; right now, we essentially decide to emit is_stmt only if emitting a non-0 line number that differs from the previous non-0 line number. The fix for this would presumably be to also emit is_stmt for the first non-line-0 instruction in a basic block; the actual solution might want to be a little more sophisticated, but it's probably a low-risk change because not many tools rely on is_stmt flags produced by LLVM.

dwblaikie commented 2 months ago

The fix for this would presumably be to also emit is_stmt for the first non-line-0 instruction in a basic block

SGTM

pogo59 commented 2 months ago

The fix for this would presumably be to also emit is_stmt for the first non-line-0 instruction in a basic block

SGTM

+1

nd commented 1 month ago

@SLTozer It looks like the fix for this issue was reverted. Should the issue be reopened, or should I file a new one?

SLTozer commented 1 month ago

I think this issue could be reopened; for full context, the fix was reverted for causing a large growth in .debug_line size; there's an alternative version that doesn't cause the same regression open at https://github.com/llvm/llvm-project/pull/108251, meaning this will be completed again once that passes review and is merged.

SLTozer commented 20 hours ago

Sorry for the long wait, but this is fixed again in https://github.com/llvm/llvm-project/commit/fe18ab983d08b9e1726314009d677517d9cd5935.