Re: [PATCH v2 0/2] Aarch64: Fix segfault when casting dummy calls

[Pedro Alves <palves at redhat dot com>]

On 10/01/2018 04:52 PM, Alan Hayward wrote:
> This is a reworking of a patch I posted in March.
> V1 had a long discussion which was then paused to wait for
> Pedro's IFUNC rewrite.
> 
> 
> Prevent the int cast in the following causing a segfault on aarch64:
> (gdb) b foo if (int)strcmp(name,"abc") == 0
> (gdb) run
> 
> 
> This is because to aarch64_push_dummy_call determines the return type
> of the function and then does not check for null pointer.
> 
> A null pointer for the return type means either 1) the call has a
> cast or 2) an error has occured.

I'd think that "1) the call has a cast" is not accurate.
If the called function has debug info, then GDB will know
it's return type.  The issue is that the called function may
not have debug information, and then GDB does not know
its return type (so its NULL), and then the only way to
call the function is to add the cast.  Right?

It kind of sounds like IFUNCs were a red herring then.  :-/

> You can see this in infcall.c:call_function_by_hand_dummy():
> 
>   CORE_ADDR funaddr = find_function_addr (function, &values_type, &ftype);
> 
>   if (values_type == NULL)
>     values_type = default_return_type;
>   if (values_type == NULL)
>     {
>       const char *name = get_function_name (funaddr,
> 					    name_buf, sizeof (name_buf));
>       error (_("'%s' has unknown return type; "
> 	       "cast the call to its declared return type"),
> 	     name);
>     }
> 
> In aarch64_push_dummy_call we do not have default_return_type, so cannot
> determine between the two cases.
> 
> (In addition, aarch64_push_dummy_call incorrectly resolves the return
> type for IFUNC).

Can you expand a bit on this IFUNC remark?


> However, aarch64_push_dummy_call only requires the return value in order
> to calculate lang_struct_return ... which has previously been calculated
> in the caller:
> 
>      This is slightly awkward, ideally the flag "lang_struct_return"
>      would be passed to the targets implementation of push_dummy_call.
>      Rather that change the target interface we call the language code
>      directly ourselves.
> 

Ah, nice, the solution was right there.  :-)

> The fix is simple:
> Patch 1: Update gdbarch interface to pass lang_struct_return.
> Patch 2: Remove incorrect code and use the passed in lang_struct_return.
> 

Since cover letters don't end up in git, this info should be
somehow migrated into the commit logs of the two patches.

Thanks,
Pedro Alves