From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15248 invoked by alias); 24 Nov 2010 00:51:08 -0000 Received: (qmail 15233 invoked by uid 22791); 24 Nov 2010 00:51:06 -0000 X-SWARE-Spam-Status: No, hits=-6.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 24 Nov 2010 00:50:49 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id oAO0ofbq002123 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 23 Nov 2010 19:50:42 -0500 Received: from host0.dyn.jankratochvil.net (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id oAO0obDK028167 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 23 Nov 2010 19:50:40 -0500 Received: from host0.dyn.jankratochvil.net (localhost.localdomain [127.0.0.1]) by host0.dyn.jankratochvil.net (8.14.4/8.14.4) with ESMTP id oAO0oara007664; Wed, 24 Nov 2010 01:50:36 +0100 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.4/8.14.4/Submit) id oAO0oYJ9007659; Wed, 24 Nov 2010 01:50:34 +0100 Date: Wed, 24 Nov 2010 00:51:00 -0000 From: Jan Kratochvil To: gdb-patches@sourceware.org Cc: Pedro Alves Subject: [patch 3/3] Fix stale tp->step_resume_breakpoint Message-ID: <20101124005034.GC7263@host0.dyn.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) 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: 2010-11/txt/msg00386.txt.bz2 Hi, this is a follow-up to: Re: [patch] Fix stale tp->step_resume_breakpoint http://sourceware.org/ml/gdb-patches/2010-11/msg00011.html On Tue, 02 Nov 2010 02:05:18 +0100, Pedro Alves wrote: > I think this raises an obvious question, and hints at > a larger issue: if you find you you need to tuck away step_resume_breakpoint, > then, how come you don't need to do the same for all the other execution > command state? (step_range_start, step_range_end, step_frame_id, > continuations, etc.). Some of them were already saved, just not into `struct inferior_thread_state' but into `struct inferior_status'. So extended now further the latter. I find these two redundant, they could be merged? I did not try to. This could be a different patch. > I'd assume that in the use case you trip on step_resume_breakpoint > troubles, you'd also be losing thread stepping state (or state > for any other execution command), thus your thread would end up > running free, forgetting about the previous command that was > going on before the infcall. Is that not the case? Tried to fix the fields I thought should be saved. Some fields I am not sure. For example `stop_signal' IMO should be saved+reset+restored but gdb.base/call-signal-resume.exp tests it is persisent across infcalls so I kept it that way. Thanks, Jan gdb/ 2010-11-24 Jan Kratochvil * defs.h (discard_all_specified_continuations): New prototype. * infrun.c (struct inferior_status) * gdb.base/step-resume-infcall.exp: New file. * gdb.base/step-resume-infcall.c: New file. --- a/gdb/defs.h +++ b/gdb/defs.h @@ -734,6 +734,8 @@ extern void add_continuation (struct thread_info *, void (*)(void *)); extern void do_all_continuations (void); extern void do_all_continuations_thread (struct thread_info *); +extern void + discard_all_specified_continuations (struct continuation **continuationsp); extern void discard_all_continuations (void); extern void discard_all_continuations_thread (struct thread_info *); --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -6167,19 +6167,26 @@ get_inferior_thread_state_regcache (struct inferior_thread_state *inf_state) struct inferior_status { /* Direct copies of the struct thread_info fields: */ + struct breakpoint *step_resume_breakpoint; CORE_ADDR step_range_start; CORE_ADDR step_range_end; struct frame_id step_frame_id; struct frame_id step_stack_frame_id; int trap_expected; int proceed_to_finish; + struct continuation *tp_continuations; + struct continuation *intermediate_continuations; int in_infcall; enum step_over_calls_kind step_over_calls; int stop_step; + int step_multi; + /* STOP_SIGNAL should not be saved/restored as tested by: + gdb.base/call-signal-resume.exp */ bpstat stop_bpstat; /* Direct copies of the struct inferior fields: */ int stop_soon; + struct continuation *inf_continuations; /* Other fields: */ enum stop_stack_kind stop_stack_dummy; @@ -6201,15 +6208,23 @@ save_inferior_status (void) struct inferior *inf = current_inferior (); /* Direct copies of the struct thread_info fields: */ + inf_status->step_resume_breakpoint = tp->step_resume_breakpoint; + tp->step_resume_breakpoint = NULL; inf_status->step_range_start = tp->step_range_start; inf_status->step_range_end = tp->step_range_end; inf_status->step_frame_id = tp->step_frame_id; inf_status->step_stack_frame_id = tp->step_stack_frame_id; inf_status->trap_expected = tp->trap_expected; inf_status->proceed_to_finish = tp->proceed_to_finish; + inf_status->tp_continuations = tp->continuations; + tp->continuations = NULL; + inf_status->intermediate_continuations = tp->intermediate_continuations; + tp->intermediate_continuations = NULL; inf_status->in_infcall = tp->in_infcall; inf_status->step_over_calls = tp->step_over_calls; inf_status->stop_step = tp->stop_step; + inf_status->step_multi = tp->step_multi; + tp->step_multi = 0; /* Save original bpstat chain here; replace it with copy of chain. If caller's caller is walking the chain, they'll be happier if we @@ -6220,6 +6235,8 @@ save_inferior_status (void) /* Direct copies of the struct inferior fields: */ inf_status->stop_soon = inf->stop_soon; + inf_status->inf_continuations = inf->continuations; + inf->continuations = NULL; /* Other fields: */ inf_status->stop_stack_dummy = stop_stack_dummy; @@ -6261,15 +6278,23 @@ restore_inferior_status (struct inferior_status *inf_status) struct inferior *inf = current_inferior (); /* Direct copies of the struct thread_info fields: */ + if (tp->step_resume_breakpoint) + tp->step_resume_breakpoint->disposition = disp_del_at_next_stop; + tp->step_resume_breakpoint = inf_status->step_resume_breakpoint; tp->step_range_start = inf_status->step_range_start; tp->step_range_end = inf_status->step_range_end; tp->step_frame_id = inf_status->step_frame_id; tp->step_stack_frame_id = inf_status->step_stack_frame_id; tp->trap_expected = inf_status->trap_expected; tp->proceed_to_finish = inf_status->proceed_to_finish; + discard_all_continuations_thread (tp); + tp->continuations = inf_status->tp_continuations; + discard_all_intermediate_continuations_thread (tp); + tp->intermediate_continuations = inf_status->intermediate_continuations; tp->in_infcall = inf_status->in_infcall; tp->step_over_calls = inf_status->step_over_calls; tp->stop_step = inf_status->stop_step; + tp->step_multi = inf_status->step_multi; /* Handle the bpstat_copy of the chain. */ bpstat_clear (&tp->stop_bpstat); @@ -6278,6 +6303,8 @@ restore_inferior_status (struct inferior_status *inf_status) /* Direct copies of the struct inferior fields: */ inf->stop_soon = inf_status->stop_soon; + discard_all_inferior_continuations (inf); + inf->continuations = inf_status->inf_continuations; /* Other fields: */ stop_stack_dummy = inf_status->stop_stack_dummy; @@ -6316,8 +6343,19 @@ make_cleanup_restore_inferior_status (struct inferior_status *inf_status) void discard_inferior_status (struct inferior_status *inf_status) { + /* Direct copies of the struct thread_info fields: */ + if (inf_status->step_resume_breakpoint) + inf_status->step_resume_breakpoint->disposition = disp_del_at_next_stop; + + discard_all_specified_continuations (&inf_status->tp_continuations); + discard_all_specified_continuations (&inf_status->intermediate_continuations); + + /* Direct copies of the struct inferior fields: */ + discard_all_specified_continuations (&inf_status->inf_continuations); + /* See save_inferior_status for info on stop_bpstat. */ bpstat_clear (&inf_status->stop_bpstat); + xfree (inf_status); } --- /dev/null +++ b/gdb/testsuite/gdb.base/step-resume-infcall.c @@ -0,0 +1,45 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2005, 2007, 2008, 2009, 2010 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 . */ + +#include + +int +cond (void) +{ + puts ("cond-hit"); + + return 1; +} + +int +func (void) +{ + return 0; /* in-func */ +} + +int +main (void) +{ + /* Variable is used so that there is always at least one instruction on the + same line after FUNC returns. */ + int i = func (); /* call-func */ + + /* Dummy calls. */ + cond (); + func (); + return 0; +} --- /dev/null +++ b/gdb/testsuite/gdb.base/step-resume-infcall.exp @@ -0,0 +1,49 @@ +# Copyright (C) 2010 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 . + +set testfile "step-resume-infcall" + +if { [prepare_for_testing ${testfile}.exp ${testfile}] } { + return -1 +} + +if ![runto_main] { + return -1 +} + +gdb_test "step" " in-func .*" +gdb_test "up" " call-func .*" +gdb_test_no_output {set $b=$pc} + +if ![runto_main] { + return -1 +} + +gdb_breakpoint {*$b if cond ()} + +set test {print $bpnum} +gdb_test_multiple $test $test { + -re " = (\[0-9\]+)\r\n$gdb_prompt $" { + set caller_bp $expect_out(1,string) + pass $test + } +} + +# Debug only: +gdb_test "disass/m" ".*" +gdb_test "info breakpoints" ".*" + +# `cond-hit' is now printed twice; but it may not be in the future. +gdb_test "next" "cond-hit.*\r\nBreakpoint $caller_bp, .* call-func .*" --- a/gdb/utils.c +++ b/gdb/utils.c @@ -840,15 +840,22 @@ do_all_continuations (void) iterate_over_threads (do_all_continuations_thread_callback, NULL); } +/* Get rid of all the continuations tracked by *CONTINUATIONSP. */ +void +discard_all_specified_continuations (struct continuation **continuationsp) +{ + struct cleanup *continuation_ptr = &(*continuationsp)->base; + + discard_my_cleanups (&continuation_ptr, NULL); + *continuationsp = NULL; +} + /* Callback for iterate over threads. */ static int discard_all_continuations_thread_callback (struct thread_info *thread, void *data) { - struct cleanup *continuation_ptr = &thread->continuations->base; - - discard_my_cleanups (&continuation_ptr, NULL); - thread->continuations = NULL; + discard_all_specified_continuations (&thread->continuations); return 0; } @@ -922,10 +929,7 @@ static int discard_all_intermediate_continuations_thread_callback (struct thread_info *thread, void *data) { - struct cleanup *continuation_ptr = &thread->intermediate_continuations->base; - - discard_my_cleanups (&continuation_ptr, NULL); - thread->intermediate_continuations = NULL; + discard_all_specified_continuations (&thread->intermediate_continuations); return 0; }