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 Thu, Nov 20, 2008 at 12:03 AM, Doug Evans <dje@google.com> wrote: > 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. > Ummm, this time with the correct patch attached. Sorry!
Attachment:
gdb-081119-infcall-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] |