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

[Kugan]

This issue also remains in 4.9 and 5.0 branches. Is this OK to backport
to the release branches.

Thanks,
Kugan

On 02/12/15 10:00, Kugan wrote:
> 
>>>
>>> gcc/ChangeLog:
>>>
>>> 2015-11-18  Kugan Vivekanandarajah  
>>>
>>> PR target/68390
>>> * config/arm/arm.c (arm_function_ok_for_sibcall): Get function type
>>> for indirect function call.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2015-11-18  Kugan Vivekanandarajah  
>>>
>>> PR target/68390
>>> * gcc.target/arm/PR68390.c: New test.
>>>
>>
>> s/PR/pr in the test name and put this in gcc.c-torture/execute instead - 
>> there is nothing ARM specific about the test. Tests in gcc.target/arm should 
>> really only be architecture specific. This isn't.
>>
>>>
>>>
>>>
>>> p.txt
>>>
>>>
>>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>>> index a379121..0dae7da 100644
>>> --- a/gcc/config/arm/arm.c
>>> +++ b/gcc/config/arm/arm.c
>>> @@ -6680,8 +6680,13 @@ arm_function_ok_for_sibcall (tree decl, tree exp)
>>>  a VFP register but then need to transfer it to a core
>>>  register.  */
>>>rtx a, b;
>>> +  tree fn_decl = decl;
>>
>> Call it decl_or_type instead - it's really that ... 
>>
>>>  
>>> -  a = arm_function_value (TREE_TYPE (exp), decl, false);
>>> +  /* If it is an indirect function pointer, get the function type.  */
>>> +  if (!decl)
>>> +   fn_decl = TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (exp)));
>>> +
>>
>> This is probably just my mail client - but please watch out for indentation.
>>
>>> +  a = arm_function_value (TREE_TYPE (exp), fn_decl, false);
>>>b = arm_function_value (TREE_TYPE (DECL_RESULT (cfun->decl)),
>>>   cfun->decl, false);
>>>if (!rtx_equal_p (a, b))
>>
>>
>> OK with those changes.
>>
>> Ramana
>>

> 

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

[Kugan]

>>
>> gcc/ChangeLog:
>>
>> 2015-11-18  Kugan Vivekanandarajah  
>>
>>  PR target/68390
>>  * config/arm/arm.c (arm_function_ok_for_sibcall): Get function type
>>  for indirect function call.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2015-11-18  Kugan Vivekanandarajah  
>>
>>  PR target/68390
>>  * gcc.target/arm/PR68390.c: New test.
>>
> 
> s/PR/pr in the test name and put this in gcc.c-torture/execute instead - 
> there is nothing ARM specific about the test. Tests in gcc.target/arm should 
> really only be architecture specific. This isn't.
> 
>>
>>
>>
>> p.txt
>>
>>
>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>> index a379121..0dae7da 100644
>> --- a/gcc/config/arm/arm.c
>> +++ b/gcc/config/arm/arm.c
>> @@ -6680,8 +6680,13 @@ arm_function_ok_for_sibcall (tree decl, tree exp)
>>   a VFP register but then need to transfer it to a core
>>   register.  */
>>rtx a, b;
>> +  tree fn_decl = decl;
> 
> Call it decl_or_type instead - it's really that ... 
> 
>>  
>> -  a = arm_function_value (TREE_TYPE (exp), decl, false);
>> +  /* If it is an indirect function pointer, get the function type.  */
>> +  if (!decl)
>> +fn_decl = TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (exp)));
>> +
> 
> This is probably just my mail client - but please watch out for indentation.
> 
>> +  a = arm_function_value (TREE_TYPE (exp), fn_decl, false);
>>b = arm_function_value (TREE_TYPE (DECL_RESULT (cfun->decl)),
>>cfun->decl, false);
>>if (!rtx_equal_p (a, b))
> 
> 
> OK with those changes.
> 
> Ramana
> 


Hi Ramana,

This issue also remains in 4.9 and 5.0 branches. Is this OK to backport
to the release branches.

Thanks,
Kugan

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

[Ramana Radhakrishnan]

On 18/11/15 00:32, Kugan wrote:
>> > Hi Ramana,
>> > 
>> > Thanks for the review. I have opened a gcc bug-report for this. I tested
>> > the attached patch for  arm-none-linux-gnueabihf and
>> > arm-none-linux-gnueabi with no new regressions. Is this OK?
>> > 
>> > 
>> > Thanks,
>> > Kugan
>> > 
>> > gcc/ChangeLog:
>> > 
>> > 2015-11-18  Kugan Vivekanandarajah  
>> > 
>> >PR target/68390
>> >* config/arm/arm.c (arm_function_ok_for_sibcall): Get function type
>> >for indirect function call.
>> > 
>> > gcc/testsuite/ChangeLog:
>> > 
>> > 2015-11-18  Kugan Vivekanandarajah  
>> > 
>> >PR target/68390
>> >* gcc.target/arm/PR68390.c: New test.
>> > 
>> > 
> Hi Ramana,
> 
> With further testing on bare-metal, I found that for the following decl
> has to be null for indirect functions.
> 
>   if (TARGET_AAPCS_BASED
>   && arm_abi == ARM_ABI_AAPCS
>   && decl
>   && DECL_WEAK (decl))
> return false;

Ok .. yes that's right.

> 
> Here is the updated patch and ChangeLog. Sorry for the noise.
> 
> Thanks,
> Kugan
> 
> 
> gcc/ChangeLog:
> 
> 2015-11-18  Kugan Vivekanandarajah  
> 
>   PR target/68390
>   * config/arm/arm.c (arm_function_ok_for_sibcall): Get function type
>   for indirect function call.
> 
> gcc/testsuite/ChangeLog:
> 
> 2015-11-18  Kugan Vivekanandarajah  
> 
>   PR target/68390
>   * gcc.target/arm/PR68390.c: New test.
> 

s/PR/pr in the test name and put this in gcc.c-torture/execute instead - there 
is nothing ARM specific about the test. Tests in gcc.target/arm should really 
only be architecture specific. This isn't.

> 
> 
> 
> p.txt
> 
> 
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index a379121..0dae7da 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -6680,8 +6680,13 @@ arm_function_ok_for_sibcall (tree decl, tree exp)
>a VFP register but then need to transfer it to a core
>register.  */
>rtx a, b;
> +  tree fn_decl = decl;

Call it decl_or_type instead - it's really that ... 

>  
> -  a = arm_function_value (TREE_TYPE (exp), decl, false);
> +  /* If it is an indirect function pointer, get the function type.  */
> +  if (!decl)
> + fn_decl = TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (exp)));
> +

This is probably just my mail client - but please watch out for indentation.

> +  a = arm_function_value (TREE_TYPE (exp), fn_decl, false);
>b = arm_function_value (TREE_TYPE (DECL_RESULT (cfun->decl)),
> cfun->decl, false);
>if (!rtx_equal_p (a, b))


OK with those changes.

Ramana

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

[Kugan]

On 17/11/15 21:05, Ramana Radhakrishnan wrote:
> 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  
>>> 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
>>> ...
>>> Breakpoint 1, arm_function_ok_for_sibcall (decl=0x0, exp=0x76104ce8)
>>> at /home/cbaylis/srcarea/gcc/gcc-git/gcc/config/arm/arm.c:6634
>>> 6634  if (cfun->machine->sibcall_blocked)
>>> (gdb) print debug_tree(exp)
>>>  >> type >> size 
>>> unit size 
>>> align 64 symtab 0 alias set -1 canonical type 0x762835e8
>>> precision 64
>>> pointer_to_this >
>>> side-effects addressable
>>> fn >> type >> 0x760e9348>
>>> ...
>>> (gdb) print stdarg_p((tree)0x760e9348)<--- 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.

Hi Ramana,

Thanks for the review. I have opened a gcc bug-report for this. I tested
the attached patch for  arm-none-linux-gnueabihf and
arm-none-linux-gnueabi with no new regressions. Is this OK?


Thanks,
Kugan

gcc/ChangeLog:

2015-11-18  Kugan Vivekanandarajah  

PR target/68390
* config/arm/arm.c (arm_function_ok_for_sibcall): Get function type
for indirect function call.

gcc/testsuite/ChangeLog:

2015-11-18  Kugan Vivekanandarajah  

PR target/68390
* gcc.target/arm/PR68390.c: New test.


diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index a379121..a4509f4 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -6681,6 +6681,10 @@ 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)
+   decl = TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (exp)));
+
   a = arm_function_value (TREE_TYPE (exp), decl, false);
   b = arm_function_value (TREE_TYPE (DECL_RESULT (cfun->decl)),
  cfun->decl, false);
diff --git a/gcc/testsuite/gcc.target/arm/PR68390.c 
b/gcc/testsuite/gcc.target/arm/PR68390.c
index e69de29..86f07fe 100644
--- a/gcc/testsuite/gcc.target/arm/PR68390.c
+++ b/gcc/testsuite/gcc.target/arm/PR68390.c
@@ -0,0 +1,27 @@
+/* { dg-do run }  */
+/* { dg-options "-O2" } */
+
+__attribute__ ((noinline))
+double direct(int x, ...)
+{
+  return x*x;
+}
+
+__attribute__ ((noinline))
+double broken(double (*indirect)(int x, ...), int v)
+{
+  return indirect(v);
+}
+
+int main ()
+{
+  double d1, d2;
+  int i = 2;
+  d1 = broken (direct, i);
+  if (d1 != i*i)
+{
+  __builtin_abort ();
+}
+  return 0;
+}
+
diff --git a/gcc/testsuite/gcc.target/arm/variadic_sibcall.c 
b/gcc/testsuite/gcc.target/arm/variadic_sibcall.c
deleted file mode 100644
index 

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

[Kugan]

> Hi Ramana,
> 
> Thanks for the review. I have opened a gcc bug-report for this. I tested
> the attached patch for  arm-none-linux-gnueabihf and
> arm-none-linux-gnueabi with no new regressions. Is this OK?
> 
> 
> Thanks,
> Kugan
> 
> gcc/ChangeLog:
> 
> 2015-11-18  Kugan Vivekanandarajah  
> 
>   PR target/68390
>   * config/arm/arm.c (arm_function_ok_for_sibcall): Get function type
>   for indirect function call.
> 
> gcc/testsuite/ChangeLog:
> 
> 2015-11-18  Kugan Vivekanandarajah  
> 
>   PR target/68390
>   * gcc.target/arm/PR68390.c: New test.
> 
> 
Hi Ramana,

With further testing on bare-metal, I found that for the following decl
has to be null for indirect functions.

  if (TARGET_AAPCS_BASED
  && arm_abi == ARM_ABI_AAPCS
  && decl
  && DECL_WEAK (decl))
return false;

Here is the updated patch and ChangeLog. Sorry for the noise.

Thanks,
Kugan


gcc/ChangeLog:

2015-11-18  Kugan Vivekanandarajah  

PR target/68390
* config/arm/arm.c (arm_function_ok_for_sibcall): Get function type
for indirect function call.

gcc/testsuite/ChangeLog:

2015-11-18  Kugan Vivekanandarajah  

PR target/68390
* gcc.target/arm/PR68390.c: New test.



diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index a379121..0dae7da 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -6680,8 +6680,13 @@ arm_function_ok_for_sibcall (tree decl, tree exp)
 a VFP register but then need to transfer it to a core
 register.  */
   rtx a, b;
+  tree fn_decl = decl;
 
-  a = arm_function_value (TREE_TYPE (exp), decl, false);
+  /* If it is an indirect function pointer, get the function type.  */
+  if (!decl)
+   fn_decl = TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (exp)));
+
+  a = arm_function_value (TREE_TYPE (exp), fn_decl, false);
   b = arm_function_value (TREE_TYPE (DECL_RESULT (cfun->decl)),
  cfun->decl, false);
   if (!rtx_equal_p (a, b))
diff --git a/gcc/testsuite/gcc.target/arm/PR68390.c 
b/gcc/testsuite/gcc.target/arm/PR68390.c
index e69de29..86f07fe 100644
--- a/gcc/testsuite/gcc.target/arm/PR68390.c
+++ b/gcc/testsuite/gcc.target/arm/PR68390.c
@@ -0,0 +1,27 @@
+/* { dg-do run }  */
+/* { dg-options "-O2" } */
+
+__attribute__ ((noinline))
+double direct(int x, ...)
+{
+  return x*x;
+}
+
+__attribute__ ((noinline))
+double broken(double (*indirect)(int x, ...), int v)
+{
+  return indirect(v);
+}
+
+int main ()
+{
+  double d1, d2;
+  int i = 2;
+  d1 = broken (direct, i);
+  if (d1 != i*i)
+{
+  __builtin_abort ();
+}
+  return 0;
+}
+

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

[Ramana Radhakrishnan]

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  
>> 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
>> ...
>> Breakpoint 1, arm_function_ok_for_sibcall (decl=0x0, exp=0x76104ce8)
>> at /home/cbaylis/srcarea/gcc/gcc-git/gcc/config/arm/arm.c:6634
>> 6634  if (cfun->machine->sibcall_blocked)
>> (gdb) print debug_tree(exp)
>>  > type > size 
>> unit size 
>> align 64 symtab 0 alias set -1 canonical type 0x762835e8
>> precision 64
>> pointer_to_this >
>> side-effects addressable
>> fn > type 
>> ...
>> (gdb) print stdarg_p((tree)0x760e9348)<--- 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

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

[Charles Baylis]

On 16 November 2015 at 22:24, Kugan  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
...
Breakpoint 1, arm_function_ok_for_sibcall (decl=0x0, exp=0x76104ce8)
at /home/cbaylis/srcarea/gcc/gcc-git/gcc/config/arm/arm.c:6634
6634  if (cfun->machine->sibcall_blocked)
(gdb) print debug_tree(exp)
 
unit size 
align 64 symtab 0 alias set -1 canonical type 0x762835e8
precision 64
pointer_to_this >
side-effects addressable
fn 
...
(gdb) print stdarg_p((tree)0x760e9348)<--- from function_type ^
$2 = true

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

[Kugan]

On 17/11/15 12:00, Charles Baylis wrote:
> On 16 November 2015 at 22:24, Kugan  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
> ...
> Breakpoint 1, arm_function_ok_for_sibcall (decl=0x0, exp=0x76104ce8)
> at /home/cbaylis/srcarea/gcc/gcc-git/gcc/config/arm/arm.c:6634
> 6634  if (cfun->machine->sibcall_blocked)
> (gdb) print debug_tree(exp)
>   type  size 
> unit size 
> align 64 symtab 0 alias set -1 canonical type 0x762835e8
> precision 64
> pointer_to_this >
> side-effects addressable
> fn  type 
> ...
> (gdb) print stdarg_p((tree)0x760e9348)<--- from function_type ^
> $2 = true
> 

How about:

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)));
+
   a = arm_function_value (TREE_TYPE (exp), decl, false);
   b = arm_function_value (TREE_TYPE (DECL_RESULT (cfun->decl)),
  cfun->decl, false);


Thanks,
Kugan

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

[Kugan]

On 17/11/15 12:00, Charles Baylis wrote:
> On 16 November 2015 at 22:24, Kugan  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
> ...
> Breakpoint 1, arm_function_ok_for_sibcall (decl=0x0, exp=0x76104ce8)
> at /home/cbaylis/srcarea/gcc/gcc-git/gcc/config/arm/arm.c:6634
> 6634  if (cfun->machine->sibcall_blocked)
> (gdb) print debug_tree(exp)
>   type  size 
> unit size 
> align 64 symtab 0 alias set -1 canonical type 0x762835e8
> precision 64
> pointer_to_this >
> side-effects addressable
> fn  type 
> ...
> (gdb) print stdarg_p((tree)0x760e9348)<--- from function_type ^
> $2 = true
> 

Hi Charles,

I wrongly thought that for indirect call we wouldn't know if it is
variadic or not. I should check stdarg_p here.

But we should really fix aapcs_allocate_return_reg as it is simply
setting  pcs_variant = arm_pcs_default without checking if this is
stdarg_p. I will send an updated patch.

Thanks,
Kugan