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]

Re: [RFA] dummy frame handling cleanup, plus inferior fun call signal handling improvement


On Mon, Dec 1, 2008 at 5:20 PM, Doug Evans <dje@google.com> wrote:
> On Mon, Dec 1, 2008 at 1:21 PM, Pedro Alves <pedro@codesourcery.com> wrote:
>> Hi Doug,
>>
>> I'd like to bring a current GDB deficiency to your attention, in
>> case it affects anything related to this patch.
>>
>> If GDB stops due to a signal instead of hitting the dummy frame
>> breakpoint, and you have set GDB to restore the state
>> automatically with "set unwindonsignal on", and the thread
>> that reported the signal (say a SIGSEGV) was *not* the same that was
>> doing the infcall, GDB will currently restore the old context to the
>> wrong thread (seen by inspection, having really tried it).
>>
>> Not having studied the patch yet, I'm just wondering if your changes
>> would make it easier or harder to fix this, or if you could be
>> extending the problem by possibly restoring things in the wrong
>> thread as well.
>
> Heh.  My patch serendipitously has the following to catch another case
> I was seeing where the current thread unexpectedly changed.
>
>  if (! ptid_equal (this_thread->ptid, inferior_thread ()->ptid))
>    {
>      /* We've switched threads.  Perhaps this shouldn't happen, but we
>         protect ourselves anyway.
>         There's no point in restoring the inferior status,
>         we're in a different thread.  */
>      discard_cleanups (inf_status_cleanup);
>      discard_inferior_status (inf_status);
>      dummy_frame_discard (dummy_frame);
>      error (_("\
> The current thread has switched while making a function call from GDB.\n\
> The state of the function call has been lost.\n\
> It may be recoverable by changing back to the original thread\n\
> and examining the state."));
>    }
>
> I need fix my patch to save the calling thread's ptid
> (this_thread->ptid) before resuming execution in case the thread dies.
> I think I shouldn't call dummy_frame_discard here too.
> And I also need to change the comment to document why the current
> thread can switch, because it _can_ happen. :-)
> I'll prepare a new version of my patch, and include a testcase to
> handle the situation you mention.  Thanks!


Here's an updated version of my patch.
It includes a testcase for the current thread changing in the middle
of a hand-called function.

Ok to check in?

2008-12-02  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 (regcache): Delete forward decl.
        (inferior_program_state,dummy_frame,cleanup): Add forward decls.
        (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_ptid.
        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.
        * gdb.base/unwindonsignal.c: New file.
        * gdb.threads/interrupted-hand-call.exp: New file.
        * gdb.threads/interrupted-hand-call.cv: New file.

Attachment: gdb-081202-infcall-3.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]