* [patch] PR debug/58123: Set correct location for TRY blocks
[not found] <54E61BE5.9090209@redhat.com>
@ 2015-02-19 18:33 ` Aldy Hernandez
2015-02-20 19:06 ` Jeff Law
2015-02-23 19:37 ` Jason Merrill
0 siblings, 2 replies; 4+ messages in thread
From: Aldy Hernandez @ 2015-02-19 18:33 UTC (permalink / raw)
To: jason merrill; +Cc: Jakub Jelinek, gcc-patches, Jan Kratochvil
[-- Attachment #1: Type: text/plain, Size: 870 bytes --]
[Ughh...I'm apparently incapable of CCing gcc-patches on the first try.
My apologies again.]
Please see the long explanation in the PR:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58123
But the short explanation for this patch is that, at the very least, we
should fix the location of the GIMPLE_TRY_FINALLY. Currently we're
preferring the input_location, instead of the location of the
TRY_FINALLY_EXPR.
An incorrect GIMPLE_TRY_FINALLY location causes the location of
location-less FINALLY statements to also have an incorrect location,
causing an inconsistent debugging experience.
As explained in the PR, I would ideally like to get rid of the kludge
where we set the location of location-less FINALLY statements to be that
of the TRY statement, but that may be by design, and/or beyond the scope
of this fix.
Tested on x86-64 Linux.
OK for mainline?
[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 2764 bytes --]
commit 119b6f5c628831706ac8fa26a906d80cf996bbe3
Author: Aldy Hernandez <aldyh@redhat.com>
Date: Wed Feb 18 13:31:41 2015 -0800
PR debug/58123
* gimplify.c (gimplify_expr): Prefer location of TRY_FINALLY_EXPR
over input_location.
testsuite/
* g++.dg/debug/dwarf2/deallocator.C: Adjust for correct try
location.
* g++.dg/gcov/gcov-2.C: Likewise.
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 1353ada..d822913 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -8244,10 +8244,10 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
TREE_CODE (*expr_p) == TRY_FINALLY_EXPR
? GIMPLE_TRY_FINALLY
: GIMPLE_TRY_CATCH);
- if (LOCATION_LOCUS (saved_location) != UNKNOWN_LOCATION)
- gimple_set_location (try_, saved_location);
- else
+ if (EXPR_HAS_LOCATION (save_expr))
gimple_set_location (try_, EXPR_LOCATION (save_expr));
+ else if (LOCATION_LOCUS (saved_location) != UNKNOWN_LOCATION)
+ gimple_set_location (try_, saved_location);
if (TREE_CODE (*expr_p) == TRY_CATCH_EXPR)
gimple_try_set_catch_is_cleanup (try_,
TRY_CATCH_IS_CLEANUP (*expr_p));
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C b/gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C
index 4057393..0fcd08e 100644
--- a/gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/deallocator.C
@@ -41,6 +41,6 @@ void foo(int i)
return;
}
// { dg-final { scan-assembler "deallocator.C:29" } }
-// { dg-final { scan-assembler "deallocator.C:31" } }
-// { dg-final { scan-assembler "deallocator.C:38" } }
-// { dg-final { scan-assembler "deallocator.C:41" } }
+// { dg-final { scan-assembler "deallocator.C:24" } }
+// { dg-final { scan-assembler "deallocator.C:34" } }
+// { dg-final { scan-assembler "deallocator.C:21" } }
diff --git a/gcc/testsuite/g++.dg/gcov/gcov-2.C b/gcc/testsuite/g++.dg/gcov/gcov-2.C
index 66d8af3..6d002f5 100644
--- a/gcc/testsuite/g++.dg/gcov/gcov-2.C
+++ b/gcc/testsuite/g++.dg/gcov/gcov-2.C
@@ -20,7 +20,7 @@ private:
void foo()
{
- C c; /* count(1) */
+ C c; /* count(2) */
c.seti (1); /* count(1) */
}
diff --git a/gcc/testsuite/g++.dg/pr58123.C b/gcc/testsuite/g++.dg/pr58123.C
new file mode 100644
index 0000000..7fe1a27
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr58123.C
@@ -0,0 +1,18 @@
+// { dg-do compile }
+// { dg-options "-fdump-tree-gimple-lineno" }
+
+// Test that the TRY block's location is the definition of "C a".
+
+class C {
+public:
+ C() {}
+ ~C() {}
+ int m() { return 0; }
+};
+int main() {
+ C a;
+ return a.m();
+}
+
+// { dg-final { scan-tree-dump-times "pr58123.C:13\.6\] try" 1 "gimple" } }
+// { dg-final { cleanup-tree-dump "gimple" } }
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch] PR debug/58123: Set correct location for TRY blocks
2015-02-19 18:33 ` [patch] PR debug/58123: Set correct location for TRY blocks Aldy Hernandez
@ 2015-02-20 19:06 ` Jeff Law
2015-02-23 19:37 ` Jason Merrill
1 sibling, 0 replies; 4+ messages in thread
From: Jeff Law @ 2015-02-20 19:06 UTC (permalink / raw)
To: Aldy Hernandez, jason merrill; +Cc: Jakub Jelinek, gcc-patches, Jan Kratochvil
On 02/19/15 10:31, Aldy Hernandez wrote:
> [Ughh...I'm apparently incapable of CCing gcc-patches on the first try.
> My apologies again.]
>
> Please see the long explanation in the PR:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58123
>
> But the short explanation for this patch is that, at the very least, we
> should fix the location of the GIMPLE_TRY_FINALLY. Currently we're
> preferring the input_location, instead of the location of the
> TRY_FINALLY_EXPR.
>
> An incorrect GIMPLE_TRY_FINALLY location causes the location of
> location-less FINALLY statements to also have an incorrect location,
> causing an inconsistent debugging experience.
>
> As explained in the PR, I would ideally like to get rid of the kludge
> where we set the location of location-less FINALLY statements to be that
> of the TRY statement, but that may be by design, and/or beyond the scope
> of this fix.
>
> Tested on x86-64 Linux.
>
> OK for mainline?
>
>
>
>
> curr
>
>
> commit 119b6f5c628831706ac8fa26a906d80cf996bbe3
> Author: Aldy Hernandez<aldyh@redhat.com>
> Date: Wed Feb 18 13:31:41 2015 -0800
>
> PR debug/58123
> * gimplify.c (gimplify_expr): Prefer location of TRY_FINALLY_EXPR
> over input_location.
> testsuite/
> * g++.dg/debug/dwarf2/deallocator.C: Adjust for correct try
> location.
> * g++.dg/gcov/gcov-2.C: Likewise.
OK.
Jeff
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch] PR debug/58123: Set correct location for TRY blocks
2015-02-19 18:33 ` [patch] PR debug/58123: Set correct location for TRY blocks Aldy Hernandez
2015-02-20 19:06 ` Jeff Law
@ 2015-02-23 19:37 ` Jason Merrill
2015-02-23 21:00 ` Aldy Hernandez
1 sibling, 1 reply; 4+ messages in thread
From: Jason Merrill @ 2015-02-23 19:37 UTC (permalink / raw)
To: Aldy Hernandez; +Cc: Jakub Jelinek, gcc-patches, Jan Kratochvil
On 02/19/2015 12:31 PM, Aldy Hernandez wrote:
> As explained in the PR, I would ideally like to get rid of the kludge
> where we set the location of location-less FINALLY statements to be that
> of the TRY statement, but that may be by design, and/or beyond the scope
> of this fix.
This reminds me of the semi-recent change to disassociate variable
cleanups with the location of the declaration. I think Jakub added a
pass specifically for this purpose? I think making the change you
suggest would make sense, but probably as a separate patch.
Jason
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch] PR debug/58123: Set correct location for TRY blocks
2015-02-23 19:37 ` Jason Merrill
@ 2015-02-23 21:00 ` Aldy Hernandez
0 siblings, 0 replies; 4+ messages in thread
From: Aldy Hernandez @ 2015-02-23 21:00 UTC (permalink / raw)
To: Jason Merrill; +Cc: Jakub Jelinek, gcc-patches, Jan Kratochvil
On 02/23/2015 10:37 AM, Jason Merrill wrote:
> On 02/19/2015 12:31 PM, Aldy Hernandez wrote:
>> As explained in the PR, I would ideally like to get rid of the kludge
>> where we set the location of location-less FINALLY statements to be that
>> of the TRY statement, but that may be by design, and/or beyond the scope
>> of this fix.
>
> This reminds me of the semi-recent change to disassociate variable
> cleanups with the location of the declaration. I think Jakub added a
> pass specifically for this purpose? I think making the change you
> suggest would make sense, but probably as a separate patch.
Absolutely, and that's why I closed the PR with the fix to the
TRY_FINALLY gimple code. That at least fixes the regression.
thanks.
Aldy
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-02-23 19:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <54E61BE5.9090209@redhat.com>
2015-02-19 18:33 ` [patch] PR debug/58123: Set correct location for TRY blocks Aldy Hernandez
2015-02-20 19:06 ` Jeff Law
2015-02-23 19:37 ` Jason Merrill
2015-02-23 21:00 ` Aldy Hernandez
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).