[v2] Don't elide all inlined frames
Commit Message
On 05/09/2018 08:17 PM, Keith Seitz wrote:
> On 04/09/2018 05:25 AM, Pedro Alves wrote:
> I'm sorry, I don't follow. The locations are gotten from `s' which is from the bpstat chain. Aside from the breakpoint's type (and whether it is a user breakpoint), the actual breakpoint is not used.
You're right, sorry about that.
>
> However, the inner for-loop (over the location->next) here can/should be removed. I've changed that.
>
> Otherwise, I'm afraid I just don't understand what you're saying. You might have to hit me with the clue stick. Sorry.
I was thinking of this comment:
/* Breakpoint that caused the stop. This is nullified if the
breakpoint ends up being deleted. See comments on
`bp_location_at' above for why do we need this field instead of
following the location's owner. */
struct breakpoint *breakpoint_at;
and noticed that the code uses "s->breakpoint_at" and somehow misread
it as still following the breakpoint's location list.
>> The part about breakint out of the outer loop no longer
>> applies as is, but, AFAICT, the current code is still letting the
>> outer stop_chain loop continue iterating even 'if (!skip_this_frame)'?
>
> The quoted code above (yours) is exactly the code in the patch. Did I send the wrong patch?
The point is that the code still continues iterating this middle
loop unnecessarily:
for (bpstat s = stop_chain; s != NULL; s = s->next)
{
I think the easiest and clearest way to address that is to move
those loops to a separate function, like this:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
From be6bc201c84fcbe94783bdf08557658b824df0d0 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 11 May 2018 16:22:09 +0100
Subject: [PATCH] split
---
gdb/inline-frame.c | 65 +++++++++++++++++++++++++++++-------------------------
1 file changed, 35 insertions(+), 30 deletions(-)
@@ -297,6 +297,35 @@ block_starting_point_at (CORE_ADDR pc, const struct block *block)
return 1;
}
+/* Loop over the stop chain and determine if execution stopped in an
+ inlined frame because of a user breakpoint. THIS_PC is the current
+ frame's PC. */
+
+static bool
+stopped_by_user_bp_inline_frame (CORE_ADDR this_pc, bpstat stop_chain)
+{
+ for (bpstat s = stop_chain; s != NULL; s = s->next)
+ {
+ struct breakpoint *bpt = s->breakpoint_at;
+
+ if (bpt != NULL && user_breakpoint_p (bpt))
+ {
+ for (bp_location *loc = s->bp_location_at;
+ loc != NULL; loc = loc->next)
+ {
+ enum bp_loc_type t = loc->loc_type;
+
+ if (loc->address == this_pc
+ && (t == bp_loc_software_breakpoint
+ || t == bp_loc_hardware_breakpoint))
+ return true;
+ }
+ }
+ }
+
+ return false;
+}
+
/* See inline-frame.h. */
void
@@ -326,38 +355,14 @@ skip_inline_frames (ptid_t ptid, bpstat stop_chain)
if (BLOCK_START (cur_block) == this_pc
|| block_starting_point_at (this_pc, cur_block))
{
- bool skip_this_frame = true;
-
- /* Loop over the stop chain and determine if execution
- stopped in an inlined frame because of a user breakpoint.
- If so do not skip the inlined frame. */
- for (bpstat s = stop_chain; s != NULL; s = s->next)
+ /* Do not skip the inlined frame if execution
+ stopped in an inlined frame because of a user
+ breakpoint. */
+ if (!stopped_by_user_bp_inline_frame (this_pc, stop_chain))
{
- struct breakpoint *bpt = s->breakpoint_at;
-
- if (bpt != NULL && user_breakpoint_p (bpt))
- {
- for (bp_location *loc = s->bp_location_at;
- loc != NULL; loc = loc->next)
- {
- enum bp_loc_type t = loc->loc_type;
-
- if (loc->address == this_pc
- && (t == bp_loc_software_breakpoint
- || t == bp_loc_hardware_breakpoint))
- {
- skip_this_frame = false;
- break;
- }
- }
- }
+ skip_count++;
+ last_sym = BLOCK_FUNCTION (cur_block);
}
-
- if (!skip_this_frame)
- break;
-
- skip_count++;
- last_sym = BLOCK_FUNCTION (cur_block);
}
else
break;