From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28304 invoked by alias); 26 Nov 2010 04:48:39 -0000 Received: (qmail 28268 invoked by uid 22791); 26 Nov 2010 04:48:31 -0000 X-SWARE-Spam-Status: No, hits=-6.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_GJ,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; Fri, 26 Nov 2010 04:48:24 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id oAQ4mFgP026244 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 25 Nov 2010 23:48:15 -0500 Received: from host0.dyn.jankratochvil.net (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id oAQ4mAVj030134 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 25 Nov 2010 23:48:14 -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 oAQ4m305032163; Fri, 26 Nov 2010 05:48:03 +0100 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.4/8.14.4/Submit) id oAQ4m2vA032154; Fri, 26 Nov 2010 05:48:02 +0100 Date: Fri, 26 Nov 2010 04:48:00 -0000 From: Jan Kratochvil To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: [patch 2/2] Fix stale tp->step_resume_breakpoint Message-ID: <20101126044756.GB25724@host0.dyn.jankratochvil.net> References: <20101124005034.GC7263@host0.dyn.jankratochvil.net> <201011241242.54099.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201011241242.54099.pedro@codesourcery.com> 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/msg00445.txt.bz2 On Wed, 24 Nov 2010 13:42:53 +0100, Pedro Alves wrote: > > +++ b/gdb/testsuite/gdb.base/step-resume-infcall.c > > @@ -0,0 +1,45 @@ > (...) > > +#include > > + > > +int > > +cond (void) > > +{ > > + puts ("cond-hit"); > > Can you make the test not rely on stdio, please? Done, didn't realize that before. (gdbserver case FAILs->PASSes as linux-nat) No regressions on {x86_64,x86_64-m32,i686}-fedora14-linux-gnu. Dropped the other fields save/restore, I was not able to reproduce problems due to them and it is now very clear/easy to extend the save/restore set. (Waiting on the fields moving cleanup approval.) Thanks, Jan gdb/ 2010-11-26 Jan Kratochvil Fix step_resume_breakpoint unsaved during an infcall. * gdbthread.h (struct thread_control_state): Move here field step_resume_breakpoint ... (struct thread_info): ... from here. * infrun.c (save_infcall_control_state): Reset control.step_resume_breakpoint to NULL. (restore_infcall_control_state, discard_infcall_control_state): Delete control.step_resume_breakpoint. * arm-linux-tdep.c, infrun.c, thread.c: Update all the references to the moved field. gdb/testsuite/ 2010-11-26 Jan Kratochvil * gdb.base/step-resume-infcall.exp: New file. * gdb.base/step-resume-infcall.c: New file. --- a/gdb/arm-linux-tdep.c +++ b/gdb/arm-linux-tdep.c @@ -791,7 +791,8 @@ arm_linux_copy_svc (struct gdbarch *gdbarch, uint32_t insn, CORE_ADDR to, fprintf_unfiltered (gdb_stdlog, "displaced: unwind pc = %lx. " "Setting momentary breakpoint.\n", (unsigned long) return_to); - gdb_assert (inferior_thread ()->step_resume_breakpoint == NULL); + gdb_assert (inferior_thread ()->control.step_resume_breakpoint + == NULL); sal = find_pc_line (return_to, 0); sal.pc = return_to; @@ -802,7 +803,7 @@ arm_linux_copy_svc (struct gdbarch *gdbarch, uint32_t insn, CORE_ADDR to, if (frame) { - inferior_thread ()->step_resume_breakpoint + inferior_thread ()->control.step_resume_breakpoint = set_momentary_breakpoint (gdbarch, sal, get_frame_id (frame), bp_step_resume); --- a/gdb/gdbthread.h +++ b/gdb/gdbthread.h @@ -37,6 +37,9 @@ struct thread_control_state { /* User/external stepping state. */ + /* Step-resume or longjmp-resume breakpoint. */ + struct breakpoint *step_resume_breakpoint; + /* Range to single step within. If this is nonzero, respond to a single-step signal by continuing @@ -151,11 +154,6 @@ struct thread_info call. See `struct thread_suspend_state'. */ struct thread_suspend_state suspend; - /* User/external stepping state. */ - - /* Step-resume or longjmp-resume breakpoint. */ - struct breakpoint *step_resume_breakpoint; - int current_line; struct symtab *current_symtab; --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -424,8 +424,8 @@ follow_fork (void) preserve the stepping state in the fork child. */ if (follow_child && should_resume) { - step_resume_breakpoint - = clone_momentary_breakpoint (tp->step_resume_breakpoint); + step_resume_breakpoint = clone_momentary_breakpoint + (tp->control.step_resume_breakpoint); step_range_start = tp->control.step_range_start; step_range_end = tp->control.step_range_end; step_frame_id = tp->control.step_frame_id; @@ -476,7 +476,8 @@ follow_fork (void) if (should_resume) { tp = inferior_thread (); - tp->step_resume_breakpoint = step_resume_breakpoint; + tp->control.step_resume_breakpoint + = step_resume_breakpoint; tp->control.step_range_start = step_range_start; tp->control.step_range_end = step_range_end; tp->control.step_frame_id = step_frame_id; @@ -530,8 +531,8 @@ follow_inferior_reset_breakpoints (void) "threads". We must update the bp's notion of which thread it is for, or it'll be ignored when it triggers. */ - if (tp->step_resume_breakpoint) - breakpoint_re_set_thread (tp->step_resume_breakpoint); + if (tp->control.step_resume_breakpoint) + breakpoint_re_set_thread (tp->control.step_resume_breakpoint); /* Reinsert all breakpoints in the child. The user may have set breakpoints after catching the fork, in which case those @@ -769,7 +770,7 @@ follow_exec (ptid_t pid, char *execd_pathname) /* If there was one, it's gone now. We cannot truly step-to-next statement through an exec(). */ - th->step_resume_breakpoint = NULL; + th->control.step_resume_breakpoint = NULL; th->control.step_range_start = 0; th->control.step_range_end = 0; @@ -3951,7 +3952,8 @@ infrun: no user watchpoint explains watchpoint SIGTRAP, ignoring\n"); || stopped_by_watchpoint || ecs->event_thread->control.trap_expected || (ecs->event_thread->control.step_range_end - && ecs->event_thread->step_resume_breakpoint == NULL)); + && (ecs->event_thread->control.step_resume_breakpoint + == NULL))); else { ecs->random_signal = !bpstat_explains_signal @@ -4019,7 +4021,7 @@ process_event_stop_test: if (ecs->event_thread->prev_pc == stop_pc && ecs->event_thread->control.trap_expected - && ecs->event_thread->step_resume_breakpoint == NULL) + && ecs->event_thread->control.step_resume_breakpoint == NULL) { /* We were just starting a new sequence, attempting to single-step off of a breakpoint and expecting a SIGTRAP. @@ -4048,7 +4050,7 @@ process_event_stop_test: && stop_pc < ecs->event_thread->control.step_range_end) && frame_id_eq (get_stack_frame_id (frame), ecs->event_thread->control.step_stack_frame_id) - && ecs->event_thread->step_resume_breakpoint == NULL) + && ecs->event_thread->control.step_resume_breakpoint == NULL) { /* The inferior is about to take a signal that will take it out of the single step range. Set a breakpoint at the @@ -4135,7 +4137,8 @@ infrun: BPSTAT_WHAT_SET_LONGJMP_RESUME (!gdbarch_get_longjmp_target)\n"); fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_CLEAR_LONGJMP_RESUME\n"); - gdb_assert (ecs->event_thread->step_resume_breakpoint != NULL); + gdb_assert (ecs->event_thread->control.step_resume_breakpoint + != NULL); delete_step_resume_breakpoint (ecs->event_thread); ecs->event_thread->control.stop_step = 1; @@ -4311,7 +4314,7 @@ infrun: not switching back to stepped thread, it has vanished\n"); return; } - if (ecs->event_thread->step_resume_breakpoint) + if (ecs->event_thread->control.step_resume_breakpoint) { if (debug_infrun) fprintf_unfiltered (gdb_stdlog, @@ -4862,10 +4865,11 @@ infrun: not switching back to stepped thread, it has vanished\n"); static int currently_stepping (struct thread_info *tp) { - return ((tp->control.step_range_end && tp->step_resume_breakpoint == NULL) - || tp->control.trap_expected - || tp->stepping_through_solib_after_catch - || bpstat_should_step ()); + return ((tp->control.step_range_end + && tp->control.step_resume_breakpoint == NULL) + || tp->control.trap_expected + || tp->stepping_through_solib_after_catch + || bpstat_should_step ()); } /* Returns true if any thread *but* the one passed in "data" is in the @@ -5010,14 +5014,14 @@ insert_step_resume_breakpoint_at_sal (struct gdbarch *gdbarch, /* There should never be more than one step-resume or longjmp-resume breakpoint per thread, so we should never be setting a new step_resume_breakpoint when one is already active. */ - gdb_assert (inferior_thread ()->step_resume_breakpoint == NULL); + gdb_assert (inferior_thread ()->control.step_resume_breakpoint == NULL); if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: inserting step-resume breakpoint at %s\n", paddress (gdbarch, sr_sal.pc)); - inferior_thread ()->step_resume_breakpoint + inferior_thread ()->control.step_resume_breakpoint = set_momentary_breakpoint (gdbarch, sr_sal, sr_id, bp_step_resume); } @@ -5094,14 +5098,14 @@ insert_longjmp_resume_breakpoint (struct gdbarch *gdbarch, CORE_ADDR pc) /* There should never be more than one step-resume or longjmp-resume breakpoint per thread, so we should never be setting a new longjmp_resume_breakpoint when one is already active. */ - gdb_assert (inferior_thread ()->step_resume_breakpoint == NULL); + gdb_assert (inferior_thread ()->control.step_resume_breakpoint == NULL); if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: inserting longjmp-resume breakpoint at %s\n", paddress (gdbarch, pc)); - inferior_thread ()->step_resume_breakpoint = + inferior_thread ()->control.step_resume_breakpoint = set_momentary_breakpoint_at_pc (gdbarch, pc, bp_longjmp_resume); } @@ -6212,6 +6216,8 @@ save_infcall_control_state (void) inf_status->thread_control = tp->control; inf_status->inferior_control = inf->control; + tp->control.step_resume_breakpoint = NULL; + /* Save original bpstat chain to INF_STATUS; replace it in TP with copy of chain. If caller's caller is walking the chain, they'll be happier if we hand them back the original chain when restore_infcall_control_state is @@ -6257,6 +6263,9 @@ restore_infcall_control_state (struct infcall_control_state *inf_status) struct thread_info *tp = inferior_thread (); struct inferior *inf = current_inferior (); + if (tp->control.step_resume_breakpoint) + tp->control.step_resume_breakpoint->disposition = disp_del_at_next_stop; + /* Handle the bpstat_copy of the chain. */ bpstat_clear (&tp->control.stop_bpstat); @@ -6301,8 +6310,13 @@ make_cleanup_restore_infcall_control_state void discard_infcall_control_state (struct infcall_control_state *inf_status) { + if (inf_status->thread_control.step_resume_breakpoint) + inf_status->thread_control.step_resume_breakpoint->disposition + = disp_del_at_next_stop; + /* See save_infcall_control_state for info on stop_bpstat. */ bpstat_clear (&inf_status->thread_control.stop_bpstat); + xfree (inf_status); } --- /dev/null +++ b/gdb/testsuite/gdb.base/step-resume-infcall.c @@ -0,0 +1,47 @@ +/* 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 + +volatile int cond_hit; + +int +cond (void) +{ + 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,52 @@ +# 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" ".*" + +gdb_test "next" "Breakpoint $caller_bp, .* call-func .*" + +# `cond-hit' is now hit twice; but it may not be in the future. It is +# currently not a bug so it is not KFAILed. +gdb_test "p cond_hit" { = [12]} --- a/gdb/thread.c +++ b/gdb/thread.c @@ -83,10 +83,10 @@ inferior_thread (void) void delete_step_resume_breakpoint (struct thread_info *tp) { - if (tp && tp->step_resume_breakpoint) + if (tp && tp->control.step_resume_breakpoint) { - delete_breakpoint (tp->step_resume_breakpoint); - tp->step_resume_breakpoint = NULL; + delete_breakpoint (tp->control.step_resume_breakpoint); + tp->control.step_resume_breakpoint = NULL; } } @@ -97,10 +97,10 @@ clear_thread_inferior_resources (struct thread_info *tp) but not any user-specified thread-specific breakpoints. We can not delete the breakpoint straight-off, because the inferior might not be stopped at the moment. */ - if (tp->step_resume_breakpoint) + if (tp->control.step_resume_breakpoint) { - tp->step_resume_breakpoint->disposition = disp_del_at_next_stop; - tp->step_resume_breakpoint = NULL; + tp->control.step_resume_breakpoint->disposition = disp_del_at_next_stop; + tp->control.step_resume_breakpoint = NULL; } bpstat_clear (&tp->control.stop_bpstat);