On Fri, Mar 22, 2024 at 02:55:43PM +0530, Ajit Agarwal wrote:
> 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.
Looks mostly good to me except some comment nits, but I'll defer
the actual ack to the rs6000 maintainers.
> + /* 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.
I thought it isn't just tail calls, even normal calls.
When the buggy C/C++ wrappers call the function with fewer arguments
than it actually has and doesn't expect the parameter save area on the
caller side because of that while the callee expects it and the callee
actually stores something in the parameter save area, it corrupts whatever
is in the caller stack frame at that location.
> + 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. */
> + unsigned int num_args = 0;
> + unsigned int hidden_length = 0;
> +
> + 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))
> + {
> + cum->hidden_string_length = 1;
> + hidden_length++;
> + }
> + }
> + }
> +
> + cum->actual_parm_length = num_args - hidden_length;
> +
> /* Check for a longcall attribute. */
> if ((!fntype && rs6000_default_long_calls)
> || (fntype
> @@ -1857,7 +1884,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
> + || (cum->hidden_string_length
> + && cum->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/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
> index 68bc45d65ba..a1d3ed00b14 100644
> --- a/gcc/config/rs6000/rs6000.h
> +++ b/gcc/config/rs6000/rs6000.h
> @@ -1490,6 +1490,14 @@ typedef struct rs6000_args
> int named; /* false for varargs params */
> int escapes; /* if function visible outside tu */
> int libcall; /* If this is a compiler generated
> call. */
> + /* Actual parameter length ignoring hidden paramter.
s/paramter/parameter/
> + This is done to C++ wrapper calling fortran module
> + which has hidden parameter that are not used. */
> + unsigned int actual_parm_length;
> + /* Hidden parameters while calling C++ wrapper to fortran
> + module. Set if there is hidden parameter in fortran
> + module while called C++ wrapper. */
modules in Fortran are something completely different.
You should IMHO talk about procedures instead of modules
in both of the above comments (multiple times even).
> + unsigned int hidden_string_length : 1;
> } CUMULATIVE_ARGS;
>
> /* Initialize a variable CUM of type CUMULATIVE_ARGS
> --
> 2.39.3
Jakub
