From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23462 invoked by alias); 26 Jan 2017 14:54:16 -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 23451 invoked by uid 89); 26 Jan 2017 14:54:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.9 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=H*MI:sk:65ec6bb, H*i:sk:65ec6bb, H*f:sk:65ec6bb, Tel X-HELO: mga03.intel.com Received: from mga03.intel.com (HELO mga03.intel.com) (134.134.136.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 26 Jan 2017 14:54:05 +0000 Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga103.jf.intel.com with ESMTP; 26 Jan 2017 06:54:00 -0800 X-ExtLoop1: 1 Received: from irsmsx106.ger.corp.intel.com ([163.33.3.31]) by fmsmga006.fm.intel.com with ESMTP; 26 Jan 2017 06:53:59 -0800 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.142]) by IRSMSX106.ger.corp.intel.com ([169.254.8.197]) with mapi id 14.03.0248.002; Thu, 26 Jan 2017 14:53:58 +0000 From: "Metzger, Markus T" To: Pedro Alves , "gdb-patches@sourceware.org" CC: Simon Marchi Subject: RE: [PATCH v2 2/2] btrace: allow recording to be started for running threads Date: Thu, 26 Jan 2017 14:54:00 -0000 Message-ID: References: <1484923135-31270-1-git-send-email-markus.t.metzger@intel.com> <1484923135-31270-3-git-send-email-markus.t.metzger@intel.com> <65ec6bb0-5528-a1cf-2b8f-e2a0ef070a6f@redhat.com> In-Reply-To: <65ec6bb0-5528-a1cf-2b8f-e2a0ef070a6f@redhat.com> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2017-01/txt/msg00577.txt.bz2 Hello Pedro, > >>> +/* Validate that we can read PTID's registers. */ > >>> + > >>> +static void > >>> +validate_registers_access_ptid (ptid_t ptid) > >>> +{ > >>> + struct cleanup *cleanup =3D save_inferior_ptid (); > >>> + > >>> + inferior_ptid =3D ptid; > >>> + validate_registers_access (); > >>> + do_cleanups (cleanup); > >>> +} > >> > >> Do we need this, we we have the new function added > >> by patch #1 ? > > > > This is a safeguard. I don't expect that we will ever throw, here. >=20 > Aren't the checks done inside the validate_registers_access function > exactly the same as done in can_access_registers_ptid? >=20 > And if that doesn't throw, and the thread is running, you have > register accesses right after: >=20 > /* Make sure we can read TP's registers. */ > validate_registers_access_ptid (tp->ptid); >=20 > regcache =3D get_thread_regcache (tp->ptid); > pc =3D regcache_read_pc (regcache); >=20 >=20 > I think I must be missing something. btrace_add_pc is called in two different flows: - when enabling tracing - when decoding trace The first checks if registers can be accessed and silently omits the call if they can't. The second calls btrace_add_pc without checking that registers can be accessed. I expected it to fail already before calling btrace_add_pc and I added this just as a safeguard. When I actually try to read the trace while the thread is running it fails in perf_event_read_bts for BTS - again while trying to read the PC. For PT, it fails here. It isn't really safe to read the trace buffer while the thread is running and generating more trace data. I would therefore add the check to btrace_fetch and remove it here. We thus allow tracing to be started and stopped while the thread is running but we require the thread to be stopped before reading the trace. OK? > >>> +# We need to enable non-stop mode for the remote case. > >>> +gdb_test_no_output "set non-stop on" > >> > >> This is too late with --target_board=3Dnative-extended-gdbserver. > >> Use instead: > >> > >> save_vars { GDBFLAGS } { > >> append GDBFLAGS " -ex \"set non-stop on\"" > >> clean_restart $testfile > >> } > > > > This test seems to run fine with --target_board=3Dnative-extended-gdbse= rver. >=20 > With that board, gdb connects to gdbserver as soon as it starts. >=20 > In the posted version of the patch, "set non-stop on" was being issued > after starting gdb. The result being, "set non-stop on" was being issued > after the initial connection. >=20 > But "set non-stop on" won't really work correctly after initial connection > It'd be possible to make work, but, currently it doesn't. > The issue is that GDB only tells the server to switch to the non-stop > variant of the protocol on the initial connection setup. After the initi= al > connection, the server continue in all-stop mode, while gdb is in non-stop > mode. This results in errors like: >=20 > Unexpected vCont reply in non-stop mode: > T05swbreak:;06:f0d8ffffff7f0000;07:10d8ffffff7f0000;10:08f9ddf7ff7f0000;t= hread: > p4c0d.4c0d;core:2; >=20 > when you try to run something. >=20 > So I'm puzzled. If the test was still passing, but gdb.log showed errors > like the above, it suggests that the test may need to have its regexps > tightened a bit. The test silently stopped when runto_main failed. With another patch that = is currently in review, this will cause an UNTESTED. Thanks, 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