From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19158 invoked by alias); 22 Aug 2013 09:42:30 -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 19149 invoked by uid 89); 22 Aug 2013 09:42:29 -0000 X-Spam-SWARE-Status: No, score=-5.0 required=5.0 tests=BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL autolearn=ham version=3.3.2 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, 22 Aug 2013 09:42:28 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1VCRPO-0001PX-NZ from Muhammad_Waqas@mentor.com ; Thu, 22 Aug 2013 02:42:26 -0700 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Thu, 22 Aug 2013 02:42:27 -0700 Received: from [137.202.157.111] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server (TLS) id 14.2.247.3; Thu, 22 Aug 2013 10:42:23 +0100 Message-ID: <5215DCEB.1010804@codesourcery.com> Date: Thu, 22 Aug 2013 09:42:00 -0000 From: Muhammad Waqas User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130804 Thunderbird/17.0.8 MIME-Version: 1.0 To: Tom Tromey CC: Pedro Alves , Yao Qi , , Subject: Re: [PATCH with testcase] Bug 11568 - delete thread-specific breakpoint on the thread exit References: <51F619CE.5040407@codesourcery.com> <51F633E5.7000302@codesourcery.com> <51F65519.2080806@codesourcery.com> <51F67992.30704@codesourcery.com> <51F7967E.3060900@codesourcery.com> <51FA4D21.4000309@redhat.com> <51FA5806.7050505@codesourcery.com> <51FB7F9E.30701@redhat.com> <51FF941E.7060705@codesourcery.com> <878v0gb5qe.fsf@fleche.redhat.com> <520093B7.8060800@codesourcery.com> In-Reply-To: <520093B7.8060800@codesourcery.com> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit X-SW-Source: 2013-08/txt/msg00624.txt.bz2 On 08/06/2013 11:12 AM, Muhammad Waqas wrote: > On 08/05/2013 06:57 PM, Tom Tromey wrote: >>>>>>> "Muhammad" == Muhammad Waqas writes: >> Muhammad> insertion_state_t; >> Muhammad> +static void remove_threaded_breakpoints (struct >> thread_info *tp, int >> Muhammad> silent); >> Muhammad> + >> >> Your patch got mangled by your mailer. This makes it hard to check the >> formatting, so please fix that. >> >> Muhammad> +static void >> Muhammad> +remove_threaded_breakpoints(struct thread_info *tp, int >> silent) >> Muhammad> +{ >> >> Needs an intro comment. >> >> Muhammad> + >> Muhammad> + if (b->thread > 0) >> Muhammad> + { >> Muhammad> + observer_attach_thread_exit >> (remove_threaded_breakpoints); >> Muhammad> + } >> >> It seems odd to re-register the observer each time. >> Why not just do it once, at initialization time? >> >> Muhammad> 2013-07-24 Muhammad Waqas >> Muhammad> Jan Kratochvil >> >> Muhammad> PR gdb/11568 >> Muhammad> *gdb.thread/thread-specific-bp.c: Newfile. >> Muhammad> *gdb.thread/thread-specific-bp.exp: Newfile. >> >> Space after "*" and in "New file". >> >> Muhammad> +set mode "All stop" >> Muhammad> + >> Muhammad> +if {[gdb_compile_pthreads \ >> Muhammad> + "${srcdir}/${subdir}/${srcfile}" \ >> Muhammad> + "${binfile}" executable {debug} ] != "" } { >> Muhammad> + return -1 >> Muhammad> +} >> Muhammad> + >> Muhammad> +clean_restart ${binfile} >> Muhammad> + >> Muhammad> +proc check_threaded_breakpoint {} { >> Muhammad> + global gdb_prompt mode >> >> Make "mode" a parameter. >> Use with_test_prefix, since otherwise the new .exp will have repeated >> test names, an gdb anti-pattern. >> >> Muhammad> +# Testing in non-stop+async mode. >> Muhammad> +set mode "non-stop\\async" >> >> It's better to simply not use an unusual character. >> >> Tom > > Thanks for reviewing patch. > > Changlog > > 2013-08-05 Muhammad Waqas > > PR gdb/11568 > * breakpoint.c (remove_threaded_breakpoints): New function. > * breakpoint.c (_initialize_breakpoint): function > remove_threaded_breakpoints registerd with thread_exit. > > Added intro comments for new function. > Fix with re-register the observer each time now it register > at inilization time only once. > > > Index: ./../../breakpoint.c > =================================================================== > RCS file: /cvs/src/src/gdb/breakpoint.c,v > retrieving revision 1.773 > diff -u -p -r1.773 breakpoint.c > --- ./../../breakpoint.c 24 Jul 2013 19:50:32 -0000 1.773 > +++ ./../../breakpoint.c 6 Aug 2013 05:56:35 -0000 > @@ -200,6 +200,9 @@ typedef enum > } > insertion_state_t; > > +static void remove_threaded_breakpoints (struct thread_info *tp, > + int silent); > + > static int remove_breakpoint (struct bp_location *, insertion_state_t); > static int remove_breakpoint_1 (struct bp_location *, > insertion_state_t); > > @@ -2928,6 +2931,24 @@ remove_breakpoints (void) > return val; > } > > +/* Used when a thread exits, it will remove breakpoints which > + are related to that thread. */ > + > +static void > +remove_threaded_breakpoints (struct thread_info *tp, int silent) > +{ > + struct breakpoint *b, *b_tmp; > + > + ALL_BREAKPOINTS_SAFE (b, b_tmp) > + { > + if (b->thread == tp->num) > + { > + b->disposition = disp_del_at_next_stop; > + b->number = 0; > + } > + } > +} > + > /* Remove breakpoints of process PID. */ > > int > @@ -16568,4 +16589,5 @@ agent-printf \"printf format string\", a > automatic_hardware_breakpoints = 1; > > observer_attach_about_to_proceed (breakpoint_about_to_proceed); > + observer_attach_thread_exit (remove_threaded_breakpoints); > } > > Changlog > > 2013-07-24 Muhammad Waqas > Jan Kratochvil > > PR gdb/11568 > * gdb.thread/thread-specific-bp.c: New file. > * gdb.thread/thread-specific-bp.exp: New file. > > Fix with testcase > > > Index: thread-specific-bp.c > =================================================================== > RCS file: thread-specific-bp.c > diff -N thread-specific-bp.c > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ thread-specific-bp.c 6 Aug 2013 05:56:57 -0000 > @@ -0,0 +1,33 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 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 > . */ > + > +#include > + > +static void * > +start (void *arg) > +{ > + return NULL; > +} > + > +int > +main (void) > +{ > + pthread_t thread; > + pthread_create (&thread, NULL, start, NULL); > + pthread_join (thread, NULL); > + return 0; /*set break here*/ > +} > Index: thread-specific-bp.exp > =================================================================== > RCS file: thread-specific-bp.exp > diff -N thread-specific-bp.exp > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ thread-specific-bp.exp 6 Aug 2013 05:57:14 -0000 > @@ -0,0 +1,98 @@ > +# Copyright (C) 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 . > +# > +# Verify that a thread-specific breakpoint is deleted when the > +# corresponding thread is gone. > + > +standard_testfile > + > +if {[gdb_compile_pthreads \ > + "${srcdir}/${subdir}/${srcfile}" \ > + "${binfile}" executable {debug} ] != "" } { > + return -1 > +} > + > +clean_restart ${binfile} > + > +proc check_threaded_breakpoint {mode} { with_test_prefix "$mode" { > + > + global gdb_prompt > + gdb_breakpoint "start" > + gdb_continue_to_breakpoint "start" > + set thre 0 > + > + gdb_test_multiple "info threads" "get thread 1 id" { > + -re "(\[0-9\]+)(\[^\n\r\]*Thread\[^\n\r\]*start.*)($gdb_prompt $)" { > + pass "thread created" > + # get the id of thread > + set thre $expect_out(1,string) > + } > + } > + gdb_breakpoint "main thread $thre" > + gdb_test "info break" ".*breakpoint.*thread $thre" "Breakpoint set" > + gdb_breakpoint [gdb_get_line_number "set break here"] > + > + # Force GDB to update its knowledge on existing threads when this > + # breakpoint is hit. Otherwise, GDB doesn't realize thread $thre > + # has exited and doesn't remove the thread specific breakpoint. > + gdb_test "commands\ninfo threads\nend" "End with.*" "add > breakpoint commands" > + gdb_test "thread $thre" "Switching to thread $thre.*" "Thread > $thre selected" > + set full_name "continue to breakpoint: set break here" > + > + send_gdb "continue\n" > + gdb_expect 10 { > + -re "(?:Breakpoint|Temporary breakpoint) .* (at|in) > .*\r\n$gdb_prompt $" { > + pass $full_name > + } > + -re ".*$gdb_prompt $" { > + fail $full_name > + } > + timeout { > + send_gdb "thread 1\n" > + exp_continue > + } > + } > + > + set test "thread-specific breakpoint is deleted" > + gdb_test_multiple "info breakpoint" $test { > + -re "thread $thre.*$gdb_prompt $" { > + fail $test > + } > + -re "$gdb_prompt $" { > + pass $test > + } > + }} > +} > + > +if ![runto_main] { > + untested "could not run to main" > + return -1 > +} > + > +# Testing in all stop mode. > +check_threaded_breakpoint "All stop" > + > +clean_restart ${binfile} > + > +gdb_test "set target-async on" ".*" "Set async mode" > +gdb_test "set non-stop on" ".*" "Set non stop mode" > + > +if ![runto_main] { > + untested "could not run to main" > + return -1 > +} > + > +# Testing in non-stop with async mode. > +check_threaded_breakpoint "non-stop with async" > ping