Re: [PATCH] Set correct source location for deallocator calls

2012-09-15 Thread H.J. Lu
On Fri, Sep 14, 2012 at 9:27 PM, Andrew Pinski pins...@gmail.com wrote: On Fri, Sep 14, 2012 at 9:25 PM, H.J. Lu hjl.to...@gmail.com wrote: On Sat, Sep 8, 2012 at 2:42 PM, Dehao Chen de...@google.com wrote: Hi, I've added a libjava unittest which verifies that this patch will not break Java

Re: [PATCH] Set correct source location for deallocator calls

2012-09-15 Thread Dehao Chen
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. 19b: Abbrev Number: 2 (DW_TAG_subprogram) 9c DW_AT_external: 1 9c DW_AT_name: bar

Re: [PATCH] Set correct source location for deallocator calls

2012-09-15 Thread H.J. Lu
On Sat, Sep 15, 2012 at 9:09 AM, Dehao Chen de...@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. 19b: Abbrev Number: 2 (DW_TAG_subprogram) 9c

Re: [PATCH] Set correct source location for deallocator calls

2012-09-15 Thread Dehao Chen
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; .

Re: [PATCH] Set correct source location for deallocator calls

2012-09-15 Thread Mark Wielaard
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

Re: [PATCH] Set correct source location for deallocator calls

2012-09-15 Thread Mark Wielaard
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. 19b: Abbrev Number: 2 (DW_TAG_subprogram) 9c

Re: [PATCH] Set correct source location for deallocator calls

2012-09-14 Thread Dehao Chen
ping... Thanks, Dehao On Sun, Sep 9, 2012 at 5:42 AM, Dehao Chen de...@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

Re: [PATCH] Set correct source location for deallocator calls

2012-09-14 Thread Andrew Haley
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

Re: [PATCH] Set correct source location for deallocator calls

2012-09-14 Thread H.J. Lu
On Sat, Sep 8, 2012 at 2:42 PM, Dehao Chen de...@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

Re: [PATCH] Set correct source location for deallocator calls

2012-09-14 Thread H.J. Lu
On Fri, Sep 14, 2012 at 9:25 PM, H.J. Lu hjl.to...@gmail.com wrote: On Sat, Sep 8, 2012 at 2:42 PM, Dehao Chen de...@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

Re: [PATCH] Set correct source location for deallocator calls

2012-09-14 Thread Andrew Pinski
On Fri, Sep 14, 2012 at 9:25 PM, H.J. Lu hjl.to...@gmail.com wrote: On Sat, Sep 8, 2012 at 2:42 PM, Dehao Chen de...@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

Re: [PATCH] Set correct source location for deallocator calls

2012-09-08 Thread Dehao Chen
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

Re: [PATCH] Set correct source location for deallocator calls

2012-09-05 Thread Andrew Haley
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)

Re: [PATCH] Set correct source location for deallocator calls

2012-09-05 Thread Andrew Pinski
On Wed, Sep 5, 2012 at 12:29 AM, Andrew Haley a...@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

Re: [PATCH] Set correct source location for deallocator calls

2012-09-05 Thread Mark Wielaard
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

Re: [PATCH] Set correct source location for deallocator calls

2012-09-04 Thread Dehao Chen
On Thu, Aug 30, 2012 at 9:33 AM, Richard Henderson r...@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

Re: [PATCH] Set correct source location for deallocator calls

2012-09-04 Thread Andrew Haley
On 09/04/2012 05:07 PM, Dehao Chen wrote: On Thu, Aug 30, 2012 at 9:33 AM, Richard Henderson r...@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

Re: [PATCH] Set correct source location for deallocator calls

2012-09-04 Thread Bryce McKinlay
On Tue, Sep 4, 2012 at 5:07 PM, Dehao Chen de...@google.com wrote: On Thu, Aug 30, 2012 at 9:33 AM, Richard Henderson r...@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,

Re: [PATCH] Set correct source location for deallocator calls

2012-09-04 Thread Andrew Haley
On 09/04/2012 05:32 PM, Bryce McKinlay wrote: On Tue, Sep 4, 2012 at 5:07 PM, Dehao Chen de...@google.com wrote: On Thu, Aug 30, 2012 at 9:33 AM, Richard Henderson r...@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

Re: [PATCH] Set correct source location for deallocator calls

2012-09-04 Thread Bryce McKinlay
On Tue, Sep 4, 2012 at 5:39 PM, Andrew Haley a...@redhat.com wrote: On 09/04/2012 05:32 PM, Bryce McKinlay wrote: On Tue, Sep 4, 2012 at 5:07 PM, Dehao Chen de...@google.com wrote: On Thu, Aug 30, 2012 at 9:33 AM, Richard Henderson r...@redhat.com wrote: On 08/30/2012 08:20 AM, Andrew Haley

Re: [PATCH] Set correct source location for deallocator calls

2012-09-04 Thread Andrew Haley
On 09/04/2012 06:08 PM, Bryce McKinlay wrote: On Tue, Sep 4, 2012 at 5:39 PM, Andrew Haley a...@redhat.com wrote: On 09/04/2012 05:32 PM, Bryce McKinlay wrote: On Tue, Sep 4, 2012 at 5:07 PM, Dehao Chen de...@google.com wrote: On Thu, Aug 30, 2012 at 9:33 AM, Richard Henderson r...@redhat.com

Re: [PATCH] Set correct source location for deallocator calls

2012-09-04 Thread Bryce McKinlay
On Tue, Sep 4, 2012 at 6:12 PM, Andrew Haley a...@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

Re: [PATCH] Set correct source location for deallocator calls

2012-09-04 Thread Andrew Haley
On 09/04/2012 06:17 PM, Bryce McKinlay wrote: On Tue, Sep 4, 2012 at 6:12 PM, Andrew Haley a...@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

Re: [PATCH] Set correct source location for deallocator calls

2012-09-04 Thread Dehao Chen
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)

Re: [PATCH] Set correct source location for deallocator calls

2012-09-04 Thread Dehao Chen
On Tue, Sep 4, 2012 at 9:22 AM, Andrew Haley a...@redhat.com wrote: On 09/04/2012 05:07 PM, Dehao Chen wrote: On Thu, Aug 30, 2012 at 9:33 AM, Richard Henderson r...@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

Re: [PATCH] Set correct source location for deallocator calls

2012-08-30 Thread Richard Henderson
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

Re: [PATCH] Set correct source location for deallocator calls

2012-08-30 Thread Bryce McKinlay
On Thu, Aug 30, 2012 at 3:28 PM, Richard Henderson r...@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

Re: [PATCH] Set correct source location for deallocator calls

2012-08-30 Thread Andrew Haley
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,

Re: [PATCH] Set correct source location for deallocator calls

2012-08-30 Thread Richard Henderson
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~

Re: [PATCH] Set correct source location for deallocator calls

2012-08-27 Thread Dehao Chen
ping Thanks, Dehao On Sat, Aug 18, 2012 at 6:02 AM, Dehao Chen de...@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

Re: [PATCH] Set correct source location for deallocator calls

2012-08-17 Thread Richard Henderson
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. This doesn't belong in guality. It belongs

Re: [PATCH] Set correct source location for deallocator calls

2012-08-17 Thread Dehao Chen
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 r...@redhat.com wrote: On 2012-08-10 20:38, Dehao Chen

Re: [PATCH] Set correct source location for deallocator calls

2012-08-16 Thread Dehao Chen
ping. Thanks, Dehao On Fri, Aug 10, 2012 at 8:38 PM, Dehao Chen de...@google.com wrote: New patch attached. Bootstrapped and passed GCC regression tests. Ok for trunk? Thanks, Dehao gcc/ChangeLog 2012-08-07 Dehao Chen de...@google.com * tree-eh.c (goto_queue_node):

Re: [PATCH] Set correct source location for deallocator calls

2012-08-10 Thread Dehao Chen
On Thu, Aug 9, 2012 at 3:06 PM, Jason Merrill ja...@redhat.com wrote: On 08/08/2012 12:32 PM, Richard Henderson wrote: On 08/08/2012 09:27 AM, Dehao Chen wrote: Then we should probably assign UNKNOWN_LOCATION for these destructor calls, what do you guys think? I think it's certainly

Re: [PATCH] Set correct source location for deallocator calls

2012-08-10 Thread Richard Henderson
On 2012-08-10 10:21, Dehao Chen wrote: Yes, cxx_maybe_build_cleanup sets it to UNKNOWN_LOCATION, but during gimplifying, it's reset to input_location: gimplify.c (gimplify_call_expr) 2486 /* For reliable diagnostics during inlining, it is necessary that 2487 every call_expr be

Re: [PATCH] Set correct source location for deallocator calls

2012-08-10 Thread Dehao Chen
On Fri, Aug 10, 2012 at 10:52 AM, Richard Henderson r...@redhat.com wrote: On 2012-08-10 10:21, Dehao Chen wrote: Yes, cxx_maybe_build_cleanup sets it to UNKNOWN_LOCATION, but during gimplifying, it's reset to input_location: gimplify.c (gimplify_call_expr) 2486 /* For reliable

Re: [PATCH] Set correct source location for deallocator calls

2012-08-10 Thread Richard Henderson
On 2012-08-10 11:01, Dehao Chen wrote: On Fri, Aug 10, 2012 at 10:52 AM, Richard Henderson r...@redhat.com wrote: On 2012-08-10 10:21, Dehao Chen wrote: Yes, cxx_maybe_build_cleanup sets it to UNKNOWN_LOCATION, but during gimplifying, it's reset to input_location: gimplify.c

Re: [PATCH] Set correct source location for deallocator calls

2012-08-10 Thread Dehao Chen
On Fri, Aug 10, 2012 at 12:04 PM, Richard Henderson r...@redhat.com wrote: On 2012-08-10 11:01, Dehao Chen wrote: On Fri, Aug 10, 2012 at 10:52 AM, Richard Henderson r...@redhat.com wrote: On 2012-08-10 10:21, Dehao Chen wrote: Yes, cxx_maybe_build_cleanup sets it to UNKNOWN_LOCATION, but

Re: [PATCH] Set correct source location for deallocator calls

2012-08-10 Thread Richard Henderson
On 2012-08-10 14:55, Dehao Chen wrote: I see your point. But the problem is that the above code is in gimplify_call_expr, in which we have no idea if it is in a TRY_FINALLY/TRY_CATCH block. That's why I suggested saving and restoring input_location while gimplifying try_finally. i.e.

Re: [PATCH] Set correct source location for deallocator calls

2012-08-10 Thread Dehao Chen
On Fri, Aug 10, 2012 at 3:11 PM, Richard Henderson r...@redhat.com wrote: On 2012-08-10 14:55, Dehao Chen wrote: I see your point. But the problem is that the above code is in gimplify_call_expr, in which we have no idea if it is in a TRY_FINALLY/TRY_CATCH block. That's why I suggested

Re: [PATCH] Set correct source location for deallocator calls

2012-08-10 Thread Dehao Chen
New patch attached. Bootstrapped and passed GCC regression tests. Ok for trunk? Thanks, Dehao gcc/ChangeLog 2012-08-07 Dehao Chen de...@google.com * tree-eh.c (goto_queue_node): New field. (record_in_goto_queue): New parameter. (record_in_goto_queue_label): New

Re: [PATCH] Set correct source location for deallocator calls

2012-08-09 Thread Jason Merrill
On 08/08/2012 12:32 PM, Richard Henderson wrote: On 08/08/2012 09:27 AM, Dehao Chen wrote: Then we should probably assign UNKNOWN_LOCATION for these destructor calls, what do you guys think? I think it's certainly plausible. I can't think what other problems such a change would cause.

Re: [PATCH] Set correct source location for deallocator calls

2012-08-08 Thread Richard Henderson
On 08/07/2012 06:25 AM, Richard Guenther wrote: (is re-setting _every_ stmt location really ok in all cases?) I'm certain that it's not, though you can't tell that from C++. Examine instead a Java test case using try-finally. In Java the contents of the finally would be incorrectly relocated

Re: [PATCH] Set correct source location for deallocator calls

2012-08-08 Thread Dehao Chen
On Wed, Aug 8, 2012 at 8:56 AM, Richard Henderson r...@redhat.com wrote: On 08/07/2012 06:25 AM, Richard Guenther wrote: (is re-setting _every_ stmt location really ok in all cases?) I'm certain that it's not, though you can't tell that from C++. Examine instead a Java test case using

Re: [PATCH] Set correct source location for deallocator calls

2012-08-08 Thread Richard Henderson
On 08/08/2012 09:27 AM, Dehao Chen wrote: On Wed, Aug 8, 2012 at 8:56 AM, Richard Henderson r...@redhat.com wrote: On 08/07/2012 06:25 AM, Richard Guenther wrote: (is re-setting _every_ stmt location really ok in all cases?) I'm certain that it's not, though you can't tell that from C++.

Re: [PATCH] Set correct source location for deallocator calls

2012-08-07 Thread Richard Guenther
On Tue, Aug 7, 2012 at 12:07 AM, Dehao Chen de...@google.com wrote: Ping... Richard, could you shed some lights on this? Thanks, Dehao On Mon, Jul 30, 2012 at 8:29 PM, Dehao Chen de...@google.com wrote: Hi, This patch fixes the source location for automatically generated calls to

Re: [PATCH] Set correct source location for deallocator calls

2012-08-06 Thread Dehao Chen
Ping... Richard, could you shed some lights on this? Thanks, Dehao On Mon, Jul 30, 2012 at 8:29 PM, Dehao Chen de...@google.com wrote: Hi, This patch fixes the source location for automatically generated calls to deallocator. For example: 19 void foo(int i) 20 { 21 for (int j = 0;

[PATCH] Set correct source location for deallocator calls

2012-07-30 Thread Dehao Chen
Hi, This patch fixes the source location for automatically generated calls to deallocator. For example: 19 void foo(int i) 20 { 21 for (int j = 0; j 10; j++) 22 { 23 t test; 24 test.foo(); 25 if (i + j) 26 { 27 test.bar(); 28 return;