From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM11-BN8-obe.outbound.protection.outlook.com (mail-bn8nam11on2040.outbound.protection.outlook.com [40.107.236.40]) by sourceware.org (Postfix) with ESMTPS id 1FF2D3858CD1 for ; Fri, 14 Jul 2023 18:55:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1FF2D3858CD1 Authentication-Results: sourceware.org; dmarc=fail (p=quarantine dis=none) header.from=amd.com Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=amd.com ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nR4UKwuShcUZ0GSoVINHeTHOA0e8TadWDCOsRl38xbS83DEUO3MooWsbLbWz6VVSVUxwB7SuC8IAAWyqf4WM/nG76XPvW48KZ+Ln+w2efJtEp4feWDj40ULjMyFZPjNcCOiBT6/7u+gxG6TCnwnvhG9OpBjcK1PQu9t5rPBe7wtQKsoDTydMmbh0z+YNdAWJoll5oaOB9iHV4tMYt6Lwg1KEdFLrt7TXfjAGM9olVEGgTETYzviWyWMIKcu9cg+FqDcHeJ1v11B7+pph1G9bc85Dy+pM/jQu+2xLwkpLGNNBrMSx4j/nrBlCLyqkm0eojo7yJfbwiTrGhdWYcOmdIw== 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=G9N1A6LArDe2E5YmyMZ2gB/aAFYLJmOPPSxOVvBNV1o=; b=jCavjGqpGYYRO+dEmcCmtufvbi1QfXher/25yDgQon2mZnsl9mzVulEIIxzDy6/73hepssnuSAMARclNys7QB1x10o4AbntIyLHzynp+sNxLLAxV1/Ky/FuatVe0DsUvPXJHmJzx3i9BhnW6Pcbpfd9gQDYcnEpfQFPumnRzroTYcCayV2mOmUFIJqrj8fkQ8VnKN7gyVrmxbLWZbJcco0Qk5/Qb/4RRARv5/0zVna/BYIW1IIrBQArkoBA9mI3rG/9/lJTTafCOwwKDosU0GC+kp6Odlyj4FPrrZ5IWIkj+430veCTnGdlTdZrL0ZI+L2nAoMxYMxD2WEbWRoHVng== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=G9N1A6LArDe2E5YmyMZ2gB/aAFYLJmOPPSxOVvBNV1o=; b=ZHXcN+nPW25SCnabnhA8UIwYCpmNiktwDIA7nvpVigwtG5z8Dkb7U63j+37iv2vqAxRi8RsBpb+cESkn9LcrV1jMkOPZebjjUeXxwitrv2FSL+v+2LxPjNXeHiepRSndbhabNLsOG0MDGftevUR5XZT9+JdpZ+rqsdOne9Oszo0= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from CH3PR12MB9079.namprd12.prod.outlook.com (2603:10b6:610:1a1::9) by CY8PR12MB7244.namprd12.prod.outlook.com (2603:10b6:930:57::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6588.28; Fri, 14 Jul 2023 18:55:39 +0000 Received: from CH3PR12MB9079.namprd12.prod.outlook.com ([fe80::5ac8:fa2c:e6ad:84d5]) by CH3PR12MB9079.namprd12.prod.outlook.com ([fe80::5ac8:fa2c:e6ad:84d5%4]) with mapi id 15.20.6588.028; Fri, 14 Jul 2023 18:55:39 +0000 Message-ID: <05217693-1a01-bffb-e74a-503b3fe3a604@amd.com> Date: Fri, 14 Jul 2023 19:54:42 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH] gdb/amd-dbgapi-target: Use inf param in detach To: Pedro Alves , gdb-patches@sourceware.org Cc: lsix@lancelotsix.com References: <20230616092528.69358-1-lancelot.six@amd.com> <2c7f2ef1-dea2-5dbe-8d3f-b9b885be3b72@palves.net> Content-Language: en-US From: Lancelot SIX In-Reply-To: <2c7f2ef1-dea2-5dbe-8d3f-b9b885be3b72@palves.net> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO2P265CA0495.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:13a::20) To CH3PR12MB9079.namprd12.prod.outlook.com (2603:10b6:610:1a1::9) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH3PR12MB9079:EE_|CY8PR12MB7244:EE_ X-MS-Office365-Filtering-Correlation-Id: acf3b6d0-feb3-49f4-e792-08db849bea0f X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 89m+ekKnHpV7TsbcCZpbb7K5WE2XD1SIxXIik3/8sPbTjNnq+yS+lJIpk5rWeELaMN6DqCMeidsTR89mxBRnE6hWHZ+TY2sDZ3Imzvc3O7oHECwB0tiLRVgSBNnvWv9plFEBRB1TLbUW2KjFhMXUBIq1XvzwGjqRZAvqdaL9GbJsBoP3sONA0RnVpVGXwTvfKKxymI46L9DqxpA6GNYBOLyh7Y1r7dFsoSVmsY7EOJD/f6638eDewaymEDUIiiLZGu2JseWUq6HznQPKjgWZDHZyQqZZoGCnu957Z0bD6x4yp7jROlKp/9BMMEwnxZ0VhIDwuRTbftHxBrRCdCiVRQ26Wc4bYfrykpvbSfTXBkuRQUGUNi8zOJEk//S1/qcIs0KY1iLMkrlBcqqLBldm6W7NKZf2DZ0BrZGTKeZ7hSOpL+lZQY8HTT2AZ6ZsHvwA70odXCXE2NGgz8r8A11A/QMKVTIT48didafPABu17vjhIZzbwNESlnVlD6R2UfTCu2ADeCaBpdFY7k6vvWhw6xCeeBFEmRveFn09f8OtKQhH3Xme6ytxHHaIOCbHBPYP7auEySPbsYCiRwKj9UeblqEdAMZR05Vi4UtsWKJ5GhKvRbmHYlwbJ6oOxu+Cd+oiLW5Jpu9GILIMhiSsaINXBA== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:CH3PR12MB9079.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230028)(4636009)(376002)(396003)(366004)(346002)(39860400002)(136003)(451199021)(5660300002)(38100700002)(41300700001)(8936002)(8676002)(316002)(86362001)(2906002)(31696002)(6512007)(53546011)(6506007)(26005)(6486002)(6666004)(478600001)(2616005)(36756003)(83380400001)(186003)(31686004)(4326008)(66476007)(66556008)(66946007)(43740500002)(45980500001);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TzNzNk0yL3JVVmh6bml2TEE5QkFhUHNhd29QSE1RT2FsZmxUNXFoRkRWL0VH?= =?utf-8?B?SnNnMDdIZ1VVMy9McTRBRldDOTFVa3BmVGVBR2ViY1I3TWh3SExVeTJiNUZP?= =?utf-8?B?ZFVNaFVhM1ljNVZHSHVaQ2FJRlBEV00rdVo4Qm1qdEd6bXBTcExuNmc4dVV6?= =?utf-8?B?MSs4ODQvVlhpRTBSaU1NQ2tUVkJOYkVIMHRoZHpOT290dDN1enBaV1VhUXNM?= =?utf-8?B?T0hDQk81MUdINTNRcWJCR0EzRjVWRjQweHBDUHJUMFpjd01Pbm5MenM0c281?= =?utf-8?B?T1NNcit2R2xnWEhyT3VzbkJuRm9mcG9jbDd4anBkS290a1BKRG5DU05HUEVm?= =?utf-8?B?V3dtUlExd0xHSXVQeHFUR2gvcytKelJ3S2krdFB5bDc2YXJaaHJJRXRQVDRm?= =?utf-8?B?SXo0ZDh3THhZM01Hc25IS21KSGloUnh5azBTaEZKdHdDMCs2WEkvWFNhTG5F?= =?utf-8?B?K2JMM2VjWm5JQS9rVDllVkdBQXdSSFg1Y1p0d0VOOVlKclJoVzlUU1lsK3RZ?= =?utf-8?B?RVlvdFh1L3VCYjY2Yys1SnZTM085aU82ckdWK1ZQeHY1cmIyZlpXcVZBOGxF?= =?utf-8?B?Wm9JMkhJd21jSWRsVW5XbGMrajcrVmFvcFQzL0pDRmt6S25YaDBVcnpYU0pO?= =?utf-8?B?WnVMY2tzK0N0dG9xdU9ETmJwODZKc1B2Q3Q2anYvRDhEOEI2VTV5L3dvM1pU?= =?utf-8?B?V0x2dzNVRVRsR0JuS3FNUEV6ZnliRkFFc2RkQUNya29va3c3dGJPcmNwaGxs?= =?utf-8?B?YlZESk9iUkxoeXhHcmV5SUgrM1RSa2ovV1pic0pSL0xrelM3WWxzTXBFRkwv?= =?utf-8?B?cS9IZVN3RmdnTmpMQ1ZWSjFJSDZZOGVlUVh5amNRR2hMYVZqQzFFdnZjeDZ3?= =?utf-8?B?a0piYWplejNVY2dSYVJJQ3hsOEluN0k3WGd5TjV1S0FyM2xlaVRuNU5WOVZ2?= =?utf-8?B?K3JacDVEdkJwc1pWMnZlN3l6Q0hROW5iMjZ5aWpHcHMxM005SDMrM1ZKQXQ1?= =?utf-8?B?TDFSU0ZFMmFsUmlhRzVQZ3ArVjZBUmpYT0R1YkVWbWNNdk1URFo1OHQ5Z2F1?= =?utf-8?B?UmNwWWNDTUpWSHc5N3p3WElETmhBTDFGek5uMlFkdVFZMGFrQ1I2WHh0SUpy?= =?utf-8?B?Zlg1NGZaMGZJQ3ZaTTR1SG90S3hqcytFSnU0cmlDN3dMWm9QakpDVEdDK25L?= =?utf-8?B?QjRzcnl3V0hHV0VyYWZoeHZqMWhqNEpNdDk1K3dLYjdzeEhaZVlEeHduSGFE?= =?utf-8?B?N3IzT3A3Z082TE9EVFhNT3l6YXdsZjdlVEpROXlOQ2NFekw5R0M0Q2trUnA2?= =?utf-8?B?dFBEaXVQZ2h0ekI3aWtRM2FmTUhUUUtHbmViREhucmtGUnVITVdXcTZVMmdM?= =?utf-8?B?eEZZbWJWUUNVLzhsZGVIVHI1ckVpazBlY2hGRzVpbWxjY0pQTHZndmU2ajVY?= =?utf-8?B?aEtsTGNpWGd4RHNDZ2doYmo5em1HL1NhWUErWTU2WmNSNlBZZEEzQnZqUFlv?= =?utf-8?B?ZmFoZGFPV0dlemwvcWkyZTdBMzIxMXlaZFRVRG5DYXlqL1VkSTZodDMwcVBX?= =?utf-8?B?ekNNYVYrVW5lNUlRdlA5eS9JRys1MFZEUTlicWUvbFVkTllVRkd1Vi9yM2Zh?= =?utf-8?B?YmppaUFkU3RrUEhQRHhFbjl6SjBIWlp4ZlJDM3lMT1FxeE5MaitOZkdFK3Ra?= =?utf-8?B?QU5QSGNGRWt6cTUrcWJ1TVZIeXNFV2R6RTZ1WmVDQTRFa0pQSXBWdzVtVXZV?= =?utf-8?B?YzZaZlRRclZiVndXcktNL3RlempEN1dhV1c1bWJ3ekM2WEpvRjJ4eThVNHpu?= =?utf-8?B?T1hjSFhDdGtuU2JTV2RyMzA2ZDB5VzFuenI5VWRJcDJGTzFmNDBrUEJBbkpn?= =?utf-8?B?Vk9abjZJTFA5bi95aHlJazhUb08rdmhNQzRWV1dkSUR3aXFkUHhmcUIrd2ZE?= =?utf-8?B?SHY1V3E5TSt5eDZvVFdXUjNTMjRrREo3RnpyK1hiMytvNnFYTGVaTEUxZ3hM?= =?utf-8?B?Y3c0clkyQVVYTkI5cUFUNFBxdGdmT3FCc1FYZlN2RGdnT1NkYzY2cTJTY1dv?= =?utf-8?B?V2VQeUlFVEEvSlpob04zdUY1ZFZCVGIrcDBWRzZ6VHhXbmt5NTAwZDl0bFFK?= =?utf-8?Q?x4r2qjMR77W3qpj6pNbIKsbRh?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: acf3b6d0-feb3-49f4-e792-08db849bea0f X-MS-Exchange-CrossTenant-AuthSource: CH3PR12MB9079.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Jul 2023 18:55:38.7092 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: pyU35ppSjwI+zV2mUvVXpmVkhYDg5GvdpH+dnqT8RjnfRpoAfdwXjmEzA1czRpqOn9kgXFeQVTOdGAw2VGlryQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR12MB7244 X-Spam-Status: No, score=-4.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 14/07/2023 19:35, Pedro Alves wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > On 2023-06-16 10:25, Lancelot SIX via Gdb-patches wrote: >> Current implementation of amd_dbgapi_target::detach (inferior *, int) >> does the following: >> >> remove_breakpoints_inf (current_inferior ()); >> detach_amd_dbgapi (inf); >> beneath ()->detach (inf, from_tty); >> >> I find that using a mix of `current_inferior ()` and `inf` disturbing. >> At this point, we know that both are the same (target_detach does assert >> that `inf == current_inferior ()` before calling target_ops::detach). >> >> To improve consistency, this patch replaces `current_inferior ()` with >> `inf` in amd_dbgapi_target::detach. >> >> Change-Id: I01b7ba2e661c25839438354b509d7abbddb7c5ed > > IMO this is a case of the target method's inferior * parameter having > been added too soon -- it would only make sense to have it if nothing in > the body of the target method implementations is relying on inf being > current_inferior on entry. But that is not the case, plenty of target_ops::detach > code has that assumption. The presence of an explicit inferior pointer should mean > that detach target method implementations that call code that depends > on the inferior being the current inferior, should be using a > scoped_restore_current_thread/inferior before calling such global-state-assuming > code. But, they don't do that, instead, we have this mixed situation. IMO, it would > be better to remove the parameter to avoid confusion and stick to the > (if explicit param, then switch global state to it if you need it) rule. > > Anyhow, your patch doesn't make it worse, so it's fine with me. > I can also go the other way around and always use `current_inferior ()` instead of the `inf` parameter in this detach implementation. What bugged me here is the inconsistency from one line to the next. Lancelot.