From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 96122 invoked by alias); 9 Dec 2016 12:22:01 -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 96058 invoked by uid 89); 9 Dec 2016 12:21:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.7 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=directors, U*patches, sk:patches, Supervisory X-HELO: mga09.intel.com Received: from mga09.intel.com (HELO mga09.intel.com) (134.134.136.24) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 09 Dec 2016 12:21:49 +0000 Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga102.jf.intel.com with ESMTP; 09 Dec 2016 04:21:46 -0800 X-ExtLoop1: 1 Received: from irsmsx108.ger.corp.intel.com ([163.33.3.3]) by fmsmga002.fm.intel.com with ESMTP; 09 Dec 2016 04:21:45 -0800 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.52]) by IRSMSX108.ger.corp.intel.com ([169.254.11.159]) with mapi id 14.03.0248.002; Fri, 9 Dec 2016 12:21:44 +0000 From: "Metzger, Markus T" To: Luis Machado , "gdb-patches@sourceware.org" Subject: RE: [PATCH 2/2] btrace: allow recording to be started for running threads Date: Fri, 09 Dec 2016 12:22:00 -0000 Message-ID: References: <1481039697-17596-1-git-send-email-markus.t.metzger@intel.com> <1481039697-17596-3-git-send-email-markus.t.metzger@intel.com> <56d6100d-2dbb-c710-4dca-a5d8254b7448@codesourcery.com> In-Reply-To: <56d6100d-2dbb-c710-4dca-a5d8254b7448@codesourcery.com> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2016-12/txt/msg00228.txt.bz2 > -----Original Message----- > From: Luis Machado [mailto:lgustavo@codesourcery.com] > Sent: Wednesday, December 7, 2016 9:21 PM > To: Metzger, Markus T ; gdb- > patches@sourceware.org > Subject: Re: [PATCH 2/2] btrace: allow recording to be started for runnin= g threads Hi Luis, Thanks for your feedback. > > + /* This is not relevant for BTRACE_FORMAT_PT since the trace wil= l already > > + start at the enable location. */ >=20 > enabled location? I rephrased the sentence to: /* This is not relevant for BTRACE_FORMAT_PT since the trace will alr= eady start at the PC at which tracing was enabled. */ > > +if { [skip_btrace_tests] } { return -1 } >=20 > unsupported "btrace not supported on this target"? Let me do this for all gdb.btrace tests in a separate patch. > > + gdb_test "thread $thread" "(running).*" >=20 > Maybe add a more meaningful test name here? "Check if thread $thread is > running"? OK. Also moved it below the with_test_prefix. It now looks like this: PASS: gdb.btrace/enable-running.exp: thread 1: is running > > + with_test_prefix "thread $thread" { > > + # Stop the thread before reading the trace. > > + gdb_test_multiple "interrupt" "interrupt" { > > + -re "interrupt\r\n$gdb_prompt " { > > + pass "interrupt" > > + } >=20 > Any reason why we couldn't use gdb_test here? Also, we a more meaningful > test name and make the name unique (by mentioning the thread number, > possibly) The thread number is contained in the test prefix so the test name is uniqu= e. PASS: gdb.btrace/enable-running.exp: thread 1: interrupt Regarding the use of gdb_test .... > > + } > > + # Wait until the thread actually stopped. > > + gdb_test_multiple "" "stopped" { > > + -re "Thread $thread.*stopped\." { > > + pass "stopped" > > + } > > + } > > Same here. Couldn't we use gdb_test? Also a more meaningful and unique > test name. ... I may be able to use it for the first part. But not here. This is non= -stop mode so we're getting an asynchronous stopped notification. gdb_test expects a = prompt at the end but we already got the prompt and now need to consume the stopped notification to ensure that the thread really stopped before we issue the n= ext command. I'm not entirely sure about using gdb_test on the first part, either. If w= e got the stopped notification fast enough, I'm not sure whether the "...$gdb_prompt = $" pattern in gdb_test would still match. According to my understanding of how dejagnu works the test may fail sporadically since there is already more in= put available after the prompt and the "... $" wouldn't match. The test prefix makes the test name unique: PASS: gdb.btrace/enable-running.exp: thread 1: stopped Regards, Markus. Intel Deutschland GmbH Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de Managing Directors: Christin Eisenschmid, Christian Lamprechter Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928