Re: [PATCH v2 03/15] Calling ifunc functions when target has no debug info but resolver has

[Simon Marchi <simark@simark.ca>]

On 2018-03-25 03:19 PM, Pedro Alves wrote:
> In v2:
>   - Added testsuite tweak.
> 
> After the previous patch, on Fedora 27 (glibc 2.26), if you try
> calling strlen in the inferior, you now get:
> 
>   (top-gdb) p strlen ("hello")
>   '__strlen_avx2' has unknown return type; cast the call to its declared return type
> 
> This is correct, because __strlen_avx2 is written in assembly.
> 
> We can improve on this though -- if the final ifunc resolved/target
> function has no debug info, but the ifunc _resolver_ does have debug
> info, we can try extracting the final function's type from the type
> that the resolver returns.  E.g.,:
> 
>   typedef size_t (*strlen_t) (const char*);
> 
>   size_t my_strlen (const char *) { /* some implementation */ }
>   strlen_t strlen_resolver (unsigned long hwcap) { return my_strlen; }
> 
>   extern size_t strlen (const char *s);
>   __typeof (strlen) strlen __attribute__ ((ifunc ("strlen_resolver")));
> 
> In the strlen example above, the resolver returns strlen_t, which is a
> typedef for pointer to a function that returns size_t.  "strlen_t" is
> the type of both the user-visible "strlen", and of the the target
> function that implements it.
> 
> This patch teaches GDB to extract that type.
> 
> This is done for actual inferior function calls (in infcall.c), and
> for ptype (in eval_call).  By the time we get to either of these
> places, we've already lost the original symbol/minsym, and only have
> values and types to work with.  Hence the changes to c-exp.y and
> evaluate_var_msym_value, to ensure that we propagate the ifunc
> minsymbol's info.
> 
> The change to make ifunc symbols have no/unknown return type exposes a
> latent problem -- gdb.compile/compile-ifunc.exp calls a no-debug-info
> function, but we did not warn about it.  The test is fixed by this
> commit too.

As usual, what you did seems to make sense, but I'm a bit lost.  I noted
some random comments.

> diff --git a/gdb/blockframe.c b/gdb/blockframe.c
> index 9be8871f756..db02b35742d 100644
> --- a/gdb/blockframe.c
> +++ b/gdb/blockframe.c
> @@ -323,6 +323,40 @@ find_pc_partial_function (CORE_ADDR pc, const char **name, CORE_ADDR *address,
>    return find_pc_partial_function_gnu_ifunc (pc, name, address, endaddr, NULL);
>  }
>  
> +/* See symtab.h.  */
> +
> +struct type *
> +find_gnu_ifunc_target_type (CORE_ADDR resolver_funaddr)
> +{
> +  /* See if we can figure out the function's return type from the type
> +     that the resolver returns.  */
> +  symbol *sym = find_pc_function (resolver_funaddr);
> +  if (sym != NULL
> +      && SYMBOL_CLASS (sym) == LOC_BLOCK
> +      && BLOCK_START (SYMBOL_BLOCK_VALUE (sym)) == resolver_funaddr)
> +    {

This looks a lot like the "find_function_type" function.  Maybe it should use it?

> @@ -864,7 +878,11 @@ call_function_by_hand_dummy (struct value *function,
>        }
>    }
>  
> -  funaddr = find_function_addr (function, &values_type);
> +  struct type *ftype = check_typedef (value_type (function));
> +  if (TYPE_CODE (ftype) == TYPE_CODE_PTR)
> +    ftype = check_typedef (TYPE_TARGET_TYPE (ftype));

Are these last operations necessary to do here?  It seems to me like find_function_addr
will do pretty much the same work and ignore the input value of ftype anyway.

> +
> +  funaddr = find_function_addr (function, &values_type, &ftype);
>    if (values_type == NULL)
>      values_type = default_return_type;
>    if (values_type == NULL)
> diff --git a/gdb/infcall.h b/gdb/infcall.h
> index a3861fb1bf3..bea1494b50d 100644
> --- a/gdb/infcall.h
> +++ b/gdb/infcall.h
> @@ -25,8 +25,15 @@
>  struct value;
>  struct type;
>  
> +/* Determine a function's address and its return type from its value.
> +   If the function is a GNU ifunc, then return the address of the
> +   target function, and set *FUNCTION_TYPE to the target function's
> +   type, and *RETVAL_TYPE to the target function's return type..
> +   Calls error() if the function is not valid for calling.  */
> +
>  extern CORE_ADDR find_function_addr (struct value *function, 
> -				     struct type **retval_type);
> +				     struct type **retval_type,
> +				     struct type **function_type = NULL);

Isn't the function's return value type always the target type of the function's
type?  If so, it seems a bit redundant to have both retval_type and function_type.
The callers easily call TYPE_TARGET_TYPE on *function_type.  Or maybe do you see
some situations where we're able to determine the reval_type but not the
function_type, in which case the retval_type would still be relevant?

Simon