[PATCH] PR19864: Revert r209764 in favor of the fix in r215766 (loop backedge jump attributed to confusing location when the last source line of a loop is inside a conditional)

[David Blaikie]

Hi aprantl, chandlerc,

The original fix for PR19864 (r209764) indirectly caused the loop
backedge of a (for \ if \ x) construct to be attributed to the 'if' line
(rather than the 'x' line where it was previously). It did this by
changing the location of the cleanup for the 'if'.

This was confusing all around - it's not the job of the 'if' to decide
where a loop backedge is located, source-wise, it was just happening by
accident because no other location was being set*. Instead, if we
specifically set the location before the backedge is emitted, its
location is now independent of that of the if's cleanup (this change was
made in r215766). So now you don't get strange nexting behavior in your
debugger where you could go "for -> if -> x -> /if/ -> for" as the
backedge was attributed to the 'if' line. Now the backedge is attributed
to the 'for' so it looks more natural (for -> if -> x -> for -> if ->
... ).

That is, unless the 'if' has cleanup (dtors) to run. In which case those
lines are attributed to the 'x' line and, in some circumstances, the
debugger may 'next' to them.

It looks as though GCC (though I don't know about LLDB) will avoid
nexting onto the dtor line somehow (I'm guessing some kind of
heuristic, but I haven't experimented too carefully to see just how/when
it works) so, even though the dtor is attributed to the 'x' line, and if
you break in the dtor and 'next' up you reach the 'x' line, if you next
from the 'if' and the condition is false, you'll end up at the 'for'
line and the dtor will have already been executed.

This breaks down when both the natural dtor /and/ exception handling
cleanup are required. GCC's output places the EH cleanup code after the
non-exceptional dtor invocation codepath. Clang places the EH cleanup
code before the non-exceptional dtor. This difference seems to cause GDB
to not skip the dtor line when 'next'ing with Clang's debug info, which
is unfortunate. I'm not sure if it's better fixed in GDB or Clang.

In brief:

Given "for / if / x"
Without any dtors in the 'if' scope, no behavior change.
With dtors:
Previous behavior  (if true): for -> if -> x -> if -> for -> ...
                  (if false): for -> if -> if -> for -> if -> if
New behavior (under GDB):
Without EH cleanup (if true): for -> if -> x -> for -> ...
                  (if false): for -> if -> for -> ...
With EH cleanup (eg: if x can throw), even if false:
                              for -> if -> x -> for -> ...

http://reviews.llvm.org/D4956

Files:
  lib/CodeGen/CGStmt.cpp
  test/CodeGenCXX/debug-info-line-if.cpp

Index: lib/CodeGen/CGStmt.cpp
===================================================================
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -501,7 +501,7 @@
 void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
   // C99 6.8.4.1: The first substatement is executed if the expression compares
   // unequal to 0.  The condition must be a scalar type.
-  LexicalScope ConditionScope(*this, S.getCond()->getSourceRange());
+  LexicalScope ConditionScope(*this, S.getSourceRange());
   RegionCounter Cnt = getPGORegionCounter(&S);
 
   if (S.getConditionVariable())
Index: test/CodeGenCXX/debug-info-line-if.cpp
===================================================================
--- test/CodeGenCXX/debug-info-line-if.cpp
+++ test/CodeGenCXX/debug-info-line-if.cpp
@@ -1,7 +1,12 @@
-// RUN: %clang_cc1 -g -std=c++11 -S -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -g -fexceptions -std=c++11 -S -emit-llvm %s -o - | FileCheck %s
 // PR19864
 extern int v[2];
 int a = 0, b = 0;
+struct foo {
+  ~foo();
+  operator bool() throw();
+};
+void body();
 int main() {
 #line 100
   for (int x : v)
@@ -26,6 +31,48 @@
 
   // CHECK: br label
   // CHECK: br label {{.*}}, !dbg [[DBG2:!.*]]
+
+#line 300
+  if (foo f = foo())
+    ++b;
+  else
+    ++a;
+
+  // The dtor of 'foo' should be attributed to the last line of the if/else
+  // scope, which is the "++a" line, as confusing as that is (especially if the
+  // ++a is never executed because 'f' evaluates to true). But GDB does the
+  // right thing and skips over this (so if you step from "++b" you step outside
+  // to the line outside the else) in this case, but it does get confused by
+  // LLVM's output when exceptions are involved. See the next test case.
+
+  // CHECK: call void @_ZN3fooD1Ev{{.*}}, !dbg [[DBG3:!.*]]
+
+#line 400
+  if (foo f = foo())
+    body(); // this might throw
+
+  // GDB currently steps from the 'foo f' line to the 'body()' line, even if 'f'
+  // is false, because the call to 'f's dtor is attribute to the 'body()' line.
+  // This is the right line for it (the end of the scope) but GDB fails to skip
+  // it when 'next'ing through the code - it seems that if we reorder the output
+  // so that the EH cleanup block (that calls the dtor, with a line table entry
+  // pointing to the end of the function) comes after the normal cleanup, GDB is
+  // fine.
+  // Not sure if this is better (or easier) fixed by changing Clang's output to
+  // match GDB, or fixing GDB's "skip dtors" heuristic (if that's what's going
+  // on on the GDB side here). If this is fixed in Clang, DBG4 and 5 line
+  // numbers should be be the inverse of what's tested below (first call should
+  // be the end-of-scope call, at line 401, second call should be the EH Block
+  // that's currently attributed to the function close '}', line 404)
+
+  // CHECK: call void @_ZN3fooD1Ev{{.*}}, !dbg [[DBG4:!.*]]
+  // CHECK: call void @_ZN3fooD1Ev{{.*}}, !dbg [[DBG5:!.*]]
+
   // CHECK: [[DBG1]] = metadata !{i32 100, i32 0, metadata !{{.*}}, null}
   // CHECK: [[DBG2]] = metadata !{i32 200, i32 0, metadata !{{.*}}, null}
+  // CHECK: [[DBG3]] = metadata !{i32 303, i32 0, metadata !{{.*}}, null}
+  // CHECK: [[DBG4]] = metadata !{i32 404, i32 0, metadata !{{.*}}, null}
+  // CHECK: [[DBG5]] = metadata !{i32 401, i32 0, metadata !{{.*}}, null}
+
+#line 404
 }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D4956.12618.patch
Type: text/x-patch
Size: 3356 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140818/10e822d5/attachment.bin>