From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10321 invoked by alias); 1 Dec 2008 20:52:07 -0000 Received: (qmail 10311 invoked by uid 22791); 1 Dec 2008 20:52:05 -0000 X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.45.13) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 01 Dec 2008 20:51:08 +0000 Received: from wpaz1.hot.corp.google.com (wpaz1.hot.corp.google.com [172.24.198.65]) by smtp-out.google.com with ESMTP id mB1Kp6nW024220 for ; Mon, 1 Dec 2008 12:51:06 -0800 Received: from rv-out-0506.google.com (rvfb25.prod.google.com [10.140.179.25]) by wpaz1.hot.corp.google.com with ESMTP id mB1Kok31002445 for ; Mon, 1 Dec 2008 12:51:05 -0800 Received: by rv-out-0506.google.com with SMTP id b25so2862556rvf.37 for ; Mon, 01 Dec 2008 12:51:04 -0800 (PST) MIME-Version: 1.0 Received: by 10.140.202.12 with SMTP id z12mr5408657rvf.183.1228164664598; Mon, 01 Dec 2008 12:51:04 -0800 (PST) In-Reply-To: References: <20081118125838.0613C412301@localhost> Date: Mon, 01 Dec 2008 20:52:00 -0000 Message-ID: Subject: Re: [RFA] dummy frame handling cleanup, plus inferior fun call signal handling improvement From: Doug Evans To: gdb-patches@sourceware.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-12/txt/msg00010.txt.bz2 Ping. On Thu, Nov 20, 2008 at 12:07 AM, Doug Evans wrote: > On Thu, Nov 20, 2008 at 12:03 AM, Doug Evans wrote: >> On Tue, Nov 18, 2008 at 4:58 AM, Doug Evans 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 ; return" where 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 ; 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 >>> >>> * 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 >> >> * 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! >