[PATCH] rs6000: Stackoverflow in optimized code on PPC (PR100799)

[PATCH] rs6000: Stackoverflow in optimized code on PPC (PR100799)

[Ajit Agarwal]

Hello All:


When using FlexiBLAS with OpenBLAS we noticed corruption of
the parameters passed to OpenBLAS functions. FlexiBLAS
basically provides a BLAS interface where each function
is a stub that forwards the arguments to a real BLAS lib,
like OpenBLAS.

Fixes the corruption of caller frame checking number of
arguments is less than equal to GP_ARG_NUM_REG (8)
excluding hidden unused DECLS.

Bootstrapped and regtested on powerpc64-linux-gnu.

Thanks & Regards
Ajit


rs6000: Stackoverflow in optimized code on PPC (PR100799)

When using FlexiBLAS with OpenBLAS we noticed corruption of
the parameters passed to OpenBLAS functions. FlexiBLAS
basically provides a BLAS interface where each function
is a stub that forwards the arguments to a real BLAS lib,
like OpenBLAS.

Fixes the corruption of caller frame checking number of
arguments is less than equal to GP_ARG_NUM_REG (8)
excluding hidden unused DECLS.

2024-03-22  Ajit Kumar Agarwal  <aagarwa1@linux.ibm.com>

gcc/ChangeLog:

        PR rtk-optimization/100799
        * config/rs600/rs600-calls.cc (rs6000_function_arg): Don't
	generate parameter save area if number of arguments passed
	less than equal to GP_ARG_NUM_REG (8) excluding hidden
	paramter.
	* function.cc (assign_parms_initialize_all): Check for hidden
	parameter in fortran code and set the flag hidden_string_length
	and actual paramter passed excluding hidden unused DECLS.
	* function.h: Add new field hidden_string_length and
	actual_parm_length in function structure.
---
 gcc/config/rs6000/rs6000-call.cc | 11 ++++++++++-
 gcc/function.cc                  | 26 ++++++++++++++++++++++++++
 gcc/function.h                   | 10 ++++++++++
 3 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/gcc/config/rs6000/rs6000-call.cc b/gcc/config/rs6000/rs6000-call.cc
index 1f8f93a2ee7..8e6e3de6804 100644
--- a/gcc/config/rs6000/rs6000-call.cc
+++ b/gcc/config/rs6000/rs6000-call.cc
@@ -1857,7 +1857,16 @@ rs6000_function_arg (cumulative_args_t cum_v, const function_arg_info &arg)
 
 	  return rs6000_finish_function_arg (mode, rvec, k);
 	}
-      else if (align_words < GP_ARG_NUM_REG)
+     /* Workaround buggy C/C++ wrappers around Fortran routines with
+	character(len=constant) arguments if the hidden string length arguments
+	are passed on the stack; if the callers forget to pass those arguments,
+	attempting to tail call in such routines leads to stack corruption.
+	Avoid return stack space for parameters <= 8 excluding hidden string
+	length argument is passed (partially or fully) on the stack in the
+	caller and the callee needs to pass any arguments on the stack.  */
+      else if (align_words < GP_ARG_NUM_REG
+	       || (cfun->hidden_string_length
+	       && cfun->actual_parm_length <= GP_ARG_NUM_REG))
 	{
 	  if (TARGET_32BIT && TARGET_POWERPC64)
 	    return rs6000_mixed_function_arg (mode, type, align_words);
diff --git a/gcc/function.cc b/gcc/function.cc
index 3cef6c17bce..1318564b466 100644
--- a/gcc/function.cc
+++ b/gcc/function.cc
@@ -2326,6 +2326,32 @@ assign_parms_initialize_all (struct assign_parm_data_all *all)
 #endif
   all->args_so_far = pack_cumulative_args (&all->args_so_far_v);
 
+  unsigned int num_args = 0;
+  unsigned int hidden_length = 0;
+
+  /* Workaround buggy C/C++ wrappers around Fortran routines with
+     character(len=constant) arguments if the hidden string length arguments
+     are passed on the stack; if the callers forget to pass those arguments,
+     attempting to tail call in such routines leads to stack corruption.
+     Avoid return stack space for parameters <= 8 excluding hidden string
+     length argument is passed (partially or fully) on the stack in the
+     caller and the callee needs to pass any arguments on the stack.  */
+  for (tree arg = DECL_ARGUMENTS (current_function_decl);
+       arg; arg = DECL_CHAIN (arg))
+    {
+      num_args++;
+      if (DECL_HIDDEN_STRING_LENGTH (arg))
+	{
+	  tree parmdef = ssa_default_def (cfun, arg);
+	  if (parmdef == NULL || has_zero_uses (parmdef))
+	    {
+	      cfun->hidden_string_length = 1;
+	      hidden_length++;
+	    }
+	}
+   }
+
+  cfun->actual_parm_length = num_args - hidden_length;
 #ifdef INCOMING_REG_PARM_STACK_SPACE
   all->reg_parm_stack_space
     = INCOMING_REG_PARM_STACK_SPACE (current_function_decl);
diff --git a/gcc/function.h b/gcc/function.h
index 19e15bd63b0..5984f0007c2 100644
--- a/gcc/function.h
+++ b/gcc/function.h
@@ -346,6 +346,11 @@ struct GTY(()) function {
   /* Last assigned dependence info clique.  */
   unsigned short last_clique;
 
+  /* Actual parameter length ignoring hidden paramter.
+     This is done to C++ wrapper calling fortran module
+     which has hidden parameter that are not used.  */
+  unsigned int actual_parm_length;
+
   /* Collected bit flags.  */
 
   /* Number of units of general registers that need saving in stdarg
@@ -442,6 +447,11 @@ struct GTY(()) function {
   /* Set for artificial function created for [[assume (cond)]].
      These should be GIMPLE optimized, but not expanded to RTL.  */
   unsigned int assume_function : 1;
+
+  /* Hidden parameters while calling C++ wrapper to fortran
+     module. Set if there is hidden parameter in fortran
+     module while called C++ wrapper.  */
+  unsigned int hidden_string_length : 1;
 };
 
 /* Add the decl D to the local_decls list of FUN.  */
-- 
2.39.3

Re: [PATCH] rs6000: Stackoverflow in optimized code on PPC (PR100799)

[Jakub Jelinek]

On Fri, Mar 22, 2024 at 01:00:21PM +0530, Ajit Agarwal wrote:
> When using FlexiBLAS with OpenBLAS we noticed corruption of
> the parameters passed to OpenBLAS functions. FlexiBLAS
> basically provides a BLAS interface where each function
> is a stub that forwards the arguments to a real BLAS lib,
> like OpenBLAS.
> 
> Fixes the corruption of caller frame checking number of
> arguments is less than equal to GP_ARG_NUM_REG (8)
> excluding hidden unused DECLS.

Thanks for working on this.

> 2024-03-22  Ajit Kumar Agarwal  <aagarwa1@linux.ibm.com>
> 
> gcc/ChangeLog:
> 
>         PR rtk-optimization/100799
>         * config/rs600/rs600-calls.cc (rs6000_function_arg): Don't

These 2 lines are 8 space indented rather than tab.

> 	generate parameter save area if number of arguments passed
> 	less than equal to GP_ARG_NUM_REG (8) excluding hidden
> 	paramter.
> 	* function.cc (assign_parms_initialize_all): Check for hidden
> 	parameter in fortran code and set the flag hidden_string_length
> 	and actual paramter passed excluding hidden unused DECLS.

s/paramter/parameter/

> 	* function.h: Add new field hidden_string_length and
> 	actual_parm_length in function structure.

Why do you need to change generic code for something that will only be
used by a single target?
I mean, why don't you add the extra members in rs6000.h (struct rs6000_args)
and initialize them in rs6000-call.cc (init_cumulative_args) -
the function.cc function you've modified is the only one which uses
INIT_CUMULATIVE_INCOMING_ARGS and in that case init_cumulative_args is
called with incoming == true, so move the stuff from function.cc there.

> --- a/gcc/config/rs6000/rs6000-call.cc
> +++ b/gcc/config/rs6000/rs6000-call.cc
> @@ -1857,7 +1857,16 @@ rs6000_function_arg (cumulative_args_t cum_v, const function_arg_info &arg)
>  
>  	  return rs6000_finish_function_arg (mode, rvec, k);
>  	}
> -      else if (align_words < GP_ARG_NUM_REG)
> +     /* Workaround buggy C/C++ wrappers around Fortran routines with
> +	character(len=constant) arguments if the hidden string length arguments
> +	are passed on the stack; if the callers forget to pass those arguments,
> +	attempting to tail call in such routines leads to stack corruption.
> +	Avoid return stack space for parameters <= 8 excluding hidden string
> +	length argument is passed (partially or fully) on the stack in the
> +	caller and the callee needs to pass any arguments on the stack.  */
> +      else if (align_words < GP_ARG_NUM_REG
> +	       || (cfun->hidden_string_length
> +	       && cfun->actual_parm_length <= GP_ARG_NUM_REG))
>  	{
>  	  if (TARGET_32BIT && TARGET_POWERPC64)
>  	    return rs6000_mixed_function_arg (mode, type, align_words);
> diff --git a/gcc/function.cc b/gcc/function.cc
> index 3cef6c17bce..1318564b466 100644
> --- a/gcc/function.cc
> +++ b/gcc/function.cc
> @@ -2326,6 +2326,32 @@ assign_parms_initialize_all (struct assign_parm_data_all *all)
>  #endif
>    all->args_so_far = pack_cumulative_args (&all->args_so_far_v);
>  
> +  unsigned int num_args = 0;
> +  unsigned int hidden_length = 0;
> +
> +  /* Workaround buggy C/C++ wrappers around Fortran routines with
> +     character(len=constant) arguments if the hidden string length arguments
> +     are passed on the stack; if the callers forget to pass those arguments,
> +     attempting to tail call in such routines leads to stack corruption.
> +     Avoid return stack space for parameters <= 8 excluding hidden string
> +     length argument is passed (partially or fully) on the stack in the
> +     caller and the callee needs to pass any arguments on the stack.  */
> +  for (tree arg = DECL_ARGUMENTS (current_function_decl);
> +       arg; arg = DECL_CHAIN (arg))
> +    {
> +      num_args++;
> +      if (DECL_HIDDEN_STRING_LENGTH (arg))
> +	{
> +	  tree parmdef = ssa_default_def (cfun, arg);
> +	  if (parmdef == NULL || has_zero_uses (parmdef))
> +	    {
> +	      cfun->hidden_string_length = 1;
> +	      hidden_length++;
> +	    }
> +	}
> +   }
> +
> +  cfun->actual_parm_length = num_args - hidden_length;
>  #ifdef INCOMING_REG_PARM_STACK_SPACE
>    all->reg_parm_stack_space
>      = INCOMING_REG_PARM_STACK_SPACE (current_function_decl);
> diff --git a/gcc/function.h b/gcc/function.h
> index 19e15bd63b0..5984f0007c2 100644
> --- a/gcc/function.h
> +++ b/gcc/function.h
> @@ -346,6 +346,11 @@ struct GTY(()) function {
>    /* Last assigned dependence info clique.  */
>    unsigned short last_clique;
>  
> +  /* Actual parameter length ignoring hidden paramter.
> +     This is done to C++ wrapper calling fortran module
> +     which has hidden parameter that are not used.  */
> +  unsigned int actual_parm_length;
> +
>    /* Collected bit flags.  */
>  
>    /* Number of units of general registers that need saving in stdarg
> @@ -442,6 +447,11 @@ struct GTY(()) function {
>    /* Set for artificial function created for [[assume (cond)]].
>       These should be GIMPLE optimized, but not expanded to RTL.  */
>    unsigned int assume_function : 1;
> +
> +  /* Hidden parameters while calling C++ wrapper to fortran
> +     module. Set if there is hidden parameter in fortran
> +     module while called C++ wrapper.  */
> +  unsigned int hidden_string_length : 1;
>  };
>  
>  /* Add the decl D to the local_decls list of FUN.  */
> -- 
> 2.39.3

	Jakub

Re: [PATCH] rs6000: Stackoverflow in optimized code on PPC (PR100799)

[Ajit Agarwal]

Hello Jakub:

Addressed the below comments and sent version 1 of the patch
for review.

Thanks & Regards
Ajit

On 22/03/24 1:15 pm, Jakub Jelinek wrote:
> On Fri, Mar 22, 2024 at 01:00:21PM +0530, Ajit Agarwal wrote:
>> When using FlexiBLAS with OpenBLAS we noticed corruption of
>> the parameters passed to OpenBLAS functions. FlexiBLAS
>> basically provides a BLAS interface where each function
>> is a stub that forwards the arguments to a real BLAS lib,
>> like OpenBLAS.
>>
>> Fixes the corruption of caller frame checking number of
>> arguments is less than equal to GP_ARG_NUM_REG (8)
>> excluding hidden unused DECLS.
> 
> Thanks for working on this.
> 
>> 2024-03-22  Ajit Kumar Agarwal  <aagarwa1@linux.ibm.com>
>>
>> gcc/ChangeLog:
>>
>>         PR rtk-optimization/100799
>>         * config/rs600/rs600-calls.cc (rs6000_function_arg): Don't
> 
> These 2 lines are 8 space indented rather than tab.
> 
>> 	generate parameter save area if number of arguments passed
>> 	less than equal to GP_ARG_NUM_REG (8) excluding hidden
>> 	paramter.
>> 	* function.cc (assign_parms_initialize_all): Check for hidden
>> 	parameter in fortran code and set the flag hidden_string_length
>> 	and actual paramter passed excluding hidden unused DECLS.
> 
> s/paramter/parameter/
> 
>> 	* function.h: Add new field hidden_string_length and
>> 	actual_parm_length in function structure.
> 
> Why do you need to change generic code for something that will only be
> used by a single target?
> I mean, why don't you add the extra members in rs6000.h (struct rs6000_args)
> and initialize them in rs6000-call.cc (init_cumulative_args) -
> the function.cc function you've modified is the only one which uses
> INIT_CUMULATIVE_INCOMING_ARGS and in that case init_cumulative_args is
> called with incoming == true, so move the stuff from function.cc there.
> 
>> --- a/gcc/config/rs6000/rs6000-call.cc
>> +++ b/gcc/config/rs6000/rs6000-call.cc
>> @@ -1857,7 +1857,16 @@ rs6000_function_arg (cumulative_args_t cum_v, const function_arg_info &arg)
>>  
>>  	  return rs6000_finish_function_arg (mode, rvec, k);
>>  	}
>> -      else if (align_words < GP_ARG_NUM_REG)
>> +     /* Workaround buggy C/C++ wrappers around Fortran routines with
>> +	character(len=constant) arguments if the hidden string length arguments
>> +	are passed on the stack; if the callers forget to pass those arguments,
>> +	attempting to tail call in such routines leads to stack corruption.
>> +	Avoid return stack space for parameters <= 8 excluding hidden string
>> +	length argument is passed (partially or fully) on the stack in the
>> +	caller and the callee needs to pass any arguments on the stack.  */
>> +      else if (align_words < GP_ARG_NUM_REG
>> +	       || (cfun->hidden_string_length
>> +	       && cfun->actual_parm_length <= GP_ARG_NUM_REG))
>>  	{
>>  	  if (TARGET_32BIT && TARGET_POWERPC64)
>>  	    return rs6000_mixed_function_arg (mode, type, align_words);
>> diff --git a/gcc/function.cc b/gcc/function.cc
>> index 3cef6c17bce..1318564b466 100644
>> --- a/gcc/function.cc
>> +++ b/gcc/function.cc
>> @@ -2326,6 +2326,32 @@ assign_parms_initialize_all (struct assign_parm_data_all *all)
>>  #endif
>>    all->args_so_far = pack_cumulative_args (&all->args_so_far_v);
>>  
>> +  unsigned int num_args = 0;
>> +  unsigned int hidden_length = 0;
>> +
>> +  /* Workaround buggy C/C++ wrappers around Fortran routines with
>> +     character(len=constant) arguments if the hidden string length arguments
>> +     are passed on the stack; if the callers forget to pass those arguments,
>> +     attempting to tail call in such routines leads to stack corruption.
>> +     Avoid return stack space for parameters <= 8 excluding hidden string
>> +     length argument is passed (partially or fully) on the stack in the
>> +     caller and the callee needs to pass any arguments on the stack.  */
>> +  for (tree arg = DECL_ARGUMENTS (current_function_decl);
>> +       arg; arg = DECL_CHAIN (arg))
>> +    {
>> +      num_args++;
>> +      if (DECL_HIDDEN_STRING_LENGTH (arg))
>> +	{
>> +	  tree parmdef = ssa_default_def (cfun, arg);
>> +	  if (parmdef == NULL || has_zero_uses (parmdef))
>> +	    {
>> +	      cfun->hidden_string_length = 1;
>> +	      hidden_length++;
>> +	    }
>> +	}
>> +   }
>> +
>> +  cfun->actual_parm_length = num_args - hidden_length;
>>  #ifdef INCOMING_REG_PARM_STACK_SPACE
>>    all->reg_parm_stack_space
>>      = INCOMING_REG_PARM_STACK_SPACE (current_function_decl);
>> diff --git a/gcc/function.h b/gcc/function.h
>> index 19e15bd63b0..5984f0007c2 100644
>> --- a/gcc/function.h
>> +++ b/gcc/function.h
>> @@ -346,6 +346,11 @@ struct GTY(()) function {
>>    /* Last assigned dependence info clique.  */
>>    unsigned short last_clique;
>>  
>> +  /* Actual parameter length ignoring hidden paramter.
>> +     This is done to C++ wrapper calling fortran module
>> +     which has hidden parameter that are not used.  */
>> +  unsigned int actual_parm_length;
>> +
>>    /* Collected bit flags.  */
>>  
>>    /* Number of units of general registers that need saving in stdarg
>> @@ -442,6 +447,11 @@ struct GTY(()) function {
>>    /* Set for artificial function created for [[assume (cond)]].
>>       These should be GIMPLE optimized, but not expanded to RTL.  */
>>    unsigned int assume_function : 1;
>> +
>> +  /* Hidden parameters while calling C++ wrapper to fortran
>> +     module. Set if there is hidden parameter in fortran
>> +     module while called C++ wrapper.  */
>> +  unsigned int hidden_string_length : 1;
>>  };
>>  
>>  /* Add the decl D to the local_decls list of FUN.  */
>> -- 
>> 2.39.3
> 
> 	Jakub
>