From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 52sNKjFjiWAEJgAAWB0awg (envelope-from ) for ; Wed, 28 Apr 2021 09:29:21 -0400 Received: by simark.ca (Postfix, from userid 112) id 9BB101F11C; Wed, 28 Apr 2021 09:29:21 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,MSGID_FROM_MTA_HEADER,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 768D21E01F for ; Wed, 28 Apr 2021 09:29:20 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id DAC0E385701E; Wed, 28 Apr 2021 13:29:19 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DAC0E385701E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1619616559; bh=SPZoesM8WDa4xrmHKOSldF6kEWt1kDZIF/pz3bovkNw=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=GJ0tBZjoEYuR4q1Wzw9CE1MvlAIoXm+FgWnwGUDHtUuTEl0GXpYEGnhUzxPXWcASM 6OfNbblzWcvv3qBtB3Hx/1OxyYur8Y5qot7UscxlWq4Sz72n+q89y2fI3ZPkwvpcQm tH8HIFX6tnA5neS42QorEeYS91UfSivXNkHCz6kQ= Received: from NAM12-DM6-obe.outbound.protection.outlook.com (mail-dm6nam12on2043.outbound.protection.outlook.com [40.107.243.43]) by sourceware.org (Postfix) with ESMTPS id E21803A71C38 for ; Wed, 28 Apr 2021 13:29:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E21803A71C38 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=OjOkv7FNuyVgOctrbTn2vk0AxxWI19CdTD0XIxNZzBd8y7DiZiBrdUGrZmsiNw/B1TrxZjGhq/vlulS0Tp2THLFkYkZEKmDjooqak0ZxxaTw/IZVRFYzfGNL37sVNHwplyDJTsqJrNw3pa3+Jx9OamtStMp0Ws4EPBVffABawG07Pcigb6ANyaNuuvXrM479L2LAeO5V8M24gc+fp2Wxd7eJRDpFfn9g8+UqubVq8zRgnM6aj5AWuf5XjitTQ4bwjnuN92LSvv4FH7OYw76Mqk3coj7V/UgoLmdoGQ3kQbDFLn7djZgILcUoB3YCd5HmVRSZYQlBVahitDV34ecgZQ== 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-SenderADCheck; bh=SPZoesM8WDa4xrmHKOSldF6kEWt1kDZIF/pz3bovkNw=; b=fQ7ARVopvyjywF6BNnM/YnfpKkgjGUdsGPU/GfudjC0GRxDyBrOXWkdCzRmXOlol+xbOu8YQe9SESn5DUh1rnNfGVMS0iW9Jt3N/Jx5V34hmwDCiQ+xHHnNNHrHUDyXkKfN1VC9BBExVeISASzAqEAPLLumOOxel33vvASkbiWLaJ46sj6Ur/YWcJtoquLkvYb/eqLwoBSR8ORrWq5WwNJ2tCFOeGRZ70o4p+cpO94TYosDE1PF4Wjy/GHKz04118WNvEMmI2Lg3rF5q4DKQm9v6anyoUxX2VRfFUzeHew3wiZK9qfb+F/RaTaWc3HBNN3eT9q66Gztd600ZcP6Pvw== 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 Received: from DM6PR12MB2762.namprd12.prod.outlook.com (2603:10b6:5:45::15) by DM5PR1201MB0011.namprd12.prod.outlook.com (2603:10b6:3:e4::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4065.25; Wed, 28 Apr 2021 13:29:15 +0000 Received: from DM6PR12MB2762.namprd12.prod.outlook.com ([fe80::49d0:1ee5:47ef:e0e5]) by DM6PR12MB2762.namprd12.prod.outlook.com ([fe80::49d0:1ee5:47ef:e0e5%7]) with mapi id 15.20.4065.023; Wed, 28 Apr 2021 13:29:15 +0000 Subject: Re: [PATCH 18/43] Add new register access interface to expr.c To: Simon Marchi , gdb-patches@sourceware.org References: <20210301144620.103016-1-Zoran.Zaric@amd.com> <20210301144620.103016-19-Zoran.Zaric@amd.com> Message-ID: <438db2e3-c39c-311a-a915-e13843085f02@amd.com> Date: Wed, 28 Apr 2021 14:29:10 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.9.1 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [2a00:23c7:5a85:6801:f5b0:44e8:2b3e:76c1] X-ClientProxiedBy: LNXP265CA0066.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:5d::30) To DM6PR12MB2762.namprd12.prod.outlook.com (2603:10b6:5:45::15) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from [IPv6:2a00:23c7:5a85:6801:f5b0:44e8:2b3e:76c1] (2a00:23c7:5a85:6801:f5b0:44e8:2b3e:76c1) by LNXP265CA0066.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:5d::30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4087.25 via Frontend Transport; Wed, 28 Apr 2021 13:29:14 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 2957af0a-7799-4eef-e7dd-08d90a499e43 X-MS-TrafficTypeDiagnostic: DM5PR1201MB0011: X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: jAUWj5vZhClhQSz1kaXSeYTn1qLoAZEF5+JdFcT4uLPQoZwj9t3qlicXlws3s+P0LLu8NHHohfhUNfqVzVsSP97gNMC5lwjnZ+g+XnAPJYPMaKhSNBXOTllfcW4mqTTogybkCWKYyW2pd8RgZNk64RHZ9VhnqRH7DcjC75SEc9Nz35nqYoJxPhB+0ovhYTeHqzEGkqcP0lmj7mU+xUT0xj2FJhugCnzZAUrgK76wzMY82/ue7aGFoZbGEKWMoZyIrZWAWJbv82cK3Ms6qROW5Lt72t3lIzx6G3iZdSEgv5YNhIM5JgHubd6AeKUdAdxCR+Wu5XVxagzgsesVAR/xsg5pxKHqQWXZc9/K3/3zkV6LE4m4j2H+qkBHcobKH8My5zfieEmZiKAmivvjj30ecT/RCF7JdIXHvUkUp2F+cpPHkKR5OCOybCCanWzsv9pCwPTuqXpaZX8BVANtuIrlEx9rAB3dK87saFk5Sp/UVrDxO6NRS9AGxM/Vg6DPV3bmFdCYRMV2QrkKjqJLUhAUhbvIXr4EIdg1rMJsOJukIeexznlilXib7bp5Qjue6yiKx6YRGYUSKCpOszHLXIOciowuPDES5Y2aMqMCkSjLPwc6aHN/puR7l+rRRaAOmx+SdFmXv5yy03gWGbv86OPfKCCM0ZsNWuSfO4B5tdSHYkxAOOkym8Rc4RQpzXiGh6hW X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM6PR12MB2762.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(396003)(376002)(346002)(366004)(136003)(39860400002)(8676002)(5660300002)(16526019)(66556008)(31686004)(66946007)(478600001)(66476007)(31696002)(6486002)(86362001)(52116002)(83380400001)(8936002)(316002)(36756003)(6666004)(2616005)(53546011)(186003)(38100700002)(2906002)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?WEo5VllRNnEvZ0RZVDlSeS96dGFFMmRKU1kxUW1iZlo4ZDN2U3MrbE5HR2h2?= =?utf-8?B?YkkzRitkMVZkZUk3cjhVNFN3akRickYzdjB6aUtXeGFpbkZBSXd3L2VhcURm?= =?utf-8?B?LzZVTUFBUUYzL0hxZHdFb2w3REhiY3NHaWlldjkwRmMrKy9wWCtMcC9zVURP?= =?utf-8?B?NGhsVytnVlVJbWZKVFBjZHdmM1EzdUx1OTdnRHJYTXpNMmVrZTFZS2NKc0s2?= =?utf-8?B?eXh4NWFsTVFyMGhmNmx2V2tRRnhOdUx3bzBVQTdLdXppc1RTRHpXaVdlZEth?= =?utf-8?B?L3NINUovUFRic2duS0hpdFRxSFhxdVNmS2oyZnc0WTFvckxtVDZoS0hDY2gr?= =?utf-8?B?bHBJaDcxaUF1ZFdYdHI3SGxDbG1ISDRKZ1RUR21JYnB6VktWWEVDblB4SjV2?= =?utf-8?B?QUpUZkxJYjNFUDYya1pTc3hWcW1IY2R4ZFlyQVhBeDJNYWpWY2tjb2MxODNC?= =?utf-8?B?WUhkbHJOb3N2bnRuVXpQY1Bkb29RMVM1S1NhVnVabDhaQnJEaHhkUlZkenVQ?= =?utf-8?B?c0w4TGxURjFhTjE1Q1huUUhLK0FCWHZOU29pVmF4QXRLNlh5ak9CM0tVVW1z?= =?utf-8?B?S0t4ZWpXYkQ2NXdmaytlck4wdTBPRm8xdGY5ZFpqZ2RibHY4TjRWM0VOelBK?= =?utf-8?B?ZnRKd3UrZlBLOFVIK2J5UkptV2RuaDdrVE4rMThPWk5OSGR0MGtqRm9FeTVB?= =?utf-8?B?ejI2WkxJWVlRZlBuZFdHVEJ4NWtzWG56R3NXaXBNTWVMMnZzN3NVbWxQMmh3?= =?utf-8?B?WXFPbG1MczdQSlBwb3hNeVRMQytVUU0rRFVtT2prYnhONE9iazBobzIyMjU3?= =?utf-8?B?MDkwUzB6cGlUdVNCNTBabHFuR2FyMzFLSS80V0hCUVZlNk0xc0c3WUlCbzVz?= =?utf-8?B?WEVmODJGbGZIaGdkK3ZKV3JIMjhqU29GRGFSS3lIZkk0NE45WEl5aC9aQUtr?= =?utf-8?B?Sm1OSFk5d3h5NXpReldWRHd2YjlXT25reGM1K0orZFIrNFd2OXJDK0N3eGZ1?= =?utf-8?B?RUg2VzZDTFJJYTJ5NnE2Z1RxcSt6L2txbnZ4ZFpQa2oyTzlXSWxTNllGU1Nw?= =?utf-8?B?UmtZeHZTWGZRMk5YSXlMN3Z0QVVGeDZhNjNPMzNuUnArWUFmenNvQ1BrbUFi?= =?utf-8?B?T2ovYkhXcUl5QncvVmFreEFrcDhidENyNmZkQm4xcjdwOXhwWnBZMEg0Qy9w?= =?utf-8?B?am02Qkc4Nk0yUWFoMkx5MjJ3ZXhoVythMXQrVUNxazVDRjQ4azV2NFFXUTNG?= =?utf-8?B?a01uUU5sUWd3U1VUQmtnVmNSd2Z4bmdibGREdlRHMnM5UzdCL0xsOGJMZi9i?= =?utf-8?B?VUw1aG5BK0lydDBjdjBHWnpvMytLNEFhTDg3aGQraXlZaWZqb1Y2c0xzdkpy?= =?utf-8?B?dUZwUk1xQkkydnlHbEkvMW5lUjVwN2RwNVFiakNnR1F5cW5RYVpTM3lHdmR5?= =?utf-8?B?SVorSFNPSzZmTVIzTGw4NFZZbWNxRy85QTBxZkZXREVsMC9PM2lyVmVKY2k1?= =?utf-8?B?bjJlbVlnclZUSmtrTm1aSUFpTnUycFAybUEvL1QydmlXamt1M2QwbU0yVUdI?= =?utf-8?B?YUo0R2FtN0VFb2JXTHQxc1hWUjB1enZ5NWtWZUVlbHA0bUlxOEt4VDA1SE0x?= =?utf-8?B?U3lwR1pXOENZSGtQaVNWMC9jdWRsQW1uZ1l1bkVQTGxhZks0QmpkZ3plUUJW?= =?utf-8?B?a2pvTnBSbENtcldEMkM4Q2F4dnFHNzAzUnNrT3ZzeERQeU1CaVJpU3kyTENw?= =?utf-8?B?eFZhZDNMeU92WVVydDRNZkxZa0NMVlVuakdUdEp1Y0o5cjZRTmRQUjd6V1Vk?= =?utf-8?B?UDFGZEc5ZTYyVExRZlhLc3pNQVAwUDVmS0ZnTjFrcEVVTCtPRGtaenAwbE53?= =?utf-8?Q?9d+/FnkdWXncl?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 2957af0a-7799-4eef-e7dd-08d90a499e43 X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB2762.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Apr 2021 13:29:15.4746 (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: r2i0EJmTQ7+DLf7227HvBqALbHGtwbWGX4aN1LfFIvi5/tF3Lzc/u+7fntBT0tJq83xDEarXJHVdvhve81LwgQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR1201MB0011 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: , From: Zoran Zaric via Gdb-patches Reply-To: Zoran Zaric Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" > > On 2021-03-01 9:45 a.m., Zoran Zaric via Gdb-patches wrote: >> DWARF expression evaluator is currently using get_frame_register_bytes >> and put_frame_register_bytes interface for register access. >> >> The problem with evaluator using this interface is that it allows a >> bleed out register access. This means that if the caller specifies a >> larger amount of data then the size of a specified register, the >> operation will continue accessing the neighboring registers until a >> full amount of data has been reached. >> >> DWARF specification does not define this behavior, so a new simplified >> register access interface is needed instead. >> >> * dwarf2/expr.c (read_from_register): New function. >> (write_to_register): New function. >> (rw_pieced_value): Now calls the read_from_register and >> write_to_register functions. >> --- >> gdb/dwarf2/expr.c | 128 ++++++++++++++++++++++++++++++++++++++-------- >> 1 file changed, 106 insertions(+), 22 deletions(-) >> >> diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c >> index c50bb3c8d90..5a1fd5b941f 100644 >> --- a/gdb/dwarf2/expr.c >> +++ b/gdb/dwarf2/expr.c >> @@ -106,6 +106,96 @@ read_addr_from_reg (struct frame_info *frame, int reg) >> return address_from_register (regnum, frame); >> } >> >> +/* Read register REGNUM's contents in a given FRAME context. >> + >> + The data read is offsetted by OFFSET, and the number of bytes read >> + is defined by LENGTH. The data is then copied into the >> + caller-managed buffer BUF. >> + >> + If the register is optimized out or unavailable for the given >> + FRAME, the OPTIMIZED and UNAVAILABLE outputs are set >> + accordingly */ >> + >> +static void >> +read_from_register (struct frame_info *frame, int regnum, >> + CORE_ADDR offset, gdb::array_view buf, >> + int *optimized, int *unavailable) >> +{ >> + struct gdbarch *gdbarch = get_frame_arch (frame); >> + int regsize = register_size (gdbarch, regnum); >> + int numregs = gdbarch_num_cooked_regs (gdbarch); >> + int length = buf.size (); >> + >> + /* If a register is wholly inside the OFFSET, skip it. */ >> + if (frame == NULL || !regsize >> + || offset + length > regsize || numregs < regnum) > > The last line is missing one column of indent. Thanks, I will fix it in the next iteration. > > Can `frame` really be NULL here? Given that where write_to_register is > used, we have: > > struct frame_info *frame = frame_find_by_id (c->frame_id); > struct gdbarch *arch = get_frame_arch (frame); > > If frame was NULL, it would segfault in get_frame_arch. This is a good point. I will remove that in the next iteration. > > Can regsize really be 0? This was part of the previously used API so my guess is that it could :) I didn't want to presume to many things if the previous code handled them. > > I don't understand the code and how it relates to the comment. What > does it mean for a register to be inside an offset? The expression > `offset + length > regsize` checks that the end of the portion we want > to read is beyond the end of the register. But there could be a part of > the portion we want to read that is within the register. The code might > be correct, but the comment needs to express the intention more clearly. This is not allowed in DWARF. The whole length needs to be contained in a register. If only some parts of it are then the location needs to be a composite and that is not covered with this low level interface. I should probably explicitly say that in the comment in the next iteration > > Is `numregs < regnum` really useful? When would you encounter that? > > Simon > This check was part of the previous interface as well. Maybe the interface was design to be conservative or maybe there was an use case where compiler generated something that resulted in an invalid regnum? Better safe then sorry I guess but, I can remove it if it doesn't make sense. Zoran