From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6716 invoked by alias); 2 Jan 2012 16:47:15 -0000 Received: (qmail 6705 invoked by uid 22791); 2 Jan 2012 16:47:13 -0000 X-SWARE-Spam-Status: No, hits=-7.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_XZ 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; Mon, 02 Jan 2012 16:46:58 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q02Gkwhk003412 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 2 Jan 2012 11:46:58 -0500 Received: from host2.jankratochvil.net (ovpn-116-32.ams2.redhat.com [10.36.116.32]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q02GkqB0025867 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO) for ; Mon, 2 Jan 2012 11:46:55 -0500 Date: Mon, 02 Jan 2012 16:47:00 -0000 From: Jan Kratochvil To: gdb-patches@sourceware.org Subject: [patch 2/2] Fix watchpoints for multi-inferior Message-ID: <20120102164652.GB10231@host2.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: 2012-01/txt/msg00048.txt.bz2 Hi, this is a new post of: [patch 4/4] hw watchpoints made multi-inferior http://sourceware.org/ml/gdb-patches/2010-12/msg00045.html Currently watchpoints somehow apply to all the inferiors but they do not work that way anyway. I believe watchpoints should be specific for each inferior. The testcase has some problems with gdbserver, it has been skipped now. No regressions on {x86_64,x86_64-m32,i686}-fedorarawhide-linux-gnu. For ppc* the testcase will be difficult, to test the issue properly one needs `awatch' (that is `watch' can hide a problem) but PPC does not feature `awatch'. Thanks, Jan gdb/ 2012-01-02 Jan Kratochvil Fix watchpoints to be specific for each inferior. * breakpoint.c (watchpoint_in_thread_scope): Verify also current_program_space. * i386-nat.c (i386_inferior_data_cleanup): New. (i386_inferior_data_get): Replace variable inf_data_local by an inferior_data call. (i386_use_watchpoints): Initialize i386_inferior_data. * linux-nat.c (linux_nat_iterate_watchpoint_lwps): Use INFERIOR_PTID specific iterate_over_lwps. gdb/testsuite/ 2012-01-02 Jan Kratochvil Fix watchpoints to be specific for each inferior. * gdb.multi/watchpoint-multi.c: New file. * gdb.multi/watchpoint-multi.exp: New file. --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -1225,9 +1225,10 @@ is_watchpoint (const struct breakpoint *bpt) static int watchpoint_in_thread_scope (struct watchpoint *b) { - return (ptid_equal (b->watchpoint_thread, null_ptid) - || (ptid_equal (inferior_ptid, b->watchpoint_thread) - && !is_executing (inferior_ptid))); + return (b->base.pspace == current_program_space + && (ptid_equal (b->watchpoint_thread, null_ptid) + || (ptid_equal (inferior_ptid, b->watchpoint_thread) + && !is_executing (inferior_ptid)))); } /* Set watchpoint B to disp_del_at_next_stop, even including its possible --- a/gdb/i386-nat.c +++ b/gdb/i386-nat.c @@ -181,16 +181,31 @@ struct i386_inferior_data struct i386_debug_reg_state state; }; +/* Per-inferior hook for register_inferior_data_with_cleanup. */ + +static void +i386_inferior_data_cleanup (struct inferior *inf, void *arg) +{ + struct i386_inferior_data *inf_data = arg; + + xfree (inf_data); +} + /* Get data specific for INFERIOR_PTID LWP. Return special data area for processes being detached. */ static struct i386_inferior_data * i386_inferior_data_get (void) { - /* Intermediate patch stub. */ - static struct i386_inferior_data inf_data_local; struct inferior *inf = current_inferior (); - struct i386_inferior_data *inf_data = &inf_data_local; + struct i386_inferior_data *inf_data; + + inf_data = inferior_data (inf, i386_inferior_data); + if (inf_data == NULL) + { + inf_data = xzalloc (sizeof (*inf_data)); + set_inferior_data (current_inferior (), i386_inferior_data, inf_data); + } if (inf->pid != ptid_get_pid (inferior_ptid)) { @@ -856,6 +871,10 @@ i386_use_watchpoints (struct target_ops *t) t->to_remove_watchpoint = i386_remove_watchpoint; t->to_insert_hw_breakpoint = i386_insert_hw_breakpoint; t->to_remove_hw_breakpoint = i386_remove_hw_breakpoint; + + if (i386_inferior_data == NULL) + i386_inferior_data + = register_inferior_data_with_cleanup (i386_inferior_data_cleanup); } void --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -1288,7 +1288,11 @@ linux_nat_iterate_watchpoint_lwps if (inf->pid == inferior_pid) { - iterate_over_lwps (minus_one_ptid, callback, callback_data); + /* Iterate all the threads of the current inferior. Without specifying + INFERIOR_PID it would iterate all threads of all inferiors, which is + inappropriate for watchpoints. */ + + iterate_over_lwps (pid_to_ptid (inferior_pid), callback, callback_data); } else { --- /dev/null +++ b/gdb/testsuite/gdb.multi/watchpoint-multi.c @@ -0,0 +1,51 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2012 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 +#include + +static volatile int a, b, c; + +static void +marker_exit (void) +{ + a = 1; +} + +static void * +start (void *arg) +{ + b = 2; + c = 3; + + return NULL; +} + +int +main (void) +{ + pthread_t thread; + int i; + + i = pthread_create (&thread, NULL, start, NULL); + assert (i == 0); + i = pthread_join (thread, NULL); + assert (i == 0); + + marker_exit (); + return 0; +} --- /dev/null +++ b/gdb/testsuite/gdb.multi/watchpoint-multi.exp @@ -0,0 +1,92 @@ +# Copyright 2012 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 . + +if [is_remote target] { + # It is KFAIL. + continue +} + +set testfile "watchpoint-multi" + +set executable ${testfile} +set srcfile ${testfile}.c +set binfile ${objdir}/${subdir}/${executable} + +if { [gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { + untested ${testfile}.exp + return -1 +} + +clean_restart $executable + +if ![runto_main] { + return +} +# Never keep/use any non-hw breakpoints to workaround a multi-inferior bug. +delete_breakpoints + +gdb_test "add-inferior" "Added inferior 2" +gdb_test "inferior 2" "witching to inferior 2 .*" +gdb_load $binfile + +if ![runto_main] { + return +} +delete_breakpoints + +# Simulate non-stop+target-async which also uses breakpoint always-inserted. +gdb_test_no_output "set breakpoint always-inserted on" +# displaced-stepping is also needed as other GDB sometimes still removes the +# breakpoints, even with always-inserted on. +gdb_test_no_output "set displaced-stepping on" + +# Debugging of this testcase: +#gdb_test_no_output "maintenance set show-debug-regs on" +#gdb_test_no_output "set debug infrun 1" + +# Do not use simple hardware watchpoint ("watch") as its false hit may be +# unnoticed by GDB if it reads it still has the same value. +gdb_test "awatch c" "Hardware access \\(read/write\\) watchpoint \[0-9\]+: c" + +gdb_breakpoint "marker_exit" + +gdb_test "inferior 1" "witching to inferior 1 .*" + +set have_awatch_b 0 +set test "awatch b" +gdb_test_multiple $test $test { + -re "Hardware access \\(read/write\\) watchpoint \[0-9\]+: b\r\n$gdb_prompt $" { + pass $test + set have_awatch_b 1 + } + -re "There are not enough available hardware resources for this watchpoint\\.\r\n$gdb_prompt $" { + untested $test + return + } +} + +gdb_test "inferior 2" "witching to inferior 2 .*" + +# FAIL would be a hit on watchpoint for `b' - that one is for the other +# inferior. +gdb_test "continue" "Hardware access \\(read/write\\) watchpoint \[0-9\]+: c\r\n\r\nOld value = 0\r\nNew value = 3\r\n.*" "catch c" + +gdb_test "continue" "Breakpoint \[0-9\]+, marker_exit .*" "catch marker_exit in inferior 2" + +gdb_test "inferior 1" "witching to inferior 1 .*" + +gdb_test "continue" "Hardware access \\(read/write\\) watchpoint \[0-9\]+: b\r\n\r\nOld value = 0\r\nNew value = 2\r\n.*" "catch b" + +gdb_test "continue" "Breakpoint \[0-9\]+, marker_exit .*" "catch marker_exit in inferior 1"