From b67a2c6fd4862b79c04c254fe2ba37c180b73d60 Mon Sep 17 00:00:00 2001 From: Yao Qi Date: Wed, 25 Jun 2014 11:52:52 +0800 Subject: [PATCH] Associate dummy_frame with ptid This patch is to add ptid into dummy_frame and extend frame_id to dummy_frame_id (which has a ptid field). With this change, GDB uses dummy_frame_id (thread ptid and frame_id) to find the dummy frames. Currently, dummy frames are looked up by frame_id, which isn't accurate in non-stop or multi-process mode. The test case gdb.multi/dummy-frame-restore.exp shows the problem and this patch can fix it. Test dummy-frame-restore.exp makes two inferiors stop at different functions, say, inferior 1 stops at f1 while inferior 2 stops at f2. Set a breakpoint to a function, do the inferior call in two inferiors, and GDB has two dummy frames of the same frame_id. When the inferior call is finished, GDB will look up a dummy frame from its stack/list and restore the inferior's regcache. Two inferiors are finished in different orders, the inferiors' states are restored differently, which is wrong. Running dummy-frame-restore.exp under un-patched GDB, we'll get two fails: FAIL: gdb.multi/dummy-frame-restore.exp: inf 2 first: after infcall: bt in inferior 2 FAIL: gdb.multi/dummy-frame-restore.exp: inf 2 first: after infcall: bt in inferior 1 With this patch applied, GDB will choose the correct dummy_frame to restore for a given inferior, because ptid is considered when looking up dummy frames. Two fails above are fixed. Regression tested on x86_64-linux, both native and gdbserver. gdb: 2014-06-27 Yao Qi * breakpoint.c (check_longjmp_breakpoint_for_call_dummy): Change parameter type to 'struct thread_info *'. Caller updated. * breakpoint.h (check_longjmp_breakpoint_for_call_dummy): Update declaration. * dummy-frame.c (struct dummy_frame_id): New. (dummy_frame_id_eq): New function. (struct dummy_frame) : Change its type to 'struct dummy_frame_id'. (dummy_frame_push): Add parameter ptid and save it in dummy_frame_id. (pop_dummy_frame_bpt): Use ptid of dummy_frame instead of inferior_ptid. (pop_dummy_frame): Assert that the ptid of dummy_frame equals to inferior_ptid. (lookup_dummy_frame): Change parameter type to 'struct dummy_frame_id *'. Callers updated. Call dummy_frame_id_eq instead of frame_id_eq. (dummy_frame_pop): Add parameter ptid. Callers updated. Update comments. Compose dummy_frame_id and pass it to lookup_dummy_frame. (dummy_frame_discard): Add parameter ptid. (dummy_frame_sniffer): Compose dummy_frame_id and call dummy_frame_id_eq instead of frame_id_eq. (fprint_dummy_frames): Print ptid. * dummy-frame.h: Remove comments. (dummy_frame_push): Add ptid in declaration. (dummy_frame_pop, dummy_frame_discard): Likewise. gdb/testsuite: 2014-06-27 Yao Qi * gdb.multi/dummy-frame-restore.exp: New. * gdb.multi/dummy-frame-restore.c: New. gdb/doc: 2014-06-27 Yao Qi * gdb.texinfo (Maintenance Commands): Update the output of 'maint print dummy-frames' command. --- gdb/ChangeLog | 31 ++++++ gdb/breakpoint.c | 8 +- gdb/breakpoint.h | 2 +- gdb/doc/ChangeLog | 5 + gdb/doc/gdb.texinfo | 4 +- gdb/dummy-frame.c | 72 +++++++++----- gdb/dummy-frame.h | 11 +-- gdb/frame.c | 2 +- gdb/infcall.c | 6 +- gdb/infrun.c | 2 +- gdb/testsuite/ChangeLog | 5 + gdb/testsuite/gdb.multi/dummy-frame-restore.c | 36 +++++++ .../gdb.multi/dummy-frame-restore.exp | 97 +++++++++++++++++++ 13 files changed, 236 insertions(+), 45 deletions(-) create mode 100644 gdb/testsuite/gdb.multi/dummy-frame-restore.c create mode 100644 gdb/testsuite/gdb.multi/dummy-frame-restore.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index c60fad8dddf..8b7c5fa3436 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,34 @@ +2014-06-27 Yao Qi + + * breakpoint.c (check_longjmp_breakpoint_for_call_dummy): + Change parameter type to 'struct thread_info *'. Caller + updated. + * breakpoint.h (check_longjmp_breakpoint_for_call_dummy): + Update declaration. + * dummy-frame.c (struct dummy_frame_id): New. + (dummy_frame_id_eq): New function. + (struct dummy_frame) : Change its type to 'struct + dummy_frame_id'. + (dummy_frame_push): Add parameter ptid and save it in + dummy_frame_id. + (pop_dummy_frame_bpt): Use ptid of dummy_frame instead of + inferior_ptid. + (pop_dummy_frame): Assert that the ptid of dummy_frame equals + to inferior_ptid. + (lookup_dummy_frame): Change parameter type to 'struct + dummy_frame_id *'. Callers updated. Call dummy_frame_id_eq + instead of frame_id_eq. + (dummy_frame_pop): Add parameter ptid. Callers updated. + Update comments. Compose dummy_frame_id and pass it to + lookup_dummy_frame. + (dummy_frame_discard): Add parameter ptid. + (dummy_frame_sniffer): Compose dummy_frame_id and call + dummy_frame_id_eq instead of frame_id_eq. + (fprint_dummy_frames): Print ptid. + * dummy-frame.h: Remove comments. + (dummy_frame_push): Add ptid in declaration. + (dummy_frame_pop, dummy_frame_discard): Likewise. + 2014-06-26 Tom Tromey * cli/cli-cmds.c (error_no_arg): Make "why" const. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index b04f3b73825..0b49dec4894 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -7460,7 +7460,7 @@ set_longjmp_breakpoint_for_call_dummy (void) } /* Verify all existing dummy frames and their associated breakpoints for - THREAD. Remove those which can no longer be found in the current frame + TP. Remove those which can no longer be found in the current frame stack. You should call this function only at places where it is safe to currently @@ -7468,12 +7468,12 @@ set_longjmp_breakpoint_for_call_dummy (void) frames. */ void -check_longjmp_breakpoint_for_call_dummy (int thread) +check_longjmp_breakpoint_for_call_dummy (struct thread_info *tp) { struct breakpoint *b, *b_tmp; ALL_BREAKPOINTS_SAFE (b, b_tmp) - if (b->type == bp_longjmp_call_dummy && b->thread == thread) + if (b->type == bp_longjmp_call_dummy && b->thread == tp->num) { struct breakpoint *dummy_b = b->related_breakpoint; @@ -7483,7 +7483,7 @@ check_longjmp_breakpoint_for_call_dummy (int thread) || frame_find_by_id (dummy_b->frame_id) != NULL) continue; - dummy_frame_discard (dummy_b->frame_id); + dummy_frame_discard (dummy_b->frame_id, tp->ptid); while (b->related_breakpoint != b) { diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index d8e88fc26b2..83ae9e6f8dd 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -1329,7 +1329,7 @@ extern void delete_longjmp_breakpoint (int thread); extern void delete_longjmp_breakpoint_at_next_stop (int thread); extern struct breakpoint *set_longjmp_breakpoint_for_call_dummy (void); -extern void check_longjmp_breakpoint_for_call_dummy (int thread); +extern void check_longjmp_breakpoint_for_call_dummy (struct thread_info *tp); extern void enable_overlay_breakpoints (void); extern void disable_overlay_breakpoints (void); diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog index 1b860710c1c..67998cacf01 100644 --- a/gdb/doc/ChangeLog +++ b/gdb/doc/ChangeLog @@ -1,3 +1,8 @@ +2014-06-27 Yao Qi + + * gdb.texinfo (Maintenance Commands): Update the output of + 'maint print dummy-frames' command. + 2014-06-24 Eli Zaretskii * gdb.texinfo (Screen Size): Improve indexing. diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 373ea07bb08..36624706a3e 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -33378,9 +33378,7 @@ Breakpoint 2, add (a=2, b=3) at @dots{} The program being debugged stopped while in a function called from GDB. @dots{} (@value{GDBP}) @kbd{maint print dummy-frames} -0x1a57c80: pc=0x01014068 fp=0x0200bddc sp=0x0200bdd6 - top=0x0200bdd4 id=@{stack=0x200bddc,code=0x101405c@} - call_lo=0x01014000 call_hi=0x01014001 +0xa8206d8: id=@{stack=0xbfffe734,code=0xbfffe73f,!special@}, ptid=process 9353 (@value{GDBP}) @end smallexample diff --git a/gdb/dummy-frame.c b/gdb/dummy-frame.c index dd2ff1285ce..35d78e96216 100644 --- a/gdb/dummy-frame.c +++ b/gdb/dummy-frame.c @@ -31,6 +31,25 @@ #include "observer.h" #include "gdbthread.h" +struct dummy_frame_id +{ + /* This frame's ID. Must match the value returned by + gdbarch_dummy_id. */ + struct frame_id id; + + /* The thread this dummy_frame relates to. */ + ptid_t ptid; +}; + +/* Return whether dummy_frame_id *ID1 and *ID2 are equal. */ + +static int +dummy_frame_id_eq (struct dummy_frame_id *id1, + struct dummy_frame_id *id2) +{ + return frame_id_eq (id1->id, id2->id) && ptid_equal (id1->ptid, id2->ptid); +} + /* Dummy frame. This saves the processor state just prior to setting up the inferior function call. Older targets save the registers on the target stack (but that really slows down function calls). */ @@ -38,9 +57,10 @@ struct dummy_frame { struct dummy_frame *next; - /* This frame's ID. Must match the value returned by - gdbarch_dummy_id. */ - struct frame_id id; + + /* An id represents a dummy frame. */ + struct dummy_frame_id id; + /* The caller's state prior to the call. */ struct infcall_suspend_state *caller_state; }; @@ -52,13 +72,14 @@ static struct dummy_frame *dummy_frame_stack = NULL; void dummy_frame_push (struct infcall_suspend_state *caller_state, - const struct frame_id *dummy_id) + const struct frame_id *dummy_id, ptid_t ptid) { struct dummy_frame *dummy_frame; dummy_frame = XCNEW (struct dummy_frame); dummy_frame->caller_state = caller_state; - dummy_frame->id = (*dummy_id); + dummy_frame->id.id = (*dummy_id); + dummy_frame->id.ptid = ptid; dummy_frame->next = dummy_frame_stack; dummy_frame_stack = dummy_frame; } @@ -83,8 +104,8 @@ pop_dummy_frame_bpt (struct breakpoint *b, void *dummy_voidp) { struct dummy_frame *dummy = dummy_voidp; - if (b->thread == pid_to_thread_id (inferior_ptid) - && b->disposition == disp_del && frame_id_eq (b->frame_id, dummy->id)) + if (b->thread == pid_to_thread_id (dummy->id.ptid) + && b->disposition == disp_del && frame_id_eq (b->frame_id, dummy->id.id)) { while (b->related_breakpoint != b) delete_breakpoint (b->related_breakpoint); @@ -107,6 +128,7 @@ pop_dummy_frame (struct dummy_frame **dummy_ptr) { struct dummy_frame *dummy = *dummy_ptr; + gdb_assert (ptid_equal (dummy->id.ptid, inferior_ptid)); restore_infcall_suspend_state (dummy->caller_state); iterate_over_breakpoints (pop_dummy_frame_bpt, dummy); @@ -125,48 +147,47 @@ pop_dummy_frame (struct dummy_frame **dummy_ptr) Return NULL if not found. */ static struct dummy_frame ** -lookup_dummy_frame (struct frame_id dummy_id) +lookup_dummy_frame (struct dummy_frame_id *dummy_id) { struct dummy_frame **dp; for (dp = &dummy_frame_stack; *dp != NULL; dp = &(*dp)->next) { - if (frame_id_eq ((*dp)->id, dummy_id)) + if (dummy_frame_id_eq (&(*dp)->id, dummy_id)) return dp; } return NULL; } -/* Pop the dummy frame DUMMY_ID, restoring program state to that before the - frame was created. +/* Find the dummy frame by DUMMY_ID and PTID, and pop it, restoring + program state to that before the frame was created. On return reinit_frame_cache has been called. - If the frame isn't found, flag an internal error. - - NOTE: This can only pop the one frame, even if it is in the middle of the - stack, because the other frames may be for different threads, and there's - currently no way to tell which stack frame is for which thread. */ + If the frame isn't found, flag an internal error. */ void -dummy_frame_pop (struct frame_id dummy_id) +dummy_frame_pop (struct frame_id dummy_id, ptid_t ptid) { struct dummy_frame **dp; + struct dummy_frame_id id = { dummy_id, ptid }; - dp = lookup_dummy_frame (dummy_id); + dp = lookup_dummy_frame (&id); gdb_assert (dp != NULL); pop_dummy_frame (dp); } -/* Drop dummy frame DUMMY_ID. Do nothing if it is not found. Do not restore - its state into inferior, just free its memory. */ +/* Find the dummy frame by DUMMY_ID and PTID and drop it. Do nothing + if it is not found. Do not restore its state into inferior, just + free its memory. */ void -dummy_frame_discard (struct frame_id dummy_id) +dummy_frame_discard (struct frame_id dummy_id, ptid_t ptid) { struct dummy_frame **dp; + struct dummy_frame_id id = { dummy_id, ptid }; - dp = lookup_dummy_frame (dummy_id); + dp = lookup_dummy_frame (&id); if (dp) remove_dummy_frame (dp); } @@ -211,13 +232,14 @@ dummy_frame_sniffer (const struct frame_unwind *self, dummy ID, assuming it is a dummy frame. */ struct frame_id this_id = gdbarch_dummy_id (get_frame_arch (this_frame), this_frame); + struct dummy_frame_id dummy_id = { this_id, inferior_ptid }; /* Use that ID to find the corresponding cache entry. */ for (dummyframe = dummy_frame_stack; dummyframe != NULL; dummyframe = dummyframe->next) { - if (frame_id_eq (dummyframe->id, this_id)) + if (dummy_frame_id_eq (&dummyframe->id, &dummy_id)) { struct dummy_frame_cache *cache; @@ -297,7 +319,9 @@ fprint_dummy_frames (struct ui_file *file) gdb_print_host_address (s, file); fprintf_unfiltered (file, ":"); fprintf_unfiltered (file, " id="); - fprint_frame_id (file, s->id); + fprint_frame_id (file, s->id.id); + fprintf_unfiltered (file, ", ptid=%s", + target_pid_to_str (s->id.ptid)); fprintf_unfiltered (file, "\n"); } } diff --git a/gdb/dummy-frame.h b/gdb/dummy-frame.h index 6db312ea0e0..bac1aac9b07 100644 --- a/gdb/dummy-frame.h +++ b/gdb/dummy-frame.h @@ -28,18 +28,13 @@ struct frame_unwind; /* Push the information needed to identify, and unwind from, a dummy frame onto the dummy frame stack. */ -/* NOTE: cagney/2004-08-02: This interface will eventually need to be - parameterized with the caller's thread - that will allow per-thread - dummy-frame stacks and, hence, per-thread inferior function - calls. */ - /* NOTE: cagney/2004-08-02: In the case of ABIs using push_dummy_code containing more than one instruction, this interface many need to be expanded so that it knowns the lower/upper extent of the dummy frame's code. */ extern void dummy_frame_push (struct infcall_suspend_state *caller_state, - const struct frame_id *dummy_id); + const struct frame_id *dummy_id, ptid_t ptid); /* Pop the dummy frame DUMMY_ID, restoring program state to that before the frame was created. @@ -50,9 +45,9 @@ extern void dummy_frame_push (struct infcall_suspend_state *caller_state, stack, because the other frames may be for different threads, and there's currently no way to tell which stack frame is for which thread. */ -extern void dummy_frame_pop (struct frame_id dummy_id); +extern void dummy_frame_pop (struct frame_id dummy_id, ptid_t ptid); -extern void dummy_frame_discard (struct frame_id dummy_id); +extern void dummy_frame_discard (struct frame_id dummy_id, ptid_t ptid); /* If the PC falls in a dummy frame, return a dummy frame unwinder. */ diff --git a/gdb/frame.c b/gdb/frame.c index e052069ed7e..8a2b8bf7e91 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -960,7 +960,7 @@ frame_pop (struct frame_info *this_frame) { /* Popping a dummy frame involves restoring more than just registers. dummy_frame_pop does all the work. */ - dummy_frame_pop (get_frame_id (this_frame)); + dummy_frame_pop (get_frame_id (this_frame), inferior_ptid); return; } diff --git a/gdb/infcall.c b/gdb/infcall.c index 685b8a4c699..52f06128fab 100644 --- a/gdb/infcall.c +++ b/gdb/infcall.c @@ -826,7 +826,7 @@ call_function_by_hand (struct value *function, int nargs, struct value **args) /* Everything's ready, push all the info needed to restore the caller (and identify the dummy-frame) onto the dummy-frame stack. */ - dummy_frame_push (caller_state, &dummy_id); + dummy_frame_push (caller_state, &dummy_id, inferior_ptid); /* Discard both inf_status and caller_state cleanups. From this point on we explicitly restore the associated state @@ -952,7 +952,7 @@ When the function is done executing, GDB will silently stop."), /* We must get back to the frame we were before the dummy call. */ - dummy_frame_pop (dummy_id); + dummy_frame_pop (dummy_id, call_thread_ptid); /* We also need to restore inferior status to that before the dummy call. */ @@ -993,7 +993,7 @@ When the function is done executing, GDB will silently stop."), { /* We must get back to the frame we were before the dummy call. */ - dummy_frame_pop (dummy_id); + dummy_frame_pop (dummy_id, call_thread_ptid); /* We also need to restore inferior status to that before the dummy call. */ diff --git a/gdb/infrun.c b/gdb/infrun.c index de5a9389091..3c58ff2e7f9 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -4443,7 +4443,7 @@ process_event_stop_test (struct execution_control_state *ecs) if (what.is_longjmp) { - check_longjmp_breakpoint_for_call_dummy (ecs->event_thread->num); + check_longjmp_breakpoint_for_call_dummy (ecs->event_thread); if (!frame_id_p (ecs->event_thread->initiating_frame)) { diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index b21c4016075..a2509340f4c 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2014-06-27 Yao Qi + + * gdb.multi/dummy-frame-restore.exp: New. + * gdb.multi/dummy-frame-restore.c: New. + 2014-06-25 Markus Metzger * gdb.btrace/gcore.exp: New. diff --git a/gdb/testsuite/gdb.multi/dummy-frame-restore.c b/gdb/testsuite/gdb.multi/dummy-frame-restore.c new file mode 100644 index 00000000000..118c5418a1a --- /dev/null +++ b/gdb/testsuite/gdb.multi/dummy-frame-restore.c @@ -0,0 +1,36 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2014 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +static void +f1 (void) +{} + +static void +f2 (void) +{} + +void +commonfun (void) +{} + +int +main (void) +{ + f1 (); + f2 (); + return 0; +} diff --git a/gdb/testsuite/gdb.multi/dummy-frame-restore.exp b/gdb/testsuite/gdb.multi/dummy-frame-restore.exp new file mode 100644 index 00000000000..a47a0b17222 --- /dev/null +++ b/gdb/testsuite/gdb.multi/dummy-frame-restore.exp @@ -0,0 +1,97 @@ +# Copyright 2014 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +standard_testfile .c +set executable ${testfile} + +# The plain remote target can't do multiple inferiors. +if [target_info exists use_gdb_stub] { + return +} + +if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug}]} { + return -1 +} + +# Inferior 1 stops at f1. + +if ![runto f1] then { + fail "Can't run to f1" + return 0 +} + +gdb_test "add-inferior -exec ${binfile}" \ + "Added inferior 2.*" \ + "add inferior 2 with -exec ${executable}" +gdb_test "inferior 2" "witching to inferior 2 .*" "switch to inferior 2" + +delete_breakpoints + +# Inferior 2 stops at f2. + +if ![runto f2] then { + fail "Can't run to f2" + return 0 +} + +gdb_breakpoint commonfun + +# Check the stack bactrace in inferior INF. PREFIX is the test +# message prefix. + +proc check_bt { inf prefix } { + with_test_prefix "$prefix" { + gdb_test "bt 1" "#0 f$inf .*" "bt in inferior $inf" + } +} + +# Do inferior call commonfun () in inferior 2 and inferior 1, but +# finish it in inferior INF1 first and then inferior INF2. Check +# the stack backtrace in each inferior is still correct after +# inferior call. + +proc test { inf1 inf2 } { + + with_test_prefix "inf $inf1 first" { + gdb_test "inferior 2" "witching to inferior 2 .*" \ + "switch to inferior 2 (1)" + check_bt 2 "before infcall" + gdb_test "p commonfun()" "Breakpoint .*The program being debugged stopped while in a function called from GDB.*" "infcall in inferior 2" + + gdb_test "inferior 1" "witching to inferior 1 .*" \ + "switch to inferior 1 (1)" + check_bt 1 "before infcall" + gdb_test "p commonfun()" "Breakpoint .*The program being debugged stopped while in a function called from GDB.*" "infcall in inferior 1" + + gdb_test "maintenance print dummy-frames" ": id={stack=.*}.*: id=.*" \ + "two dummy frames" + + gdb_test "inferior $inf1" "witching to inferior $inf1 .*" \ + "switch to inferior $inf1 (2)" + gdb_test "finish" "Run till exit from #0 commonfun .*" \ + "finish in inferior $inf1" + check_bt $inf1 "after infcall" + + gdb_test "inferior $inf2" "witching to inferior $inf2 .*" \ + "switch to inferior $inf2 (2)" + gdb_test "finish" "Run till exit from #0 commonfun .*" \ + "finish in inferior $inf2" + check_bt $inf2 "after infcall" + } +} + +# Do the test in different orders. +test 1 2 +test 2 1 -- 2.30.2