From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from AUS01-ME3-obe.outbound.protection.outlook.com (mail-me3aus01olkn2150.outbound.protection.outlook.com [40.92.63.150]) by sourceware.org (Postfix) with ESMTPS id 4F2C13858430 for ; Thu, 21 Apr 2022 14:29:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4F2C13858430 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=JTidVqRobBc//nzXb8zeFqSdALd8FSPwCbK3WtpzQW5TKzdrVuhTR7VizT5pTpqxKbSt+u0ot4ENxGGnM/13IjMw89nQbutwvwRRAZuxQda/dEPAea1Q7bdY5iHUHzmkfmkvWtn5C/6pzfgOaAKg5pqQ7nrJseA5AzCHkZ2Kxcb89H5FN6avUxhF75Fk98GMMSY2sCls7fg/uTAjVpsD6gK9I1EdIWX9GeHEpD1Ykho8hSL8QnjcyZmoVu7K7ArhtTuICDOpSO8fQEomNAj/JEtFxdo+GuXqtMI8yJ4kIoqe1w1AZlxCKrlAxAx+WbfVFTCabAipgkyRIS/Gq8Jtig== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=CfD9GaXcZKc/yOFOR3ED5PDSAZjeLhI7M49H31kcmWI=; b=TeS+V0iE0EVRPYFDknAT9nGr0foo4LJPi3N95pablhk/RMFHAbBvJgaRv8uog8MIAL9/zK+y511aCXFFWofx8HQndwlgIBZFFlQ/BS61YQqsJQ5wrj1vPoh0Mjo9z4b0CAlVXrNugsBRmUkGcJHRTH5VT4kjDHtziLOKf+uMqjOmBKk+caVDmtKEFKCLlu2GZ0onNoHxPiSMLj0m0Cvmz0O053754jaNIySz7PelR+SdarWXZn0OdkJEXoZiBqvwhzM2B37dWx9G3ycUXW+SfI/r00v1XD6XQ4o7/KNshJs/ChRE//dUGQBcdj7jd3fOFjFOZHPrw+RFp5a/HS7mng== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none Received: from MEAP282MB0293.AUSP282.PROD.OUTLOOK.COM (2603:10c6:220:71::10) by SY4P282MB2267.AUSP282.PROD.OUTLOOK.COM (2603:10c6:10:f4::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5186.15; Thu, 21 Apr 2022 14:29:44 +0000 Received: from MEAP282MB0293.AUSP282.PROD.OUTLOOK.COM ([fe80::5804:c2bb:bc94:3cec]) by MEAP282MB0293.AUSP282.PROD.OUTLOOK.COM ([fe80::5804:c2bb:bc94:3cec%6]) with mapi id 15.20.5164.025; Thu, 21 Apr 2022 14:29:44 +0000 Date: Thu, 21 Apr 2022 22:29:34 +0800 From: Enze Li To: lsix@lancelotsix.com Cc: gdb-patches@sourceware.org Subject: Re: [PATCH RESEND v3] gdb: fix using clear command to delete non-user breakpoints(PR cli/7161) Message-ID: In-Reply-To: References: <87sfqez4de.fsf@tromey.com> Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-TMN: [bDqye6MZ6sLMINmhmZNnmUrt1Knthu3H] X-ClientProxiedBy: HK2PR04CA0074.apcprd04.prod.outlook.com (2603:1096:202:15::18) To MEAP282MB0293.AUSP282.PROD.OUTLOOK.COM (2603:10c6:220:71::10) X-Microsoft-Original-Message-ID: <20220421222259.7ea899b0@asus> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 1e4251c5-c209-45ca-43ac-08da23a360ee X-MS-TrafficTypeDiagnostic: SY4P282MB2267:EE_ X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: T8L70HXuVc34Jvk0oYBcRqet9vOVWzyNXRyu1ABuuGtUczHL8R1GZbXHWEB8udM1A2Eqo3KjKQ54iYqJmwcBKBnbVVu68UvjSWwjTh/ayfA4nTgoyWN+ND31btCdr1W35+ZCdLWYrkdLjYVhZfOKGbMOPn5LJoXQt4elfm3xfNVZzjUnFfGToa8Jl7s2sReRMcf5OKDrwaKThoeOtyCrI1vDVgSF0KbkQhQxEfD8g0558suOoKUtkhVnF+TvxCfqBWJaNC4j5WmpO+4IU7yXgVN9Ag7V+/kRdxul9jTtQuiojy+6C1S1Sz9uXJfuaA0ku0AMT0AIPWBh8RdpgZFoh+MJ0qXK1Wfn6NRUWnjJdSDlPH93RtgjL16eiJG0cNBfjJuM5mUvSAEgWUvVMLV6uRhlnVHKbRv9V8HaKeZmeRjBN54nPqWwUtzLVS6YgfzijV09JB3GEuAMB5w4NGGynvpz1RI0Wk++jlSDXEUvZ48XSTVkrId+Vo+6IdjV9xLI1fRZQ/3QNEFYDEJ62j4AtXw+DnlhpODFzvg4f5Tmn0tfGf87iEp+o9vqi3zI4xrZhlukornZ2zMvVvIPHnRS7RIiV6Tt5dqYeSN3gUsiLewlcmClNbbe8S10wgoraKRE X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?HbH4MhLSpuYY1jcaM0WO1BViEihmBvFAXkfy7f/LfedsiU3VMsvQFs/aPYS0?= =?us-ascii?Q?nYYtF3At7Jncx/lLoOQ+r+y5T3Fwyf2jVdNxGgEX6E7SuhRQ43U3OhuqC7VP?= =?us-ascii?Q?R3ayDcsRARU5DUwkurkTPpd3jr9U3n7pr/6GQ+GvlKuC3Nw4mU8oy45fYY0H?= =?us-ascii?Q?8DD2fSIC0MTnOPJqFdclvg0I46sRSYt0SLSQRWZigOM8SYvHhLQOSIRW/oZ2?= =?us-ascii?Q?+vQvFqXyJdIeqJrkjLu6eO4KUVgPa43GeVmHdQkYv+NL73/5LvFmLP4ZfM9d?= =?us-ascii?Q?9WX7k843e2BTA0LpzshOQuf9/4OzUsBKEr1ZPaN6NRChKHuRVFnOa+7Yjc7V?= =?us-ascii?Q?/eW31TnTJ4Lsw85St96RfVKgs6SwtOeClbC0qK9eOw3sUqbkBg9RM+NPBm3j?= =?us-ascii?Q?XH2bzQ7gfJq+UxFzP0NThhO9PfGpNN4BE42YmR1ZAjWngsXXNUYWM4sPYjN/?= =?us-ascii?Q?puAb//3EO75YS4K35Chb5qUfFB0e6yWWhJObt2Ye/5QRQuUMMu/I7kLuXKkS?= =?us-ascii?Q?RmHKRUXJX8tD4fQUb6hQSHNVIKf09a6Js8J+E+2hrfD57PYcsY6BvjGvbvSq?= =?us-ascii?Q?M81t/3mTpmDu0mfHSEk71KzNPXapURG4rKjNeLqeJc7l9KUMPL0BKXtqaLK7?= =?us-ascii?Q?drTumY/Aet1irag6oSnwHjUse9xAiGoT48KWuUgMS4bUB3HlTpbkDjh1EpxH?= =?us-ascii?Q?em53N8DCGxQWxYb12n68OL0puwAWVETipC23O2mvs8SiNIy8Y01ITPA0HBNP?= =?us-ascii?Q?2fndo2CFd8Wqwy4Ezrn32yFDYWlFaGvMlXseJ/kycNuFtrqyBfrqxixIhrsB?= =?us-ascii?Q?hf4Kq6psUYng3wXLdTin6/SpdN1qmcOqYy2AwbkJxEeatfCN1NInrFTBuCEv?= =?us-ascii?Q?j36YQr27dS5PuBenOjFK2Ux5L71Pq5nIxz2DnzBi//vTcqW0yaXLtLCk+zSe?= =?us-ascii?Q?HrH6/aY+7fmb+h0bEo1R0aEEkS3G03+mUZ6Ibc7/YdyxBZiZZLwL37fm33pV?= =?us-ascii?Q?cmV+gCEHEKS3sZLTyxrQ3CbSO3/fcX7pt36YFbAiyJhp2/RpCs8knil1FCwS?= =?us-ascii?Q?14wTIyoSTSbr+Nl+MlybhNB1OU/bkQihrE0zwe2slnyLXPnZau9r7ATUlsft?= =?us-ascii?Q?F78H44r/l2d7PTbfem3gIz8W++WIFlM/kmn7CR1ovmL6SHs0eRZ3rg6puF74?= =?us-ascii?Q?1LC7TI1sc17Hk4c5rJlnLT4WFdKf5lOAzjmmcil5MLy7Ge+kKq3UBXtw7ipa?= =?us-ascii?Q?dSYs/GKVq0GyOXkgx6H98dlpeonx7RfMMNn7A3qXamK9LaXg+B3E0oK8yfi7?= =?us-ascii?Q?PF8NGFCZDtq9N/AO7lnQmFXyJ6B+4uNKL0dF/qrVkNNwRk5AxXjbTeOvLZg1?= =?us-ascii?Q?Z1Caire0JVs8lYGUsd8V77pg2NfwjQLXgam8jMTG0E72yz/D59guupmutyiO?= =?us-ascii?Q?6l6iYGHscX8=3D?= X-OriginatorOrg: sct-15-20-4755-11-msonline-outlook-746f3.templateTenant X-MS-Exchange-CrossTenant-Network-Message-Id: 1e4251c5-c209-45ca-43ac-08da23a360ee X-MS-Exchange-CrossTenant-AuthSource: MEAP282MB0293.AUSP282.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Apr 2022 14:29:44.2340 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: SY4P282MB2267 X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Apr 2022 14:29:51 -0000 Hi Lancelot, Thank you for the review. I'm very sorry for the long wait, I dunno what's going on with my email. I didn't receive this email from you and gdb-patches@list until I happened to see your reply on the gdb-patches web page[1]. I copied the content from the web page and replied to you here, please see below. [1] https://sourceware.org/pipermail/gdb-patches/2022-April/187959.html >Lancelot SIX lsix@lancelotsix.com >Sun Apr 17 22:29:29 GMT 2022 > >Hi, > >I think Tom already OK-ed this patch, but I have minor comments, if >you do not mind. > >> + >> +proc get_maint_info_bp { var } { >> + global expect_out >> + global gdb_prompt >> + >> + gdb_test_multiple "maint info break -1" "find address of >> internal bp $var" { > >In this proc, the `var` param is only used in the messages, not in the >actual command. Did you mean "maint info break $var" here? > Yes, I didn't notice the problem at the time. >> + -re ".*(0x\[0-9a-f\]+).*$gdb_prompt $" { >> + return $expect_out(1,string) >> + } >> + timeout { >> + perror "couldn't find address of $var" >> + return "" >> + } >> + } >> + return "" >> +} > >> + >> +standard_testfile .c >> + >> +# This testcase just needs a "Hello world" source file, reuse >> +# gdb.base/main.c instead of adding a new one. >> +if { [gdb_compile "${srcdir}/${subdir}/main.c" "${binfile}" >> executable {debug}] != "" } { >> + untested "failed to compile" >> + return -1 >> +} >> + >> +# Start with a fresh gdb. >> +clean_restart ${binfile} >> + >> +if ![runto_main] then { >> + return 0 >> +} >> + >> +gdb_test "break main.c:21" \ >> + ".*Breakpoint.* at .*" \ >> + "set breakpoint" > >If I run this test, I have: > > (gdb) break -qualified main > Breakpoint 1 at 0x401020: file .../gdb/testsuite/gdb.base/main.c, > line 21. (gdb) run > Starting program: > .../gdb/testsuite/outputs/gdb.base/clear_non_user_bp/clear_non_user_bp > [Thread debugging using libthread_db enabled] Using host libthread_db > library ".../lib/libthread_db.so.1". > > Breakpoint 1, main () at .../gdb/testsuite/gdb.base/main.c:21 > 21 return 0; > (gdb) break main.c:21 > Note: breakpoint 1 also set at pc 0x401020. > Breakpoint 2 at 0x401020: file .../gdb/testsuite/gdb.base/main.c, > line 21. (gdb) PASS: gdb.base/clear_non_user_bp.exp: set breakpoint > >So you already have a user breakpoint inserted (which also happens to >be at the same address as the breakpoint you want to insert). I am >not sure this adds much to your testcase (but I might be wrong). > >Also, I do not know how maintainers feel about this (I am not one), but >I feel that using gdb.base/main.c which is not directly linked to this >testcase makes it easy to have main.c accidentally edited with this >file not being touched. With a plain line number, we might end up with >unexpected test changes. I really do not expect this file to change >any time soon, but who knows, an update in the license header might >lead to the line you are looking for not being 21 anymore. > >If you really need this breakpoint, I guess I would go for something >like: > > gdb_break [gdb_get_line_number "return 0;"] > I also noticed this "Note" message a few hours ago, and I was thinking of deleting this piece of code, which is actually possible. >> + >> +set bp_addr [get_maint_info_bp "-1"] >> + >> +gdb_test "maint info break -1" \ >> + "-1.*shlib events.*keep y.*$bp_addr.*" \ >> + "maint info breakpoint -1 error" > >The last param is the name of the test, not an error message. If the >test was to fail, the output would be: > > FAIL: main info breakpoint -1 error > >"FAIL" and "error" would be somewhat redundant. I would suggest >dropping the last argument altogether since, if missing, the framework >uses the 1st parameter (the command) as default value. > >Also, I feel that both main > >> + >> +gdb_test "clear *$bp_addr" \ >> + "No breakpoint at \\*$bp_addr." \ >> + "clear internal breakpoint error" > >Here, I would suggest just dropping the " error" part of the testname. >"$bp_addr" is likely to change from one configuration to another, >making diffing .sum files difficult. > Thank you so much for providing such detailed instructions and advice, it was really helpful. I'll cc you when the next version is ready if you don't mind. Thanks again, Enze