Message ID | 20161129150758.29912-1-simon.marchi@ericsson.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 11944 invoked by alias); 29 Nov 2016 15:08:33 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 11899 invoked by uid 89); 29 Nov 2016 15:08:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00 autolearn=ham version=3.3.2 spammy=involve, HX-Exchange-Antispam-Report-Test:22074186197030, HX-Envelope-From:sk:simon.m, Continuing X-HELO: sesbmg22.ericsson.net Received: from sesbmg22.ericsson.net (HELO sesbmg22.ericsson.net) (193.180.251.48) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 29 Nov 2016 15:08:22 +0000 Received: from ESESSHC008.ericsson.se (Unknown_Domain [153.88.183.42]) by (Symantec Mail Security) with SMTP id 34.AC.03096.2E99D385; Tue, 29 Nov 2016 16:08:19 +0100 (CET) Received: from EUR01-VE1-obe.outbound.protection.outlook.com (153.88.183.145) by oa.msg.ericsson.com (153.88.183.42) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 29 Nov 2016 16:08:18 +0100 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=simon.marchi@ericsson.com; Received: from elxcz23q12-y4.ca.am.ericsson.se (192.75.88.130) by DB4PR07MB394.eurprd07.prod.outlook.com (10.141.236.17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.761.5; Tue, 29 Nov 2016 15:08:16 +0000 From: Simon Marchi <simon.marchi@ericsson.com> To: <gdb-patches@sourceware.org> CC: <markus.t.metzger@intel.com>, Simon Marchi <simon.marchi@ericsson.com> Subject: [PATCH] Prevent turning record on while threads are running (PR 20869) Date: Tue, 29 Nov 2016 10:07:58 -0500 Message-ID: <20161129150758.29912-1-simon.marchi@ericsson.com> MIME-Version: 1.0 Content-Type: text/plain X-ClientProxiedBy: SN1PR02CA0038.namprd02.prod.outlook.com (10.165.224.176) To DB4PR07MB394.eurprd07.prod.outlook.com (10.141.236.17) X-MS-Office365-Filtering-Correlation-Id: 0887589a-2ac7-4b1f-590e-08d418698c4f X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001); SRVR:DB4PR07MB394; X-Microsoft-Exchange-Diagnostics: 1; DB4PR07MB394; 3:LBVyqi1J+YyI+arGzaTlFTWNCCjIA+gM+JBqQcdhfu77dV0Pmx8pbUPX5aY/cTjI2gEdCQgaWR57uJRej57yVp3F4E4+v7j1ToqpU6BmyPSDLzeLWj856IxQUD60xwkmj2zaplmpYmXs59T9gBtrZqCVdmlV0S6MUJ199fCRa4hA3sVVSZBTn650wyVNseNBn5qu5NaaTxJD0b1cTpdZkJXUS3SsrRIndKy/2xbr/G66yNOpzjmiK1hwmZ2mL8RAtrCe3QP4r7u3iW4+IGxh2w== X-Microsoft-Exchange-Diagnostics: 1; DB4PR07MB394; 25:rZb0/NQ8CsqRmAdAbfPxgDjo212+a0T6lDHY6alICjDvr3sawvajj1vwMxjNp4/CezX8Xb55JiNI3j8FfZeLJKTqmdb/GLz9Nw8KMGz59yOicRD6VJIxMkMi5XtDY14XKyRiMS9+1aH+Vx+KUYM6hBpv1p/OThybHdyBqRa+L4Av3c4ybZ3YCTXBePUAMIhzjjWJGbTCzyPC6tnbSR2JuDMGsSgk4ip9N3wKbEP9VoHODFx1QQTLghkDCpsU7Hvd9DLW020TguQehIVhybOPDY6qS6Bu2B+Rb8Qk1d+RrlejSjxYKTcWykWD3KJUbv8YLRBXRAQs1A/rwdm6jUl6IM68VQJbVUV4gva3pjNw1U3WCxzn9EY5cgCW3L878t8kgjKaCmR6YkgV95PHhs113CYBkMqXOlWzQBN60/3hQgHn9UyYmO8Q5HIF1hXm7BllmR13H3IOxTopSqGho6LFjbSfimuTgsWO1SwcMoMZV3VeGq732En8f8yZmLlvukIwiYp2rSl5SvJVbKzXbl9nQ8kF2svAjoAoa3Vs/2vopewMjS4Wv0h5q7Shw+FD+7oNrb8UeU6fkgfKfC/B9vvC9lm3FMEasD/5DPKabBXXsUDifiUMSH+rhpDw3ZvuXG70kJkkEqae/36uLgxdTpORY/cl/jCgQ9CNv7Hi7iX204U7d+mI4H+ujzFutF0yiOZgOSqPTqPz0e0tVyt+MfPfTB1vlNAVCdLEsMu6hPeKn5R3kZqtnhTWJhKNopmKiq+fTO7fsfuwR7J1ZpWaWMZ1BNxtprERCPL7kxEWWN0P4kxZRU8wFOtR10oiXh959ZRG9a+3SnsXDuT79NnZkFqvpOc8QIq7wuhgWnpXOFOTY1U= X-Microsoft-Exchange-Diagnostics: 1; DB4PR07MB394; 31:xujnNdenPcLAQey+tvbtG8gBGu1tHlrhOzJiVQCrFhIrPSf7tmvVAhj92hhMxmo8GiA/BmmGeE1ncB/fckAAHEf9f3koERnxj2WleoVFl6TO/rQ3cIbb4mW13wo1MTmq8q/MBhbu172NXbBm61eiXI/jUbT4xmpoocFsaCXWley9usPXGiCjLI0VvuvvxD4jHtkz2CySVWCD7IYgGR4IKo7Sk/K53aXCx0G5ZaB3L7mMBXDtc10ju/ioGD2cqwlVS675Xx5tcpFV2+rJCLB9SQiYfa44udHI9HjuX+pBggRnAjGm/Cht1Oi+pDzqZMG0; 20:twJTZxmjSBj28/pYd8SAgHmqSv6EkBpphaI0rRPVmvDzzrXpPHGXp/mD/RGqAG+Ca2Q5ZfiAQZLnKQrvhE2En9Vi/vjZyqbPX61XnI3itbbxPkn+AKSSx4zM15oCrz7KJn1811DH35763IZgruO6/3Bu1ZqcoHL2AkbQ2hePm//00B8Npw6bfoCRyWW086BUUNoTvr0rzhuhx5ZFoqT5bpmucy+gzI2taKEG3f+NiFlycNe+BZdiHK7RAtXWr/y0cEQ/F3uTOsttE7OvL1vloehf4y4sa0j9H8+VLhLWnkgur5fWqfF4m/dZGuVIdh8QHlweloMoh+rSQGyGXb+KI8Kceu5CPnAoJG2bT0tsn/2HzFBk1Hm3lw5QzlJKPp+UmXxa4dBbRC8k30lofYTAt9Fm3yokGidCNVy7BAQtv8GF5VIamCDBS2BM1NZCXVCIlKnX42LDsu3JQpt7JTZ2t5sEy1ZlfKv9iu3ZK7NchXcfwnE44hMZ1XWe/8aFdK6O X-Microsoft-Antispam-PRVS: <DB4PR07MB3943CF10395BBDD0977673CED8D0@DB4PR07MB394.eurprd07.prod.outlook.com> X-Exchange-Antispam-Report-Test: UriScan:(250305191791016)(22074186197030); X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(6060326)(6040361)(6045199)(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001)(6041248)(6061324)(20161123564025)(20161123562025)(20161123560025)(20161123555025); SRVR:DB4PR07MB394; BCL:0; PCL:0; RULEID:; SRVR:DB4PR07MB394; X-Microsoft-Exchange-Diagnostics: 1; DB4PR07MB394; 4:cWzOZw7Ah6I75aZ64j90Krv59SKGbo4XgrYoSt7gQu/wpfSJbaneBakEOL30eEo9VMIaQGWmiUfhDUufHNK3pwjzd2413EzBvwIIicSwZT0pSl+hA1A/kqaPqb1CdcKulDpzSgUdBAGprte7+u8dJI5UdL2tQrAPACXBlQCwK09Ojlx/Ciind2EkZiwUpTvy/4M0PPFuz92rwbXbSqKb1vXD8l31PNhQ+ZWN3aWxP3YEb8Lv5Gs6oM/6yX+QqcJIu7+WR/TQ6tdGV4bPYYwTQdGGAfAr1u9CokmJjZaPYWC+6ej1XkCUu6LyHmM4j9fmUDSaXTOAA7079PYkrKZwr+wxZ3K/h7pW+g79RGOa/zhOJdfwv9Z/szKfcIk/vM1wv9TErp4zjGsrTcDvjHvYC6Nd1YBR8rWqMSnM9GpcPNAYI3izU+l8OLqZ8lKT1D4XhAlMqTGhlCrkM29nRdqN+UZAuE+aHGQ+1k4rjG/7igDXsJUyzowu2z19hpP/2pOtzVLpE8BqYKvVQvLnhuFZ6qeWwz5NRX9nBj2XA3paRufYK3/C3gOiz/tB1POHkDndD5pDCjPz7kuR8kcVHCxRBfGrdUc9j+1P3ZbaKnXl3HrYZtFRApFczccoGP8/zjH4ixHD7HZRYPUPajAtI/jfuv4NZpj1FCg4F4PrBD2dXzwg1mKE+/fcwx4MnJDExT6O X-Forefront-PRVS: 01415BB535 X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(4630300001)(6009001)(7916002)(54534003)(199003)(189002)(33646002)(305945005)(86362001)(6916009)(6666003)(4001430100002)(66066001)(48376002)(107886002)(6486002)(50466002)(733004)(47776003)(7846002)(7736002)(575784001)(39450400002)(8676002)(81156014)(81166006)(36756003)(39380400001)(39410400001)(39400400001)(110136003)(38730400001)(97736004)(5660300001)(50226002)(106356001)(50986999)(6116002)(2906002)(3846002)(5003940100001)(189998001)(92566002)(4326007)(1076002)(42186005)(105586002)(68736007)(101416001)(2351001)(2004002); DIR:OUT; SFP:1101; SCL:1; SRVR:DB4PR07MB394; H:elxcz23q12-y4.ca.am.ericsson.se; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; Received-SPF: None (protection.outlook.com: ericsson.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; DB4PR07MB394; 23:3qjXlop/AiVLsVd2F0t9QYHDsTxvbKOU586U6orFxY?= =?us-ascii?Q?oobdu9qEIaXvv8u5uw+Dv6/eP9s/lkOZ40dvYMNgrU+joVH9KqdpbGN4sMuv?= =?us-ascii?Q?s7AyulefznJX9M00xDl8bhMQkvAoaCudJ1YVNx7IeZpDUGCOFYKy5/Y8QaLs?= =?us-ascii?Q?P5BZ1Sas8xnSMrCJIBTQEPaM9uIDLVRZJLOlC6DJDrAieDsBXDF3RUisN7hD?= =?us-ascii?Q?6gKHt6/oLKTDSJg0mD88T+Wkr3C/M2fR6b8wmJ8hn90ULl+rcCEFcKlINFg5?= =?us-ascii?Q?gzKZzx97KXhxxkJ/9aYBgmUshuJ9fjujn0fI156/rdAHPACfAYNbLuu+Il2h?= =?us-ascii?Q?8KjoFUoc/FuQwLDM57bRieLU++SZjgOIpQVXEgpDkXoNIbmcQbKJ2NXHTe63?= =?us-ascii?Q?KXTcSgr6301imi6Nk4sKzCFC+GY0vqoboFIDmyhTuuiwPTi1YtvHKCdfBhlB?= =?us-ascii?Q?lmsHTp4et/7Au3JERXrag1je74aNNWJfSauA0DwtKppWKDQLSYJ6LWiau8iB?= =?us-ascii?Q?0aS2uw8tFXuHpPoqocDSVRE01TKyYYEVez2QIB29APxmEYRc9/pQ7HqZycwq?= =?us-ascii?Q?RrErkd6wpKNUljjimyvzyH0APnKGDGIhUH4/q4bVqW++4HiFCMmKVzJkyJj4?= =?us-ascii?Q?f2C+NTWYe0ek5wAQFAMlVbp+05+v6qSCc2y7trXxcGzPG4Uv0eGc7yjvPZZ5?= =?us-ascii?Q?ItHfWYcX1U5kCXrNTdVUFCW5HnQP0qHbojgcbOep7YKTqOmbm2OPClMzfszK?= =?us-ascii?Q?SFVv4xWQ3ayudhQ2s56oibOkkmsb69lpE3o4ZHGrFI+io6Y6V+yN/g844V1L?= =?us-ascii?Q?qgrBMo4IX7T1CNT9ZxF2DIsoANGZvMyP/QN3pgyzVDKaJDWupmzAazie9wHk?= =?us-ascii?Q?+FIWLm3rCM8gUDsBqb3h1gj0J0jp9XYDdrw2tBlxqEKNawkdDsDyWIf9Yvf7?= =?us-ascii?Q?v9WnrEUjcQe+wtarcOP1Y+tk/5wHumf3jhs6rPE2knwLTLbwkTg0cBFDxZu+?= =?us-ascii?Q?n8dyTuntfkWcEiz7lZHlIZHg+cyCfmJkeblqpOeO8r8H+UCAK+GlZsK3ISd8?= =?us-ascii?Q?0bjZpfVkwkQxFkTkhW2NLO4omR5W8XAfjKfIoRArpjmqCPDeJqUZ9LSmEOLG?= =?us-ascii?Q?JPcSRJzSeitE6jNbF+YbF6olyPgDzhB1JlI2GqaGclkmgDV+qHVMlgoiO1sz?= =?us-ascii?Q?OG5xdgsPQlTSlPtL/w2mCwU6LRoWoAI/SalngiTiLtL/5VJgfwRI/bepkewc?= =?us-ascii?Q?Z3VWwBE+QAG6Vwmuk=3D?= X-Microsoft-Exchange-Diagnostics: 1; DB4PR07MB394; 6:0qWnak+r90Ml6qoxYJQ/0t3Lz4AcBwhW9LnaNGoT3siVDiRZ47saZCijLWAhdGuK/HcBbTIYkNIi1jTw17AKw9x0iuyYyCCAVH3DHj+xaOlVbC/S9MGA825FNHAwj2kqPIfywa5ljyP4eWO6EJ0LMIMvFEOxkXOq8VN1jayIC2h3bjfOSxSNYfRcxQhEduTYB5PgJxx0UUxqXpC6lJiTAXhJIghMY8nVk5kocROHheKuXM1ywegnR69EwSQ65m/lFefDucuyfh8BgsByVs7Ik/I8JorGkjzALm2UtHlbu1J0vWfVYG9Wa+uAWPU/OYEMM6Wui0Z7sE1GTftlQC6bA0IGCBzriV2+r6854FUF8JL0LA+on2u64ez9OO6F/Y0Cduq4XX0DCo2rg6diWBVbbdzTTHt+KRfZg6B7ga2+JoqGsNU54DmjeUqm9ld04iaFFyrRSQgrcuzp97Nci82O6g==; 5:3NMocjsq7LsLMIr4CvK/l1qywehB9dKdwb16pguJG49QZv4fsVpikO2UrsJFymP9Kq37MmWXFSDMzc7NgFYS+AjkQ0bsM/zdNMfNkryM/xUbdaHk4DuhiE83UcR9w6cN6ZPA1hBciB1S/dPIE/cho5o0ZqPP6XLyK07gi5x2sUI=; 24:l64vhkVnmyR8MP8OuZT1Q+9/Ll6XpP/nZUtv6KGQmo6k5Fjt1xD5/voaeqxXjUtDIVL9RDUmEnisU7ZjZuS8RlcFyxnUapDEPX2xu8R4tOU= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1; DB4PR07MB394; 7:4aX1Yv5YGmpggoHbeqod/e9mmrlgRDoIQSxawUHh1X0InDkkticbSw2SPzdBI/AVRA7Ta7cYRlj8sqy/TRN5ZLjMDOtcGmTHVmtoq07XAcBQBt7SMzPOFt6ZVDq3dnVwrlMjJyBqKNKJBae+IRP1CBGbhvz2OjaBT1XebT4bAHtjA2E50FVKKvijeu2MbNOT/b2HRk54yxqPWuzqFjuT6sQNR7ZScZpCCrpwN4nFAC7rEAxPdufwdEve9lMOFpjAUXjADRdxAkejPZwykaaaGn3lSXzziR/L8PIBB+l14ygDIAA4wJkx0I2EPg5gCZDVIDoX3MhlYpFA4MI9RvK1HaJdx1rh5yVYqZxFUQE7SQGJsPOysL6sbXXCOn8+zj4HnajtKn+emcwz31/j8zlrbXMyyu2JI+jfpNBQBIPd5hX8R/o6zrB4lDFyghnRdrjxxFM5ma9aLE/B7iXDuobrhg== X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Nov 2016 15:08:16.0807 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB4PR07MB394 X-OriginatorOrg: ericsson.com X-IsSubscribed: yes |
Commit Message
Simon Marchi
Nov. 29, 2016, 3:07 p.m. UTC
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
Comments
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 <ctype.h> > > @@ -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 <http://www.gnu.org/licenses/>. */ > + > +#include <unistd.h> > + > +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 <http://www.gnu.org/licenses/>. */ > + > +# 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.
On 16-11-29 10:58 AM, Luis Machado wrote: >> +if ![supports_reverse] { > > Add an explicit untested call here? Right, adding: untested "reverse debugging not supported" >> +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. You are right, it helps when reading the test. The command by itself doesn't convey why we are using doing that command. How about: proc_with_prefix test_record_while_running { } { gdb_test "continue &" "Continuing." "resume target" gdb_test \ "record" \ "Can't enable record while the program is running. Use \"interrupt\" to stop it first." \ "switch record on while target is running" } PASS: gdb.reverse/record-while-running.exp: test_record_while_running: resume target PASS: gdb.reverse/record-while-running.exp: test_record_while_running: switch record on while target is running I added proc_with_prefix, I think it can help by giving some context to the messages. Thanks for the feedback, Simon
On 11/29/2016 10:42 AM, Simon Marchi wrote: > On 16-11-29 10:58 AM, Luis Machado wrote: >>> +if ![supports_reverse] { >> >> Add an explicit untested call here? > > Right, adding: > > untested "reverse debugging not supported" > >>> +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. > > You are right, it helps when reading the test. The command by itself doesn't > convey why we are using doing that command. How about: > > proc_with_prefix test_record_while_running { } { > gdb_test "continue &" "Continuing." "resume target" > gdb_test \ > "record" \ > "Can't enable record while the program is running. Use \"interrupt\" to stop it first." \ > "switch record on while target is running" > } > > PASS: gdb.reverse/record-while-running.exp: test_record_while_running: resume target > PASS: gdb.reverse/record-while-running.exp: test_record_while_running: switch record on while target is running > > I added proc_with_prefix, I think it can help by giving some context to the messages. > > Thanks for the feedback, > > Simon > The above looks good to me. Thanks, Luis
On 11/29/2016 04:47 PM, Luis Machado wrote: > On 11/29/2016 10:42 AM, Simon Marchi wrote: >> On 16-11-29 10:58 AM, Luis Machado wrote: >>>> +if ![supports_reverse] { >>> >>> Add an explicit untested call here? >> >> Right, adding: >> >> untested "reverse debugging not supported" Shouldn't it be "unsupported" ? > The above looks good to me. Agreed. Thanks, Pedro Alves
On 11/30/2016 09:36 AM, Pedro Alves wrote: > On 11/29/2016 04:47 PM, Luis Machado wrote: >> On 11/29/2016 10:42 AM, Simon Marchi wrote: >>> On 16-11-29 10:58 AM, Luis Machado wrote: >>>>> +if ![supports_reverse] { >>>> >>>> Add an explicit untested call here? >>> >>> Right, adding: >>> >>> untested "reverse debugging not supported" > > Shouldn't it be "unsupported" ? > It does make more sense. But it is also untested, because it is unsupported.
On 16-11-30 11:11 AM, Luis Machado wrote: > On 11/30/2016 09:36 AM, Pedro Alves wrote: >> Shouldn't it be "unsupported" ? >> > > It does make more sense. But it is also untested, because it is unsupported. > I also think it makes sense. Changing locally.
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 <ctype.h> @@ -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 <http://www.gnu.org/licenses/>. */ + +#include <unistd.h> + +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 <http://www.gnu.org/licenses/>. */ + +# Test that trying to turn on recording while the target is running is correctly +# handled. + +if ![supports_reverse] { + 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." +} + +test_record_while_running