This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On Tue, Nov 18, 2008 at 4:58 AM, Doug Evans <dje@google.com> wrote: > Hi. This patch was in progress when I wrote > http://sourceware.org/ml/gdb-patches/2008-11/msg00391.html > This patch subsumes that one. > > There are two things this patch does: > 1) properly pop dummy frames more often > 2) make the inferior resumable after an inferior function call gets a > signal without having to remember to do "signal 0" > > 1) properly pop dummy frames more often > > Ulrich asked: >> I agree that it would be better if call_function_by_hand were to call >> dummy_frame_pop in this case. However, why exactly is this a bug? > > It's a bug because it's confusing to not pop the frame in the places > where one is able to. Plus it's, well, unclean. > I recognize that the dummy frame stack can't be precisely managed because > the user can do things like: > > set $orig_pc = $pc > set $orig_sp = $sp > break my_fun > call my_fun() > set $pc = $orig_pc > set $sp = $orig_sp > continue > > [This doesn't always work, but you get the idea.] > At the "continue", the inferior function call no longer exists and > gdb's mechanisms for removing the dummy frame from dummy_frame_stack > will never trigger (until the program is resumed and cleanup_dummy_frames > is called). But we can still keep things clean > and unconfusing for the common case. > > 2) make the inferior resumable after an inferior function call gets a > signal without having to remember to do "signal 0" > > If an inferior function call gets a signal (say SIGSEGV or SIGABRT), > one should be able to easily resume program execution after popping the stack. > It works today, but after manually popping the stack (e.g. with > "frame <dummy> ; return" where <dummy> is the dummy stack frame's number) > one has to remember to resume the program with "signal 0". > This is because stop_signal doesn't get restored. > Maybe there's a reason it shouldn't be, or maybe should be made an option, > but the current behaviour seems user-unfriendly for the normal case. > > Restoring stop_signal also has the benefit that if an inferior function > call is made after getting a signal, and the inferior function call > doesn't complete (e.g. has a breakpoint and doesn't complete right away), > the user can resume the program (after popping the inf fun call off the > stack) with "continue" without having to remember what the signal was > and remember to use "signal N". > > [BTW, IWBN if there was a command that did the equivalent of > "frame <dummy> ; return", named say "abandon", so that one didn't have > to go to the trouble of finding the dummy frame's stack number. > "abandon" would have the additional benefit that if the stack > was corrupted and one couldn't find the dummy frame, it would still > work since everything it needs is in dummy_frame_stack - it doesn't need > to examine the inferior's stack, except maybe for some sanity checking. > Continuing the inferior may still not be possible, but it does give the > user a more straightforward way to abandon an inferior function call > than exists today.] > > The bulk of the patch goes into making (2) work in a clean way. > Right now the inferior state that can be restored is a collection of > inferior_status, regcache, and misc. things like stop_pc (see the > assignment of stop_pc in normal_stop() when stop_stack_dummy). > The patch organizes the state into two pieces: inferior program state > (registers, stop_pc, stop_signal) and gdb session state > (the rest of inferior_status). > Program state is recorded on the dummy frame stack and is always > restored, either when the inferior function call completes or > when the stack frame is manually popped. > inferior_status contains the things that only get restored > if either the inferior function call successfully completes or > it gets a signal and unwindonsignal is set. > > P.S. I've removed one copy of the regcache. Hopefully I've structured > things in a way that doesn't lose. > > NOTES: > - this adds the unwindonsignal.exp testcase from before, plus a new > testcase that verifies one can resume the inferior after abandoning > an inferior function call that gets a signal > > It's a big patch so presumably there are issues with it. > Comments? > > [tested on i386-linux and x86_64-linux] > > 2008-11-18 Doug Evans <dje@google.com> > > * dummy-frame.c (dummy_frame): Replace regcache member with > caller_state. > (dummy_frame_push): Replace caller_regcache arg with caller_state. > Return pointer to created dummy frame. All callers updated. > (remove_dummy_frame,do_dummy_frame_cleanup,pop_dummy_frame, > assert_dummy_present,pop_dummy_frame_below,lookup_dummy_id, > dummy_frame_discard,do_pop_dummy_frame_cleanup, > make_cleanup_pop_dummy_frame): New functions. > (dummy_frame_pop): Rewrite. Verify requested frame is in the > dummy frame stack. Restore program state. Discard dummy frames below > the one being deleted. > (dummy_frame_sniffer): Update. > * dummy-frame.h (dummy_frame_push): Update prototype. > (dummy_frame_discard,make_cleanup_pop_dummy_frame): Declare. > * frame.c (frame_pop): dummy_frame_pop now does all the work for > DUMMY_FRAMEs. > * infcall.c (call_function_by_hand): Replace caller_regcache, > caller_regcache_cleanup with caller_state, caller_state_cleanup. > New locals dummy_frame, dummy_frame_cleanup. > Ensure dummy frame is popped/discarded for all possible exit paths. > * inferior.h (inferior_program_state): Declare (opaque type). > (save_inferior_program_state,restore_inferior_program_state, > make_cleanup_restore_inferior_program_state, > discard_inferior_program_state, > get_inferior_program_state_regcache): Declare. > (save_inferior_status): Update prototype. > * infrun.c: #include "dummy-frame.h" > (normal_stop): When stopped for the completion of an inferior function > call, verify the expected stack frame kind and use dummy_frame_pop. > (inferior_program_state): New struct. > (save_inferior_program_state,restore_inferior_program_state, > make_cleanup_restore_inferior_program_state, > discard_inferior_program_state, > get_inferior_program_state_regcache): New functions. > (save_inferior_status): Remove arg restore_stack_info. > All callers updated. Remove saving of state now saved by > save_inferior_program_state. > (restore_inferior_status): Remove restoration of state now done by > restore_inferior_program_state. > (discard_inferior_status): Remove freeing of registers, now done by > discard_inferior_program_state. > > * gdb.base/call-signal-resume.exp: New file. > * gdb.base/call-signals.c: New file. > * gdb.base/unwindonsignal.exp: New file. ref: http://sourceware.org/ml/gdb-patches/2008-11/msg00454.html Blech. I went too far in trying to keep dummy_frame_stack accurate. One can (theoretically) have multiple hand-function-calls outstanding in multiple threads (and soon in multiple processes presumably). Attached is a new version of the patch that goes back to having dummy_frame_pop only popping the requested frame (and similarily for dummy_frame_discard). I wrote a testcase that calls functions in multiple threads (with scheduler-locking on) by setting a breakpoint on the function being called. I think there's a bug in scheduler-locking support as when I make the second call in the second thread, gdb makes the first thread step over the breakpoint and then resume, and control returns to call_function_by_hand with inferior_ptid changed to the first thread. To protect call_function_by_hand from this it now flags an error if the thread changes. [I'll submit the testcase separately once I can get it to pass, unless folks want it to see it.] How's this? 2008-11-18 Doug Evans <dje@google.com> * dummy-frame.c (dummy_frame): Replace regcache member with caller_state. (dummy_frame_push): Replace caller_regcache arg with caller_state. Return pointer to created dummy frame. All callers updated. (remove_dummy_frame,do_dummy_frame_cleanup,pop_dummy_frame_from_ptr, lookup_dummy,lookup_dummy_id, pop_dummy_frame,dummy_frame_discard, do_pop_dummy_frame_cleanup,make_cleanup_pop_dummy_frame): New functions. (dummy_frame_pop): Rewrite. Verify requested frame is in the dummy frame stack. Restore program state. (cleanup_dummy_frames): Rewrite. (dummy_frame_sniffer): Update. * dummy-frame.h (dummy_frame_push): Update prototype. (dummy_frame_discard,make_cleanup_pop_dummy_frame): Declare. * frame.c (frame_pop): dummy_frame_pop now does all the work for DUMMY_FRAMEs. * infcall.c (call_function_by_hand): Replace caller_regcache, caller_regcache_cleanup with caller_state, caller_state_cleanup. New locals dummy_frame, dummy_frame_cleanup, this_thread. Ensure dummy frame is popped/discarded for all possible exit paths. Detect program stopping in a different thread. * inferior.h (inferior_program_state): Declare (opaque type). (save_inferior_program_state,restore_inferior_program_state, make_cleanup_restore_inferior_program_state, discard_inferior_program_state, get_inferior_program_state_regcache): Declare. (save_inferior_status): Update prototype. * infrun.c: #include "dummy-frame.h" (normal_stop): When stopped for the completion of an inferior function call, verify the expected stack frame kind and use dummy_frame_pop. (inferior_program_state): New struct. (save_inferior_program_state,restore_inferior_program_state, do_restore_inferior_program_state_cleanup, make_cleanup_restore_inferior_program_state, discard_inferior_program_state, get_inferior_program_state_regcache): New functions. (inferior_status): Remove members stop_signal, stop_pc, registers, restore_stack_info. (save_inferior_status): Remove arg restore_stack_info. All callers updated. Remove saving of state now saved by save_inferior_program_state. (restore_inferior_status): Remove restoration of state now done by restore_inferior_program_state. (discard_inferior_status): Remove freeing of registers, now done by discard_inferior_program_state. * gdb.base/call-signal-resume.exp: New file. * gdb.base/call-signals.c: New file. * gdb.base/unwindonsignal.exp: New file.
Attachment:
gdb-081119-sym-info-2.patch.txt
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |