https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58123
Aldy Hernandez <aldyh at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Last reconfirmed| |2015-02-18
Ever confirmed|0 |1
--- Comment #7 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
Confirmed.
There are a few curious things with this bug.
First, the reason we jump to line 11 is because the EH code, as it expands
try/finally blocks, can't find the location of the destructor calls, so it uses
the location of the enclosing TRY block. So for something like this:
try {
foo();
} finally {
C::~C (&var);
}
...lower_try_finally_onedest(), will set the location of the destructor call to
the try location:
if (LOCATION_LOCUS (gimple_location (stmt)) == UNKNOWN_LOCATION)
{
tree block = gimple_block (stmt);
gimple_set_location (stmt, gimple_location (tf->try_finally_expr));
gimple_set_block (stmt, block);
}
Putting this aside for a second, my question is, do we really want a debugging
experience where we jump from back from the end of scope, back to the
declaration of the class? Because one way of fixing this would be to get rid
of the above code that pushes the try location onto finally statements that are
locationless. Then we could hit "next" on the debugger and go from line 8 -> 9
-> 10 -> 11 -> 12, without backtracking to some definition.
Second, the reason we don't have a location for the destructor call is
apparently by design, so we don't do this jumping around I am arguing against.
The patch for PR debug/49951 specifically removes the location of the
destructor so we don't jump around:
/* build_delete sets the location of the destructor call to the
current location, even though the destructor is going to be
called later, at the end of the current scope. This can lead to
a "jumpy" behaviour for users of debuggers when they step around
the end of the block. So let's unset the location of the
destructor call instead. */
if (cleanup != NULL && EXPR_P (cleanup))
SET_EXPR_LOCATION (cleanup, UNKNOWN_LOCATION);
But alas, as I explain above, the try/finally code fills in the missing
location for the destructor with the location of the try.
Third, be that as it may with the above two points, the TRY location is
actually wrong in the trees:
[a.cc:11:40] try <-- ***THIS SHOULD BE LINE 8***
{
[a.cc:9:14] C::C ([a.cc:9:14] &b);
[a.cc:9:14] try
{
[a.cc:10:14] C::C ([a.cc:10:14] &c);
[a.cc:10:14] try
{
[a.cc:11:22] D.2371 = C::m ([a.cc:11:22] &a);
[a.cc:11:30] D.2372 = C::m ([a.cc:11:30] &b);
[a.cc:11:25] D.2373 = D.2371 + D.2372;
[a.cc:11:38] D.2374 = C::m ([a.cc:11:38] &c);
[a.cc:11:39] D.2370 = D.2373 + D.2374;
[a.cc:11:39] goto <D.2375>;
}
finally
{
C::~C (&c);
}
}
finally
{
C::~C (&b);
}
}
finally
{
C::~C (&a);
}
This is actually a problem in the gimplifier. The code gimplifying the
TRY_FINALLY_EXPR has:
if (LOCATION_LOCUS (saved_location) != UNKNOWN_LOCATION)
gimple_set_location (try_, saved_location);
else
gimple_set_location (try_, EXPR_LOCATION (save_expr));
which means we are preferring the saved_location (gathered generically from
input_location) over the TRY_FINALLY_EXPR's location.
IMO, this should be (untested):
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));
This prefers the TRY_FINALLY_EXPR's location if available, else it uses the
input_location upon entry.
With this untested patch, we do the jumpy thing Jan was expecting-- going back
to line 8. But I question whether we want this at all. Instead, do we even
want to fill in unknown location finally statements with the location of the
try block?
It seems like there are two possible solutions (a) avoid filling in missing
locations with the try block's location (b) fix the gimplifier to use the
TRY_FINALLY_EXPR's location if available.
Either way, I think the gimplifier is wrong and should be fixed with (b)
independently of this problem.
Jason, Jakub?
