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

[Pedro Alves <palves@redhat.com>]

On 04/01/2018 05:22 AM, Simon Marchi wrote:

>> +/* 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?

Good idea.  That lives in infcall.c currently, but I moved it along.

> 
>> @@ -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.

You're right, I did this.

> 
>> +
>> +  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?

Yeah, find_function_addr handles a case where the function's type is not
really a function, but an integer:

  else if (TYPE_CODE (ftype) == TYPE_CODE_INT)
    {

This is reached when you call a function by address, like

  (gdb) print 0x1()

In this case, the caller wouldn't be able to use
TYPE_TARGET_TYPE on function_type.  Instead it gets a 
NULL retval_type.

I think it may be possible to handle that case by making
it return the type we use for non-debug functions in function_type:

  objfile_type->nodebug_text_symbol
    = init_type (objfile, TYPE_CODE_FUNC, TARGET_CHAR_BIT,
		 "<text variable, no debug info>");

but that type is currently tied to an objfile, seemingly for no
good reason.  But I'm not sure.  It'd needs experimentation
and reworking nodebug_text_symbol, which I'd prefer to leave
for another day.

Here's what I'm squashing into the patch locally.

From 2e89a1ef27e7eac203092346da942d16bc5a606c Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 10 Apr 2018 22:46:49 +0100
Subject: [PATCH] find_function_type

---
 gdb/blockframe.c | 47 ++++++++++++++++++++++++++---------------------
 gdb/infcall.c    | 24 ++++--------------------
 gdb/infcall.h    |  2 +-
 gdb/symtab.h     |  5 +++++
 4 files changed, 36 insertions(+), 42 deletions(-)

diff --git a/gdb/blockframe.c b/gdb/blockframe.c
index db02b35742d..e6938a341ae 100644
--- a/gdb/blockframe.c
+++ b/gdb/blockframe.c
@@ -325,32 +325,37 @@ find_pc_partial_function (CORE_ADDR pc, const char **name, CORE_ADDR *address,
 
 /* See symtab.h.  */
 
+struct type *
+find_function_type (CORE_ADDR pc)
+{
+  struct symbol *sym = find_pc_function (pc);
+
+  if (sym != NULL && BLOCK_START (SYMBOL_BLOCK_VALUE (sym)) == pc)
+    return SYMBOL_TYPE (sym);
+
+  return 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)
+  struct type *resolver_type = find_function_type (resolver_funaddr);
+  if (resolver_type != NULL)
     {
-      struct type *resolver_type = SYMBOL_TYPE (sym);
-      if (TYPE_CODE (resolver_type) == TYPE_CODE_FUNC)
+      /* Get the return type of the resolver.  */
+      struct type *resolver_ret_type
+	= check_typedef (TYPE_TARGET_TYPE (resolver_type));
+
+      /* If we found a pointer to function, then the resolved type
+	 is the type of the pointed-to function.  */
+      if (TYPE_CODE (resolver_ret_type) == TYPE_CODE_PTR)
 	{
-	  /* Get the return type of the resolver.  */
-	  struct type *resolver_ret_type
-	    = check_typedef (TYPE_TARGET_TYPE (resolver_type));
-
-	  /* If we found a pointer to function, then the resolved type
-	     is the type of the pointed-to function.  */
-	  if (TYPE_CODE (resolver_ret_type) == TYPE_CODE_PTR)
-	    {
-	      struct type *resolved_type
-		= TYPE_TARGET_TYPE (resolver_ret_type);
-	      if (TYPE_CODE (check_typedef (resolved_type)) == TYPE_CODE_FUNC)
-		return resolved_type;
-	    }
+	  struct type *resolved_type
+	    = TYPE_TARGET_TYPE (resolver_ret_type);
+	  if (TYPE_CODE (check_typedef (resolved_type)) == TYPE_CODE_FUNC)
+	    return resolved_type;
 	}
     }
 
diff --git a/gdb/infcall.c b/gdb/infcall.c
index d89d8ca7e8c..b233e369f27 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -229,20 +229,6 @@ value_arg_coerce (struct gdbarch *gdbarch, struct value *arg,
   return value_cast (type, arg);
 }
 
-/* Return the type of a function with its first instruction exactly at
-   the PC address.  Return NULL otherwise.  */
-
-static struct type *
-find_function_type (CORE_ADDR pc)
-{
-  struct symbol *sym = find_pc_function (pc);
-
-  if (sym != NULL && BLOCK_START (SYMBOL_BLOCK_VALUE (sym)) == pc)
-    return SYMBOL_TYPE (sym);
-
-  return NULL;
-}
-
 /* See infcall.h.  */
 
 CORE_ADDR
@@ -737,13 +723,12 @@ call_function_by_hand_dummy (struct value *function,
 			     void *dummy_dtor_data)
 {
   CORE_ADDR sp;
-  struct type *values_type, *target_values_type;
+  struct type *target_values_type;
   unsigned char struct_return = 0, hidden_first_param_p = 0;
   CORE_ADDR struct_addr = 0;
   struct infcall_control_state *inf_status;
   struct cleanup *inf_status_cleanup;
   struct infcall_suspend_state *caller_state;
-  CORE_ADDR funaddr;
   CORE_ADDR real_pc;
   CORE_ADDR bp_addr;
   struct frame_id dummy_id;
@@ -878,11 +863,10 @@ call_function_by_hand_dummy (struct value *function,
       }
   }
 
-  struct type *ftype = check_typedef (value_type (function));
-  if (TYPE_CODE (ftype) == TYPE_CODE_PTR)
-    ftype = check_typedef (TYPE_TARGET_TYPE (ftype));
+  type *ftype;
+  type *values_type;
+  CORE_ADDR funaddr = find_function_addr (function, &values_type, &ftype);
 
-  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 bea1494b50d..8b2195019c9 100644
--- a/gdb/infcall.h
+++ b/gdb/infcall.h
@@ -28,7 +28,7 @@ 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..
+   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, 
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 22b52019ee3..83ff6f226d8 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1675,6 +1675,11 @@ extern int find_pc_partial_function_gnu_ifunc (CORE_ADDR pc, const char **name,
 extern int find_pc_partial_function (CORE_ADDR, const char **, CORE_ADDR *,
 				     CORE_ADDR *);
 
+/* Return the type of a function with its first instruction exactly at
+   the PC address.  Return NULL otherwise.  */
+
+extern struct type *find_function_type (CORE_ADDR pc);
+
 /* See if we can figure out the function's actual type from the type
    that the resolver returns.  RESOLVER_FUNADDR is the address of the
    ifunc resolver.  */
-- 
2.14.3