From patchwork Mon Oct 9 10:16:16 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 23405 Received: (qmail 115769 invoked by alias); 9 Oct 2017 10:16:23 -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 115698 invoked by uid 89); 9 Oct 2017 10:16:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=pedro, Pedro, continuing, barrier 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 ESMTP; Mon, 09 Oct 2017 10:16:21 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C03B481E06 for ; Mon, 9 Oct 2017 10:16:20 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com C03B481E06 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves@redhat.com Received: from cascais.lan (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 34A1969FBC for ; Mon, 9 Oct 2017 10:16:20 +0000 (UTC) From: Pedro Alves To: gdb-patches@sourceware.org Subject: [PATCH 1/2] Multi-arch exec, more register reading avoidance Date: Mon, 9 Oct 2017 11:16:16 +0100 Message-Id: <1507544177-24880-2-git-send-email-palves@redhat.com> In-Reply-To: <1507544177-24880-1-git-send-email-palves@redhat.com> References: <1507544177-24880-1-git-send-email-palves@redhat.com> As mentioned in commit bf93d7ba9931 ("Add thread after updating gdbarch when exec'ing"), we should avoid doing register reads after a process does an exec and before we've updated that inferior's gdbarch. Otherwise, we may interpret the registers using the wrong registers using the wrong architecture. There's still (at least) one case where we still read registers post-exec with the pre-exec architecture. That's when infrun decides it needs to switch context to the exec'ing thread. I.e., if the exec event is processed at a time when the current thread is not already the exec'ing thread, then we get (with the test added by this commit): continue Continuing. Truncated register 50 in remote 'g' packet Truncated register 50 in remote 'g' packet (gdb) FAIL: gdb.multi/multi-arch-exec.exp: selected_thread=2: follow_exec_mode=same: continue across exec that changes architecture The fix is to avoid reading registers when switching context in this case. (I'd be nice to get rid of the constant stop_pc reading when switching threads, but that'd be deeper change.) gdb/ChangeLog: yyyy-mm-dd Pedro Alves * infrun.c (handle_inferior_event_1) : Skip reading registers when switching context. gdb/testsuite/ChangeLog: yyyy-mm-dd Pedro Alves * gdb.multi/multi-arch-exec.c: Include and . (barrier): New. (thread_start, all_started): New functions. (main): Spawn new thread and wait until it is scheduled. * gdb.multi/multi-arch-exec.exp: Build $srcfile1 with the pthreads option. (do_test): Add 'selected_thread' parameter. Run to all_started instead of main. Explicitly set the breakpoint at main. Switch to the SELECTED_THREAD thread. (top level): Test handling the exec event with either the main thread or the second thread selected. --- gdb/infrun.c | 5 ++++- gdb/testsuite/gdb.multi/multi-arch-exec.c | 27 ++++++++++++++++++++++++++ gdb/testsuite/gdb.multi/multi-arch-exec.exp | 30 ++++++++++++++++++++++------- 3 files changed, 54 insertions(+), 8 deletions(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index 4628118..0e30490 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -5271,8 +5271,11 @@ Cannot fill $_exitsignal with the correct signal number.\n")); if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_EXECD\n"); + /* Note we can't read registers yet (the stop_pc), because we + don't yet know the inferior's post-exec architecture. + 'stop_pc' is explicitly read below instead. */ if (!ptid_equal (ecs->ptid, inferior_ptid)) - context_switch (ecs->ptid); + switch_to_thread_no_regs (ecs->event_thread); /* Do whatever is necessary to the parent branch of the vfork. */ handle_vfork_child_exec_or_exit (1); diff --git a/gdb/testsuite/gdb.multi/multi-arch-exec.c b/gdb/testsuite/gdb.multi/multi-arch-exec.c index 1a443de..f43a5a8 100644 --- a/gdb/testsuite/gdb.multi/multi-arch-exec.c +++ b/gdb/testsuite/gdb.multi/multi-arch-exec.c @@ -20,11 +20,32 @@ #include #include #include +#include + +#define NUM_THREADS 1 + +static pthread_barrier_t barrier; + +static void * +thread_start (void *arg) +{ + pthread_barrier_wait (&barrier); + + while (1) + sleep (1); + return NULL; +} + +static void +all_started (void) +{ +} int main (int argc, char ** argv) { char prog[PATH_MAX]; + pthread_t thread; int len; strcpy (prog, argv[0]); @@ -33,6 +54,12 @@ main (int argc, char ** argv) memcpy (prog + len - 15, "multi-arch-exec-hello", 21); prog[len + 6] = 0; + pthread_barrier_init (&barrier, NULL, NUM_THREADS + 1); + pthread_create (&thread, NULL, thread_start, NULL); + + pthread_barrier_wait (&barrier); + all_started (); + execl (prog, prog, (char *) NULL); diff --git a/gdb/testsuite/gdb.multi/multi-arch-exec.exp b/gdb/testsuite/gdb.multi/multi-arch-exec.exp index 27a0d60..891eb14 100644 --- a/gdb/testsuite/gdb.multi/multi-arch-exec.exp +++ b/gdb/testsuite/gdb.multi/multi-arch-exec.exp @@ -53,7 +53,7 @@ if [istarget "s390*-*-*"] { } if { [prepare_for_testing "failed to prepare" ${exec1} "${srcfile1}" \ - [list debug \ + [list debug pthreads \ additional_flags=${march1}]] } { return -1 } @@ -76,22 +76,38 @@ if { [prepare_for_testing "failed to prepare" ${exec2} "${srcfile2}" \ return -1 } -proc do_test { mode } { +proc do_test { mode selected_thread } { global exec1 clean_restart ${exec1} - if ![runto_main] then { - fail "couldn't run to main" + if ![runto all_started] then { + fail "couldn't run to all_started" return -1 } + # Delete the breakpoint at 'all_started' otherwise GDB mayb + # switch context back to thread 1 to step over the breakpoint. + delete_breakpoints + + # A location for this breakpoint should be found in the new + # post-exec image too. + gdb_breakpoint main + + gdb_test "thread $selected_thread" "Switching to thread $selected_thread .*" + gdb_test_no_output "set follow-exec-mode $mode" # Test that GDB updates the target description / arch successfuly # after the exec. - gdb_test "continue" "Breakpoint 1, main.*" "continue across exec that changes architecture" + gdb_test "continue" "Breakpoint 2, main.*" "continue across exec that changes architecture" } -foreach follow_exec_mode {"same" "new"} { - do_test $follow_exec_mode +# Test handling the exec event with either the main thread or the +# second thread selected. This tries to ensure that GDB doesn't read +# registers off of the execing thread before figuring out its +# architecture. +foreach_with_prefix selected_thread {1 2} { + foreach_with_prefix follow_exec_mode {"same" "new"} { + do_test $follow_exec_mode $selected_thread + } }