Re: [RFAv2 3/3] Make symtab.c better styled.

[Philippe Waroquiers <philippe dot waroquiers at skynet dot be>]

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