From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 72386 invoked by alias); 29 Nov 2016 15:58:50 -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 72363 invoked by uid 89); 29 Nov 2016 15:58:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 29 Nov 2016 15:58:39 +0000 Received: from svr-orw-mbx-03.mgc.mentorg.com ([147.34.90.203]) by relay1.mentorg.com with esmtp id 1cBknm-0003nX-7Q from Luis_Gustavo@mentor.com ; Tue, 29 Nov 2016 07:58:38 -0800 Received: from [172.30.5.15] (147.34.91.1) by svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Tue, 29 Nov 2016 07:58:35 -0800 Subject: Re: [PATCH] Prevent turning record on while threads are running (PR 20869) References: <20161129150758.29912-1-simon.marchi@ericsson.com> To: Simon Marchi , CC: Reply-To: Luis Machado From: Luis Machado Message-ID: Date: Tue, 29 Nov 2016 15:58:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161129150758.29912-1-simon.marchi@ericsson.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) To svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) X-IsSubscribed: yes X-SW-Source: 2016-11/txt/msg00960.txt.bz2 On 11/29/2016 09:07 AM, Simon Marchi wrote: > As stated in the bug description, trying to do "record" while threads > are running leads to a broken state. I tried to see in the code if > there was anything to try to support the use case of enabling record > while the program is running, but didn't find anything. > > When full/software recording is active, we are single stepping each > instruction. Transitioning from the state where threads are running > freely (no recording) to recording enabled should therefore involve a > step where we stop them to initiate the single stepping. I can't find > anything that looks like this, so my conclusion is that this was never a > supported use case (please correct me if I'm wrong). > > The easy solution is to prevent the user for enabling record if a thread > is running. > > I also wanted to know whether it was supported to start btrace bts/pt > recording with threads running. When I try it with btrace bts, it works > halfway. "record btrace bts" gives me the error: > > Couldn't get registers: No such process. > > If I do the command again, gdb doesn't complain and then I'm able to > interrupt the target and use reverse-next. However, since the > initialization function was interrupted halfway, I am not sure that > everything is setup correctly. If we want to allow it, we would first > need to look into this issue. > > I have therefore put a check for running threads in record_preopen, > which affects all recording methods. It can always be moved to > record_full_open if we only want to affect record full. > > No regression on the buildbot. > > gdb/ChangeLog: > > * record.c: Include gdbthread.h. > (record_preopen): Check if any thread is running. > > gdb/testsuite: > > * gdb.reverse/record-while-running.c: New file. > * gdb.reverse/record-while-running.exp: New file. > --- > gdb/record.c | 11 ++++++ > gdb/testsuite/gdb.reverse/record-while-running.c | 29 ++++++++++++++++ > gdb/testsuite/gdb.reverse/record-while-running.exp | 40 ++++++++++++++++++++++ > 3 files changed, 80 insertions(+) > create mode 100644 gdb/testsuite/gdb.reverse/record-while-running.c > create mode 100644 gdb/testsuite/gdb.reverse/record-while-running.exp > > diff --git a/gdb/record.c b/gdb/record.c > index 34ebd1b..e0bc133 100644 > --- a/gdb/record.c > +++ b/gdb/record.c > @@ -26,6 +26,7 @@ > #include "common/common-utils.h" > #include "cli/cli-utils.h" > #include "disasm.h" > +#include "gdbthread.h" > > #include > > @@ -89,6 +90,16 @@ record_preopen (void) > if (find_record_target () != NULL) > error (_("The process is already being recorded. Use \"record stop\" to " > "stop recording first.")); > + > + iterate_over_threads([] (struct thread_info *tp, void *) -> int { > + if (tp->state == thread_state::THREAD_RUNNING) > + error (_ ("Can't enable record while the program is running. Use " > + "\"interrupt\" to stop it first.")); > + > + return 0; > + }, NULL); > + > + > } > > /* See record.h. */ > diff --git a/gdb/testsuite/gdb.reverse/record-while-running.c b/gdb/testsuite/gdb.reverse/record-while-running.c > new file mode 100644 > index 0000000..f00ceb6 > --- /dev/null > +++ b/gdb/testsuite/gdb.reverse/record-while-running.c > @@ -0,0 +1,29 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2016 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 > + > +int > +main () > +{ > + int i; > + > + for (i = 0; i < 30; i++) > + sleep (1); > + > + return 0; > +} > diff --git a/gdb/testsuite/gdb.reverse/record-while-running.exp b/gdb/testsuite/gdb.reverse/record-while-running.exp > new file mode 100644 > index 0000000..57e1df3 > --- /dev/null > +++ b/gdb/testsuite/gdb.reverse/record-while-running.exp > @@ -0,0 +1,40 @@ > +# Copyright 2016 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 . */ > + > +# Test that trying to turn on recording while the target is running is correctly > +# handled. > + > +if ![supports_reverse] { Add an explicit untested call here? > + return > +} > + > +standard_testfile > + > +if { [prepare_for_testing $testfile.exp $testfile $srcfile] } { > + fail "failed to compile" > + return > +} > + > +if { ![runto_main] } { > + fail "couldn't run to main" > + return > +} > + > +proc test_record_while_running { } { > + gdb_test "continue &" "Continuing." > + gdb_test "record" "Can't enable record while the program is running. Use \"interrupt\" to stop it first." I have mixed feelings with the above test names. I'd know what to look for in case of failure, but more explicit test names wouldn't hurt for a quick inspection of the logs. "move thread" "switch record on when thread is moving" Feel free to pick it up though. Not a hard requirement.