Re. the cruelty of macros: I have a case in my backlog where a macro expands to multiple statements; we emit the same source location for each, but put a separate is_stmt flag on each statement. Literally true but not helpful to the debugging experience.
Given that a debugger would be stepping through source as it was originally provided, my feeling is that we should treat the whole mass as a single megastatement whose source location is the location of the macro invocation. --paulr > -----Original Message----- > From: [email protected] [mailto:cfe-commits- > [email protected]] On Behalf Of David Blaikie > Sent: Friday, May 09, 2014 7:45 PM > To: [email protected] > Subject: r208468 - Add FIXME describing the limitation of using column > info to disambiguate inlining. > > Author: dblaikie > Date: Fri May 9 21:44:57 2014 > New Revision: 208468 > > URL: http://llvm.org/viewvc/llvm-project?rev=208468&view=rev > Log: > Add FIXME describing the limitation of using column info to disambiguate > inlining. > > Also tidy up, simplify, and extend the test coverage to demonstrate the > limitations. This test should now fail if the bugs are fixed (& > hopefully whoever ends up in this situation sees the FIXMEs and realizes > that the test needs to be updated to positively test their change that > has fixed some or all of these issues). > > I do wonder whether I could demonstrate breakage without a macro here, > but any way I slice it I can't think of a way to get two calls to the > same function on the same line/column in non-macro C++ - implicit > conversions happen at the same location as an explicit function, but > you'd never get an implicit conversion on the result of an explicit call > to the same implicit conversion operator (since the value is already > converted to the desired result)... > > Modified: > cfe/trunk/lib/CodeGen/CGExpr.cpp > cfe/trunk/test/CodeGenCXX/debug-info-same-line.cpp > > Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp > URL: http://llvm.org/viewvc/llvm- > project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=208468&r1=208467&r2=208468& > view=diff > ======================================================================== > ====== > --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Fri May 9 21:44:57 2014 > @@ -2862,6 +2862,10 @@ RValue CodeGenFunction::EmitCallExpr(con > SourceLocation Loc = E->getLocStart(); > // Force column info to be generated so we can differentiate > // multiple call sites on the same line in the debug info. > + // FIXME: This is insufficient. Two calls coming from the same > macro > + // expansion will still get the same line/column and break debug > info. It's > + // possible that LLVM can be fixed to not rely on this uniqueness, > at which > + // point this workaround can be removed. > const FunctionDecl* Callee = E->getDirectCallee(); > bool ForceColumnInfo = Callee && Callee->isInlineSpecified(); > DI->EmitLocation(Builder, Loc, ForceColumnInfo); > @@ -3131,6 +3135,10 @@ RValue CodeGenFunction::EmitCall(QualTyp > > // Force column info to differentiate multiple inlined call sites on > // the same line, analoguous to EmitCallExpr. > + // FIXME: This is insufficient. Two calls coming from the same macro > expansion > + // will still get the same line/column and break debug info. It's > possible > + // that LLVM can be fixed to not rely on this uniqueness, at which > point this > + // workaround can be removed. > bool ForceColumnInfo = false; > if (const FunctionDecl* FD = dyn_cast_or_null<const > FunctionDecl>(TargetDecl)) > ForceColumnInfo = FD->isInlineSpecified(); > > Modified: cfe/trunk/test/CodeGenCXX/debug-info-same-line.cpp > URL: http://llvm.org/viewvc/llvm- > project/cfe/trunk/test/CodeGenCXX/debug-info-same- > line.cpp?rev=208468&r1=208467&r2=208468&view=diff > ======================================================================== > ====== > --- cfe/trunk/test/CodeGenCXX/debug-info-same-line.cpp (original) > +++ cfe/trunk/test/CodeGenCXX/debug-info-same-line.cpp Fri May 9 > 21:44:57 2014 > @@ -6,118 +6,59 @@ > > #define INLINE inline __attribute__((always_inline)) > > -INLINE int > -product (int x, int y) > -{ > - int result = x * y; > - return result; > -} > - > -INLINE int > -sum (int a, int b) > -{ > - int result = a + b; > - return result; > -} > +int i; > > -int > -strange_max (int m, int n) > -{ > - if (m > n) > - return m; > - else if (n > m) > - return n; > - else > - return 0; > +INLINE void sum(int a, int b) { > + i = a + b; > } > > -int > -foo (int i, int j) > -{ > - if (strange_max (i, j) == i) > - return product (i, j); > - else if (strange_max (i, j) == j) > - return sum (i, j); > - else > - return product (sum (i, i), sum (j, j)); > +void noinline(int x, int y) { > + i = x + y; > } > > -int > -main(int argc, char const *argv[]) > -{ > +#define CALLS sum(9, 10), sum(11, 12) > > - int array[3]; > - int n; > - > - array[0] = foo (1238, 78392); > - array[1] = foo (379265, 23674); > - array[2] = foo (872934, 234); > - > - n = strange_max(array[0], strange_max(array[1], array[2])); > +inline void inlsum(int t, int u) { > + i = t + u; > +} > > - return n & 0xf; > +int main() { > + sum(1, 2), sum(3, 4); > + noinline(5, 6), noinline(7, 8); > + CALLS; > + inlsum(13, 14), inlsum(15, 16); > } > > -// CHECK: define {{.*}} @_Z3fooii > -// i > -// CHECK: call void @llvm.dbg.declare > -// j > -// CHECK: call void @llvm.dbg.declare > -// x > -// CHECK: call void @llvm.dbg.declare > -// y > -// CHECK: call void @llvm.dbg.declare > -// result > -// CHECK: call void @llvm.dbg.declare > - > -// CHECK: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, > metadata ![[A_MD:[0-9]+]]), !dbg ![[A_DI:[0-9]+]] > -// CHECK: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, > metadata ![[B_MD:[0-9]+]]), !dbg ![[B_DI:[0-9]+]] > -// result > -// CHECK: call void @llvm.dbg.declare > - > -// We want to see a distinct !dbg node. > -// CHECK-NOT: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, > metadata ![[A_MD]]), !dbg ![[A_DI]] > -// CHECK: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, > metadata ![[A_MD]]), !dbg !{{.*}} > -// CHECK-NOT: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, > metadata ![[B_MD]]), !dbg ![[B_DI]] > -// CHECK: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, > metadata ![[B_MD]]), !dbg !{{.*}} > -// result > -// CHECK: call void @llvm.dbg.declare > - > -// We want to see a distinct !dbg node. > -// CHECK-NOT: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, > metadata ![[A_MD]]), !dbg ![[A_DI]] > -// CHECK: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, > metadata ![[A_MD]]), !dbg !{{.*}} > -// CHECK-NOT: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, > metadata ![[B_MD]]), !dbg ![[B_DI]] > -// CHECK: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, > metadata ![[B_MD]]), !dbg !{{.*}} > -// result > -// CHECK: call void @llvm.dbg.declare > - > -// Again: we want to see a distinct !dbg node. > -// CHECK: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, > metadata ![[X_MD:[0-9]+]]), !dbg ![[X_DI:[0-9]+]] > -// CHECK: call void @llvm.dbg.declare(metadata !{i32* %{{.*}}}, > metadata ![[Y_MD:[0-9]+]]), !dbg ![[Y_DI:[0-9]+]] > -// result > -// CHECK: call void @llvm.dbg.declare > - > - > -// CHECK: define {{.*}} @main > -// CHECK: call {{.*}} @_Z3fooii > -// CHECK: call {{.*}} @_Z3fooii > -// CHECK: call {{.*}} @_Z3fooii > -// CHECK: store > -// CHECK: getelementptr > -// We want to see the same !dbg node for non-inlined functions. > -// Needed for GDB compatibility. > -// CHECK: load {{.*}} !dbg ![[DBG:.*]] > -// CHECK: load {{.*}} !dbg ![[DBG]] > -// CHECK: load {{.*}} !dbg ![[DBG]] > -// CHECK: call {{.*}} @_Z11strange_maxii(i32 {{.*}}, i32 {{.*}}), !dbg > ![[DBG]] > -// CHECK: call {{.*}} @_Z11strange_maxii(i32 {{.*}}, i32 {{.*}}), !dbg > ![[DBG]] > - > - > -// Verify that product() has its own inlined_at location at column 15. > -// CHECK-DAG: ![[A_MD]] = metadata{{.*}}[ DW_TAG_arg_variable ] [a] > -// CHECK-DAG: ![[B_MD]] = metadata{{.*}}[ DW_TAG_arg_variable ] [b] > -// CHECK-DAG: ![[X_MD]] = metadata{{.*}}[ DW_TAG_arg_variable ] [x] > -// CHECK-DAG: ![[Y_MD]] = metadata{{.*}}[ DW_TAG_arg_variable ] [y] > -// CHECK-DAG: ![[X_DI]] = metadata !{i32 {{[0-9]+}}, i32 {{[0-9]+}}, > metadata !{{[0-9]+}}, metadata ![[PRODUCT:[0-9]+]]} > -// CHECK-DAG: [[PRODUCT]] = metadata !{i32 {{.*}}, i32 16, metadata > !{{.*}}, null} > -// CHECK-DAG: ![[Y_DI]] = metadata !{i32 {{[0-9]+}}, i32 {{[0-9]+}}, > metadata !{{[0-9]+}}, metadata ![[PRODUCT]]} > +// CHECK-LABEL: @main > +// CHECK: = add {{.*}} !dbg [[FIRST_INLINE:![0-9]*]] > +// CHECK: = add {{.*}} !dbg [[SECOND_INLINE:![0-9]*]] > + > +// Check that we don't give column information (and thus end up with > distinct > +// line entries) for two non-inlined calls on the same line. > +// CHECK: call {{.*}}noinline{{.*}}(i32 5, i32 6), !dbg [[NOINLINE:![0- > 9]*]] > +// CHECK: call {{.*}}noinline{{.*}}(i32 7, i32 8), !dbg [[NOINLINE]] > + > +// FIXME: These should be separate locations but because the two calls > have the > +// same line /and/ column, they get coalesced into a single inlined > call by > +// accident. We need discriminators or some other changes to LLVM to > cope with > +// this. (this is, unfortunately, an LLVM test disguised as a Clang > test - since > +// inlining is forced to happen here). It's possible this could be > fixed in > +// Clang, but I doubt it'll be the right place for the fix. > +// CHECK: = add {{.*}} !dbg [[FIRST_MACRO_INLINE:![0-9]*]] > +// CHECK: = add {{.*}} !dbg [[FIRST_MACRO_INLINE]] > + > +// Even if the functions are marked inline but do not get inlined, they > +// shouldn't use column information, and thus should be at the same > debug > +// location. > +// CHECK: call {{.*}}inlsum{{.*}}(i32 13, i32 14), !dbg > [[INL_FIRST:![0-9]*]] > +// CHECK: call {{.*}}inlsum{{.*}}(i32 15, i32 16), !dbg > [[INL_SECOND:![0-9]*]] > + > +// [[FIRST_INLINE]] = > +// [[SECOND_INLINE]] = > + > +// FIXME: These should be the same location since the functions appear > on the > +// same line and were not inlined - they needlessly have column > information > +// intended to disambiguate inlined calls, which is going to confuse > GDB as it > +// doesn't cope well with column information. > +// [[INL_FIRST]] = > +// [[INL_SECOND]] = > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
