From: Jakub Jelinek <jakub@redhat.com>
To: Ajit Agarwal <aagarwa1@linux.ibm.com>
Cc: "Kewen.Lin" <linkw@linux.ibm.com>,
Segher Boessenkool <segher@kernel.crashing.org>,
Peter Bergner <bergner@linux.ibm.com>,
David Edelsohn <dje.gcc@gmail.com>,
Michael Meissner <meissner@linux.ibm.com>,
gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] rs6000: Stackoverflow in optimized code on PPC (PR100799)
Date: Fri, 22 Mar 2024 08:45:30 +0100 [thread overview]
Message-ID: <Zf03GuS7isaNmIYp@tucnak> (raw)
In-Reply-To: <17782aae-98cd-41e0-ac3c-18e0ed68f432@linux.ibm.com>
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
next prev parent reply other threads:[~2024-03-22 7:45 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-22 7:30 Ajit Agarwal
2024-03-22 7:45 ` Jakub Jelinek [this message]
2024-03-22 9:27 ` Ajit Agarwal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zf03GuS7isaNmIYp@tucnak \
--to=jakub@redhat.com \
--cc=aagarwa1@linux.ibm.com \
--cc=bergner@linux.ibm.com \
--cc=dje.gcc@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=linkw@linux.ibm.com \
--cc=meissner@linux.ibm.com \
--cc=segher@kernel.crashing.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).