From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12065 invoked by alias); 25 Apr 2013 05:48:36 -0000 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 Received: (qmail 12051 invoked by uid 89); 25 Apr 2013 05:48:35 -0000 X-Spam-SWARE-Status: No, score=-4.7 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL autolearn=ham version=3.3.1 Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 25 Apr 2013 05:48:33 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1UVF2k-0002M6-Ri from Luis_Gustavo@mentor.com ; Wed, 24 Apr 2013 22:48:30 -0700 Received: from NA1-MAIL.mgc.mentorg.com ([147.34.98.181]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Wed, 24 Apr 2013 22:48:30 -0700 Received: from [172.30.7.227] ([172.30.7.227]) by NA1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.3959); Wed, 24 Apr 2013 22:48:29 -0700 Message-ID: <5178C3A6.7010302@codesourcery.com> Date: Thu, 25 Apr 2013 14:13:00 -0000 From: Luis Machado Reply-To: lgustavo@codesourcery.com User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130329 Thunderbird/17.0.5 MIME-Version: 1.0 To: Pedro Alves CC: "'gdb-patches@sourceware.org'" Subject: Re: [PATCH, ppc] Fix hw *points for embedded ppc in a threaded environment References: <516EC58C.5060501@codesourcery.com> <51755821.8020907@codesourcery.com> <51755A5F.3060009@redhat.com> <51756D2B.5050204@redhat.com> <51758960.2090702@redhat.com> <51768D08.4090709@codesourcery.com> <5176B3D9.2050702@redhat.com> <5176CD03.3070800@codesourcery.com> <5177F7D6.6070908@redhat.com> In-Reply-To: <5177F7D6.6070908@redhat.com> Content-Type: multipart/mixed; boundary="------------010106060208020308070202" X-Virus-Found: No X-SW-Source: 2013-04/txt/msg00771.txt.bz2 This is a multi-part message in MIME format. --------------010106060208020308070202 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1514 > One idea for less intrusive debugging would be to make it so > the kernel doesn't notified of new threads/clones at all, IOW, not > force parent/child clones to stop to report the child's existence to > a ptracer (unless it wants to). Having the kernel auto-copy debug > resources in the new child is a requirement for that, and given > ptrace options and other things are copied as well, I can't > really call it broken outright. > > I'd suggest just mixing something like > > Older kernels did not make new threads inherit their parent thread's > debug state, so we always clear the slot and replicate the debug state > ourselves, ensuring compatibility with all kernels. > > into the existing comment. > The comment looked thorough enough that i went with it. > It does not: > > +# Run to `main' where we begin our tests. > +if ![runto_main] then { > + fail "Failed to run to main" > +} > + Really done now. > Let's call "unsupported" or even FAIL then, so it can draw attention. > We could also end up in that situation due to a bug somewhere in the > test or gdb. Something like 'fail "no hardware watchpoints"'. Done. >> +void empty_cycle (void) > > '\n' after first void. Done. > >> +for { set i 1 } { $i <= $TRIGGERS} { incr i} { > -------^ -----^ > Missing spaces. > Done. >> +# Move the threads and hit the watchpoints >> +# TRIGGERS times. > > Nit, that fits in a single line. Done. > > Otherwise OK. Thanks! I'll check it in later today. Luis --------------010106060208020308070202 Content-Type: text/x-patch; name="hw_watch.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="hw_watch.diff" Content-length: 12272 gdb/ 2013-04-25 Luis Machado * ppc-linux-nat.c (ppc_linux_new_thread): Clear the new thread's debug state prior to replicating existing hardware watchpoints or breakpoints. gdb/testsuite/ 2013-04-25 Luis Machado gdb/testsuite * gdb.threads/wp-replication.c: New file. * gdb.threads/wp-replication.exp: New file. Index: gdb-head/gdb/ppc-linux-nat.c =================================================================== --- gdb-head.orig/gdb/ppc-linux-nat.c 2013-04-23 16:34:19.726438757 +0200 +++ gdb-head/gdb/ppc-linux-nat.c 2013-04-25 07:42:30.325296537 +0200 @@ -2178,7 +2178,18 @@ ppc_linux_new_thread (struct lwp_info *l /* Copy that thread's breakpoints and watchpoints to the new thread. */ for (i = 0; i < max_slots_number; i++) if (hw_breaks[i].hw_break) - booke_insert_point (hw_breaks[i].hw_break, tid); + { + /* Older kernels did not make new threads inherit their parent + thread's debug state, so we always clear the slot and replicate + the debug state ourselves, ensuring compatibility with all + kernels. */ + + /* The ppc debug resource accounting is done through "slots". + Ask the kernel the deallocate this specific *point's slot. */ + ptrace (PPC_PTRACE_DELHWDEBUG, tid, 0, hw_breaks[i].slot); + + booke_insert_point (hw_breaks[i].hw_break, tid); + } } else ptrace (PTRACE_SET_DEBUGREG, tid, 0, saved_dabr_value); Index: gdb-head/gdb/testsuite/gdb.threads/wp-replication.c =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gdb-head/gdb/testsuite/gdb.threads/wp-replication.c 2013-04-25 07:23:24.281316900 +0200 @@ -0,0 +1,163 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2009-2013 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 . + + Check that hardware watchpoints get correctly replicated to all + existing threads when hardware watchpoints are created. This test + creates one hardware watchpoint per thread until a maximum is + reached. It originally addresses a deficiency seen on embedded + powerpc targets with slotted hardware *point designs. +*/ + +#include +#include +#include +#include +#include + +#ifndef NR_THREADS +#define NR_THREADS 4 /* Set by the testcase. */ +#endif + +#ifndef X_INCR_COUNT +#define X_INCR_COUNT 10 /* Set by the testcase. */ +#endif + +void *thread_function (void *arg); /* Function executed by each thread. */ + +/* Used to hold threads back until wp-replication.exp is ready. */ +int test_ready = 0; + +/* Used to hold threads back until every thread has had a chance of causing + a watchpoint trigger. This prevents a situation in GDB where it may miss + watchpoint triggers when threads exit while other threads are causing + watchpoint triggers. */ +int can_terminate = 0; + +/* Used to push the program out of the waiting loop after the + testcase is done counting the number of hardware watchpoints + available for our target. */ +int watch_count_done = 0; + +/* Number of watchpoints GDB is capable of using (this is provided + by GDB during the test run). */ +int hw_watch_count = 0; + +/* Array with elements we can create watchpoints for. */ +static int watched_data[NR_THREADS]; +pthread_mutex_t data_mutex; + +/* Wait function to keep threads busy while the testcase does + what it needs to do. */ +void +empty_cycle (void) +{ + usleep (1); +} + +int +main () +{ + int res; + pthread_t threads[NR_THREADS]; + int i; + + while (watch_count_done == 0) + { + /* GDB will modify the value of "i" at runtime and we will + get past this point. */ + empty_cycle (); + } + + pthread_mutex_init (&data_mutex, NULL); + + for (i = 0; i < NR_THREADS; i++) + { + res = pthread_create (&threads[i], + NULL, thread_function, + (void *) (intptr_t) i); + if (res != 0) + { + fprintf (stderr, "error in thread %d create\n", i); + abort (); + } + } + + for (i = 0; i < NR_THREADS; ++i) + { + res = pthread_join (threads[i], NULL); + if (res != 0) + { + fprintf (stderr, "error in thread %d join\n", i); + abort (); + } + } + + exit (EXIT_SUCCESS); +} + +/* Easy place for a breakpoint. + wp-replication.exp uses this to track when all threads are running + instead of, for example, the program keeping track + because we don't need the program to know when all threads are running, + instead we need gdb to know when all threads are running. + There is a delay between when a thread has started and when the thread + has been registered with gdb. */ + +void +thread_started (void) +{ +} + +void * +thread_function (void *arg) +{ + int i, j; + long thread_number = (long) arg; + + thread_started (); + + /* Don't start incrementing X until wp-replication.exp is ready. */ + while (!test_ready) + usleep (1); + + pthread_mutex_lock (&data_mutex); + + for (i = 0; i < NR_TRIGGERS_PER_THREAD; i++) + { + for (j = 0; j < hw_watch_count; j++) + { + /* For debugging. */ + printf ("Thread %ld changing watch_thread[%d] data" + " from %d -> %d\n", thread_number, j, + watched_data[j], watched_data[j] + 1); + /* Increment the watched data field. */ + watched_data[j]++; + } + } + + pthread_mutex_unlock (&data_mutex); + + /* Hold the threads here to work around a problem GDB has evaluating + watchpoints right when a DSO event shows up (PR breakpoints/10116). + Sleep a little longer (than, say, 1, 5 or 10) to avoid consuming + lots of cycles while the other threads are trying to execute the + loop. */ + while (!can_terminate) + usleep (100); + + pthread_exit (NULL); +} Index: gdb-head/gdb/testsuite/gdb.threads/wp-replication.exp =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gdb-head/gdb/testsuite/gdb.threads/wp-replication.exp 2013-04-25 07:24:16.245315977 +0200 @@ -0,0 +1,151 @@ +# This testcase is part of GDB, the GNU debugger. + +# Copyright 2009-2013 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 . + +# Check that hardware watchpoints get correctly replicated to all +# existing threads when hardware watchpoints are created. This test +# creates one hardware watchpoint per thread until a maximum is +# reached. It originally addresses a deficiency seen on embedded +# powerpc targets with slotted hardware *point designs. + +set NR_THREADS 10 +set NR_TRIGGERS_PER_THREAD 2 + +# This test verifies that a hardware watchpoint gets replicated to +# every existing thread and is detected properly. This test is +# only meaningful on a target with hardware watchpoint support. +if {[skip_hw_watchpoint_tests]} { + return 0 +} + +standard_testfile +if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "additional_flags=-DNR_THREADS=$NR_THREADS -DNR_TRIGGERS_PER_THREAD=$NR_TRIGGERS_PER_THREAD"]] != "" } { + return -1 +} + +clean_restart ${binfile} + +# Force hardware watchpoints to be used. +gdb_test_no_output "set can-use-hw-watchpoints 1" "" + +# Run to `main' where we begin our tests. +if ![runto_main] then { + fail "Failed to run to main" + return 0 +} + +# First, break at empty_cycle. +gdb_test "break empty_cycle" \ + "Breakpoint 2 at .*: file .*${srcfile}, line .*" \ + "Breakpoint on empty_cycle" + +# Set some default values. +set hwatch_count 0 +set done 0 + +# Count the number of hardware watchpoints available on +# this target. +while { $done == 0 } { + + gdb_test "continue" \ + ".*Breakpoint 2, empty_cycle \\(\\) at .*${srcfile}.*" \ + "Continue to empty_cycle to insert watchpoint $hwatch_count" + + # Some targets do resource counting as we insert watchpoints. + # Such targets won't cause a watchpoint insertion failure, but + # will switch to software watchpoints silently. We check for + # both cases here. + gdb_test_multiple "watch watched_data\[$hwatch_count\]" \ + "watch watched_data\[$hwatch_count\]" { + -re "Hardware watchpoint .*$gdb_prompt $" { + } + -re "Watchpoint .*$gdb_prompt $" { + set done 1 + break + } + } + + gdb_test_multiple "continue" "watchpoint created successfully" { + -re ".*Breakpoint 2, empty_cycle \\(\\).*$gdb_prompt $" { + incr hwatch_count + } + -re ".*Could not insert hardware watchpoint.*$gdb_prompt $" { + set done 1 + break + } + } +} + +# Target cannot insert hardware watchpoints. It should have reported +# (through board settings) that it did not support them in the first place. +# Just exit. +if { $hwatch_count == 0} { + fail "No hardware watchpoints available" + return 0 +} + +# Set the testcase's internal variable indicating the number of +# hardware watchpoints the target supports. +gdb_test_no_output "set var hw_watch_count=${hwatch_count}" \ + "set var hw_watch_count=${hwatch_count}" + +# At this point, we know how many hardware watchpoints +# the target supports. Use that to do further testing. +delete_breakpoints + +# Break out of the empty_cycle loop by changing the +# controlling variable. +gdb_test_no_output "set var watch_count_done=1" \ + "set var watch_count_done=1" + +# Prepare to create all the threads. +gdb_test "break thread_started" \ + "Breakpoint \[0-9\]+ at .*: file .*${srcfile}, line .*" \ + "Breakpoint on thread_started" + +# Move all threads to where they're supposed to be for testing. +for { set i 0 } { $i < $NR_THREADS } { incr i } { + + # We want to set the maximum number of hardware watchpoints + # and make sure the target can handle that without an error. + # That will show us the watchpoints got replicated to all the + # threads correctly, and that no new watchpoints got created + # in the background for a specific thread. + if {$i < $hwatch_count} { + gdb_test "watch watched_data\[$i\]" \ + "Hardware watchpoint .*" \ + "watch watched_data\[$i\]" + } else { + verbose -log "Not setting watchpoint for watched_data\[$i\]\n" + } + + gdb_test continue "Continuing\\..*Breakpoint \[0-9\]+, thread_started \\(\\) at .*$srcfile.*" \ + "Thread $i hit breakpoint at thread_started" +} + +# Let the threads run and change the watched data, leading +# to watchpoint triggers. +gdb_test_no_output "set var test_ready=1" \ + "set var test_ready=1" + +# Set the number of expected watchpoint triggers. +set TRIGGERS [expr "$NR_THREADS * $hwatch_count * $NR_TRIGGERS_PER_THREAD"] + +# Move the threads and hit the watchpoints TRIGGERS times. +for { set i 1 } { $i <= $TRIGGERS } { incr i } { + gdb_test continue "Continuing\\..*Hardware watchpoint \[0-9\]+: watched_data\[\[0-9\]+\].*Old value = \[0-9\]+.*New value = \[0-9\]+.*thread_function \\(arg=$hex\\) at .*$srcfile.*" \ + "Continue to watchpoint trigger $i out of ${TRIGGERS} on watched_data" +} --------------010106060208020308070202--