From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR03-AM7-obe.outbound.protection.outlook.com (mail-am7eur03on2081.outbound.protection.outlook.com [40.107.105.81]) by sourceware.org (Postfix) with ESMTPS id F2C363858D28 for ; Wed, 30 Nov 2022 10:19:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org F2C363858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=WzW9kJ34l77OzvYpPIcGegbpkBpC0AQBDIESI+0tP2JpGTxdkZFoG+mMSvRVidJSXgBjM/euxupknTCth15xdixYeAJZxl5F2IvEGA0s1hFaUwa8FdVdAq62ik5sLa8x4VprLILdd4Vb/t42Cy+zQHDoTx+JM845STwW8B8+huWxvfx7KGVtexK7ugRJhxotKZgzoXab1Da7I5Bhe5L6DPffiTHgz6vbxtrjN8bBVl92D2s8VWZij7+fYKgw9pwIkAzpsoP3w2aNtAaY6ZqTb1FUZnNPVr/PMVABFcprd6DQ7Y1d50MNHxxsQ5aXBW/Q+XJT6Gz71iPSdP128qrNew== 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=vaOi+vnm25cAD0r6sag0YIStToei0gIRsYK6UAi93v8=; b=GGnPm183iQYRAwKmONSM4uR5zuaqWfo8V6NlJjtJrNt9Gf/XffCkGxTJDBFIyB1qRP+cUNSE0WTBSKqbynm/dWbyJYv3ly5KLjfk3fW/6AvUK5tEBExhPs5N5I+uJkuv4TQBtGPm+/tdGkZxtBzE2IQwl4pmxqg3mfXBwAY3MT1UWq9/HOEUWrcCdZ6mfTav6a84FZCpbBMtnBsKu0L3xm494RqnDXBQlKnMUucqgQQR1+sNVOvX7ukPscpYpDFRRmLoxIwhj/UzZt7uQZ3iOvFX1ZqcOPzw8Ex8Z2KqmblZiAN9N4S4s+zLP/KYPI4RSteqzaTrzueOnEhMgL5UPg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=vaOi+vnm25cAD0r6sag0YIStToei0gIRsYK6UAi93v8=; b=UCB7PKqLSZUO9g2HcVPJe6Xsya8QTmB9g6UqXpzK4oiQT/x+Zu1Ipf3fRm7oZ3HqKy7XhoahEV1xRi1DRkrDyOY2zJJwcrKrR8IkRAvUHq9DsYcRbhIzRCbHvgaD+scslkhVBXr14RsflgLWvttyDgindJYFYoyFKcU+rrNajaE= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; Received: from VI1PR08MB3919.eurprd08.prod.outlook.com (2603:10a6:803:c4::31) by AS2PR08MB10054.eurprd08.prod.outlook.com (2603:10a6:20b:649::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5880.8; Wed, 30 Nov 2022 10:19:11 +0000 Received: from VI1PR08MB3919.eurprd08.prod.outlook.com ([fe80::fe5c:b195:a2ad:b19c]) by VI1PR08MB3919.eurprd08.prod.outlook.com ([fe80::fe5c:b195:a2ad:b19c%4]) with mapi id 15.20.5880.008; Wed, 30 Nov 2022 10:19:10 +0000 Message-ID: <12c60e71-27ae-62ae-ba15-5e4e9df40507@arm.com> Date: Wed, 30 Nov 2022 10:19:09 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [PATCH v2 3/4] gdb: dwarf2 generic implementation for caching function data Content-Language: en-US To: Torbjorn SVENSSON , Tomas Vanek , gdb-patches@sourceware.org Cc: Yvan Roux References: <20221118155252.113476-1-torbjorn.svensson@foss.st.com> <20221118155252.113476-4-torbjorn.svensson@foss.st.com> <4461f93e-b29d-80bc-3249-04cbcb94be99@foss.st.com> <66d62a68-fc39-5288-1ee2-63c03b85ba47@foss.st.com> From: Luis Machado In-Reply-To: <66d62a68-fc39-5288-1ee2-63c03b85ba47@foss.st.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-ClientProxiedBy: LO4P123CA0327.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:18c::8) To VI1PR08MB3919.eurprd08.prod.outlook.com (2603:10a6:803:c4::31) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: VI1PR08MB3919:EE_|AS2PR08MB10054:EE_ X-MS-Office365-Filtering-Correlation-Id: 23ef6cb6-e094-47df-75ca-08dad2bc5244 NoDisclaimer: true X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: FgTGVEoyditfEW/oTUQkjaiGO0So+56QrolmbkqfeMwSFhHGn0Li1uglDNUuuEwxO6oAP7S+fbLk6IjMItKX+HF4R4XS+5YXU4i3l6NB4+BoIP8bz3pKCRW4Ao+z6HLorAmLHDt6yHLISz2WUpWcVSrLVn19zcZAqpwmMZ3TL44s8uYGE7LPrCaIBKpCIkNrwa2zJ5ohsMS1o9gM0gvpcAiILJgLPDcl3V+V0XAWwGYIAjHGBzds2NYj4Ux6dN7S48p0MRKz1Q52KJHi6tmRU8gz1wE+bKcsNB/gfHJc7aFVlokP3TeihQ28mtFT7fsQ9a/ZxooPkNUNzFSe92m64+hOFI24Dn0Iay/E7fV1ON3jqudE2fwSb2l1ZCcq2nMcAn2pJdmmM+sz2rGpDtOCdWS/x099ly/8HtRhktpmnWpkK2bB3E08NxuisCfh+Osj+XDBsBkY4/dE1rXkPh04Ik9i9mS5rlQ5CLen7I/i8uqhZrcQQaNc7+5xOXmMGx0PNTVjFK+FxKTPz9lDAMov670vmQdJZbAuIraTkye2tv0cRkxF4vJVhskXRB/4GNtf/1+ZfwtRJWgIZJt0umZg3aNCBUzxwuaSJl1yntyj3+uhN8wQTFfqah4qu560H9wE2p7czvTmmy0C74HbRxcxE6mI5aLzplM4r2oRmv4KtcORSBQirVv2mNQiGTzLn4Rcg+dRcKIdFsSfMLE3Sn5UV55EtECccbDxsR8HSw4sitktlozmRO5tvHUtXYdJ61Bm X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VI1PR08MB3919.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(4636009)(136003)(376002)(39860400002)(346002)(366004)(396003)(451199015)(8936002)(5660300002)(66574015)(110136005)(316002)(26005)(2616005)(186003)(36756003)(8676002)(53546011)(4326008)(66556008)(66946007)(30864003)(38100700002)(83380400001)(4001150100001)(86362001)(44832011)(6512007)(66476007)(41300700001)(2906002)(31696002)(6486002)(6506007)(31686004)(66899015)(478600001)(41533002)(43740500002)(45980500001);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?c0VuK3I5U0ZnSFJlVjlsUUdESzFWdkp5SVduTnFxZC9xOHFxbVk0QUZVMmJ2?= =?utf-8?B?Q1VJZ3lod0R3YmJrQXpHS1lRcFYwQ1R0RGpOeDJpV1lEWVBjVXNSbUpiQWhI?= =?utf-8?B?ZzUwWWx1bFAvbGFpN3JoMHFRTGNIcWRRUUVzM1h5UFFqelFLR3VvNnpKdnZB?= =?utf-8?B?dGJOaXhRZzEvcTdWN1IvYW15Z1I4YkppQ3NXdTRUS0pDN01KL0grNEplSkZv?= =?utf-8?B?SStEcE53Y2hwcUdPOFRvd1pjWEFjbi9LU0pycmdBNzVqZG92Ui8wNkVZU2VH?= =?utf-8?B?V0Z6M01URW1KTWMzQjFvMjNCVE84dSs0U09zUDUyeXE3NWNmOEFSMkJoZEQ3?= =?utf-8?B?ZzlEVC9namVnWjJUZXN2eWllakg5NUJwbVRTOVJnWlJWWmRMNWlHVmlpYUdQ?= =?utf-8?B?MGFOcjE2enZuZWVyOEQ3RU5pbGd5L2ZuVnV3Nkt4dldPQTRqZTlCQTlPUnAx?= =?utf-8?B?NTA5QXVTU1JyRXhtYXJTQk5HV2ljNm5CRlMwUzkxUHlrN1BkSkROblNsTXVy?= =?utf-8?B?T0loRFZ3a2x3RzRiOVIrc0drQ3JjRVV5ZUpndjhENnJpREVraGhubGF1MzYz?= =?utf-8?B?Qzh0a3dDTkpRZndaQ1IyVm9RQ2pwd2F6OThOeHNqcm50TlMrTDcvVmhoTFRC?= =?utf-8?B?MFhtcGI0L3JSNFhQZjVFU2VkM0YvN2hDVDRIQXVLb3lzVVNHMjJwS0lFODZF?= =?utf-8?B?bGJndTFwaGZqN3YyNFVFdWlJWmVPVUN4cHRXY2szK1JDRVh1dXBDVGdoa0ZD?= =?utf-8?B?ZE5tRzMvZ2ZncFRGUTBRK29RL3lHVUNIUEUrbldZMlhrTTNpWGxqUVhHLzQ5?= =?utf-8?B?MzUrbXIwN3FjZERNKzBUTjJ6eFFTL21IcE5vRWZ1a0xmRmxHSmpjWENPOVdh?= =?utf-8?B?RW1hMWNXNUJzVXQ5WnZhcXNXdldHd01ZcWNGd2M4Q1loZW9PNVg3UUJ5a3RQ?= =?utf-8?B?SGQxdmlVWVZCclU5WDNQUE8xSkM3ZFVBZWo2eUxnTm9kOHFZYmw5cjNkMnZQ?= =?utf-8?B?aUtqbW90TVBUYU1vTjJxZ1VZYlZzaUN2eGJoTnNoQnhNUzlYYW1yVUVnRXZT?= =?utf-8?B?cVRuNS9oaDJRUVhZVmNpZlhpZllFVUJldGpNNEhmbXNxVnR6alJubkVQbmY1?= =?utf-8?B?NGlHclJQTHJlS3BNeDY5WmkyejJVdVczVlcwU0FxYzVVMGJzWkJJZmNaT3RW?= =?utf-8?B?eGV3b3BhUmpCR00vMmNhQTVURGFZZkhyc09pNU1tWXNXKzJpVVdHckdDNEw3?= =?utf-8?B?S3VacXRWeVBRMXVkVTgyVkROWjI1blFsTCt2SGNTN0NqelE4bVlHc2tRcldo?= =?utf-8?B?OCtDYXBldmFLUzNnNnpPYzdSUTZhWDJ1anpvaFBnNkVmWEVlMEJSeHpNKzZa?= =?utf-8?B?RUo1Tm5RK0h0UlRZWEd0TmoxZjdkVTN6ZWRKOE5DbG9HTzFTMXN0Tk5xZzJ2?= =?utf-8?B?VmRjcnJwZVZnTFJQanNhSzBQb0FWcERhUVFyekVHcU5vYzNGMm9aRjI5L09k?= =?utf-8?B?K201SmNhUVVVMi9zVlhLN2s1ZUsyaTBQU2Q0MTJabzhsRmJnN3ZDOWNWY2h1?= =?utf-8?B?YnhEWXZEdVdIMkh1TXRaQlE5bDRQMFBwVjJuTklXdHhjelRsS1hsQUpZeVQw?= =?utf-8?B?ZlBqbUxVdUFRRkNmcWN5UjMwN2tkdWdCekhKZE5yNmkxdXZrMHEvcDBHa1B5?= =?utf-8?B?ZkN5UnRYYTNVcStEUTluZFM5M0ltM0VySTlaV1FyeitNYldlOXlwSGRkalp4?= =?utf-8?B?b0JLVnphZ255R1dzZUt4V2lwZ1dZNS9UZDBmRjU1SFc0VlFMRUZuZEdINU91?= =?utf-8?B?SjYvMHdXTnJySDlRVndtWHhRN21PNEhMNDdML1FxVWZSa2c0dGF3TUVGRVhz?= =?utf-8?B?K2ppSXVmMkF2QktSMHhXZmxwZ0NXMzRnTWh1TDk2RE4ra3pzblhVd2xqc0M5?= =?utf-8?B?aENSS01sNDVEbnd1b25rQXhhYW9LTUdBVllZakJjT3BwKzhvWUp5akVORGFO?= =?utf-8?B?MEVpK01aVFZBdk51TnZ4VXFUZmpPbzdkcjYxVHVtTCt2WUVQbEVaWkRpTUo0?= =?utf-8?B?cVdmUFNObkRjUjJYb1MrN2FBL0lWWXlnRmI0eldYek94RGwwclVDTW4rYUh2?= =?utf-8?Q?YvH8QFJ24KEU3inunoaDBL7sB?= X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-Network-Message-Id: 23ef6cb6-e094-47df-75ca-08dad2bc5244 X-MS-Exchange-CrossTenant-AuthSource: VI1PR08MB3919.eurprd08.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Nov 2022 10:19:10.7561 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: 7vy7f9xusnPxYbFqPH6a1eAfmD7X/vONvipx7kyWouHdhV8EHfnY24dZuP3g3XIp7YG2n3mVqxLwiC2arX6vgg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS2PR08MB10054 X-Spam-Status: No, score=-10.2 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,FORGED_SPF_HELO,GIT_PATCH_0,KAM_DMARC_NONE,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,TXREP autolearn=ham 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 11/30/22 10:16, Torbjorn SVENSSON wrote: > Hi, > > On 2022-11-29 17:24, Tomas Vanek wrote: >> Hi Torbjorn, >> >> On 29/11/2022 16:19, Torbjorn SVENSSON wrote: >>> Hi, >>> >>> I've had a long discussion with Luis on IRC regarding the points mentioned here, but I'll reply to the list now in order to get more eyes on the topic. >>> >>> >>> On 2022-11-21 22:16, Luis Machado wrote: >>>> Hi, >>>> >>>> On 11/18/22 15:52, Torbjörn SVENSSON wrote: >>>>> When there is no dwarf2 data for a register, a function can be called >>>>> to provide the value of this register.  In some situations, it might >>>>> not be trivial to determine the value to return and it would cause a >>>>> performance bottleneck to do the computation each time. >>>>> >>>>> This patch allows the called function to have a "cache" object that it >>>>> can use to store some metadata between calls to reduce the performance >>>>> impact of the complex logic. >>>>> >>>>> The cache object is unique for each function and frame, so if there are >>>>> more than one function pointer stored in the dwarf2_frame_cache->reg >>>>> array, then the appropriate pointer will be supplied (the type is not >>>>> known by the dwarf2 implementation). >>>>> >>>>> dwarf2_frame_get_fn_data can be used to retrieve the function unique >>>>> cache object. >>>>> dwarf2_frame_allocate_fn_data can be used to allocate and retrieve the >>>>> function unqiue cache object. >>>> >>>> unqiue -> unique >>>> >>>>> >>>>> Signed-off-by: Torbjörn SVENSSON >>>>> Signed-off-by: Yvan Roux >>>>> --- >>>>>   gdb/dwarf2/frame.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++ >>>>>   gdb/dwarf2/frame.h | 20 +++++++++++++++++-- >>>>>   2 files changed, 66 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c >>>>> index 3f884abe1d5..bff3b706e7e 100644 >>>>> --- a/gdb/dwarf2/frame.c >>>>> +++ b/gdb/dwarf2/frame.c >>>>> @@ -831,6 +831,14 @@ dwarf2_fetch_cfa_info (struct gdbarch *gdbarch, CORE_ADDR pc, >>>>>   } >>>>>   >>>>> +struct dwarf2_frame_fn_data >>>>> +{ >>>>> +  struct value *(*fn) (frame_info_ptr this_frame, void **this_cache, >>>>> +               int regnum); >>>>> +  void *data; >>>>> +  struct dwarf2_frame_fn_data* next; >>>>> +}; >>>>> + >>>> >>>> I'm wondering if we really need to have a function pointer here. Isn't the cache supposed to be frame-wide and not >>>> function-specific? >>>> >>>> If we don't need it, the cache just becomes an opaque data pointer. >>>> >>>>>   struct dwarf2_frame_cache >>>>>   { >>>>>     /* DWARF Call Frame Address.  */ >>>>> @@ -862,6 +870,8 @@ struct dwarf2_frame_cache >>>>>        dwarf2_tailcall_frame_unwind unwinder so this field does not apply for >>>>>        them.  */ >>>>>     void *tailcall_cache; >>>>> + >>>>> +  struct dwarf2_frame_fn_data *fn_data; >>>>>   }; >>>>>   static struct dwarf2_frame_cache * >>>>> @@ -1221,6 +1231,44 @@ dwarf2_frame_prev_register (frame_info_ptr this_frame, void **this_cache, >>>>>       } >>>>>   } >>>>> +void *dwarf2_frame_get_fn_data (frame_info_ptr this_frame, void **this_cache, >>>>> +                fn_prev_register fn) >>>>> +{ >>>>> +  struct dwarf2_frame_fn_data *fn_data = nullptr; >>>>> +  struct dwarf2_frame_cache *cache >>>>> +    = dwarf2_frame_cache (this_frame, this_cache); >>>>> + >>>>> +  /* Find the object for the function.  */ >>>>> +  for (fn_data = cache->fn_data; fn_data; fn_data = fn_data->next) >>>>> +    if (fn_data->fn == fn) >>>>> +      return fn_data->data; >>>>> + >>>>> +  return nullptr; >>>>> +} >>>>> + >>>>> +void *dwarf2_frame_allocate_fn_data (frame_info_ptr this_frame, >>>>> +                     void **this_cache, >>>>> +                     fn_prev_register fn, unsigned long size) >>>>> +{ >>>>> +  struct dwarf2_frame_fn_data *fn_data = nullptr; >>>>> +  struct dwarf2_frame_cache *cache >>>>> +    = dwarf2_frame_cache (this_frame, this_cache); >>>>> + >>>>> +  /* First try to find an existing object.  */ >>>>> +  void *data = dwarf2_frame_get_fn_data (this_frame, this_cache, fn); >>>>> +  if (data) >>>>> +    return data; >>>>> + >>>>> +  /* No object found, lets create a new instance.  */ >>>>> +  fn_data = FRAME_OBSTACK_ZALLOC (struct dwarf2_frame_fn_data); >>>>> +  fn_data->fn = fn; >>>>> +  fn_data->data = frame_obstack_zalloc (size); >>>>> +  fn_data->next = cache->fn_data; >>>>> +  cache->fn_data = fn_data; >>>>> + >>>>> +  return fn_data->data; >>>>> +} >>>> >>>> And if we only have a data pointer, we can return a reference to it through the argument, and then DWARF can cache it. >>>> >>>> We could even have a destructor/cleanup that can get called once the frames are destroyed. >>> >>> I don't think we can do that without introducing a lot more changes to the common code. My changes are designed in a way that would only have an impact on arm (as they are the only users for the functionality right now) and not for every target out there that GDB supports. If going for a simpler solution, it would mean that every target needs to be re-tested in order to get the confirmation that the implementation would not break some other target. >>> >>> >>>> >>>>> + >>>>>   /* Proxy for tailcall_frame_dealloc_cache for bottom frame of a virtual tail >>>>>      call frames chain.  */ >>>>> diff --git a/gdb/dwarf2/frame.h b/gdb/dwarf2/frame.h >>>>> index 06c8a10c178..444afd9f8eb 100644 >>>>> --- a/gdb/dwarf2/frame.h >>>>> +++ b/gdb/dwarf2/frame.h >>>>> @@ -66,6 +66,9 @@ enum dwarf2_frame_reg_rule >>>>>   /* Register state.  */ >>>>> +typedef struct value *(*fn_prev_register) (frame_info_ptr this_frame, >>>>> +                       void **this_cache, int regnum); >>>>> + >>>>>   struct dwarf2_frame_state_reg >>>>>   { >>>>>     /* Each register save state can be described in terms of a CFA slot, >>>>> @@ -78,8 +81,7 @@ struct dwarf2_frame_state_reg >>>>>         const gdb_byte *start; >>>>>         ULONGEST len; >>>>>       } exp; >>>>> -    struct value *(*fn) (frame_info_ptr this_frame, void **this_cache, >>>>> -             int regnum); >>>>> +    fn_prev_register fn; >>>>>     } loc; >>>>>     enum dwarf2_frame_reg_rule how; >>>>>   }; >>>>> @@ -262,4 +264,18 @@ extern int dwarf2_fetch_cfa_info (struct gdbarch *gdbarch, CORE_ADDR pc, >>>>>                     const gdb_byte **cfa_start_out, >>>>>                     const gdb_byte **cfa_end_out); >>>>> + >>>>> +/* Allocate a new instance of the function unique data.  */ >>>>> + >>>>> +extern void *dwarf2_frame_allocate_fn_data (frame_info_ptr this_frame, >>>>> +                        void **this_cache, >>>>> +                        fn_prev_register fn, >>>>> +                        unsigned long size); >>>>> + >>>>> +/* Retrieve the function unique data for this frame.  */ >>>>> + >>>>> +extern void *dwarf2_frame_get_fn_data (frame_info_ptr this_frame, >>>>> +                       void **this_cache, >>>>> +                       fn_prev_register fn); >>>>> + >>>>>   #endif /* dwarf2-frame.h */ >>>> >>>> As we've discussed before, I think the cache idea is nice if we have to deal with targets with multiple CFA's (in our case, we have either 4 SP's or 2 SP's, plus aliases). >>>> >>>> DWARF doesn't seem to support this at the moment, and the function HOW for DWARF is not smart enough to remember a previously-fetched value. So it seems we have room >>>> for some improvement, unless there is enough reason elsewhere about why we shouldn't have a cache. >>> >>> >>> This patch does not provide a cache or anything, it just provides a way for the callback function to save some additional data between calls for the same frame. >>> The code above is generic in the way that it has one data object per function and frame. The reason for this implementation is that it's rather easy to ensure that the data object is okay for the function that uses it without any inter-dependencies with some other function that might be called for some other register on the same frame. You could even consider having a shared function to be a callback function. In the case of a shared function, that would mean that the object would be large and public and then it would simply make more sense to make the dwarf2 object public and extend it instead. >>> My approach ensures that each callback function has its own data and the data structure is "private" to the function. It's possible to share the struct for the data object between 2 functions, but it's not possible to share the instance of the struct between 2 functions. >> >> Sorry, the per-function-pointers looks like an overkill to me. >> Maybe I'm just an old school programmer and don't like associative arrays... >> - frame unwinders use a generic pointer and ensuring the proper type cast is fully in responsibility of the implementation. >> - we need just to replicate the similar functionality for architecture dependent handling of dwarf2 frames >> - functions assigned to a dwarf2 frame by how = DWARF2_FRAME_REG_FN are never isolated functions from different parts >> of code: a gdbarch can set only one initializer by dwarf2_frame_set_init_reg() and it sets all functions >> - if we ever have more than one function assigned in one dwarf2 frame, seems me likely that all functions would prefer a single cache over isolated ones > > > Based on the points above, can please answer the below questions? > > a) Why is there a function pointer per register if it's always going to be a single one? It would have made more sense to simply have a single bool telling if the function should be called or not if your suggestion would make sense. > > b) If you want a shared cache object, then why is all the cache types private to the compile unit? You can't have something shared across 2 compile units that are just defined in the .c file. > > > Don't get me wrong, I'm not against defining the cache types in the public namespace. By doing so, it will have a large impact on the GDB code base and it will require a lot more testing than the change I'm proposing. > - Is it work the extra risk? > - Who can actually do the testing on all the targets to make sure that nothing broke? > What I see is an area of gdb where we can make some general improvements to help all targets. Testing wouldn't be that much more complicated, specially if we create a new HOW and leave the old one as-is. > Kind regards, > Torbjörn > > >> >>> >>> >>>> It would be nice to have some opinions from others, so we can potentially shape this in a way that makes it useful for the general case. >>> >>> Yes. Please give me some more feedback to work on! >>> >>> Kind regards, >>> Torbjörn >> >> regards >>      Tomas