| Message ID | CAO2gOZWhueDAShNLbNcJpkHA6QqXk1LNZhEMFvfT77aTZTCH9w@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
ping.... Thanks, Dehao On Sat, Aug 18, 2012 at 6:02 AM, Dehao Chen <dehao@google.com> wrote: > Hi, Richard, > > Thanks for the review. I've addressed most of the issues except the > java unittest (see comments below). The new patch is attached in the > end of this email. > > Thanks, > Dehao > > On Fri, Aug 17, 2012 at 8:47 AM, Richard Henderson <rth@redhat.com> wrote: >> On 2012-08-10 20:38, Dehao Chen wrote: >>> + // { dg-final { scan-assembler "1 28 0" } } >> >> This test case isn't going to work except with dwarf2, and with gas. >> You can use -dA so that you can scan for file.c:line. There are >> other examples in the testsuite. > > Done. >> >> This doesn't belong in guality. It belongs in g++.dg/debug/. > > Done. > >> It would be nice if you could add a java testcase to see that it >> doesn't regress. > > I spend a whole day working on this, but find it very difficult to add > such a java test because: > > * First, libjava testsuits are all runtime tests, i.e., it compiles > the byte code to native code, execute it, and compares the output to > expected output. There is no way to scan the assembly. > * Though there is a way to derive the line number at runtime in java > (using Exception().getStackTrace()), this method only works on VM, and > the gcj generated native code does not get the lineno. > > Any suggestions on this? > >> >>> ! record_in_goto_queue_label (struct leh_tf_state *tf, treemple stmt, >>> tree label, >>> ! location_t location) >> >> BTW, for the future, please fix your mailer to not wrap lines. > > Okay, I'll try. The problem is that we have to send mail in plain txt. > And in "plain text mode" gmail wraps each line to 80 characters and > wouldn't allow you change that... > >> I'll quibble with the wording here. It reads as if we want to force >> all calls to have UNKNOWN_LOC, whereas all we want is to prevent any >> calls that already have UNKNOWN_LOC from gaining input_loc via gimplify_call_expr. > > Done. > > New Patch: > gcc/ChangeLog > * tree-eh.c (goto_queue_node): New field. > (record_in_goto_queue): New parameter. > (record_in_goto_queue_label): New parameter. > (lower_try_finally_dup_block): New parameter. > (maybe_record_in_goto_queue): Update source location. > (lower_try_finally_copy): Likewise. > (honor_protect_cleanup_actions): Likewise. > * gimplify.c (gimplify_expr): Reset the location to unknown. > > gcc/testsuite/ChangeLog > 2012-08-07 Dehao Chen <dehao@google.com> > > * g++.dg/debug/dwarf2/deallocator.C: New test. > > Index: gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C > =================================================================== > --- gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C (revision 0) > +++ gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C (revision 0) > @@ -0,0 +1,33 @@ > +// Test that debug info generated for auto-inserted deallocator is > +// correctly attributed. > +// This patch scans for the lineno directly from assembly, which may > +// differ between different architectures. Because it mainly tests > +// FE generated debug info, without losing generality, only x86 > +// assembly is scanned in this test. > +// { dg-do compile { target { i?86-*-* x86_64-*-* } } } > +// { dg-options "-O2 -fno-exceptions -g -dA" } > + > +struct t { > + t (); > + ~t (); > + void foo(); > + void bar(); > +}; > + > +int bar(); > + > +void foo(int i) > +{ > + for (int j = 0; j < 10; j++) > + { > + t test; > + test.foo(); > + if (i + j) > + { > + test.bar(); > + return; > + } > + } > + return; > +} > +// { dg-final { scan-assembler "1 28 0" } } > Index: gcc/tree-eh.c > =================================================================== > --- gcc/tree-eh.c (revision 190209) > +++ gcc/tree-eh.c (working copy) > @@ -321,6 +321,7 @@ > struct goto_queue_node > { > treemple stmt; > + location_t location; > gimple_seq repl_stmt; > gimple cont_stmt; > int index; > @@ -560,7 +561,8 @@ > record_in_goto_queue (struct leh_tf_state *tf, > treemple new_stmt, > int index, > - bool is_label) > + bool is_label, > + location_t location) > { > size_t active, size; > struct goto_queue_node *q; > @@ -583,6 +585,7 @@ > memset (q, 0, sizeof (*q)); > q->stmt = new_stmt; > q->index = index; > + q->location = location; > q->is_label = is_label; > } > > @@ -590,7 +593,8 @@ > TF is not null. */ > > static void > -record_in_goto_queue_label (struct leh_tf_state *tf, treemple stmt, tree label) > +record_in_goto_queue_label (struct leh_tf_state *tf, treemple stmt, tree label, > + location_t location) > { > int index; > treemple temp, new_stmt; > @@ -629,7 +633,7 @@ > since with a GIMPLE_COND we have an easy access to the then/else > labels. */ > new_stmt = stmt; > - record_in_goto_queue (tf, new_stmt, index, true); > + record_in_goto_queue (tf, new_stmt, index, true, location); > } > > /* For any GIMPLE_GOTO or GIMPLE_RETURN, decide whether it leaves a try_finally > @@ -649,19 +653,22 @@ > { > case GIMPLE_COND: > new_stmt.tp = gimple_op_ptr (stmt, 2); > - record_in_goto_queue_label (tf, new_stmt, gimple_cond_true_label (stmt)); > + record_in_goto_queue_label (tf, new_stmt, gimple_cond_true_label (stmt), > + EXPR_LOCATION (*new_stmt.tp)); > new_stmt.tp = gimple_op_ptr (stmt, 3); > - record_in_goto_queue_label (tf, new_stmt, gimple_cond_false_label (stmt)); > + record_in_goto_queue_label (tf, new_stmt, gimple_cond_false_label (stmt), > + EXPR_LOCATION (*new_stmt.tp)); > break; > case GIMPLE_GOTO: > new_stmt.g = stmt; > - record_in_goto_queue_label (tf, new_stmt, gimple_goto_dest (stmt)); > + record_in_goto_queue_label (tf, new_stmt, gimple_goto_dest (stmt), > + gimple_location (stmt)); > break; > > case GIMPLE_RETURN: > tf->may_return = true; > new_stmt.g = stmt; > - record_in_goto_queue (tf, new_stmt, -1, false); > + record_in_goto_queue (tf, new_stmt, -1, false, gimple_location (stmt)); > break; > > default: > @@ -866,13 +873,19 @@ > Make sure to record all new labels found. */ > > static gimple_seq > -lower_try_finally_dup_block (gimple_seq seq, struct leh_state *outer_state) > +lower_try_finally_dup_block (gimple_seq seq, struct leh_state *outer_state, > + location_t loc) > { > gimple region = NULL; > gimple_seq new_seq; > + gimple_stmt_iterator gsi; > > new_seq = copy_gimple_seq_and_replace_locals (seq); > > + for (gsi = gsi_start (new_seq); !gsi_end_p (gsi); gsi_next (&gsi)) > + if (gimple_location (gsi_stmt (gsi)) == UNKNOWN_LOCATION) > + gimple_set_location (gsi_stmt (gsi), loc); > + > if (outer_state->tf) > region = outer_state->tf->try_finally_expr; > collect_finally_tree_1 (new_seq, region); > @@ -967,7 +980,8 @@ > gimple_try_set_cleanup (tf->top_p, gimple_eh_else_n_body (eh_else)); > } > else if (this_state) > - finally = lower_try_finally_dup_block (finally, outer_state); > + finally = lower_try_finally_dup_block (finally, outer_state, > + UNKNOWN_LOCATION); > finally_may_fallthru = gimple_seq_may_fallthru (finally); > > /* If this cleanup consists of a TRY_CATCH_EXPR with TRY_CATCH_IS_CLEANUP > @@ -1184,7 +1198,7 @@ > > if (tf->may_fallthru) > { > - seq = lower_try_finally_dup_block (finally, state); > + seq = lower_try_finally_dup_block (finally, state, tf_loc); > lower_eh_constructs_1 (state, &seq); > gimple_seq_add_seq (&new_stmt, seq); > > @@ -1200,7 +1214,7 @@ > if (eh_else) > seq = gimple_eh_else_e_body (eh_else); > else > - seq = lower_try_finally_dup_block (finally, state); > + seq = lower_try_finally_dup_block (finally, state, tf_loc); > lower_eh_constructs_1 (state, &seq); > > emit_post_landing_pad (&eh_seq, tf->region); > @@ -1250,7 +1264,7 @@ > x = gimple_build_label (lab); > gimple_seq_add_stmt (&new_stmt, x); > > - seq = lower_try_finally_dup_block (finally, state); > + seq = lower_try_finally_dup_block (finally, state, q->location); > lower_eh_constructs_1 (state, &seq); > gimple_seq_add_seq (&new_stmt, seq); > > Index: gcc/gimplify.c > =================================================================== > --- gcc/gimplify.c (revision 190209) > +++ gcc/gimplify.c (working copy) > @@ -7434,6 +7434,15 @@ > gimple_seq eval, cleanup; > gimple try_; > > + /* Calls to destructors are generated automatically in FINALL/CATCH > + block. They should have location as UNKNOWN_LOCATION. However, > + gimplify_call_expr will reset these call stmts to input_location > + if it finds stmt's location is unknown. To prevent resetting for > + destructors, we set the input_location to unknown. > + Note that this only affects the destructor calls in FINALL/CATCH > + block, and will automatically reset to its original value by the > + end of gimplify_expr. */ > + input_location = UNKNOWN_LOCATION; > eval = cleanup = NULL; > gimplify_and_add (TREE_OPERAND (*expr_p, 0), &eval); > gimplify_and_add (TREE_OPERAND (*expr_p, 1), &cleanup);
On 08/17/2012 03:02 PM, Dehao Chen wrote: > I spend a whole day working on this, but find it very difficult to add > such a java test because: > > * First, libjava testsuits are all runtime tests, i.e., it compiles > the byte code to native code, execute it, and compares the output to > expected output. There is no way to scan the assembly. > * Though there is a way to derive the line number at runtime in java > (using Exception().getStackTrace()), this method only works on VM, and > the gcj generated native code does not get the lineno. > > Any suggestions on this? Hmm, not from me, unfortunately. Cc'ing the java list for clues. I won't hang up the main patch for this though. >> BTW, for the future, please fix your mailer to not wrap lines. > > Okay, I'll try. The problem is that we have to send mail in plain txt. > And in "plain text mode" gmail wraps each line to 80 characters and > wouldn't allow you change that... In that case use a text/plain attachment (which, not having tried it myself, may require you use a .txt suffix on the patch file). Most mail readers will show those inline. It's certainly better than having actually corrupt data sent to the list. > +// { dg-options "-O2 -fno-exceptions -g -dA" } ... > +// { dg-final { scan-assembler "1 28 0" } } You're still scanning for the .loc line, not the "test.c:28" comment added by -dA. To understand the problem, go back to your build tree, edit auto-host.h and undefine HAVE_AS_DWARF2_DEBUG_LINE. Then rerun the testsuite with RUNTESTFLAGS=dwarf2.exp. > + /* Calls to destructors are generated automatically in FINALL/CATCH > + block. They should have location as UNKNOWN_LOCATION. However, > + gimplify_call_expr will reset these call stmts to input_location > + if it finds stmt's location is unknown. To prevent resetting for > + destructors, we set the input_location to unknown. > + Note that this only affects the destructor calls in FINALL/CATCH > + block, and will automatically reset to its original value by the > + end of gimplify_expr. */ s/FINALL/FINALLY/g r~
On Thu, Aug 30, 2012 at 3:28 PM, Richard Henderson <rth@redhat.com> wrote: > On 08/17/2012 03:02 PM, Dehao Chen wrote: >> I spend a whole day working on this, but find it very difficult to add >> such a java test because: >> >> * First, libjava testsuits are all runtime tests, i.e., it compiles >> the byte code to native code, execute it, and compares the output to >> expected output. There is no way to scan the assembly. >> * Though there is a way to derive the line number at runtime in java >> (using Exception().getStackTrace()), this method only works on VM, and >> the gcj generated native code does not get the lineno. >> >> Any suggestions on this? > > Hmm, not from me, unfortunately. Cc'ing the java list for clues. > I won't hang up the main patch for this though. libjava calls out to addr2line to get the line number and source file name for stack traces. As long as it can find addr2line you should get a line number - but some platforms don't have it. Ref: libjava/stacktrace.cc and libjava/gnu/gcj/runtime/NameFinder.java
On 08/30/2012 03:28 PM, Richard Henderson wrote: > On 08/17/2012 03:02 PM, Dehao Chen wrote: >> I spend a whole day working on this, but find it very difficult to add >> such a java test because: >> >> * First, libjava testsuits are all runtime tests, i.e., it compiles >> the byte code to native code, execute it, and compares the output to >> expected output. There is no way to scan the assembly. >> * Though there is a way to derive the line number at runtime in java >> (using Exception().getStackTrace()), this method only works on VM, and >> the gcj generated native code does not get the lineno. >> >> Any suggestions on this? > > Hmm, not from me, unfortunately. Cc'ing the java list for clues. > I won't hang up the main patch for this though. Fair enough. As Bryce said, line numbers should work if you have addr2line installed. Can't we scan the assembly? Is the problem simply that the logic to scan the assembly code isn't present in the libgcj testsuite? Andrew.
On 08/30/2012 08:20 AM, Andrew Haley wrote: > Is the problem simply that the logic to > scan the assembly code isn't present in the libgcj testsuite? Yes, exactly. r~
On Thu, Aug 30, 2012 at 9:33 AM, Richard Henderson <rth@redhat.com> wrote: > On 08/30/2012 08:20 AM, Andrew Haley wrote: >> Is the problem simply that the logic to >> scan the assembly code isn't present in the libgcj testsuite? > > Yes, exactly. For this case, I don't think that we want a testcase to rely on addr2line in the system. So how about that that we add a test when assembly scan is available in libgcj testsuit? Thanks, Dehao > > > r~
On 09/04/2012 05:07 PM, Dehao Chen wrote: > On Thu, Aug 30, 2012 at 9:33 AM, Richard Henderson <rth@redhat.com> wrote: >> On 08/30/2012 08:20 AM, Andrew Haley wrote: >>> Is the problem simply that the logic to >>> scan the assembly code isn't present in the libgcj testsuite? >> >> Yes, exactly. > > For this case, I don't think that we want a testcase to rely on > addr2line in the system. So how about that that we add a test when > assembly scan is available in libgcj testsuit? Fine by me. I guess you can just copy the scanning code from the gcc testsuite. Andrew.
On Tue, Sep 4, 2012 at 5:07 PM, Dehao Chen <dehao@google.com> wrote: > On Thu, Aug 30, 2012 at 9:33 AM, Richard Henderson <rth@redhat.com> wrote: >> On 08/30/2012 08:20 AM, Andrew Haley wrote: >>> Is the problem simply that the logic to >>> scan the assembly code isn't present in the libgcj testsuite? >> >> Yes, exactly. > > For this case, I don't think that we want a testcase to rely on > addr2line in the system. So how about that that we add a test when > assembly scan is available in libgcj testsuit? Once Ian Lance Taylor's libbacktrace patch is integrated (see: http://gcc.gnu.org/ml/gcc/2012-08/msg00317.html), we'll be able to get rid of the code that calls addr2line from libgcj. So, I think it would be fine to write a Java test case using Throwable.getStackTrace(). Whichever approach is easiest for you is fine. Bryce
On 09/04/2012 05:32 PM, Bryce McKinlay wrote: > On Tue, Sep 4, 2012 at 5:07 PM, Dehao Chen <dehao@google.com> wrote: >> On Thu, Aug 30, 2012 at 9:33 AM, Richard Henderson <rth@redhat.com> wrote: >>> On 08/30/2012 08:20 AM, Andrew Haley wrote: >>>> Is the problem simply that the logic to >>>> scan the assembly code isn't present in the libgcj testsuite? >>> >>> Yes, exactly. >> >> For this case, I don't think that we want a testcase to rely on >> addr2line in the system. So how about that that we add a test when >> assembly scan is available in libgcj testsuit? > > Once Ian Lance Taylor's libbacktrace patch is integrated (see: > http://gcc.gnu.org/ml/gcc/2012-08/msg00317.html), we'll be able to get > rid of the code that calls addr2line from libgcj. As I understand it, Ian Taylor's backtrace patch is intended for use in gcc development, and as he puts it "Since its use in GCC would be purely for GCC developers, it's not essential that it be fully portable." Not for gcj runtime. Andrew.
On Tue, Sep 4, 2012 at 5:39 PM, Andrew Haley <aph@redhat.com> wrote: > On 09/04/2012 05:32 PM, Bryce McKinlay wrote: >> On Tue, Sep 4, 2012 at 5:07 PM, Dehao Chen <dehao@google.com> wrote: >>> On Thu, Aug 30, 2012 at 9:33 AM, Richard Henderson <rth@redhat.com> wrote: >>>> On 08/30/2012 08:20 AM, Andrew Haley wrote: >>>>> Is the problem simply that the logic to >>>>> scan the assembly code isn't present in the libgcj testsuite? >>>> >>>> Yes, exactly. >>> >>> For this case, I don't think that we want a testcase to rely on >>> addr2line in the system. So how about that that we add a test when >>> assembly scan is available in libgcj testsuit? >> >> Once Ian Lance Taylor's libbacktrace patch is integrated (see: >> http://gcc.gnu.org/ml/gcc/2012-08/msg00317.html), we'll be able to get >> rid of the code that calls addr2line from libgcj. > > As I understand it, Ian Taylor's backtrace patch is intended for use in > gcc development, and as he puts it "Since its use in GCC would > be purely for GCC developers, it's not essential that it be fully > portable." Not for gcj runtime. He's also planning to use it for libgo, and other gcc runtime libs have indicated interest. It doesn't have to work on all platforms, and I can't see how it would be any less portable than addr2line! Bryce
On 09/04/2012 06:08 PM, Bryce McKinlay wrote: > On Tue, Sep 4, 2012 at 5:39 PM, Andrew Haley <aph@redhat.com> wrote: >> On 09/04/2012 05:32 PM, Bryce McKinlay wrote: >>> On Tue, Sep 4, 2012 at 5:07 PM, Dehao Chen <dehao@google.com> wrote: >>>> On Thu, Aug 30, 2012 at 9:33 AM, Richard Henderson <rth@redhat.com> wrote: >>>>> On 08/30/2012 08:20 AM, Andrew Haley wrote: >>>>>> Is the problem simply that the logic to >>>>>> scan the assembly code isn't present in the libgcj testsuite? >>>>> >>>>> Yes, exactly. >>>> >>>> For this case, I don't think that we want a testcase to rely on >>>> addr2line in the system. So how about that that we add a test when >>>> assembly scan is available in libgcj testsuit? >>> >>> Once Ian Lance Taylor's libbacktrace patch is integrated (see: >>> http://gcc.gnu.org/ml/gcc/2012-08/msg00317.html), we'll be able to get >>> rid of the code that calls addr2line from libgcj. >> >> As I understand it, Ian Taylor's backtrace patch is intended for use in >> gcc development, and as he puts it "Since its use in GCC would >> be purely for GCC developers, it's not essential that it be fully >> portable." Not for gcj runtime. > > He's also planning to use it for libgo, and other gcc runtime libs > have indicated interest. It doesn't have to work on all platforms, and > I can't see how it would be any less portable than addr2line! I certainly can. Maybe once it's shaken-down so it's at least as robust as what we have now it'll be OK. I suspect it hasn't had much testing with, for example, unwinding through signal handlers. Andrew.
On Tue, Sep 4, 2012 at 6:12 PM, Andrew Haley <aph@redhat.com> wrote: >> He's also planning to use it for libgo, and other gcc runtime libs >> have indicated interest. It doesn't have to work on all platforms, and >> I can't see how it would be any less portable than addr2line! > > I certainly can. Maybe once it's shaken-down so it's at least as > robust as what we have now it'll be OK. I suspect it hasn't had much > testing with, for example, unwinding through signal handlers. libgcj wouldn't actually use it for unwinding, we already have all that. We'd just use it to read DWARF debug info and give us the source code line numbers.
On 09/04/2012 06:17 PM, Bryce McKinlay wrote: > On Tue, Sep 4, 2012 at 6:12 PM, Andrew Haley <aph@redhat.com> wrote: > >>> He's also planning to use it for libgo, and other gcc runtime libs >>> have indicated interest. It doesn't have to work on all platforms, and >>> I can't see how it would be any less portable than addr2line! >> >> I certainly can. Maybe once it's shaken-down so it's at least as >> robust as what we have now it'll be OK. I suspect it hasn't had much >> testing with, for example, unwinding through signal handlers. > > libgcj wouldn't actually use it for unwinding, we already have all > that. We'd just use it to read DWARF debug info and give us the source > code line numbers. OK, as long as that's all it does. I think I was perhaps a bit misled by its description of "a stack backtrace library". It certainly looks like a nicer approach than addr2line, but is going to be much less well-ported. I guess we'll see how it goes. Andrew.
Looks like even with addr2line properly installed, the gcj generated
code cannot get the correct source file/lineno. Do I need to pass in
anything to gcj to let it know where addr2line is?
Thanks,
Dehao
#javac stacktrace.java
#java stacktrace
stacktrace.e(stacktrace.java:42)
stacktrace.d(stacktrace.java:38)
stacktrace.c(stacktrace.java:31)
stacktrace.b(stacktrace.java:26)
stacktrace.a(stacktrace.java:19)
stacktrace.main(stacktrace.java:12)
#gcj *.class -o stacktrace.exe
#./stacktrace.exe
stacktrace.e(stacktrace.exe:-1)
stacktrace.d(stacktrace.exe:-1)
stacktrace.c(stacktrace.exe:-1)
stacktrace.b(stacktrace.exe:-1)
stacktrace.a(stacktrace.exe:-1)
stacktrace.main(stacktrace.exe:-1)
The java code is shown below:
stacktrace.java
/* This test should test the stacktrace functionality.
We only print ClassName and MethName since the other information
like FileName and LineNumber are not consistent while building
native or interpreted and we want to test the output inside the dejagnu
test environment.
Also, we have to make the methods public since they might be optimized away
with inline's and then the -O3/-O2 execution might fail.
*/
public class stacktrace {
public static void main(String args[]) {
try {
new stacktrace().a();
} catch (TopException e) {
}
}
public void a() throws TopException {
try {
b();
} catch (MiddleException e) {
throw new TopException(e);
}
}
public void b() throws MiddleException {
c();
}
public void c() throws MiddleException {
try {
d();
} catch (BottomException e) {
throw new MiddleException(e);
}
}
public void d() throws BottomException {
e();
}
public void e() throws BottomException {
throw new BottomException();
}
}
class TopException extends Exception {
TopException(Throwable cause) {
super(cause);
}
}
class MiddleException extends Exception {
MiddleException(Throwable cause) {
super(cause);
}
}
class BottomException extends Exception {
BottomException() {
StackTraceElement stack[] = this.getStackTrace();
for (int i = 0; i < stack.length; i++) {
String className = stack[i].getClassName();
String methodName = stack[i].getMethodName();
System.out.println(className + "." + methodName + "(" +
stack[i].getFileName() + ":" +
stack[i].getLineNumber() + ")");
}
}
}
On Tue, Sep 4, 2012 at 9:22 AM, Andrew Haley <aph@redhat.com> wrote: > On 09/04/2012 05:07 PM, Dehao Chen wrote: >> On Thu, Aug 30, 2012 at 9:33 AM, Richard Henderson <rth@redhat.com> wrote: >>> On 08/30/2012 08:20 AM, Andrew Haley wrote: >>>> Is the problem simply that the logic to >>>> scan the assembly code isn't present in the libgcj testsuite? >>> >>> Yes, exactly. >> >> For this case, I don't think that we want a testcase to rely on >> addr2line in the system. So how about that that we add a test when >> assembly scan is available in libgcj testsuit? > > Fine by me. I guess you can just copy the scanning code from the gcc > testsuite. I tried that, but it is not trivial, and simply copying "proc scan-assembler" to libjava seems ugly. Do libjava people really think it's worth to add scan-assembler and other premitives in gcc testsuite into libjava testsuite? If yes, I'll leave it to the TODO list. Thanks, Dehao > > Andrew. >
On 09/04/2012 09:31 PM, Dehao Chen wrote: > Looks like even with addr2line properly installed, the gcj generated > code cannot get the correct source file/lineno. Do I need to pass in > > #javac stacktrace.java > #java stacktrace > stacktrace.e(stacktrace.java:42) > stacktrace.d(stacktrace.java:38) > stacktrace.c(stacktrace.java:31) > stacktrace.b(stacktrace.java:26) > stacktrace.a(stacktrace.java:19) > stacktrace.main(stacktrace.java:12) > #gcj *.class -o stacktrace.exe > #./stacktrace.exe > stacktrace.e(stacktrace.exe:-1) > stacktrace.d(stacktrace.exe:-1) > stacktrace.c(stacktrace.exe:-1) > stacktrace.b(stacktrace.exe:-1) > stacktrace.a(stacktrace.exe:-1) > stacktrace.main(stacktrace.exe:-1) Works for me: [aph@nighthawk ~]$ gcj stacktrace.java --main=stacktrace -g [aph@nighthawk ~]$ ./a.out stacktrace.e(stacktrace.java:42) stacktrace.d(stacktrace.java:38) stacktrace.c(stacktrace.java:31) stacktrace.b(stacktrace.java:26) stacktrace.a(stacktrace.java:19) stacktrace.main(stacktrace.java:12) Aren't you just compiling without -g ? There is no debuginfo. Andrew.
On Wed, Sep 5, 2012 at 12:29 AM, Andrew Haley <aph@redhat.com> wrote: > On 09/04/2012 09:31 PM, Dehao Chen wrote: >> Looks like even with addr2line properly installed, the gcj generated >> code cannot get the correct source file/lineno. Do I need to pass in >> >> #javac stacktrace.java >> #java stacktrace >> stacktrace.e(stacktrace.java:42) >> stacktrace.d(stacktrace.java:38) >> stacktrace.c(stacktrace.java:31) >> stacktrace.b(stacktrace.java:26) >> stacktrace.a(stacktrace.java:19) >> stacktrace.main(stacktrace.java:12) >> #gcj *.class -o stacktrace.exe >> #./stacktrace.exe >> stacktrace.e(stacktrace.exe:-1) >> stacktrace.d(stacktrace.exe:-1) >> stacktrace.c(stacktrace.exe:-1) >> stacktrace.b(stacktrace.exe:-1) >> stacktrace.a(stacktrace.exe:-1) >> stacktrace.main(stacktrace.exe:-1) > > Works for me: > > [aph@nighthawk ~]$ gcj stacktrace.java --main=stacktrace -g > [aph@nighthawk ~]$ ./a.out > stacktrace.e(stacktrace.java:42) > stacktrace.d(stacktrace.java:38) > stacktrace.c(stacktrace.java:31) > stacktrace.b(stacktrace.java:26) > stacktrace.a(stacktrace.java:19) > stacktrace.main(stacktrace.java:12) > > Aren't you just compiling without -g ? There is no debuginfo. The other thing that might be needed is a newer addr2line which works correctly with the dwarf2(4) that GCC outputs. Thanks, Andrew
On Tue, 2012-09-04 at 18:17 +0100, Bryce McKinlay wrote: > libgcj wouldn't actually use it for unwinding, we already have all > that. We'd just use it to read DWARF debug info and give us the source > code line numbers. Casey Marshell did also write that part some time ago, but it was never finished/integrated. http://gcc.gnu.org/ml/java-patches/2004-q3/msg00350.html Cheers, Mark
Index: gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C =================================================================== --- gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C (revision 0) +++ gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C (revision 0) @@ -0,0 +1,33 @@ +// Test that debug info generated for auto-inserted deallocator is +// correctly attributed. +// This patch scans for the lineno directly from assembly, which may +// differ between different architectures. Because it mainly tests +// FE generated debug info, without losing generality, only x86 +// assembly is scanned in this test. +// { dg-do compile { target { i?86-*-* x86_64-*-* } } } +// { dg-options "-O2 -fno-exceptions -g -dA" } + +struct t { + t (); + ~t (); + void foo(); + void bar(); +}; + +int bar(); + +void foo(int i) +{ + for (int j = 0; j < 10; j++) + { + t test; + test.foo(); + if (i + j) + { + test.bar(); + return; + } + } + return; +} +// { dg-final { scan-assembler "1 28 0" } } Index: gcc/tree-eh.c =================================================================== --- gcc/tree-eh.c (revision 190209) +++ gcc/tree-eh.c (working copy) @@ -321,6 +321,7 @@ struct goto_queue_node { treemple stmt; + location_t location; gimple_seq repl_stmt; gimple cont_stmt; int index; @@ -560,7 +561,8 @@ record_in_goto_queue (struct leh_tf_state *tf, treemple new_stmt, int index, - bool is_label) + bool is_label, + location_t location) { size_t active, size; struct goto_queue_node *q; @@ -583,6 +585,7 @@ memset (q, 0, sizeof (*q)); q->stmt = new_stmt; q->index = index; + q->location = location; q->is_label = is_label; } @@ -590,7 +593,8 @@ TF is not null. */ static void -record_in_goto_queue_label (struct leh_tf_state *tf, treemple stmt, tree label) +record_in_goto_queue_label (struct leh_tf_state *tf, treemple stmt, tree label, + location_t location) { int index; treemple temp, new_stmt; @@ -629,7 +633,7 @@ since with a GIMPLE_COND we have an easy access to the then/else labels. */ new_stmt = stmt; - record_in_goto_queue (tf, new_stmt, index, true); + record_in_goto_queue (tf, new_stmt, index, true, location); } /* For any GIMPLE_GOTO or GIMPLE_RETURN, decide whether it leaves a try_finally @@ -649,19 +653,22 @@ { case GIMPLE_COND: new_stmt.tp = gimple_op_ptr (stmt, 2); - record_in_goto_queue_label (tf, new_stmt, gimple_cond_true_label (stmt)); + record_in_goto_queue_label (tf, new_stmt, gimple_cond_true_label (stmt), + EXPR_LOCATION (*new_stmt.tp)); new_stmt.tp = gimple_op_ptr (stmt, 3); - record_in_goto_queue_label (tf, new_stmt, gimple_cond_false_label (stmt)); + record_in_goto_queue_label (tf, new_stmt, gimple_cond_false_label (stmt), + EXPR_LOCATION (*new_stmt.tp)); break; case GIMPLE_GOTO: new_stmt.g = stmt; - record_in_goto_queue_label (tf, new_stmt, gimple_goto_dest (stmt)); + record_in_goto_queue_label (tf, new_stmt, gimple_goto_dest (stmt), + gimple_location (stmt)); break; case GIMPLE_RETURN: tf->may_return = true; new_stmt.g = stmt; - record_in_goto_queue (tf, new_stmt, -1, false); + record_in_goto_queue (tf, new_stmt, -1, false, gimple_location (stmt)); break; default: @@ -866,13 +873,19 @@ Make sure to record all new labels found. */ static gimple_seq -lower_try_finally_dup_block (gimple_seq seq, struct leh_state *outer_state) +lower_try_finally_dup_block (gimple_seq seq, struct leh_state *outer_state, + location_t loc) { gimple region = NULL; gimple_seq new_seq; + gimple_stmt_iterator gsi; new_seq = copy_gimple_seq_and_replace_locals (seq); + for (gsi = gsi_start (new_seq); !gsi_end_p (gsi); gsi_next (&gsi)) + if (gimple_location (gsi_stmt (gsi)) == UNKNOWN_LOCATION) + gimple_set_location (gsi_stmt (gsi), loc); + if (outer_state->tf) region = outer_state->tf->try_finally_expr; collect_finally_tree_1 (new_seq, region); @@ -967,7 +980,8 @@ gimple_try_set_cleanup (tf->top_p, gimple_eh_else_n_body (eh_else)); } else if (this_state) - finally = lower_try_finally_dup_block (finally, outer_state); + finally = lower_try_finally_dup_block (finally, outer_state, + UNKNOWN_LOCATION); finally_may_fallthru = gimple_seq_may_fallthru (finally); /* If this cleanup consists of a TRY_CATCH_EXPR with TRY_CATCH_IS_CLEANUP @@ -1184,7 +1198,7 @@ if (tf->may_fallthru) { - seq = lower_try_finally_dup_block (finally, state); + seq = lower_try_finally_dup_block (finally, state, tf_loc); lower_eh_constructs_1 (state, &seq); gimple_seq_add_seq (&new_stmt, seq); @@ -1200,7 +1214,7 @@ if (eh_else) seq = gimple_eh_else_e_body (eh_else); else - seq = lower_try_finally_dup_block (finally, state); + seq = lower_try_finally_dup_block (finally, state, tf_loc); lower_eh_constructs_1 (state, &seq); emit_post_landing_pad (&eh_seq, tf->region); @@ -1250,7 +1264,7 @@ x = gimple_build_label (lab); gimple_seq_add_stmt (&new_stmt, x); - seq = lower_try_finally_dup_block (finally, state); + seq = lower_try_finally_dup_block (finally, state, q->location); lower_eh_constructs_1 (state, &seq); gimple_seq_add_seq (&new_stmt, seq); Index: gcc/gimplify.c =================================================================== --- gcc/gimplify.c (revision 190209) +++ gcc/gimplify.c (working copy) @@ -7434,6 +7434,15 @@ gimple_seq eval, cleanup; gimple try_; + /* Calls to destructors are generated automatically in FINALL/CATCH + block. They should have location as UNKNOWN_LOCATION. However, + gimplify_call_expr will reset these call stmts to input_location + if it finds stmt's location is unknown. To prevent resetting for + destructors, we set the input_location to unknown. + Note that this only affects the destructor calls in FINALL/CATCH + block, and will automatically reset to its original value by the + end of gimplify_expr. */ + input_location = UNKNOWN_LOCATION; eval = cleanup = NULL; gimplify_and_add (TREE_OPERAND (*expr_p, 0), &eval); gimplify_and_add (TREE_OPERAND (*expr_p, 1), &cleanup);