From patchwork Thu May 21 23:19:11 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 6880 Received: (qmail 19465 invoked by alias); 21 May 2015 23:59:06 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 19455 invoked by uid 89); 21 May 2015 23:59:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.9 required=5.0 tests=AWL, BAYES_50, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_PASS, T_RP_MATCHES_RCVD, UNSUBSCRIBE_BODY autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 21 May 2015 23:59:03 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t4LNJVgu000813 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 21 May 2015 19:19:31 -0400 Received: from brno.lan (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t4LNJEJD027592 for ; Thu, 21 May 2015 19:19:30 -0400 From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH 15/18] Disable displaced stepping if trying it fails Date: Fri, 22 May 2015 00:19:11 +0100 Message-Id: <1432250354-2721-16-git-send-email-palves@redhat.com> In-Reply-To: <1432250354-2721-1-git-send-email-palves@redhat.com> References: <1432250354-2721-1-git-send-email-palves@redhat.com> Running the testsuite with "maint set target-non-stop on" shows: (gdb) PASS: gdb.base/valgrind-infcall.exp: continue #98 (false warning) continue Continuing. dl_main (phdr=..., auxv=) at rtld.c:2302 2302 LIBC_PROBE (init_complete, 2, LM_ID_BASE, r); Cannot access memory at address 0x400532 (gdb) PASS: gdb.base/valgrind-infcall.exp: continue #99 (false warning) p gdb_test_infcall () $1 = 1 (gdb) FAIL: gdb.base/valgrind-infcall.exp: p gdb_test_infcall () Even though that was a native GNU/Linux test run, this test spawns Valgrind and connects to it with "target remote". The error above is actually orthogonal to target-non-stop. The real issue is that that enables displaced stepping, and displaced stepping doesn't work with Valgrind, because we can't write to the inferior memory (thus can't copy the instruction to the scratch pad area). I'm sure there will be other targets with the same issue, so trying to identify Valgrind wouldn't be sufficient. The fix is to try setting up the displaced step anyway. If we get a MEMORY_ERROR, we disable displaced stepping for that inferior, and fall back to doing an in-line step-over. If "set displaced-stepping" is "on" (as opposed to "auto), GDB warns displaced stepping failed ("on" is mainly useful for the testsuite, not for users). Tested on x86_64 Fedora 20. gdb/ChangeLog: 2015-05-21 Pedro Alves * inferior.h (struct inferior) : New field. * infrun.c (use_displaced_stepping_now_p): New parameter 'inf'. Return false if dispaced stepping failed before. (resume): Pass the current inferior to use_displaced_stepping_now_p. Wrap displaced_step_prepare in TRY/CATCH. If we get a MEMORY_ERROR, set the inferior's displaced_stepping_failed flag, and fall back to an in-line step-over. gdb/testsuite/ChangeLog: 2015-05-21 Pedro Alves * gdb.base/valgrind-disp-step.c: New file. * gdb.base/valgrind-disp-step.exp: New file. v4: - no changes. v3: - displaced_stepping_failed flag moved from "struct inferior" to "struct displaced_step_inferior_state", and renamed "failed_before". - use_displaced_stepping_now_p in v2 eliminated. Simply change the prototype of use_displaced_stepping to take a thread pointer instead. - TRY/CATCH around displaced_step_prepare moved to separate function. v2: - warn if displaced stepping fails, and "set displaced-stepping" is "on" (but not "auto"). --- gdb/infrun.c | 112 ++++++++++++++++++---- gdb/testsuite/gdb.base/valgrind-disp-step.c | 32 +++++++ gdb/testsuite/gdb.base/valgrind-disp-step.exp | 131 ++++++++++++++++++++++++++ 3 files changed, 255 insertions(+), 20 deletions(-) create mode 100644 gdb/testsuite/gdb.base/valgrind-disp-step.c create mode 100644 gdb/testsuite/gdb.base/valgrind-disp-step.exp diff --git a/gdb/infrun.c b/gdb/infrun.c index d9fc4e5..c321e03 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -1462,6 +1462,10 @@ struct displaced_step_inferior_state /* The process this displaced step state refers to. */ int pid; + /* True if preparing a displaced step ever failed. If so, we won't + try displaced stepping for this inferior again. */ + int failed_before; + /* If this is not null_ptid, this is the thread carrying out a displaced single-step in process PID. This thread's state will require fixing up once it has completed its step. */ @@ -1632,16 +1636,24 @@ show_can_use_displaced_stepping (struct ui_file *file, int from_tty, } /* Return non-zero if displaced stepping can/should be used to step - over breakpoints. */ + over breakpoints of thread TP. */ static int -use_displaced_stepping (struct gdbarch *gdbarch) +use_displaced_stepping (struct thread_info *tp) { + struct regcache *regcache = get_thread_regcache (tp->ptid); + struct gdbarch *gdbarch = get_regcache_arch (regcache); + struct displaced_step_inferior_state *displaced_state; + + displaced_state = get_displaced_stepping_state (ptid_get_pid (tp->ptid)); + return (((can_use_displaced_stepping == AUTO_BOOLEAN_AUTO && target_is_non_stop_p ()) || can_use_displaced_stepping == AUTO_BOOLEAN_TRUE) && gdbarch_displaced_step_copy_insn_p (gdbarch) - && find_record_target () == NULL); + && find_record_target () == NULL + && (displaced_state == NULL + || !displaced_state->failed_before)); } /* Clean out any stray displaced stepping state. */ @@ -1695,7 +1707,7 @@ displaced_step_dump_bytes (struct ui_file *file, Returns 1 if preparing was successful -- this thread is going to be stepped now; or 0 if displaced stepping this thread got queued. */ static int -displaced_step_prepare (ptid_t ptid) +displaced_step_prepare_throw (ptid_t ptid) { struct cleanup *old_cleanups, *ignore_cleanups; struct thread_info *tp = find_thread_ptid (ptid); @@ -1805,6 +1817,50 @@ displaced_step_prepare (ptid_t ptid) return 1; } +/* Wrapper for displaced_step_prepare_throw that disabled further + attempts at displaced stepping if we get a memory error. */ + +static int +displaced_step_prepare (ptid_t ptid) +{ + int prepared = -1; + + TRY + { + prepared = displaced_step_prepare_throw (ptid); + } + CATCH (ex, RETURN_MASK_ERROR) + { + struct displaced_step_inferior_state *displaced_state; + + if (ex.error != MEMORY_ERROR) + throw_exception (ex); + + if (debug_infrun) + { + fprintf_unfiltered (gdb_stdlog, + "infrun: disabling displaced stepping: %s\n", + ex.message); + } + + /* Be verbose if "set displaced-stepping" is "on", silent if + "auto". */ + if (can_use_displaced_stepping == AUTO_BOOLEAN_TRUE) + { + warning (_("disabling displaced stepping: %s\n"), + ex.message); + } + + /* Disable further displaced stepping attempts. */ + displaced_state + = get_displaced_stepping_state (ptid_get_pid (ptid)); + displaced_state->failed_before = 1; + } + END_CATCH + + return prepared; +} + static void write_memory_ptid (ptid_t ptid, CORE_ADDR memaddr, const gdb_byte *myaddr, int len) @@ -1935,6 +1991,7 @@ static void keep_going_pass_signal (struct execution_control_state *ecs); static void prepare_to_wait (struct execution_control_state *ecs); static int keep_going_stepped_thread (struct thread_info *tp); static int thread_still_needs_step_over (struct thread_info *tp); +static void stop_all_threads (void); /* Are there any pending step-over requests? If so, run all we can now and return true. Otherwise, return false. */ @@ -1955,8 +2012,6 @@ start_step_over (void) struct execution_control_state *ecs = &ecss; enum step_over_what step_what; int must_be_in_line; - struct regcache *regcache = get_thread_regcache (tp->ptid); - struct gdbarch *gdbarch = get_regcache_arch (regcache); next = thread_step_over_chain_next (tp); @@ -1968,7 +2023,7 @@ start_step_over (void) step_what = thread_still_needs_step_over (tp); must_be_in_line = ((step_what & STEP_OVER_WATCHPOINT) || ((step_what & STEP_OVER_BREAKPOINT) - && !use_displaced_stepping (gdbarch))); + && !use_displaced_stepping (tp))); /* We currently stop all threads of all processes to step-over in-line. If we need to start a new in-line step-over, let @@ -2417,15 +2472,15 @@ resume (enum gdb_signal sig) We can't use displaced stepping when we are waiting for vfork_done event, displaced stepping breaks the vfork child similarly as single step software breakpoint. */ - if (use_displaced_stepping (gdbarch) - && tp->control.trap_expected + if (tp->control.trap_expected + && use_displaced_stepping (tp) && !step_over_info_valid_p () && sig == GDB_SIGNAL_0 && !current_inferior ()->waiting_for_vfork_done) { - struct displaced_step_inferior_state *displaced; + int prepared = displaced_step_prepare (inferior_ptid); - if (!displaced_step_prepare (inferior_ptid)) + if (prepared == 0) { if (debug_infrun) fprintf_unfiltered (gdb_stdlog, @@ -2435,14 +2490,32 @@ resume (enum gdb_signal sig) discard_cleanups (old_cleanups); return; } + else if (prepared < 0) + { + /* Fallback to stepping over the breakpoint in-line. */ + + if (target_is_non_stop_p ()) + stop_all_threads (); + + set_step_over_info (get_regcache_aspace (regcache), + regcache_read_pc (regcache), 0); + + step = maybe_software_singlestep (gdbarch, pc); + + insert_breakpoints (); + } + else if (prepared > 0) + { + struct displaced_step_inferior_state *displaced; - /* Update pc to reflect the new address from which we will execute - instructions due to displaced stepping. */ - pc = regcache_read_pc (get_thread_regcache (inferior_ptid)); + /* Update pc to reflect the new address from which we will + execute instructions due to displaced stepping. */ + pc = regcache_read_pc (get_thread_regcache (inferior_ptid)); - displaced = get_displaced_stepping_state (ptid_get_pid (inferior_ptid)); - step = gdbarch_displaced_step_hw_singlestep (gdbarch, - displaced->step_closure); + displaced = get_displaced_stepping_state (ptid_get_pid (inferior_ptid)); + step = gdbarch_displaced_step_hw_singlestep (gdbarch, + displaced->step_closure); + } } /* Do we need to do it the hard way, w/temp breakpoints? */ @@ -2570,8 +2643,8 @@ resume (enum gdb_signal sig) } if (debug_displaced - && use_displaced_stepping (gdbarch) && tp->control.trap_expected + && use_displaced_stepping (tp) && !step_over_info_valid_p ()) { struct regcache *resume_regcache = get_thread_regcache (tp->ptid); @@ -7361,8 +7434,7 @@ keep_going_pass_signal (struct execution_control_state *ecs) watchpoint. The instruction copied to the scratch pad would still trigger the watchpoint. */ if (remove_bp - && (remove_wps - || !use_displaced_stepping (get_regcache_arch (regcache)))) + && (remove_wps || !use_displaced_stepping (ecs->event_thread))) { set_step_over_info (get_regcache_aspace (regcache), regcache_read_pc (regcache), remove_wps); diff --git a/gdb/testsuite/gdb.base/valgrind-disp-step.c b/gdb/testsuite/gdb.base/valgrind-disp-step.c new file mode 100644 index 0000000..baba74e --- /dev/null +++ b/gdb/testsuite/gdb.base/valgrind-disp-step.c @@ -0,0 +1,32 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2015 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 . */ + +static int +foo (void) +{ +} + +int +main (void) +{ + foo (); /* stop 0 */ + foo (); /* stop 1 */ + foo (); /* stop 2 */ + foo (); /* stop 3 */ + foo (); /* stop 4 */ + return 0; +} diff --git a/gdb/testsuite/gdb.base/valgrind-disp-step.exp b/gdb/testsuite/gdb.base/valgrind-disp-step.exp new file mode 100644 index 0000000..7e6803b --- /dev/null +++ b/gdb/testsuite/gdb.base/valgrind-disp-step.exp @@ -0,0 +1,131 @@ +# Copyright 2012-2015 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 . + +# Step over breakpoints with displaced stepping on, against Valgrind. +# We can't really use displaced stepping with Valgrind, so what this +# really tests is that GDB falls back to in-line stepping +# automatically instead of getting stuck or crashing. + +if [is_remote target] { + # The test always runs locally. + return 0 +} + +standard_testfile .c +if {[build_executable "failed to build" $testfile $srcfile {debug}] == -1} { + return -1 +} + +set test "spawn valgrind" +set cmd "valgrind --vgdb-error=0 $binfile" +set res [remote_spawn host $cmd] +if { $res < 0 || $res == "" } { + verbose -log "Spawning $cmd failed." + unsupported $test + return -1 +} +pass $test +# Declare GDB now as running. +set gdb_spawn_id $res + +# GDB started by vgdb stops already after the startup is executed, like with +# non-extended gdbserver. It is also not correct to run/attach the inferior. +set use_gdb_stub 1 + +set test "valgrind started" +# The trailing '.' differs for different memcheck versions. +gdb_test_multiple "" $test { + -re "Memcheck, a memory error detector\\.?\r\n" { + pass $test + } + -re "valgrind: failed to start tool 'memcheck' for platform '.*': No such file or directory" { + unsupported $test + return -1 + } + -re "valgrind: wrong ELF executable class" { + unsupported $test + return -1 + } + -re "command not found" { + # The spawn succeeded, but then valgrind was not found - e.g. if + # we spawned SSH to a remote system. + unsupported $test + return -1 + } + -re "valgrind: Bad option.*--vgdb-error=0" { + # valgrind is not >= 3.7.0. + unsupported $test + return -1 + } +} + +set test "vgdb prompt" +# The trailing '.' differs for different memcheck versions. +gdb_test_multiple "" $test { + -re " (target remote | \[^\r\n\]*/vgdb \[^\r\n\]*)\r\n" { + set vgdbcmd $expect_out(1,string) + pass $test + } +} + +# Do not kill valgrind. +set valgrind_pid [exp_pid -i [board_info host fileid]] +unset gdb_spawn_id +set board [host_info name] +unset_board_info fileid + +clean_restart $testfile + +gdb_test "$vgdbcmd" " in \\.?_start .*" "target remote for vgdb" + +gdb_test "monitor v.set gdb_output" "valgrind output will go to gdb.*" + +gdb_test_no_output "set displaced-stepping off" +gdb_breakpoint "main" "breakpoint at main" +gdb_test "continue" " stop 0 .*" "continue to main" +delete_breakpoints + +set curr_stop 0 +foreach displaced { "off" "on" } { + with_test_prefix "displaced $displaced" { + + gdb_test_no_output "set displaced-stepping $displaced" + + foreach go { "once" "twice" } { + with_test_prefix $go { + gdb_test "break" "Breakpoint .* at .*" "set breakpoint" + + # Whether we should see a warning. + set should_warn [expr {$go == "once" && $displaced == "on"}] + + incr curr_stop + + set msg "step over breakpoint" + set pattern " stop $curr_stop .*$gdb_prompt $" + gdb_test_multiple "next" $msg { + -re "warning: disabling displaced stepping.*$pattern" { + gdb_assert $should_warn $msg + } + -re "$pattern" { + gdb_assert !$should_warn $msg + } + } + } + } + } +} + +# Only if valgrind got stuck. +remote_exec host "kill -9 ${valgrind_pid}"