Patchwork [GOOGLE] More strict checking for call args

login
register
mail settings
Submitter Dehao Chen
Date May 30, 2013, 10:47 p.m.
Message ID <CAO2gOZWs7maFPDo=EZUe1mPARfNFxxnA5Yg3z0Wo0WS1+2ji2Q@mail.gmail.com>
Download mbox | patch
Permalink /patch/247812/
State New
Headers show

Comments

Dehao Chen - May 30, 2013, 10:47 p.m.
This patch makes more strict check of call args to make sure the
number of args match.

Bootstrapped and passed regression tests.

OK for google branches?

Thanks,
Dehao
Xinliang David Li - May 30, 2013, 11:10 p.m.
On Thu, May 30, 2013 at 3:47 PM, Dehao Chen <dehao@google.com> wrote:
> This patch makes more strict check of call args to make sure the
> number of args match.
>
> Bootstrapped and passed regression tests.
>
> OK for google branches?
>
> Thanks,
> Dehao
>
> Index: gcc/gimple-low.c
> ===================================================================
> --- gcc/gimple-low.c (revision 199414)
> +++ gcc/gimple-low.c (working copy)
> @@ -254,9 +254,13 @@ gimple_check_call_args (gimple stmt, tree fndecl)
>    && !fold_convertible_p (DECL_ARG_TYPE (p), arg)))
>              return false;
>   }
> +      if (p != NULL)
> + return false;
>      }
>    else if (parms)
>      {
> +      if (list_length (parms) - nargs != 1)
> + return false;

This does not seem to be correct for vararg functions.

David


>        for (i = 0, p = parms; i < nargs; i++, p = TREE_CHAIN (p))
>   {
>    tree arg;
Duncan Sands - May 31, 2013, 8:15 a.m.
Hi Dehao,

On 31/05/13 00:47, Dehao Chen wrote:
> This patch makes more strict check of call args to make sure the
> number of args match.
>
> Bootstrapped and passed regression tests.

did you thoroughly test Fortran?  The Fortran front-end has long had an
unfortunate tendency to eg declare a function as taking 4 int arguments,
but in the call pass it one argument (an array of length 4, consisting
of ints).  It would be great if all such nastiness has been fixed.  There
are also a few cases in which it declares a builtin as taking, say, an
int,float pair, but passes a float,int pair in the call.  I fixed a couple
of instances of this a while back, but I still have one outstanding patch.

Ciao, Duncan.

>
> OK for google branches?
>
> Thanks,
> Dehao
>
> Index: gcc/gimple-low.c
> ===================================================================
> --- gcc/gimple-low.c (revision 199414)
> +++ gcc/gimple-low.c (working copy)
> @@ -254,9 +254,13 @@ gimple_check_call_args (gimple stmt, tree fndecl)
>     && !fold_convertible_p (DECL_ARG_TYPE (p), arg)))
>               return false;
>    }
> +      if (p != NULL)
> + return false;
>       }
>     else if (parms)
>       {
> +      if (list_length (parms) - nargs != 1)
> + return false;
>         for (i = 0, p = parms; i < nargs; i++, p = TREE_CHAIN (p))
>    {
>     tree arg;
>
Xinliang David Li - May 31, 2013, 6:46 p.m.
Those cases you mentioned may lead to problems in inlining, indirect
target promotion transformations etc too, so it is better to use the
new guard against them.

David

On Fri, May 31, 2013 at 1:15 AM, Duncan Sands <baldrick@free.fr> wrote:
> Hi Dehao,
>
>
> On 31/05/13 00:47, Dehao Chen wrote:
>>
>> This patch makes more strict check of call args to make sure the
>> number of args match.
>>
>> Bootstrapped and passed regression tests.
>
>
> did you thoroughly test Fortran?  The Fortran front-end has long had an
> unfortunate tendency to eg declare a function as taking 4 int arguments,
> but in the call pass it one argument (an array of length 4, consisting
> of ints).  It would be great if all such nastiness has been fixed.  There
> are also a few cases in which it declares a builtin as taking, say, an
> int,float pair, but passes a float,int pair in the call.  I fixed a couple
> of instances of this a while back, but I still have one outstanding patch.
>
> Ciao, Duncan.
>
>
>>
>> OK for google branches?
>>
>> Thanks,
>> Dehao
>>
>> Index: gcc/gimple-low.c
>> ===================================================================
>> --- gcc/gimple-low.c (revision 199414)
>> +++ gcc/gimple-low.c (working copy)
>> @@ -254,9 +254,13 @@ gimple_check_call_args (gimple stmt, tree fndecl)
>>     && !fold_convertible_p (DECL_ARG_TYPE (p), arg)))
>>               return false;
>>    }
>> +      if (p != NULL)
>> + return false;
>>       }
>>     else if (parms)
>>       {
>> +      if (list_length (parms) - nargs != 1)
>> + return false;
>>         for (i = 0, p = parms; i < nargs; i++, p = TREE_CHAIN (p))
>>    {
>>     tree arg;
>>
>

Patch

Index: gcc/gimple-low.c
===================================================================
--- gcc/gimple-low.c (revision 199414)
+++ gcc/gimple-low.c (working copy)
@@ -254,9 +254,13 @@  gimple_check_call_args (gimple stmt, tree fndecl)
   && !fold_convertible_p (DECL_ARG_TYPE (p), arg)))
             return false;
  }
+      if (p != NULL)
+ return false;
     }
   else if (parms)
     {
+      if (list_length (parms) - nargs != 1)
+ return false;
       for (i = 0, p = parms; i < nargs; i++, p = TREE_CHAIN (p))
  {
   tree arg;