Re: Incorrect code due to indirect tail call of varargs function with hard float ABI

[Ramana Radhakrishnan <ramana dot radhakrishnan at foss dot arm dot com>]

Hi Kugan,

It does look like an issue.

Please open a bug report.

> 
> 
> On 17/11/15 12:00, Charles Baylis wrote:
>> On 16 November 2015 at 22:24, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
>>
>>> Please note that we have a sibcall from "broken" to "indirect".
>>>
>>> "direct" is variadic function so it is conforming to AAPCS base standard.
>>>
>>> "broken" is a non-variadic function and will return the value in
>>> floating point register for TARGET_HARD_FLOAT. Thus we should not be
>>> doing sibcall here.
>>>
>>> Attached patch fixes this. Bootstrap and regression testing is ongoing.
>>> Is this OK if no issues with the testing?
>>
>> Hi Kugan,
>>
>> It looks like this patch should work, but I think this is an overly
>> conservative fix, as it prevents all sibcalls for hardfloat targets.
>> It would be better if only variadic sibcalls were prevented on
>> hardfloat. You can check for variadic calls by checking the
>> function_type in the call expression (exp) using stdarg_p().
>>
>> As an example to show how to test for variadic function calls, this is
>> how to test it in gdb:
>>
>> (gdb) b arm_function_ok_for_sibcall
>> Breakpoint 1 at 0xdae59c: file
>> /home/cbaylis/srcarea/gcc/gcc-git/gcc/config/arm/arm.c, line 6634.
>> (gdb) r
>> <snip>...
>> Breakpoint 1, arm_function_ok_for_sibcall (decl=0x0, exp=0x7ffff6104ce8)
>>     at /home/cbaylis/srcarea/gcc/gcc-git/gcc/config/arm/arm.c:6634
>> 6634      if (cfun->machine->sibcall_blocked)
>> (gdb) print debug_tree(exp)
>>  <call_expr 0x7ffff6104ce8
>>     type <real_type 0x7ffff62835e8 double sizes-gimplified DF
>>         size <integer_cst 0x7ffff626cf18 constant 64>
>>         unit size <integer_cst 0x7ffff626cf30 constant 8>
>>         align 64 symtab 0 alias set -1 canonical type 0x7ffff62835e8
>> precision 64
>>         pointer_to_this <pointer_type 0x7ffff62837e0>>
>>     side-effects addressable
>>     fn <ssa_name 0x7ffff6275708
>>         type <pointer_type 0x7ffff60e9540 type <function_type 0x7ffff60e9348>
>> <snip>...
>> (gdb) print stdarg_p((tree)0x7ffff60e9348)    <--- from function_type ^^^^^
>> $2 = true
>>
> 
> How about:



A run time testcase and a changelog would also be needed.

> 
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index a379121..2376d66 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -6681,6 +6681,13 @@ arm_function_ok_for_sibcall (tree decl, tree exp)
>          register.  */
>        rtx a, b;
> 
> +      /* If it is an indirect function pointer, get the function type.  */
> +      if (!decl
> +         && POINTER_TYPE_P (TREE_TYPE (CALL_EXPR_FN (exp)))
> +         && (TREE_CODE (TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (exp))))
> +             == FUNCTION_TYPE))
> +       decl = TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (exp)));
> +

If decl is null it's guaranteed to be an indirect function call - drop the additional checks in the if clause.


>        a = arm_function_value (TREE_TYPE (exp), decl, false);
>        b = arm_function_value (TREE_TYPE (DECL_RESULT (cfun->decl)),
>                               cfun->decl, false);
> 


Please resubmit with a testcase, Changelog and after testing.


regards
Ramana