From patchwork Thu May 16 20:12:18 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Franco de Carvalho X-Patchwork-Id: 32726 Received: (qmail 76087 invoked by alias); 16 May 2019 20:12:40 -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 76023 invoked by uid 89); 16 May 2019 20:12:40 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-15.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.1 spammy=tracked, new_argv, execvp, meaningful X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0a-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.156.1) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 16 May 2019 20:12:36 +0000 Received: from pps.filterd (m0098394.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id x4GKCXaJ066825 for ; Thu, 16 May 2019 16:12:35 -0400 Received: from e31.co.us.ibm.com (e31.co.us.ibm.com [32.97.110.149]) by mx0a-001b2d01.pphosted.com with ESMTP id 2she92h235-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Thu, 16 May 2019 16:12:34 -0400 Received: from localhost by e31.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 16 May 2019 21:12:34 +0100 Received: from b03cxnp08028.gho.boulder.ibm.com (9.17.130.20) by e31.co.us.ibm.com (192.168.1.131) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; (version=TLSv1/SSLv3 cipher=AES256-GCM-SHA384 bits=256/256) Thu, 16 May 2019 21:12:32 +0100 Received: from b03ledav006.gho.boulder.ibm.com (b03ledav006.gho.boulder.ibm.com [9.17.130.237]) by b03cxnp08028.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id x4GKCTci28180758 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 16 May 2019 20:12:29 GMT Received: from b03ledav006.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 7B8E3C6061; Thu, 16 May 2019 20:12:29 +0000 (GMT) Received: from b03ledav006.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 3E9A5C605D; Thu, 16 May 2019 20:12:29 +0000 (GMT) Received: from pedro.localdomain (unknown [9.18.235.49]) by b03ledav006.gho.boulder.ibm.com (Postfix) with ESMTP; Thu, 16 May 2019 20:12:29 +0000 (GMT) Received: by pedro.localdomain (Postfix, from userid 1000) id 6077B3C0449; Thu, 16 May 2019 17:12:26 -0300 (-03) From: Pedro Franco de Carvalho To: gdb-patches@sourceware.org Cc: uweigand@de.ibm.com Subject: [PATCH 2/2] x86 linux: Update debug register state after exec events Date: Thu, 16 May 2019 17:12:18 -0300 In-Reply-To: <20190516201218.29403-1-pedromfc@linux.ibm.com> References: <20190516201218.29403-1-pedromfc@linux.ibm.com> MIME-Version: 1.0 x-cbid: 19051620-8235-0000-0000-00000E979D69 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00011106; HX=3.00000242; KW=3.00000007; PH=3.00000004; SC=3.00000285; SDB=6.01204287; UDB=6.00632196; IPR=6.00985218; MB=3.00026920; MTD=3.00000008; XFM=3.00000015; UTC=2019-05-16 20:12:32 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 19051620-8236-0000-0000-0000459727C5 Message-Id: <20190516201218.29403-3-pedromfc@linux.ibm.com> This patch changes the linux native x86 target so that it updates its debug register state after an exec call is detected, by overriding the architecture-specific low target method low_post_exec from linux_nat_target. Previously, an exec event could cause the debug register state to become inconsistent. The infrun logic tries to re-insert symbolic watchpoints and breakpoints after an exec event, in case that they are still meaningful in the new program. However, it assumes watchpoints and breakpoints are cleared across the exec event, including those that use hardware debug registers, so it doesn't try to remove them before re-inserting them. From the target's perspective, this is seen as two successive calls to insert_watchpoint, with no remove_watchpoint in between, as it is sometimes done when the inferior stops and is resumed again. The first insertion comes before the inferior is resumed and eventually calls exec, the second happens after the exec call when infrun updates the symbolic breakpoints and watchpoints that are still meaningful. This double insertion can cause the state of the debug registers tracked by the low target to become inconsistent. In some cases this can cause a watchpoint not to be installed. The x86 linux native target keeps track of requests for equivalent hardware watchpoints with a reference count so that it doesn't need to duplicate debug register usage. This type of request doesn't cause the debug registers in the threads to be updated, since the target assumes the thread already has the debug registers properly configured. If a program self-execs, a global symbol can have the same address and length in its new instance. This will cause the double insertion across an exec call to not have any effect, so the watchpoint won't trigger if the second instance of the program writes to the watched region. This issue is reflected by the testcase included in this patch. This specific issue probably doesn't happen in ARM and s390, and possibly other targets, since they don't seem to handle duplicated hardware watchpoints specially, but any debug register state tracked by these targets is likely to be incorrect, and could cause other issues, so they should also redefine low_post_exec to reset the debug register state that they track. The new testcase will likely pass for these. The Power linux native target currently has other issues that need to be addressed first. This patch makes no changes to the x86 linux-low gdbserver stub, because when the generic linux stub detects an exec event, and if exec events are configured to be reported to the client ("exec-events+" in the query packet), it deletes the process state and recreates one, which causes the debug register data of the linux x86 stub to be cleared. Linux stubs for other arches that support hardware watchpoints can use this same mechanism. When the generic linux stub detects an exec event, but these are not configured to be reported back to the client, the process state is not re-initialized. However, if infrun isn't notified of the exec event in the first place, it won't try to insert the watchpoint twice without a removal in-between, so the debug register state in the stub will match what infrun knows about the watchpoints, even though it doesn't match what is actually installed in the threads, if the watchpoint was cleared by the exec call. gdb/ChangeLog: 2019-05-16 Pedro Franco de Carvalho * x86-nat.h (x86_cleanup_dregs): Declare overloaded function with a pid argument. Adjust the comment for the original function. * x86-nat.c (x86_cleanup_dregs): Define overloaded function. Change the original version to use it. * x86-linux-nat.h (x86_linux_nat_target) : Override and define. gdb/testsuite/ChangeLog: 2019-05-16 Pedro Franco de Carvalho * gdb.base/watchpoint-exec.exp: New testcase. * gdb.base/watchpoint-exec.c: New file. --- gdb/testsuite/gdb.base/watchpoint-exec.c | 48 ++++++++++++++++++++ gdb/testsuite/gdb.base/watchpoint-exec.exp | 53 ++++++++++++++++++++++ gdb/x86-linux-nat.h | 3 ++ gdb/x86-nat.c | 15 ++++-- gdb/x86-nat.h | 8 +++- 5 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 gdb/testsuite/gdb.base/watchpoint-exec.c create mode 100644 gdb/testsuite/gdb.base/watchpoint-exec.exp diff --git a/gdb/testsuite/gdb.base/watchpoint-exec.c b/gdb/testsuite/gdb.base/watchpoint-exec.c new file mode 100644 index 0000000000..56d3dbe69f --- /dev/null +++ b/gdb/testsuite/gdb.base/watchpoint-exec.c @@ -0,0 +1,48 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright (C) 2019 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 . */ + +/* This program exits if its first argument is --stop, otherwise it + tries to exec the first argument, passing --stop. In both cases it + writes to a global variable, which GDB will watch in this test. It + is meant to be called with its own pathname, so that it writes to + the variable, execs itself once, writes to the variable again, and + exits. */ + +#include +#include + +int watched_var = 0; + +int main (int argc, char *argv[]) +{ + char *new_argv[3] = {"", "--stop", NULL}; + + if (argc != 2) + return 1; + + new_argv[0] = argv[0]; + + watched_var = 1; + + if (strcmp (argv[1], "--stop") == 0) + return 0; + + if (execvp (argv[1], &new_argv[0]) < 0) + return 1; + + return 0; +} diff --git a/gdb/testsuite/gdb.base/watchpoint-exec.exp b/gdb/testsuite/gdb.base/watchpoint-exec.exp new file mode 100644 index 0000000000..d366add292 --- /dev/null +++ b/gdb/testsuite/gdb.base/watchpoint-exec.exp @@ -0,0 +1,53 @@ +# Copyright (C) 2019 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 . + +# This file is part of the gdb testsuite. + +# Test if a hardware watchpoint triggers before and after a program +# self-execs. This should happen since the watched global symbol must +# be valid after the exec. + +if {[skip_hw_watchpoint_tests]} { + return +} + +# The testsuite doesn't currently allow sending args to the stub. +if [use_gdb_stub] { + unsupported "using gdb stub" + return +} + +standard_testfile + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} { + return +} + +gdb_test_no_output "set args $binfile" "" + +runto_main + +# We don't want to hit the breakpoint from runto_main after the exec. +delete_breakpoints + +gdb_test "watch watched_var" "Hardware watchpoint.*: watched_var.*" + +set watchpoint_hit_re \ + "Hardware watchpoint.*: watched_var.*Old value = 0.*New value = 1" + +gdb_test "continue" "$watchpoint_hit_re.*" "continue before exec" + +gdb_test "continue" "is executing new program.*$watchpoint_hit_re.*" \ + "continue after exec" diff --git a/gdb/x86-linux-nat.h b/gdb/x86-linux-nat.h index 887e30eecd..9f024d4649 100644 --- a/gdb/x86-linux-nat.h +++ b/gdb/x86-linux-nat.h @@ -65,6 +65,9 @@ struct x86_linux_nat_target : public x86_nat_target void low_forget_process (pid_t pid) override { x86_forget_process (pid); } + void low_post_exec (struct lwp_info *lwp) override + { x86_cleanup_dregs (lwp->ptid.pid ()); } + void low_prepare_to_resume (struct lwp_info *lwp) override { x86_linux_prepare_to_resume (lwp); } diff --git a/gdb/x86-nat.c b/gdb/x86-nat.c index cd9ce17e8d..71f7c35ca9 100644 --- a/gdb/x86-nat.c +++ b/gdb/x86-nat.c @@ -133,13 +133,22 @@ x86_forget_process (pid_t pid) } /* Clear the reference counts and forget everything we knew about the - debug registers. */ + debug registers of process PID. */ void -x86_cleanup_dregs (void) +x86_cleanup_dregs (pid_t pid) { /* Starting from scratch has the same effect. */ - x86_forget_process (inferior_ptid.pid ()); + x86_forget_process (pid); +} + +/* Convenience overloaded function that calls x86_cleanup_dregs with + the pid of inferior_ptid. */ + +void +x86_cleanup_dregs (void) +{ + x86_cleanup_dregs (inferior_ptid.pid ()); } /* Insert a watchpoint to watch a memory region which starts at diff --git a/gdb/x86-nat.h b/gdb/x86-nat.h index baa4218a87..08df339a33 100644 --- a/gdb/x86-nat.h +++ b/gdb/x86-nat.h @@ -36,7 +36,13 @@ extern void x86_set_debug_register_length (int len); -/* Use this function to reset the x86-nat.c debug register state. */ +/* Use this function to reset the x86-nat.c debug register state for + PID. */ + +extern void x86_cleanup_dregs (pid_t pid); + +/* Use this function to reset the x86-nat.c debug register state for + the pid of inferior_ptid. */ extern void x86_cleanup_dregs (void);