* Re: [PATCH] Set correct source location for deallocator calls [not found] ` <CAO2gOZWhueDAShNLbNcJpkHA6QqXk1LNZhEMFvfT77aTZTCH9w@mail.gmail.com> @ 2012-08-30 14:28 ` Richard Henderson 2012-08-30 14:45 ` Bryce McKinlay 2012-08-30 15:20 ` Andrew Haley 0 siblings, 2 replies; 29+ messages in thread From: Richard Henderson @ 2012-08-30 14:28 UTC (permalink / raw) To: Dehao Chen; +Cc: Jason Merrill, Richard Guenther, gcc-patches, David Li, java 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~ ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-08-30 14:28 ` [PATCH] Set correct source location for deallocator calls Richard Henderson @ 2012-08-30 14:45 ` Bryce McKinlay 2012-08-30 15:20 ` Andrew Haley 1 sibling, 0 replies; 29+ messages in thread From: Bryce McKinlay @ 2012-08-30 14:45 UTC (permalink / raw) To: Richard Henderson Cc: Dehao Chen, Jason Merrill, Richard Guenther, gcc-patches, David Li, java 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-08-30 14:28 ` [PATCH] Set correct source location for deallocator calls Richard Henderson 2012-08-30 14:45 ` Bryce McKinlay @ 2012-08-30 15:20 ` Andrew Haley 2012-08-30 16:33 ` Richard Henderson 1 sibling, 1 reply; 29+ messages in thread From: Andrew Haley @ 2012-08-30 15:20 UTC (permalink / raw) To: Richard Henderson Cc: Dehao Chen, Jason Merrill, Richard Guenther, gcc-patches, David Li, 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-08-30 15:20 ` Andrew Haley @ 2012-08-30 16:33 ` Richard Henderson 2012-09-04 16:07 ` Dehao Chen 0 siblings, 1 reply; 29+ messages in thread From: Richard Henderson @ 2012-08-30 16:33 UTC (permalink / raw) To: Andrew Haley Cc: Dehao Chen, Jason Merrill, Richard Guenther, gcc-patches, David Li, java 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~ ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-08-30 16:33 ` Richard Henderson @ 2012-09-04 16:07 ` Dehao Chen 2012-09-04 16:22 ` Andrew Haley 2012-09-04 16:32 ` Bryce McKinlay 0 siblings, 2 replies; 29+ messages in thread From: Dehao Chen @ 2012-09-04 16:07 UTC (permalink / raw) To: Richard Henderson Cc: Andrew Haley, Jason Merrill, Richard Guenther, gcc-patches, David Li, java 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~ ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-04 16:07 ` Dehao Chen @ 2012-09-04 16:22 ` Andrew Haley 2012-09-04 20:40 ` Dehao Chen 2012-09-04 16:32 ` Bryce McKinlay 1 sibling, 1 reply; 29+ messages in thread From: Andrew Haley @ 2012-09-04 16:22 UTC (permalink / raw) To: Dehao Chen Cc: Richard Henderson, Jason Merrill, Richard Guenther, gcc-patches, David Li, java 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-04 16:22 ` Andrew Haley @ 2012-09-04 20:40 ` Dehao Chen 0 siblings, 0 replies; 29+ messages in thread From: Dehao Chen @ 2012-09-04 20:40 UTC (permalink / raw) To: Andrew Haley Cc: Richard Henderson, Jason Merrill, Richard Guenther, gcc-patches, David Li, java 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. > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-04 16:07 ` Dehao Chen 2012-09-04 16:22 ` Andrew Haley @ 2012-09-04 16:32 ` Bryce McKinlay 2012-09-04 16:40 ` Andrew Haley 1 sibling, 1 reply; 29+ messages in thread From: Bryce McKinlay @ 2012-09-04 16:32 UTC (permalink / raw) To: Dehao Chen Cc: Richard Henderson, Andrew Haley, Jason Merrill, Richard Guenther, gcc-patches, David Li, java 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-04 16:32 ` Bryce McKinlay @ 2012-09-04 16:40 ` Andrew Haley 2012-09-04 17:08 ` Bryce McKinlay 0 siblings, 1 reply; 29+ messages in thread From: Andrew Haley @ 2012-09-04 16:40 UTC (permalink / raw) To: Bryce McKinlay Cc: Dehao Chen, Richard Henderson, Jason Merrill, Richard Guenther, gcc-patches, David Li, java 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-04 16:40 ` Andrew Haley @ 2012-09-04 17:08 ` Bryce McKinlay 2012-09-04 17:12 ` Andrew Haley 0 siblings, 1 reply; 29+ messages in thread From: Bryce McKinlay @ 2012-09-04 17:08 UTC (permalink / raw) To: Andrew Haley Cc: Dehao Chen, Richard Henderson, Jason Merrill, Richard Guenther, gcc-patches, David Li, java 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-04 17:08 ` Bryce McKinlay @ 2012-09-04 17:12 ` Andrew Haley 2012-09-04 17:17 ` Bryce McKinlay 0 siblings, 1 reply; 29+ messages in thread From: Andrew Haley @ 2012-09-04 17:12 UTC (permalink / raw) To: Bryce McKinlay Cc: Dehao Chen, Richard Henderson, Jason Merrill, Richard Guenther, gcc-patches, David Li, java 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-04 17:12 ` Andrew Haley @ 2012-09-04 17:17 ` Bryce McKinlay 2012-09-04 17:21 ` Andrew Haley 2012-09-05 9:58 ` Mark Wielaard 0 siblings, 2 replies; 29+ messages in thread From: Bryce McKinlay @ 2012-09-04 17:17 UTC (permalink / raw) To: Andrew Haley Cc: Dehao Chen, Richard Henderson, Jason Merrill, Richard Guenther, gcc-patches, David Li, java 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-04 17:17 ` Bryce McKinlay @ 2012-09-04 17:21 ` Andrew Haley 2012-09-04 20:32 ` Dehao Chen 2012-09-05 9:58 ` Mark Wielaard 1 sibling, 1 reply; 29+ messages in thread From: Andrew Haley @ 2012-09-04 17:21 UTC (permalink / raw) To: Bryce McKinlay Cc: Dehao Chen, Richard Henderson, Jason Merrill, Richard Guenther, gcc-patches, David Li, java 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-04 17:21 ` Andrew Haley @ 2012-09-04 20:32 ` Dehao Chen 2012-09-05 7:29 ` Andrew Haley 0 siblings, 1 reply; 29+ messages in thread From: Dehao Chen @ 2012-09-04 20:32 UTC (permalink / raw) To: Andrew Haley Cc: Bryce McKinlay, Richard Henderson, Jason Merrill, Richard Guenther, gcc-patches, David Li, java 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() + ")"); } } } ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-04 20:32 ` Dehao Chen @ 2012-09-05 7:29 ` Andrew Haley 2012-09-05 7:31 ` Andrew Pinski 0 siblings, 1 reply; 29+ messages in thread From: Andrew Haley @ 2012-09-05 7:29 UTC (permalink / raw) To: Dehao Chen; +Cc: Richard Guenther, gcc-patches, java 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-05 7:29 ` Andrew Haley @ 2012-09-05 7:31 ` Andrew Pinski 0 siblings, 0 replies; 29+ messages in thread From: Andrew Pinski @ 2012-09-05 7:31 UTC (permalink / raw) To: Andrew Haley; +Cc: Dehao Chen, Richard Guenther, gcc-patches, java 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-04 17:17 ` Bryce McKinlay 2012-09-04 17:21 ` Andrew Haley @ 2012-09-05 9:58 ` Mark Wielaard 2012-09-08 21:42 ` Dehao Chen 1 sibling, 1 reply; 29+ messages in thread From: Mark Wielaard @ 2012-09-05 9:58 UTC (permalink / raw) To: Bryce McKinlay Cc: Andrew Haley, Dehao Chen, Richard Henderson, Jason Merrill, Richard Guenther, gcc-patches, David Li, java 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-05 9:58 ` Mark Wielaard @ 2012-09-08 21:42 ` Dehao Chen 2012-09-14 15:14 ` Dehao Chen ` (2 more replies) 0 siblings, 3 replies; 29+ messages in thread From: Dehao Chen @ 2012-09-08 21:42 UTC (permalink / raw) To: Mark Wielaard Cc: Bryce McKinlay, Andrew Haley, Richard Henderson, Jason Merrill, Richard Guenther, gcc-patches, David Li, java [-- Attachment #1: Type: text/plain, Size: 1017 bytes --] Hi, I've added a libjava unittest which verifies that this patch will not break Java debug info. I've also incorporated Richard's review in the previous mail. Attached is the new patch, which passed bootstrap and all gcc/libjava testsuites on x86. Is it ok for trunk? Thanks, Dehao gcc/ChangeLog: 2012-09-08 Dehao Chen <dehao@google.com> * 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-09-08 Dehao Chen <dehao@google.com> * g++.dg/debug/dwarf2/deallocator.C: New test. libjava/ChangeLog: 2012-09-08 Dehao Chen <dehao@google.com> * testsuite/libjava.lang/sourcelocation.java: New cases. * testsuite/libjava.lang/sourcelocation.out: New cases. [-- Attachment #2: patch.txt --] [-- Type: text/plain, Size: 8676 bytes --] Index: libjava/testsuite/libjava.lang/sourcelocation.java =================================================================== --- libjava/testsuite/libjava.lang/sourcelocation.java (revision 0) +++ libjava/testsuite/libjava.lang/sourcelocation.java (revision 0) @@ -0,0 +1,18 @@ +/* This test should test the source location attribution. + We print the line number of different parts of the program to make sure + that the source code attribution is correct. + To make this test pass, one need to have up-to-date addr2line installed + to parse the dwarf4 data format. +*/ +public class sourcelocation { + public static void main(String args[]) { + try { + System.out.println(new Exception().getStackTrace()[0].getLineNumber()); + throw new Exception(); + } catch (Exception e) { + System.out.println(new Exception().getStackTrace()[0].getLineNumber()); + } finally { + System.out.println(new Exception().getStackTrace()[0].getLineNumber()); + } + } +} Index: libjava/testsuite/libjava.lang/sourcelocation.out =================================================================== --- libjava/testsuite/libjava.lang/sourcelocation.out (revision 0) +++ libjava/testsuite/libjava.lang/sourcelocation.out (revision 0) @@ -0,0 +1,3 @@ +10 +13 +15 Index: libjava/testsuite/libjava.lang/sourcelocation.jar =================================================================== Cannot display: file marked as a binary type. svn:mime-type = application/octet-stream Property changes on: libjava/testsuite/libjava.lang/sourcelocation.jar ___________________________________________________________________ Added: svn:mime-type + application/octet-stream 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 "deallocator.C:28" } } Index: gcc/tree-eh.c =================================================================== --- gcc/tree-eh.c (revision 191083) +++ gcc/tree-eh.c (working copy) @@ -321,6 +321,7 @@ static bitmap eh_region_may_contain_throw_map; struct goto_queue_node { treemple stmt; + location_t location; gimple_seq repl_stmt; gimple cont_stmt; int index; @@ -560,7 +561,8 @@ static void 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 @@ record_in_goto_queue (struct leh_tf_state *tf, memset (q, 0, sizeof (*q)); q->stmt = new_stmt; q->index = index; + q->location = location; q->is_label = is_label; } @@ -590,7 +593,8 @@ record_in_goto_queue (struct leh_tf_state *tf, 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 @@ static void 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 @@ maybe_record_in_goto_queue (struct leh_state *stat { 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 @@ frob_into_branch_around (gimple tp, eh_region regi 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 @@ honor_protect_cleanup_actions (struct leh_state *o 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 @@ lower_try_finally_copy (struct leh_state *state, s 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 @@ lower_try_finally_copy (struct leh_state *state, s 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 @@ lower_try_finally_copy (struct leh_state *state, s 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 191083) +++ gcc/gimplify.c (working copy) @@ -7429,6 +7429,15 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gi gimple_seq eval, cleanup; gimple try_; + /* Calls to destructors are generated automatically in FINALLY/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 FINALLY/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); ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-08 21:42 ` Dehao Chen @ 2012-09-14 15:14 ` Dehao Chen 2012-09-14 15:20 ` Andrew Haley 2012-09-15 4:25 ` H.J. Lu 2 siblings, 0 replies; 29+ messages in thread From: Dehao Chen @ 2012-09-14 15:14 UTC (permalink / raw) To: Mark Wielaard Cc: Bryce McKinlay, Andrew Haley, Richard Henderson, Jason Merrill, Richard Guenther, GCC Patches, David Li, java ping... Thanks, Dehao On Sun, Sep 9, 2012 at 5:42 AM, Dehao Chen <dehao@google.com> wrote: > Hi, > > I've added a libjava unittest which verifies that this patch will not > break Java debug info. I've also incorporated Richard's review in the > previous mail. Attached is the new patch, which passed bootstrap and > all gcc/libjava testsuites on x86. > > Is it ok for trunk? > > Thanks, > Dehao > > gcc/ChangeLog: > 2012-09-08 Dehao Chen <dehao@google.com> > > * 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-09-08 Dehao Chen <dehao@google.com> > > * g++.dg/debug/dwarf2/deallocator.C: New test. > > libjava/ChangeLog: > 2012-09-08 Dehao Chen <dehao@google.com> > > * testsuite/libjava.lang/sourcelocation.java: New cases. > * testsuite/libjava.lang/sourcelocation.out: New cases. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-08 21:42 ` Dehao Chen 2012-09-14 15:14 ` Dehao Chen @ 2012-09-14 15:20 ` Andrew Haley 2012-09-15 4:25 ` H.J. Lu 2 siblings, 0 replies; 29+ messages in thread From: Andrew Haley @ 2012-09-14 15:20 UTC (permalink / raw) To: Dehao Chen Cc: Mark Wielaard, Bryce McKinlay, Richard Henderson, Jason Merrill, Richard Guenther, gcc-patches, David Li, java On 09/08/2012 10:42 PM, Dehao Chen wrote: > I've added a libjava unittest which verifies that this patch will not > break Java debug info. I've also incorporated Richard's review in the > previous mail. Attached is the new patch, which passed bootstrap and > all gcc/libjava testsuites on x86. > > Is it ok for trunk? Yes, thanks. Andrew. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-08 21:42 ` Dehao Chen 2012-09-14 15:14 ` Dehao Chen 2012-09-14 15:20 ` Andrew Haley @ 2012-09-15 4:25 ` H.J. Lu 2012-09-15 4:27 ` H.J. Lu 2012-09-15 4:27 ` Andrew Pinski 2 siblings, 2 replies; 29+ messages in thread From: H.J. Lu @ 2012-09-15 4:25 UTC (permalink / raw) To: Dehao Chen Cc: Mark Wielaard, Bryce McKinlay, Andrew Haley, Richard Henderson, Jason Merrill, Richard Guenther, gcc-patches, David Li, java On Sat, Sep 8, 2012 at 2:42 PM, Dehao Chen <dehao@google.com> wrote: > Hi, > > I've added a libjava unittest which verifies that this patch will not > break Java debug info. I've also incorporated Richard's review in the > previous mail. Attached is the new patch, which passed bootstrap and > all gcc/libjava testsuites on x86. > > Is it ok for trunk? > > Thanks, > Dehao > > gcc/ChangeLog: > 2012-09-08 Dehao Chen <dehao@google.com> > > * 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-09-08 Dehao Chen <dehao@google.com> > > * g++.dg/debug/dwarf2/deallocator.C: New test. > > libjava/ChangeLog: > 2012-09-08 Dehao Chen <dehao@google.com> > > * testsuite/libjava.lang/sourcelocation.java: New cases. > * testsuite/libjava.lang/sourcelocation.out: New cases. On Linux/x86, I got FAIL: sourcelocation -O3 -findirect-dispatch output - source compiled test FAIL: sourcelocation -O3 output - source compiled test FAIL: sourcelocation -findirect-dispatch output - source compiled test FAIL: sourcelocation output - source compiled test spawn [open ...]^M -1 -1 -1 PASS: sourcelocation -findirect-dispatch execution - source compiled test FAIL: sourcelocation -findirect-dispatch output - source compiled test -- H.J. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-15 4:25 ` H.J. Lu @ 2012-09-15 4:27 ` H.J. Lu 2012-09-15 4:27 ` Andrew Pinski 1 sibling, 0 replies; 29+ messages in thread From: H.J. Lu @ 2012-09-15 4:27 UTC (permalink / raw) To: Dehao Chen Cc: Mark Wielaard, Bryce McKinlay, Andrew Haley, Richard Henderson, Jason Merrill, Richard Guenther, gcc-patches, David Li, java On Fri, Sep 14, 2012 at 9:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Sat, Sep 8, 2012 at 2:42 PM, Dehao Chen <dehao@google.com> wrote: >> Hi, >> >> I've added a libjava unittest which verifies that this patch will not >> break Java debug info. I've also incorporated Richard's review in the >> previous mail. Attached is the new patch, which passed bootstrap and >> all gcc/libjava testsuites on x86. >> >> Is it ok for trunk? >> >> Thanks, >> Dehao >> >> gcc/ChangeLog: >> 2012-09-08 Dehao Chen <dehao@google.com> >> >> * 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-09-08 Dehao Chen <dehao@google.com> >> >> * g++.dg/debug/dwarf2/deallocator.C: New test. >> >> libjava/ChangeLog: >> 2012-09-08 Dehao Chen <dehao@google.com> >> >> * testsuite/libjava.lang/sourcelocation.java: New cases. >> * testsuite/libjava.lang/sourcelocation.out: New cases. > > On Linux/x86, I got > > FAIL: sourcelocation -O3 -findirect-dispatch output - source compiled test > FAIL: sourcelocation -O3 output - source compiled test > FAIL: sourcelocation -findirect-dispatch output - source compiled test > FAIL: sourcelocation output - source compiled test > > spawn [open ...]^M > -1 > -1 > -1 > PASS: sourcelocation -findirect-dispatch execution - source compiled test > FAIL: sourcelocation -findirect-dispatch output - source compiled test > > I am using binutils mainline 20120914 from CVS. -- H.J. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-15 4:25 ` H.J. Lu 2012-09-15 4:27 ` H.J. Lu @ 2012-09-15 4:27 ` Andrew Pinski 2012-09-15 12:55 ` H.J. Lu 1 sibling, 1 reply; 29+ messages in thread From: Andrew Pinski @ 2012-09-15 4:27 UTC (permalink / raw) To: H.J. Lu Cc: Dehao Chen, Mark Wielaard, Bryce McKinlay, Andrew Haley, Richard Henderson, Jason Merrill, Richard Guenther, gcc-patches, David Li, java On Fri, Sep 14, 2012 at 9:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Sat, Sep 8, 2012 at 2:42 PM, Dehao Chen <dehao@google.com> wrote: >> Hi, >> >> I've added a libjava unittest which verifies that this patch will not >> break Java debug info. I've also incorporated Richard's review in the >> previous mail. Attached is the new patch, which passed bootstrap and >> all gcc/libjava testsuites on x86. >> >> Is it ok for trunk? >> >> Thanks, >> Dehao >> >> gcc/ChangeLog: >> 2012-09-08 Dehao Chen <dehao@google.com> >> >> * 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-09-08 Dehao Chen <dehao@google.com> >> >> * g++.dg/debug/dwarf2/deallocator.C: New test. >> >> libjava/ChangeLog: >> 2012-09-08 Dehao Chen <dehao@google.com> >> >> * testsuite/libjava.lang/sourcelocation.java: New cases. >> * testsuite/libjava.lang/sourcelocation.out: New cases. > > On Linux/x86, I got > > FAIL: sourcelocation -O3 -findirect-dispatch output - source compiled test > FAIL: sourcelocation -O3 output - source compiled test > FAIL: sourcelocation -findirect-dispatch output - source compiled test > FAIL: sourcelocation output - source compiled test > > spawn [open ...]^M > -1 > -1 > -1 > PASS: sourcelocation -findirect-dispatch execution - source compiled test > FAIL: sourcelocation -findirect-dispatch output - source compiled test I bet you have an older addr2line installed. Thanks, Andrew Pinski ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-15 4:27 ` Andrew Pinski @ 2012-09-15 12:55 ` H.J. Lu 2012-09-15 16:09 ` Dehao Chen 0 siblings, 1 reply; 29+ messages in thread From: H.J. Lu @ 2012-09-15 12:55 UTC (permalink / raw) To: Andrew Pinski Cc: Dehao Chen, Mark Wielaard, Bryce McKinlay, Andrew Haley, Richard Henderson, Jason Merrill, Richard Guenther, gcc-patches, David Li, java On Fri, Sep 14, 2012 at 9:27 PM, Andrew Pinski <pinskia@gmail.com> wrote: > On Fri, Sep 14, 2012 at 9:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Sat, Sep 8, 2012 at 2:42 PM, Dehao Chen <dehao@google.com> wrote: >>> Hi, >>> >>> I've added a libjava unittest which verifies that this patch will not >>> break Java debug info. I've also incorporated Richard's review in the >>> previous mail. Attached is the new patch, which passed bootstrap and >>> all gcc/libjava testsuites on x86. >>> >>> Is it ok for trunk? >>> >>> Thanks, >>> Dehao >>> >>> gcc/ChangeLog: >>> 2012-09-08 Dehao Chen <dehao@google.com> >>> >>> * 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-09-08 Dehao Chen <dehao@google.com> >>> >>> * g++.dg/debug/dwarf2/deallocator.C: New test. >>> >>> libjava/ChangeLog: >>> 2012-09-08 Dehao Chen <dehao@google.com> >>> >>> * testsuite/libjava.lang/sourcelocation.java: New cases. >>> * testsuite/libjava.lang/sourcelocation.out: New cases. >> >> On Linux/x86, I got >> >> FAIL: sourcelocation -O3 -findirect-dispatch output - source compiled test >> FAIL: sourcelocation -O3 output - source compiled test >> FAIL: sourcelocation -findirect-dispatch output - source compiled test >> FAIL: sourcelocation output - source compiled test >> >> spawn [open ...]^M >> -1 >> -1 >> -1 >> PASS: sourcelocation -findirect-dispatch execution - source compiled test >> FAIL: sourcelocation -findirect-dispatch output - source compiled test > > I bet you have an older addr2line installed. > I am using addr2line from binutils 20120914. -- H.J. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-15 12:55 ` H.J. Lu @ 2012-09-15 16:09 ` Dehao Chen 2012-09-15 18:06 ` H.J. Lu 2012-09-15 22:39 ` Mark Wielaard 0 siblings, 2 replies; 29+ messages in thread From: Dehao Chen @ 2012-09-15 16:09 UTC (permalink / raw) To: H.J. Lu Cc: Andrew Pinski, Mark Wielaard, Bryce McKinlay, Andrew Haley, Richard Henderson, Jason Merrill, Richard Guenther, GCC Patches, David Li, java I tried the up-to-date addr2line on any "gcc -g" generated code, it does not work either. This is because in the new dwarf, the DW_AT_high_pc now actually means the size. e.g. <1><9b>: Abbrev Number: 2 (DW_TAG_subprogram) <9c> DW_AT_external : 1 <9c> DW_AT_name : bar <a0> DW_AT_decl_file : 1 <a1> DW_AT_decl_line : 8 <a2> DW_AT_linkage_name: (indirect string, offset: 0x7b): _Z3barv <a6> DW_AT_type : <0x8d> <aa> DW_AT_low_pc : 0x400583 <b2> DW_AT_high_pc : 0x37 0x0 <ba> DW_AT_frame_base : 1 byte block: 9c (DW_OP_call_frame_cfa) <bc> DW_AT_GNU_all_call_sites: 1 <bc> DW_AT_sibling : <0xff> However, addr2line still thinks DW_AT_high_pc means "high_pc". I think we should wait for binutil to catch up with gcc. Thanks, Dehao On Sat, Sep 15, 2012 at 8:55 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Fri, Sep 14, 2012 at 9:27 PM, Andrew Pinski <pinskia@gmail.com> wrote: >> On Fri, Sep 14, 2012 at 9:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> On Sat, Sep 8, 2012 at 2:42 PM, Dehao Chen <dehao@google.com> wrote: >>>> Hi, >>>> >>>> I've added a libjava unittest which verifies that this patch will not >>>> break Java debug info. I've also incorporated Richard's review in the >>>> previous mail. Attached is the new patch, which passed bootstrap and >>>> all gcc/libjava testsuites on x86. >>>> >>>> Is it ok for trunk? >>>> >>>> Thanks, >>>> Dehao >>>> >>>> gcc/ChangeLog: >>>> 2012-09-08 Dehao Chen <dehao@google.com> >>>> >>>> * 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-09-08 Dehao Chen <dehao@google.com> >>>> >>>> * g++.dg/debug/dwarf2/deallocator.C: New test. >>>> >>>> libjava/ChangeLog: >>>> 2012-09-08 Dehao Chen <dehao@google.com> >>>> >>>> * testsuite/libjava.lang/sourcelocation.java: New cases. >>>> * testsuite/libjava.lang/sourcelocation.out: New cases. >>> >>> On Linux/x86, I got >>> >>> FAIL: sourcelocation -O3 -findirect-dispatch output - source compiled test >>> FAIL: sourcelocation -O3 output - source compiled test >>> FAIL: sourcelocation -findirect-dispatch output - source compiled test >>> FAIL: sourcelocation output - source compiled test >>> >>> spawn [open ...]^M >>> -1 >>> -1 >>> -1 >>> PASS: sourcelocation -findirect-dispatch execution - source compiled test >>> FAIL: sourcelocation -findirect-dispatch output - source compiled test >> >> I bet you have an older addr2line installed. >> > > I am using addr2line from binutils 20120914. > > > -- > H.J. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-15 16:09 ` Dehao Chen @ 2012-09-15 18:06 ` H.J. Lu 2012-09-15 22:03 ` Dehao Chen 2012-09-15 22:39 ` Mark Wielaard 1 sibling, 1 reply; 29+ messages in thread From: H.J. Lu @ 2012-09-15 18:06 UTC (permalink / raw) To: Dehao Chen Cc: Andrew Pinski, Mark Wielaard, Bryce McKinlay, Andrew Haley, Richard Henderson, Jason Merrill, Richard Guenther, GCC Patches, David Li, java On Sat, Sep 15, 2012 at 9:09 AM, Dehao Chen <dehao@google.com> wrote: > I tried the up-to-date addr2line on any "gcc -g" generated code, it > does not work either. This is because in the new dwarf, the > DW_AT_high_pc now actually means the size. e.g. > > <1><9b>: Abbrev Number: 2 (DW_TAG_subprogram) > <9c> DW_AT_external : 1 > <9c> DW_AT_name : bar > <a0> DW_AT_decl_file : 1 > <a1> DW_AT_decl_line : 8 > <a2> DW_AT_linkage_name: (indirect string, offset: 0x7b): _Z3barv > <a6> DW_AT_type : <0x8d> > <aa> DW_AT_low_pc : 0x400583 > <b2> DW_AT_high_pc : 0x37 0x0 > <ba> DW_AT_frame_base : 1 byte block: 9c (DW_OP_call_frame_cfa) > <bc> DW_AT_GNU_all_call_sites: 1 > <bc> DW_AT_sibling : <0xff> > > However, addr2line still thinks DW_AT_high_pc means "high_pc". I think > we should wait for binutil to catch up with gcc. > So, the meaning of DW_AT_high_pc in DWARF4 is different from DWARF3? -- H.J. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-15 18:06 ` H.J. Lu @ 2012-09-15 22:03 ` Dehao Chen 2012-09-15 22:31 ` Mark Wielaard 0 siblings, 1 reply; 29+ messages in thread From: Dehao Chen @ 2012-09-15 22:03 UTC (permalink / raw) To: H.J. Lu Cc: Andrew Pinski, Mark Wielaard, Bryce McKinlay, Andrew Haley, Richard Henderson, Jason Merrill, Richard Guenther, GCC Patches, David Li, java Yeah, in dwarf2out.c: 4590 add_AT_low_high_pc (dw_die_ref die, const char *lbl_low, const char *lbl_high, ...... 4604 if (dwarf_version < 4) 4605 attr.dw_attr_val.val_class = dw_val_class_lbl_id; 4606 else 4607 attr.dw_attr_val.val_class = dw_val_class_high_pc; . dw_val_class_lbl_id is handled: 7984 case dw_val_class_lbl_id: 7985 dw2_asm_output_addr (DWARF2_ADDR_SIZE, AT_lbl (a), "%s", name); 7986 break; dw_val_class_high_pc is handled: 8027 case dw_val_class_high_pc: 8028 dw2_asm_output_delta (DWARF2_ADDR_SIZE, AT_lbl (a), 8029 get_AT_low_pc (die), "DW_AT_high_pc"); 8030 break; The dwarf4 specification says: If the value of the DW_AT_high_pc is of class address, it is the relocated address of the first location past the last instruction associated with the entity; if it is of class constant, the value is an unsigned integer offset which when added to the low PC gives the address of the first location past the last instruction associated with the entity. However, I'm not sure how to tell how the DW_AT_high_pc's class is represented... Maybe it's the bug in the gcc dwarf implementation? Dehao On Sun, Sep 16, 2012 at 2:06 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Sat, Sep 15, 2012 at 9:09 AM, Dehao Chen <dehao@google.com> wrote: >> I tried the up-to-date addr2line on any "gcc -g" generated code, it >> does not work either. This is because in the new dwarf, the >> DW_AT_high_pc now actually means the size. e.g. >> >> <1><9b>: Abbrev Number: 2 (DW_TAG_subprogram) >> <9c> DW_AT_external : 1 >> <9c> DW_AT_name : bar >> <a0> DW_AT_decl_file : 1 >> <a1> DW_AT_decl_line : 8 >> <a2> DW_AT_linkage_name: (indirect string, offset: 0x7b): _Z3barv >> <a6> DW_AT_type : <0x8d> >> <aa> DW_AT_low_pc : 0x400583 >> <b2> DW_AT_high_pc : 0x37 0x0 >> <ba> DW_AT_frame_base : 1 byte block: 9c (DW_OP_call_frame_cfa) >> <bc> DW_AT_GNU_all_call_sites: 1 >> <bc> DW_AT_sibling : <0xff> >> >> However, addr2line still thinks DW_AT_high_pc means "high_pc". I think >> we should wait for binutil to catch up with gcc. >> > > So, the meaning of DW_AT_high_pc in DWARF4 is different > from DWARF3? > > -- > H.J. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-15 22:03 ` Dehao Chen @ 2012-09-15 22:31 ` Mark Wielaard 0 siblings, 0 replies; 29+ messages in thread From: Mark Wielaard @ 2012-09-15 22:31 UTC (permalink / raw) To: Dehao Chen Cc: H.J. Lu, Andrew Pinski, Bryce McKinlay, Andrew Haley, Richard Henderson, Jason Merrill, Richard Guenther, GCC Patches, David Li, java On Sun, Sep 16, 2012 at 06:03:24AM +0800, Dehao Chen wrote: > The dwarf4 specification says: > > If the value of the DW_AT_high_pc is of class address, it is the > relocated address of the first location past the last instruction > associated with the entity; if it is of class constant, the value is > an unsigned integer offset which when added to the low PC gives the > address of the first location past the last instruction associated > with the entity. > > However, I'm not sure how to tell how the DW_AT_high_pc's class is > represented... You look at the form in which the attribute is encoded. If it is DW_FORM_addr then it is of class address, otherwise (DW_FORM_data1, DW_FORM_data2, DW_FORM_data4, DW_FORM_data8, DW_FORM_sdata or DW_FORM_udata) it is of class constant. Cheers, Mark ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Set correct source location for deallocator calls 2012-09-15 16:09 ` Dehao Chen 2012-09-15 18:06 ` H.J. Lu @ 2012-09-15 22:39 ` Mark Wielaard 1 sibling, 0 replies; 29+ messages in thread From: Mark Wielaard @ 2012-09-15 22:39 UTC (permalink / raw) To: Dehao Chen Cc: H.J. Lu, Andrew Pinski, Bryce McKinlay, Andrew Haley, Richard Henderson, Jason Merrill, Richard Guenther, GCC Patches, David Li, java On Sun, Sep 16, 2012 at 12:09:04AM +0800, Dehao Chen wrote: > I tried the up-to-date addr2line on any "gcc -g" generated code, it > does not work either. This is because in the new dwarf, the > DW_AT_high_pc now actually means the size. e.g. > > <1><9b>: Abbrev Number: 2 (DW_TAG_subprogram) > <9c> DW_AT_external : 1 > <9c> DW_AT_name : bar > <a0> DW_AT_decl_file : 1 > <a1> DW_AT_decl_line : 8 > <a2> DW_AT_linkage_name: (indirect string, offset: 0x7b): _Z3barv > <a6> DW_AT_type : <0x8d> > <aa> DW_AT_low_pc : 0x400583 > <b2> DW_AT_high_pc : 0x37 0x0 > <ba> DW_AT_frame_base : 1 byte block: 9c (DW_OP_call_frame_cfa) > <bc> DW_AT_GNU_all_call_sites: 1 > <bc> DW_AT_sibling : <0xff> > > However, addr2line still thinks DW_AT_high_pc means "high_pc". I think > we should wait for binutil to catch up with gcc. I thought it had some months ago: http://sourceware.org/ml/binutils/2012-04/msg00447.html If that patch is present in your binutils and addr2line still doesn't work as intented there might be a bug or some other place to update. Thanks, Mark ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2012-09-15 22:39 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CAO2gOZXfnETUe4wqjT7p6fd61hXreu9PDfqKxNz+HxpE0E7K0g@mail.gmail.com> [not found] ` <CAO2gOZX_hZ1m0xKvxUg+6gWTOX4V5ok-vvCEg3Vhfpt=qXj8-g@mail.gmail.com> [not found] ` <CAFiYyc00wud5Oa3k51ntm8KeZHZnfm4rpnBsOJjURZMcoqFdTA@mail.gmail.com> [not found] ` <50228C38.5080703@redhat.com> [not found] ` <CAO2gOZUbkTsnJ2fwgeNhcRmxkzzJEifECeiF=_YRUjeKDeRMdA@mail.gmail.com> [not found] ` <502294A1.3060800@redhat.com> [not found] ` <50243480.7090803@redhat.com> [not found] ` <CAO2gOZURCuWUk2MVwCwmwrijNzWxJt3q=HUpU7=Qv6zB9e-uqA@mail.gmail.com> [not found] ` <50254A50.8070208@redhat.com> [not found] ` <CAO2gOZXhHuFGJ0z=jvkYKZ84EoDaPSYVjxS7QzGore56SyhWyQ@mail.gmail.com> [not found] ` <50255B35.9020705@redhat.com> [not found] ` <CAO2gOZWe+qMrVyvOoo7Ek-di0NKZxxU4Z=pqrDCqqCCAfctZOw@mail.gmail.com> [not found] ` <50258712.4070002@redhat.com> [not found] ` <CAO2gOZUQQjmKtooyXXAfgFoNVbeNwtT+P=E7pQ6jC=e07Nsr2g@mail.gmail.com> [not found] ` <CAO2gOZX-Gn+b6LEzps5zxhgmwQWcJ-zFqG=a7hesW6fbVyxZYQ@mail.gmail.com> [not found] ` <502E6774.8050609@redhat.com> [not found] ` <CAO2gOZWhueDAShNLbNcJpkHA6QqXk1LNZhEMFvfT77aTZTCH9w@mail.gmail.com> 2012-08-30 14:28 ` [PATCH] Set correct source location for deallocator calls Richard Henderson 2012-08-30 14:45 ` Bryce McKinlay 2012-08-30 15:20 ` Andrew Haley 2012-08-30 16:33 ` Richard Henderson 2012-09-04 16:07 ` Dehao Chen 2012-09-04 16:22 ` Andrew Haley 2012-09-04 20:40 ` Dehao Chen 2012-09-04 16:32 ` Bryce McKinlay 2012-09-04 16:40 ` Andrew Haley 2012-09-04 17:08 ` Bryce McKinlay 2012-09-04 17:12 ` Andrew Haley 2012-09-04 17:17 ` Bryce McKinlay 2012-09-04 17:21 ` Andrew Haley 2012-09-04 20:32 ` Dehao Chen 2012-09-05 7:29 ` Andrew Haley 2012-09-05 7:31 ` Andrew Pinski 2012-09-05 9:58 ` Mark Wielaard 2012-09-08 21:42 ` Dehao Chen 2012-09-14 15:14 ` Dehao Chen 2012-09-14 15:20 ` Andrew Haley 2012-09-15 4:25 ` H.J. Lu 2012-09-15 4:27 ` H.J. Lu 2012-09-15 4:27 ` Andrew Pinski 2012-09-15 12:55 ` H.J. Lu 2012-09-15 16:09 ` Dehao Chen 2012-09-15 18:06 ` H.J. Lu 2012-09-15 22:03 ` Dehao Chen 2012-09-15 22:31 ` Mark Wielaard 2012-09-15 22:39 ` Mark Wielaard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).