Re: [PATCH v2 1/1] gdb: Preserve IFUNC marker when finding inferior functions

[Simon Marchi <simark@simark.ca>]

> @@ -356,6 +361,39 @@ proc misc_tests {resolver_attr resolver_debug final_debug} {
>      }
>  }
>  
> +# Test that GDB resolves a GNU IFUNC minimal symbol when it uses
> +# find_function_in_inferior to make an internal inferior call.  String
> +# literals are copied into the inferior with a call to malloc, so a
> +# no-debug IFUNC malloc exercises the minimal-symbol fallback.
> +
> +proc_with_prefix test_inferior_call {} {
> +    global srcdir subdir
> +    global infcall_file infcall_src
> +    global infcall_malloc_file infcall_malloc_src
> +
> +    set executable $infcall_file
> +    set binfile [standard_output_file $executable]
> +    set malloc_obj [standard_output_file ${infcall_malloc_file}.o]
> +
> +    if { [gdb_compile ${srcdir}/${subdir}/${infcall_malloc_src} \
> +	      $malloc_obj object {}] != ""
> +	 || [gdb_compile [list ${srcdir}/${subdir}/${infcall_src} \
> +			      $malloc_obj] \
> +		 $binfile executable {debug}] != "" } {
> +	untested "failed to compile inferior call testcase"
> +	return
> +    }

I think that the test should also cover the case where we do have debug
info.  However, the gdb.base/gnu-ifunc.exp test is already written in
a such a way that it tests all imaginable combinations:

# Test all the combinations of:
#
# - An ifunc resolver with the same name as the ifunc symbol vs an
#   ifunc resolver with a different name as the ifunc symbol.
#
# - ifunc resolver compiled with and without debug info.  This ensures
#   that GDB understands that a function not a regular function by
#   looking at the STT_GNU_IFUNC type in the elf symbols.  DWARF has
#   no way to express the STT_GNU_IFUNC type.
#
# - ifunc target function (resolved) compiled with and without debug
#   info.
foreach_with_prefix resolver_attr {0 1} {
    foreach_with_prefix resolver_debug {0 1} {
	foreach_with_prefix final_debug {0 1} {
	    if { [build $resolver_attr $resolver_debug $final_debug] != 0 } {
		misc_tests $resolver_attr $resolver_debug $final_debug
		set-break $resolver_attr $resolver_debug $final_debug
	    }
	}
    }
}

Could you somehow hook the new infcall tests into that, so that we also
test infcalls in all imaginable situations.

> diff --git a/gdb/valops.c b/gdb/valops.c
> index ab6fd5079e1..7d305871efc 100644
> --- a/gdb/valops.c
> +++ b/gdb/valops.c
> @@ -133,11 +133,15 @@ find_function_in_inferior (const char *name, struct objfile **objf_p)
>  	  struct gdbarch *gdbarch = objfile->arch ();
>  
>  	  struct type *type;
> +	  struct type *resolved_type;
>  	  CORE_ADDR maddr;
>  	  type = lookup_pointer_type (builtin_type (gdbarch)->builtin_char);
>  	  type = lookup_function_type (type);
>  	  type = lookup_pointer_type (type);
> -	  maddr = msymbol.value_address ();
> +	  resolved_type = find_minsym_type_and_address (msymbol.minsym, objfile,
> +							&maddr);
> +	  if (resolved_type->is_gnu_ifunc ())
> +	    type->target_type ()->set_is_gnu_ifunc (true);

Calling find_minsym_type_and_address just to know if the minsym is an
ifunc seems rather heavyweight for nothing.  Can't we just check that
the minsym type is mst_text_gnu_ifunc?

I assume that the `maddr` output by find_minsym_type_and_address would
always be equal to `msymbol.value_address ()`.

Side-note: this "find_function_in_inferior" (at least the fallback)
really assumes that we are looking for a function returning "char *", so
that function seems misnamed (it could easily be mis-used).

Simon