* [patch] Fix another stale frame_info *
@ 2012-02-23 8:38 Jan Kratochvil
2012-02-24 15:37 ` Pedro Alves
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kratochvil @ 2012-02-23 8:38 UTC (permalink / raw)
To: gdb-patches
Hi,
ISTM one cannot safely initialize FRAME_CALLER_PC unconditionally.
No regressions on {x86_64,x86_64-m32,i686}-fedora17-linux-gnu.
I will check it in.
Thanks,
Jan
gdb/
2012-02-23 Jan Kratochvil <jan.kratochvil@redhat.com>
* breakpoint.c (until_break_command): New variables frame_caller_id,
frame_caller_pc and frame_caller_arch. Use them after
set_momentary_breakpoint.
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -9853,12 +9853,19 @@ until_break_command (char *arg, int from_tty, int anywhere)
struct symtabs_and_lines sals;
struct symtab_and_line sal;
struct frame_info *frame = get_selected_frame (NULL);
+ struct frame_id frame_caller_id = frame_unwind_caller_id (frame);
+ /* Initialize it just to avoid a GCC false warning. */
+ CORE_ADDR frame_caller_pc = 0;
+ struct gdbarch *frame_caller_arch = frame_unwind_caller_arch (frame);
struct breakpoint *breakpoint;
struct breakpoint *breakpoint2 = NULL;
struct cleanup *old_chain;
int thread;
struct thread_info *tp;
+ if (frame_id_p (frame_caller_id))
+ frame_caller_pc = frame_unwind_caller_pc (frame);
+
clear_proceed_status ();
/* Set a breakpoint where the user wants it and at return from
@@ -9903,17 +9912,15 @@ until_break_command (char *arg, int from_tty, int anywhere)
/* Keep within the current frame, or in frames called by the current
one. */
- if (frame_id_p (frame_unwind_caller_id (frame)))
+ if (frame_id_p (frame_caller_id))
{
- sal = find_pc_line (frame_unwind_caller_pc (frame), 0);
- sal.pc = frame_unwind_caller_pc (frame);
- breakpoint2 = set_momentary_breakpoint (frame_unwind_caller_arch (frame),
- sal,
- frame_unwind_caller_id (frame),
- bp_until);
+ sal = find_pc_line (frame_caller_pc, 0);
+ sal.pc = frame_caller_pc;
+ breakpoint2 = set_momentary_breakpoint (frame_caller_arch, sal,
+ frame_caller_id, bp_until);
make_cleanup_delete_breakpoint (breakpoint2);
- set_longjmp_breakpoint (tp, frame_unwind_caller_id (frame));
+ set_longjmp_breakpoint (tp, frame_caller_id);
make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] Fix another stale frame_info *
2012-02-23 8:38 [patch] Fix another stale frame_info * Jan Kratochvil
@ 2012-02-24 15:37 ` Pedro Alves
2012-02-24 15:41 ` Jan Kratochvil
0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2012-02-24 15:37 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
On 02/23/2012 08:37 AM, Jan Kratochvil wrote:
> + if (frame_id_p (frame_caller_id))
> + frame_caller_pc = frame_unwind_caller_pc (frame);
> +
Yeah. If we swap the order we do things, we can avoid needing this
bit. WDYT? It seems slightly cleaner to me. Tested on x86_64 Fedora 16.
2012-02-24 Jan Kratochvil <jan.kratochvil@redhat.com>
Pedro Alves <palves@redhat.com>
* breakpoint.c (until_break_command): Install breakpoints after
all frame manipulations.
---
gdb/breakpoint.c | 41 ++++++++++++++++++++++++-----------------
1 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3740157..fa09238 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -9853,6 +9853,8 @@ until_break_command (char *arg, int from_tty, int anywhere)
struct symtabs_and_lines sals;
struct symtab_and_line sal;
struct frame_info *frame = get_selected_frame (NULL);
+ struct gdbarch *gdbarch = get_frame_arch (frame);
+ struct frame_id stack_frame_id = get_stack_frame_id (frame);
struct breakpoint *breakpoint;
struct breakpoint *breakpoint2 = NULL;
struct cleanup *old_chain;
@@ -9883,32 +9885,25 @@ until_break_command (char *arg, int from_tty, int anywhere)
resolve_sal_pc (&sal);
- if (anywhere)
- /* If the user told us to continue until a specified location,
- we don't specify a frame at which we need to stop. */
- breakpoint = set_momentary_breakpoint (get_frame_arch (frame), sal,
- null_frame_id, bp_until);
- else
- /* Otherwise, specify the selected frame, because we want to stop
- only at the very same frame. */
- breakpoint = set_momentary_breakpoint (get_frame_arch (frame), sal,
- get_stack_frame_id (frame),
- bp_until);
-
- old_chain = make_cleanup_delete_breakpoint (breakpoint);
-
tp = inferior_thread ();
thread = tp->num;
+ old_chain = make_cleanup (null_cleanup, NULL);
+
+ /* Installing a breakpoint invalidates the frame chain (as it may
+ need to switch threads), so do any frame handling first. */
+
/* Keep within the current frame, or in frames called by the current
one. */
if (frame_id_p (frame_unwind_caller_id (frame)))
{
- sal = find_pc_line (frame_unwind_caller_pc (frame), 0);
- sal.pc = frame_unwind_caller_pc (frame);
+ struct symtab_and_line sal2;
+
+ sal2 = find_pc_line (frame_unwind_caller_pc (frame), 0);
+ sal2.pc = frame_unwind_caller_pc (frame);
breakpoint2 = set_momentary_breakpoint (frame_unwind_caller_arch (frame),
- sal,
+ sal2,
frame_unwind_caller_id (frame),
bp_until);
make_cleanup_delete_breakpoint (breakpoint2);
@@ -9917,6 +9912,18 @@ until_break_command (char *arg, int from_tty, int anywhere)
make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
}
+ if (anywhere)
+ /* If the user told us to continue until a specified location,
+ we don't specify a frame at which we need to stop. */
+ breakpoint = set_momentary_breakpoint (gdbarch, sal,
+ null_frame_id, bp_until);
+ else
+ /* Otherwise, specify the selected frame, because we want to stop
+ only at the very same frame. */
+ breakpoint = set_momentary_breakpoint (gdbarch, sal,
+ stack_frame_id, bp_until);
+ make_cleanup_delete_breakpoint (breakpoint);
+
proceed (-1, TARGET_SIGNAL_DEFAULT, 0);
/* If we are running asynchronously, and proceed call above has
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] Fix another stale frame_info *
2012-02-24 15:37 ` Pedro Alves
@ 2012-02-24 15:41 ` Jan Kratochvil
2012-02-24 15:54 ` Pedro Alves
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kratochvil @ 2012-02-24 15:41 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Fri, 24 Feb 2012 16:28:04 +0100, Pedro Alves wrote:
> Yeah. If we swap the order we do things, we can avoid needing this
> bit. WDYT? It seems slightly cleaner to me. Tested on x86_64 Fedora 16.
In general yes just this patch is not there yet.
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
[...]
> if (frame_id_p (frame_unwind_caller_id (frame)))
> {
> - sal = find_pc_line (frame_unwind_caller_pc (frame), 0);
> - sal.pc = frame_unwind_caller_pc (frame);
> + struct symtab_and_line sal2;
> +
> + sal2 = find_pc_line (frame_unwind_caller_pc (frame), 0);
> + sal2.pc = frame_unwind_caller_pc (frame);
> breakpoint2 = set_momentary_breakpoint (frame_unwind_caller_arch (frame),
> - sal,
> + sal2,
> frame_unwind_caller_id (frame),
> bp_until);
> make_cleanup_delete_breakpoint (breakpoint2);
Here is still in the code:
set_longjmp_breakpoint (tp, frame_unwind_caller_id (frame));
Thanks,
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] Fix another stale frame_info *
2012-02-24 15:41 ` Jan Kratochvil
@ 2012-02-24 15:54 ` Pedro Alves
2012-02-24 16:05 ` Jan Kratochvil
0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2012-02-24 15:54 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
On 02/24/2012 03:37 PM, Jan Kratochvil wrote:
> On Fri, 24 Feb 2012 16:28:04 +0100, Pedro Alves wrote:
>> Yeah. If we swap the order we do things, we can avoid needing this
>> bit. WDYT? It seems slightly cleaner to me. Tested on x86_64 Fedora 16.
>
> In general yes just this patch is not there yet.
>
>
>> --- a/gdb/breakpoint.c
>> +++ b/gdb/breakpoint.c
> [...]
>> if (frame_id_p (frame_unwind_caller_id (frame)))
>> {
>> - sal = find_pc_line (frame_unwind_caller_pc (frame), 0);
>> - sal.pc = frame_unwind_caller_pc (frame);
>> + struct symtab_and_line sal2;
>> +
>> + sal2 = find_pc_line (frame_unwind_caller_pc (frame), 0);
>> + sal2.pc = frame_unwind_caller_pc (frame);
>> breakpoint2 = set_momentary_breakpoint (frame_unwind_caller_arch (frame),
>> - sal,
>> + sal2,
>> frame_unwind_caller_id (frame),
>> bp_until);
>> make_cleanup_delete_breakpoint (breakpoint2);
>
> Here is still in the code:
> set_longjmp_breakpoint (tp, frame_unwind_caller_id (frame));
Whoops. Re-tested on x86_64 Fedora 16.
2012-02-24 Jan Kratochvil <jan.kratochvil@redhat.com>
Pedro Alves <palves@redhat.com>
* breakpoint.c (until_break_command): Install breakpoints after
all frame manipulations.
---
gdb/breakpoint.c | 48 ++++++++++++++++++++++++++++--------------------
1 files changed, 28 insertions(+), 20 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3740157..1d5f069 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -9853,6 +9853,9 @@ until_break_command (char *arg, int from_tty, int anywhere)
struct symtabs_and_lines sals;
struct symtab_and_line sal;
struct frame_info *frame = get_selected_frame (NULL);
+ struct gdbarch *gdbarch = get_frame_arch (frame);
+ struct frame_id stack_frame_id = get_stack_frame_id (frame);
+ struct frame_id caller_frame_id = frame_unwind_caller_id (frame);
struct breakpoint *breakpoint;
struct breakpoint *breakpoint2 = NULL;
struct cleanup *old_chain;
@@ -9883,40 +9886,45 @@ until_break_command (char *arg, int from_tty, int anywhere)
resolve_sal_pc (&sal);
- if (anywhere)
- /* If the user told us to continue until a specified location,
- we don't specify a frame at which we need to stop. */
- breakpoint = set_momentary_breakpoint (get_frame_arch (frame), sal,
- null_frame_id, bp_until);
- else
- /* Otherwise, specify the selected frame, because we want to stop
- only at the very same frame. */
- breakpoint = set_momentary_breakpoint (get_frame_arch (frame), sal,
- get_stack_frame_id (frame),
- bp_until);
-
- old_chain = make_cleanup_delete_breakpoint (breakpoint);
-
tp = inferior_thread ();
thread = tp->num;
+ old_chain = make_cleanup (null_cleanup, NULL);
+
+ /* Installing a breakpoint invalidates the frame chain (as it may
+ need to switch threads), so do any frame handling first. */
+
/* Keep within the current frame, or in frames called by the current
one. */
- if (frame_id_p (frame_unwind_caller_id (frame)))
+ if (frame_id_p (caller_frame_id))
{
- sal = find_pc_line (frame_unwind_caller_pc (frame), 0);
- sal.pc = frame_unwind_caller_pc (frame);
+ struct symtab_and_line sal2;
+
+ sal2 = find_pc_line (frame_unwind_caller_pc (frame), 0);
+ sal2.pc = frame_unwind_caller_pc (frame);
breakpoint2 = set_momentary_breakpoint (frame_unwind_caller_arch (frame),
- sal,
- frame_unwind_caller_id (frame),
+ sal2,
+ caller_frame_id,
bp_until);
make_cleanup_delete_breakpoint (breakpoint2);
- set_longjmp_breakpoint (tp, frame_unwind_caller_id (frame));
+ set_longjmp_breakpoint (tp, caller_frame_id);
make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
}
+ if (anywhere)
+ /* If the user told us to continue until a specified location,
+ we don't specify a frame at which we need to stop. */
+ breakpoint = set_momentary_breakpoint (gdbarch, sal,
+ null_frame_id, bp_until);
+ else
+ /* Otherwise, specify the selected frame, because we want to stop
+ only at the very same frame. */
+ breakpoint = set_momentary_breakpoint (gdbarch, sal,
+ stack_frame_id, bp_until);
+ make_cleanup_delete_breakpoint (breakpoint);
+
proceed (-1, TARGET_SIGNAL_DEFAULT, 0);
/* If we are running asynchronously, and proceed call above has
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] Fix another stale frame_info *
2012-02-24 15:54 ` Pedro Alves
@ 2012-02-24 16:05 ` Jan Kratochvil
2012-02-24 16:32 ` Pedro Alves
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kratochvil @ 2012-02-24 16:05 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Fri, 24 Feb 2012 16:52:01 +0100, Pedro Alves wrote:
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -9853,6 +9853,9 @@ until_break_command (char *arg, int from_tty, int anywhere)
> struct symtabs_and_lines sals;
> struct symtab_and_line sal;
> struct frame_info *frame = get_selected_frame (NULL);
> + struct gdbarch *gdbarch = get_frame_arch (frame);
> + struct frame_id stack_frame_id = get_stack_frame_id (frame);
> + struct frame_id caller_frame_id = frame_unwind_caller_id (frame);
OK with it.
'gdbarch' should be called 'frame_gdbarch' as this function deals both with
FRAME and previous-of-FRAME (caller_frame_id).
Thanks,
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] Fix another stale frame_info *
2012-02-24 16:05 ` Jan Kratochvil
@ 2012-02-24 16:32 ` Pedro Alves
0 siblings, 0 replies; 6+ messages in thread
From: Pedro Alves @ 2012-02-24 16:32 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
On 02/24/2012 04:00 PM, Jan Kratochvil wrote:
> OK with it.
>
> 'gdbarch' should be called 'frame_gdbarch' as this function deals both with
> FRAME and previous-of-FRAME (caller_frame_id).
Agreed. Applied with that rename.
Thanks.
2012-02-24 Jan Kratochvil <jan.kratochvil@redhat.com>
Pedro Alves <palves@redhat.com>
* breakpoint.c (until_break_command): Install breakpoints after
all frame manipulations.
---
gdb/breakpoint.c | 48 ++++++++++++++++++++++++++++--------------------
1 files changed, 28 insertions(+), 20 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3740157..b5c30ee 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -9853,6 +9853,9 @@ until_break_command (char *arg, int from_tty, int anywhere)
struct symtabs_and_lines sals;
struct symtab_and_line sal;
struct frame_info *frame = get_selected_frame (NULL);
+ struct gdbarch *frame_gdbarch = get_frame_arch (frame);
+ struct frame_id stack_frame_id = get_stack_frame_id (frame);
+ struct frame_id caller_frame_id = frame_unwind_caller_id (frame);
struct breakpoint *breakpoint;
struct breakpoint *breakpoint2 = NULL;
struct cleanup *old_chain;
@@ -9883,40 +9886,45 @@ until_break_command (char *arg, int from_tty, int anywhere)
resolve_sal_pc (&sal);
- if (anywhere)
- /* If the user told us to continue until a specified location,
- we don't specify a frame at which we need to stop. */
- breakpoint = set_momentary_breakpoint (get_frame_arch (frame), sal,
- null_frame_id, bp_until);
- else
- /* Otherwise, specify the selected frame, because we want to stop
- only at the very same frame. */
- breakpoint = set_momentary_breakpoint (get_frame_arch (frame), sal,
- get_stack_frame_id (frame),
- bp_until);
-
- old_chain = make_cleanup_delete_breakpoint (breakpoint);
-
tp = inferior_thread ();
thread = tp->num;
+ old_chain = make_cleanup (null_cleanup, NULL);
+
+ /* Installing a breakpoint invalidates the frame chain (as it may
+ need to switch threads), so do any frame handling first. */
+
/* Keep within the current frame, or in frames called by the current
one. */
- if (frame_id_p (frame_unwind_caller_id (frame)))
+ if (frame_id_p (caller_frame_id))
{
- sal = find_pc_line (frame_unwind_caller_pc (frame), 0);
- sal.pc = frame_unwind_caller_pc (frame);
+ struct symtab_and_line sal2;
+
+ sal2 = find_pc_line (frame_unwind_caller_pc (frame), 0);
+ sal2.pc = frame_unwind_caller_pc (frame);
breakpoint2 = set_momentary_breakpoint (frame_unwind_caller_arch (frame),
- sal,
- frame_unwind_caller_id (frame),
+ sal2,
+ caller_frame_id,
bp_until);
make_cleanup_delete_breakpoint (breakpoint2);
- set_longjmp_breakpoint (tp, frame_unwind_caller_id (frame));
+ set_longjmp_breakpoint (tp, caller_frame_id);
make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
}
+ if (anywhere)
+ /* If the user told us to continue until a specified location,
+ we don't specify a frame at which we need to stop. */
+ breakpoint = set_momentary_breakpoint (frame_gdbarch, sal,
+ null_frame_id, bp_until);
+ else
+ /* Otherwise, specify the selected frame, because we want to stop
+ only at the very same frame. */
+ breakpoint = set_momentary_breakpoint (frame_gdbarch, sal,
+ stack_frame_id, bp_until);
+ make_cleanup_delete_breakpoint (breakpoint);
+
proceed (-1, TARGET_SIGNAL_DEFAULT, 0);
/* If we are running asynchronously, and proceed call above has
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-02-24 16:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-23 8:38 [patch] Fix another stale frame_info * Jan Kratochvil
2012-02-24 15:37 ` Pedro Alves
2012-02-24 15:41 ` Jan Kratochvil
2012-02-24 15:54 ` Pedro Alves
2012-02-24 16:05 ` Jan Kratochvil
2012-02-24 16:32 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox