From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14005 invoked by alias); 18 Jun 2013 18:10:03 -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 13995 invoked by uid 89); 18 Jun 2013 18:10:03 -0000 X-Spam-SWARE-Status: No, score=-8.1 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Tue, 18 Jun 2013 18:10:02 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r5II9wTh014266 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 18 Jun 2013 14:09:59 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r5II9uZs005690; Tue, 18 Jun 2013 14:09:57 -0400 Message-ID: <51C0A274.9010808@redhat.com> Date: Tue, 18 Jun 2013 18:16:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6 MIME-Version: 1.0 To: Hui Zhu CC: Tom Tromey , gdb-patches@sourceware.org, Keith Seitz , Yao Qi Subject: Re: [RFC] PR 15075 dprintf interferes with "next" References: <1361192891-29341-1-git-send-email-yao@codesourcery.com> <8738wpd3qe.fsf@fleche.redhat.com> <5176C14B.6010603@redhat.com> <51774714.9060306@codesourcery.com> <51969A92.80003@redhat.com> <519CBE2B.7060007@redhat.com> <51ACD6ED.7040604@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-06/txt/msg00448.txt.bz2 On 06/07/2013 04:15 AM, Hui Zhu wrote: > 2013-06-07 Yao Qi > Hui Zhu > Pedro Alves > > PR breakpoints/15075 > PR breakpoints/15434 > * breakpoint.c (bpstat_stop_status): Call > b->ops->after_condition_true. > (update_dprintf_command_list): Don't append "continue" command > to the command list of dprintf breakpoint. > (base_breakpoint_after_condition_true): New function. > (base_breakpoint_ops): Add base_breakpoint_after_condition_true. > (dprintf_create_breakpoints_sal, > dprintf_after_condition_true): New functions. Context is split in multiple lines with '()'s, not ','s: (dprintf_create_breakpoints_sal) (dprintf_after_condition_true): New functions. > (initialize_breakpoint_ops): Set dprintf_create_breakpoints_sal > and dprintf_after_condition_true. > * breakpoint.h (breakpoint_ops): Add after_condition_true. > > }; > > /* Default breakpoint_ops methods. */ > @@ -13351,6 +13353,76 @@ dprintf_print_recreate (struct breakpoin > print_recreate_thread (tp, fp); > } > > +/* Implement the "create_breakpoints_sal" breakpoint_ops method for > + dprintf. */ > + > +static void > +dprintf_create_breakpoints_sal (struct gdbarch *gdbarch, > + struct linespec_result *canonical, > + struct linespec_sals *lsal, > + char *cond_string, > + char *extra_string, > + enum bptype type_wanted, > + enum bpdisp disposition, > + int thread, > + int task, int ignore_count, > + const struct breakpoint_ops *ops, > + int from_tty, int enabled, > + int internal, unsigned flags) > +{ > + struct breakpoint *b; > + > + create_breakpoints_sal_default (gdbarch, canonical, lsal, > + cond_string, extra_string, > + type_wanted, > + disposition, thread, task, > + ignore_count, ops, from_tty, > + enabled, internal, flags); > + > + b = get_breakpoint (breakpoint_count); > + gdb_assert (b != NULL); > + > + breakpoint_set_silent (b, 0); > +} When I tried it last, I didn't find making the dprintf explicitly silent necessary, given: /* Print nothing for this entry if we don't stop or don't print. */ if (!bs->stop || !bs->print) bs->print_it = print_it_noop; So did it turn out really necessary for some reason? Please don't leave such changes between revisions unexplained. > +gdb_test "next" "\\+\\+x\;.*\/\* Next without dprintf.*" "next 1" > +gdb_test "next" "\\+\\+x\;.*\/\* Set dprintf here.*" "next 2" > --- /dev/null > +++ b/gdb/testsuite/gdb.base/dprintf-non-stop.c > @@ -0,0 +1,30 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright (C) 2013 Free Software Foundation, Inc. > + Contributed by Hui Zhu ( IMO, we should stop adding these (like glibc has done so too): http://sourceware.org/ml/gdb-patches/2013-05/msg00253.html ) > --- /dev/null > +++ b/gdb/testsuite/gdb.base/dprintf-non-stop.exp > @@ -0,0 +1,54 @@ > +# Copyright (C) 2013 Free Software Foundation, Inc. > +# Contributed by Hui Zhu > + > +# 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] then { > + unsupported "Dprintf with non-stop is not supported." That's not exactly true. It's supported, but testing is racy at the moment. Write instead: if [is_remote target] then { # Testing with remote/non-stop is racy at the moment. unsupported "Testing dprintf with remote/non-stop is not supported." return 0 } > + return 0 > +} > + > +standard_testfile > + > +if [prepare_for_testing "failed to prepare for dprintf with non-stop" \ > + ${testfile} ${srcfile} {debug}] { > + return -1 > +} > + > +gdb_test_no_output "set target-async on" > +gdb_test_no_output "set non-stop on" > + > +if ![runto main] { > + fail "Can't run to main" > + return -1 > +} > + > +gdb_test "dprintf foo,\"At foo entry\\n\"" "Dprintf .*" > + > +send_gdb "continue &\n" > +exec sleep 1 > + > +set test "interrupt" > +gdb_test_multiple $test $test { > + -re "interrupt\r\n$gdb_prompt " { > + pass $test Hmm, this still looks racy to me, even on native targets. "continue &" produces a gdb prompt. gdb_test_multiple inside has a match for the prompt: -re "\r\n$gdb_prompt $" { if ![string match "" $message] then { fail "$message" } set result 1 } So if the expect happens to read continue & (gdb) from the buffer, we'll hit the fail. Doesn't the read1 hack catch this? We need to consume the prompt after that "continue&". > + } > +} > + > +set test "inferior stopped" > +gdb_test_multiple "" $test { > + -re "\r\n\\\[.* \[0-9\]+\\\] #1 stopped\\\.\r\n" { > + pass $test > + } > +} Likewise? -- Pedro Alves