From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 74925 invoked by alias); 9 Feb 2019 10:36:05 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 74914 invoked by uid 89); 9 Feb 2019 10:36:05 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-3.4 required=5.0 tests=BAYES_50,GIT_PATCH_2,KAM_STOCKGEN,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=sometime, H*i:sk:393f282, 4667,8, msymbolminsym X-HELO: mailsec105.isp.belgacom.be Received: from mailsec105.isp.belgacom.be (HELO mailsec105.isp.belgacom.be) (195.238.20.101) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 09 Feb 2019 10:36:02 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=skynet.be; i=@skynet.be; q=dns/txt; s=securemail; t=1549708561; x=1581244561; h=message-id:subject:from:to:date:in-reply-to:references: mime-version:content-transfer-encoding; bh=lF6D3kBn5yjK6qx4Neqb7+s7WgN022+qGzs0m/kIvX8=; b=fto1YAvi1WO6jxjuqrcqXoQTtI8zjnXTU/0YODItpO+6ZwifwTih7CuT A8IEBHpA2gjSKY2U/rpVb8zOLIY1Mw==; Received: from 147.122-130-109.adsl-dyn.isp.belgacom.be (HELO md) ([109.130.122.147]) by relay.skynet.be with ESMTP/TLS/AES256-GCM-SHA384; 09 Feb 2019 11:35:58 +0100 Message-ID: <1549708558.12345.4.camel@skynet.be> Subject: Re: [RFAv2 3/3] Make symtab.c better styled. From: Philippe Waroquiers To: Pedro Alves , gdb-patches@sourceware.org Date: Sat, 09 Feb 2019 10:36:00 -0000 In-Reply-To: <393f2825-8cc8-b71a-4e61-1edfd944cb84@redhat.com> References: <20190112222835.16932-1-philippe.waroquiers@skynet.be> <20190112222835.16932-4-philippe.waroquiers@skynet.be> <393f2825-8cc8-b71a-4e61-1edfd944cb84@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2019-02/txt/msg00105.txt.bz2 On Thu, 2019-02-07 at 18:58 +0000, Pedro Alves wrote: > On 01/12/2019 10:28 PM, Philippe Waroquiers wrote: > > > @@ -4667,8 +4685,15 @@ print_msymbol_info (struct bound_minimal_symbol msymbol) > > else > > tmp = hex_string_custom (BMSYMBOL_VALUE_ADDRESS (msymbol), > > 16); > > - printf_filtered ("%s %s\n", > > - tmp, MSYMBOL_PRINT_NAME (msymbol.minsym)); > > + fputs_styled (tmp, address_style.style (), gdb_stdout); > > + fputs_filtered (" ", gdb_stdout); > > + if (msymbol_type_text_p (msymbol)) > > + fputs_styled (MSYMBOL_PRINT_NAME (msymbol.minsym), > > + function_name_style.style (), > > + gdb_stdout); > > + else > > + fputs_filtered (MSYMBOL_PRINT_NAME (msymbol.minsym), gdb_stdout); > > + fputs_filtered ("\n", gdb_stdout); > > Should this use the existing msymbol_is_function instead? That is a good question. Note that there was a v3 where if (msymbol_type_text_p (msymbol)) is replaced by if (msymbol.minsym->text_p ()) The below summarizes the behavior difference if we change the patch to rather use msymbol_is_function instead of msymbol.minsym->text_p ():                             patch   msymbol_is_function                             -----   -------------------   mst_unknown   mst_text,     text_p  function   mst_text_gnu_ifunc,       text_p  function   mst_data_gnu_ifunc,     text_p  maybe   mst_slot_got_plt,     text_p  maybe   mst_data,                 data_p  maybe   mst_bss,     data_p  maybe   mst_abs,     data_p  maybe   mst_solib_trampoline,     text_p  function   mst_file_text,     text_p  function   mst_file_data,     data_p  maybe   mst_file_bss,     data_p  maybe   nr_minsym_types The 'patch' first colummn indicates how the patch classifies a msymbol type.  The 'msymbol_is_function' describes the behaviour if msymbol_is_function is used instead of 'text_p'. maybe indicates msymbol_is_function returns True if the msymbol address can be converted to a different address using gdbarch_convert_from_func_ptr_addr. When trying to use msymbol_is_function, I however do not see any difference of behavior on debian/amd64with a small executable. The logic I used for the patch was: there are already 2 places that define the set of msymbol types that are data, so I assumed the rest was text, and the resulting type set looked plausible. It is however somewhat bizarre to have msymbol_is_function that will sometime say that data is a function (and then 'gives back' another address than the msymbol address). So, not very clear which one is better/correct, as this msymbol concept is not very clear to me, and I see no difference of behavior to decide which one looks better. Thanks Philippe