debug info fail -- range based for loops and other stuff

[chandlerc]

Bugzilla Link	19864
Version	unspecified
OS	Linux
CC	@adrian-prantl,@dwblaikie,@echristo,@hfinkel,@jmorse

Extended Description

So, I came here to file a bug about debug info for range-based for loops. I wrote the following test case:

#include <vector>
#include <iostream>

int main() {
  std::vector<int> v = {13, 21, 8, 3, 34, 1, 5, 2};
  std::cout << "v = {" << v[0] << ", " << v[1] << ", " << v[2] << ", " << v[3]
            << ", " << v[4] << ", " << v[5] << ", " << v[6] << ", " << v[7]
            << "}\n";

  int size = v.size();
  if (v.size() > 3) {
    int a = 0, b = 0;
    for (int x : v)
      if (x >= size)
        ++b;
      else if (x >= 0)
        ++a;
    std::cout << "a = " << a << "\n";
    std::cout << "b = " << b << "\n";
  }
}

Then I compiled and debugged it:

% clang++ -std=c++11 -g -o x x.cc
% gdb --args ./x
GNU gdb (GDB) 7.6.50.20130820-cvs
<snip>
Reading symbols from .../x...done.
(gdb) break main
Breakpoint 1 at 0x400b92: file x.cc, line 5.
(gdb) r
Starting program: .../x
<snip>

Breakpoint 1, main () at x.cc:5
5         std::vector<int> v = {13, 21, 8, 3, 34, 1, 5, 2};
(gdb) n
6         std::cout << "v = {" << v[0] << ", " << v[1] << ", " << v[2] << ", " << v[3]
(gdb) 
7                   << ", " << v[4] << ", " << v[5] << ", " << v[6] << ", " << v[7]
(gdb) 
6         std::cout << "v = {" << v[0] << ", " << v[1] << ", " << v[2] << ", " << v[3]
(gdb) 
7                   << ", " << v[4] << ", " << v[5] << ", " << v[6] << ", " << v[7]
(gdb) 
6         std::cout << "v = {" << v[0] << ", " << v[1] << ", " << v[2] << ", " << v[3]
(gdb) 
7                   << ", " << v[4] << ", " << v[5] << ", " << v[6] << ", " << v[7]
(gdb) 
6         std::cout << "v = {" << v[0] << ", " << v[1] << ", " << v[2] << ", " << v[3]
(gdb) 
7                   << ", " << v[4] << ", " << v[5] << ", " << v[6] << ", " << v[7]
(gdb) 
6         std::cout << "v = {" << v[0] << ", " << v[1] << ", " << v[2] << ", " << v[3]

WAT. I have no idea what happened here. But again, this is not why I wanted to file the bug, its just something we should fix. Continuing with GDB...

(gdb) 
v = {13, 21, 8, 3, 34, 1, 5, 2}
10        int size = v.size();
(gdb) 
11        if (v.size() > 3) {
(gdb) 
12          int a = 0, b = 0;
(gdb) 
13          for (int x : v)
(gdb) 
14            if (x >= size)
(gdb) 
15              ++b;
(gdb) 
17              ++a;

BUG: We never execute both ++b and ++a. This is a bogus line to jump to, I think it's actually trying to jump to the "end" of the body of the range based for loop, and has this as the source location or something. The behavior of this program is entirely correct, but the debugger is lying to me.

(gdb) 
13          for (int x : v)
(gdb) 
14            if (x >= size)
(gdb) 
15              ++b;
(gdb) 
17              ++a;

And it happens on every loop where we execute "++b".

(gdb) 
13          for (int x : v)
(gdb) 
14            if (x >= size)
(gdb) 
15              ++b;
(gdb) 
17              ++a;
(gdb) 
13          for (int x : v)
(gdb) 
14            if (x >= size)
(gdb) 
16            else if (x >= 0)
(gdb) 
17              ++a;
(gdb) 
13          for (int x : v)

But we only see one stop at that line when we execute "++a".

Let me know what else I can provide to help fix!

[chandlerc]

assigned to @adrian-prantl

[echristo]

I think Adrian had done something here recently?

[adrian-prantl]

Looks like the branch to the for increment is associated with the "end" of the for loop, which makes little sense in the absence of a compound statement.

if.end67: ; preds = %if.end, %if.then63
br label %for.inc, !dbg !​3047

!​3047 = metadata !{i32 17, i32 0, metadata !​3041, null}

[adrian-prantl]

Strictly speaking it was the continuation block of the IfStmt.
Please check out CFE r209764.

[dwblaikie]

Breakpoint 1, main () at x.cc:5
5 std::vector v = {13, 21, 8, 3, 34, 1, 5, 2};
(gdb) n
6 std::cout << "v = {" << v[0] << ", " << v[1] << ", " << v[2] << ",
" << v[3]
(gdb)
7 << ", " << v[4] << ", " << v[5] << ", " << v[6] << ", "
<< v[7]
(gdb)
6 std::cout << "v = {" << v[0] << ", " << v[1] << ", " << v[2] << ",
" << v[3]
(gdb)
7 << ", " << v[4] << ", " << v[5] << ", " << v[6] << ", "
<< v[7]
(gdb)
6 std::cout << "v = {" << v[0] << ", " << v[1] << ", " << v[2] << ",
" << v[3]
(gdb)
7 << ", " << v[4] << ", " << v[5] << ", " << v[6] << ", "
<< v[7]
(gdb)
6 std::cout << "v = {" << v[0] << ", " << v[1] << ", " << v[2] << ",
" << v[3]
(gdb)
7 << ", " << v[4] << ", " << v[5] << ", " << v[6] << ", "
<< v[7]
(gdb)
6 std::cout << "v = {" << v[0] << ", " << v[1] << ", " << v[2] << ",
" << v[3]

WAT. I have no idea what happened here. But again, this is not why I wanted
to file the bug, its just something we should fix.

So to explain this - we were attributing (in clang) the location of a function call (such as the overloaded op<< above) to the beginning of the expression (eg: the 'x' in 'x << y'). So the jumping behavior was produced by evaluating an operand (v[n]) then making the function call (all of which would be attributed to the first character in the whole expression, due to precedence.

This has been improved to attribute overloaded operators to the location of the operator, so each op<< will be attributed to the <<, not the start of its LHS.

So now we get the behavior:

(gdb)
5 std::vector v = {13, 21, 8, 3, 34, 1, 5, 2};
(gdb)
6 std::cout << "v = {" << v[0] << ", " << v[1] << ", " << v[2] << ", " << v[3]
(gdb)
7 << ", " << v[4] << ", " << v[5] << ", " << v[6] << ", " << v[7]
(gdb)
8 << "}\n";

Which looks totally sane.

This would still get a bit jumpy if you'd happened to put the << on the end of the previous line (then it would've gone to the next line to evaluate the operand, then to the previous line to call <<, then back to the next line, etc)...

[dwblaikie]

A lot of the discussion on the topic is summarized here:

http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150209/123042.html

the meaty bits:

TL;DR; We should put cleanup code on the right line (whatever that may be,
I think the end of the scope is best) with is_stmt 0. GDB essentially
ignores is_stmt 0 line table entries and attributes those lines to the
previous line entry which is wrong but fairly beningly so. (it'll just mean
the backtrace is at the last line we codegen'd which is /probably/ the if
block instead of the else block, if the else block is empty) ASan can do
the right thing and we can file a bug on GDB in case they care. But this
gets all the /stepping/ behavior as right as we can get it, really (empty
blocks like the first two cases will still step right past them and get a 1
2 1 2 1 2 sort of behavior, but that seems OK)

That said, implementing this means plumbing is_stmt through from the
frontend, and that seems like more work than I'm willing to put in right
now - but at least it gives us a plan for if/when this bug becomes a
priority for anyone to implement.

One other thing in favor of the is_stmt solution is that it's fairly robust

• almost all the other solutions depend on block ordering for their GDB
  behavior (zero doesn't - but it seems to have some rather bad behavior,
  perhaps that's just GDB bugs, though). The GDB behavior of not breaking
  when stepping into the middle of a line doesn't seem entirely unreasonable,
  and that's all that's 'saving' any of the current solutions (including
  GCC's) and requires specific block ordering for it to do so. Using is_stmt
  0 is block ordering independent with some minor GDB bugs (GDB just
  completely ignores the is_stmt 0 line entries, as though they weren't there
  at all - causing them to be wherever the previous line entry is, getting
  that GDB "don't break in the middle of a line" behavior).

Sound plausible to everyone?

1: for // loop twice
2: if // true on the first call, false on the second
3: CALL;
4: else
5: CALL;

CALL=, exceptions

GCC: 1 2 5 1 2 5 1 (dtor: 5)
ToT: 1 2 1 2 1 (dtor: 2)
End: 1 2 1 2 1 (dtor: 5)

Zero: 1 2 1 2 1 (dtor: 2)
no stmt: <same as ToT, already has is_stmt 0 there by coincidence>

CALL=, no-exceptions

GCC: 1 2 5 1 2 5 1 (dtor: 5)
ToT: 1 2 1 2 1 (dtor: 2)
End: 1 2 5 1 2 5 1 (dtor: 5)

Zero: 1 2 0x400603 (dtor: 0x400608) ??
no stmt: <same as ToT, already has is_stmt 0 there by coincidence>

CALL=func(), exceptions

GCC: 1 2 3 1 2 5 1 (dtor: 5)
ToT: 1 2 3 2 1 2 5 2 1 (dtor: 2)
End: 1 2 3 1 2 1 (dtor: 5)*

Zero: 1 2 3 0x4007ed (dtor: 0x4007f2) ??
no stmt: 1 2 3 1 2 5 1 (dtor: 5)

CALL=func(), no-exceptions

GCC: 1 2 3 1 2 5 1 (dtor: 5)
ToT: 1 2 3 2 1 2 5 2 1 (dtor: 2)
End: 1 2 3 1 2 5 1 (dtor: 5)

Zero: 1 2 3 0x40060b (dtor: 0x400610)
no stmt: 1 2 3 1 2 5 1 (dtor: 5)

• because the EH cleanup code appears before the else block, the else
  block is a line continuation and stepping to it skips over :/ is_stmt might
  help? Let's see. Yep. Once I make the EH cleanup block is_stmt 0 and the
  else block is_stmt 1 (on the same source line) I get the obvious 1 2 3 1 2
  5 1 (dtor: 5) behavior.

[dwblaikie]

*** Bug llvm/llvm-bugzilla-archive#45609 has been marked as a duplicate of this bug. ***

[dwblaikie]

mentioned in issue llvm/llvm-bugzilla-archive#20747

[dwblaikie]

mentioned in issue llvm/llvm-bugzilla-archive#45603

[dwblaikie]

mentioned in issue llvm/llvm-bugzilla-archive#45609

[jimingham]

mentioned in issue llvm/llvm-bugzilla-archive#46032

[dwblaikie]

mentioned in issue llvm/llvm-bugzilla-archive#46032

[dwblaikie]

mentioned in issue llvm/llvm-bugzilla-archive#47232

[llvmbot]

@llvm/issue-subscribers-debuginfo