drop va_list from formals if requested

drop va_list from formals if requested

[Alexandre Oliva]

I'm working on a feature that involves creating wrappers for some
functions, using alternate means to pass variable argument lists to
the wrapped versions.  The split-out (wrapped) function shouldn't be
stdarg, and though comments in m_always_copy_start in
ipa_param_adjustments suggested that making it negative would cause
variable arguments to be dropped, this was not the case, and AFAICT
there was no way to accomplish that.

Perhaps there was no such intent implied by the comments, but that
sounded like a reasonable plan to me, so I went ahead an implemented
it.  With this patch, a negative m_always_copy_start drops the
variable argument list marker from a cloned function's type, and the
stdarg flag from the cloned cfun data structure.

Regstrapped on x86_64-linux-gnu.  Ok to install?


for  gcc/ChangeLog

	* ipa-param-manipulation.c (build_adjusted_function_type): Add
	skip_valist, obey it.
	(ipa_param_adjustments::build_new_function_type): Take
	negative m_always_copy_start as skip_valist.
	(ipa_param_body_adjustmnets::modify_formal_parameters):
	Likewise.
	* tee-inline.c (initialize_cfun): Clear stdarg if function
	type doesn't support it.
---
 gcc/ipa-param-manipulation.c |   17 ++++++++++++-----
 gcc/tree-inline.c            |    2 +-
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
index 26b02d7aa95c9..75b5a47a7ae8b 100644
--- a/gcc/ipa-param-manipulation.c
+++ b/gcc/ipa-param-manipulation.c
@@ -283,14 +283,17 @@ fill_vector_of_new_param_types (vec<tree> *new_types, vec<tree> *otypes,
 
 static tree
 build_adjusted_function_type (tree orig_type, vec<tree> *new_param_types,
-			      bool method2func, bool skip_return)
+			      bool method2func, bool skip_return,
+			      bool skip_valist)
 {
   tree new_arg_types = NULL;
   if (TYPE_ARG_TYPES (orig_type))
     {
       gcc_checking_assert (new_param_types);
-      bool last_parm_void = (TREE_VALUE (tree_last (TYPE_ARG_TYPES (orig_type)))
-			     == void_type_node);
+      bool last_parm_void = (skip_valist
+			     || (TREE_VALUE (tree_last (TYPE_ARG_TYPES
+							(orig_type)))
+				 == void_type_node));
       unsigned len = new_param_types->length ();
       for (unsigned i = 0; i < len; i++)
 	new_arg_types = tree_cons (NULL_TREE, (*new_param_types)[i],
@@ -458,8 +461,10 @@ ipa_param_adjustments::build_new_function_type (tree old_type,
   else
     new_param_types_p = NULL;
 
+  bool skip_valist = m_always_copy_start < 0;
   return build_adjusted_function_type (old_type, new_param_types_p,
-				       method2func_p (old_type), m_skip_return);
+				       method2func_p (old_type), m_skip_return,
+				       skip_valist);
 }
 
 /* Build variant of function decl ORIG_DECL which has no return value if
@@ -1309,8 +1314,10 @@ ipa_param_body_adjustments::modify_formal_parameters ()
      through tree_function_versioning, not when modifying function body
      directly.  */
   gcc_assert (!m_adjustments || !m_adjustments->m_skip_return);
+  bool skip_valist = m_adjustments && m_adjustments->m_always_copy_start < 0;
   tree new_type = build_adjusted_function_type (orig_type, &m_new_types,
-						m_method2func, false);
+						m_method2func, false,
+						skip_valist);
 
   TREE_TYPE (m_fndecl) = new_type;
   DECL_VIRTUAL_P (m_fndecl) = 0;
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index f605e763f4a61..0e9bb62242177 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -2793,7 +2793,7 @@ initialize_cfun (tree new_fndecl, tree callee_fndecl, profile_count count)
   cfun->va_list_fpr_size = src_cfun->va_list_fpr_size;
   cfun->has_nonlocal_label = src_cfun->has_nonlocal_label;
   cfun->calls_eh_return = src_cfun->calls_eh_return;
-  cfun->stdarg = src_cfun->stdarg;
+  cfun->stdarg = src_cfun->stdarg && stdarg_p (TREE_TYPE (new_fndecl));
   cfun->after_inlining = src_cfun->after_inlining;
   cfun->can_throw_non_call_exceptions
     = src_cfun->can_throw_non_call_exceptions;

-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

Re: drop va_list from formals if requested

[Martin Jambor]

Hi,

On Tue, Jul 13 2021, Alexandre Oliva wrote:
> I'm working on a feature that involves creating wrappers for some
> functions, using alternate means to pass variable argument lists to
> the wrapped versions.  The split-out (wrapped) function shouldn't be
> stdarg, and though comments in m_always_copy_start in
> ipa_param_adjustments suggested that making it negative would cause
> variable arguments to be dropped, this was not the case, and AFAICT
> there was no way to accomplish that.
>
> Perhaps there was no such intent implied by the comments, but that
> sounded like a reasonable plan to me, so I went ahead an implemented
> it.  With this patch, a negative m_always_copy_start drops the
> variable argument list marker from a cloned function's type, and the
> stdarg flag from the cloned cfun data structure.
>

It probably was my general intent, at least the comments really suggest
so :-)

However, my intent also was that functions which should have all their
arguments copied as they are (and only the return value dropped) should
be marked with NULL m_adj_params and m_always_copy_start set to zero.
Unfortunately, I did not make that work in time, I only vaguely remember
that ipa-split was somehow problematic.  I decided that I would not be
making a huge patch even bigger and settled on the idea that, at least
temporarily, m_always_copy_start would always be the initial number of
parameters.

As a consequence I also got lazy and used that assumption in
ipa_param_adjustments::modify_call where m_always_copy_start is meant to
be the original number of PARM_DECLs for which we may attempt to
generate debug info in:

      for (tree old_parm = DECL_ARGUMENTS (old_decl);
	   old_parm && i < old_nargs && ((int) i) < m_always_copy_start;
	   old_parm = DECL_CHAIN (old_parm), i++)

Fortunately, my recent re-write made that count available through other
means and we can now just use the orig_nargs local variable which is
already correctly computed at the beginning of the function.

> Regstrapped on x86_64-linux-gnu.  Ok to install?

Generally speaking, I agree that relaxing this assumption in general is
a good idea and so the patch is OK with this one additional fix
described above (I did not bootstrap and test it myself but it should
work (TM)).

Note that there are asserts in that check that m_always_copy_start is
either negative or the initial count of arguments so even after this
change m_always_copy_start cannot have an arbitrary value (and with the
exception of allowing zero I think that is a reasonable limitation).

>
> for  gcc/ChangeLog
>
> 	* ipa-param-manipulation.c (build_adjusted_function_type): Add
> 	skip_valist, obey it.
> 	(ipa_param_adjustments::build_new_function_type): Take
> 	negative m_always_copy_start as skip_valist.
> 	(ipa_param_body_adjustmnets::modify_formal_parameters):
> 	Likewise.
> 	* tee-inline.c (initialize_cfun): Clear stdarg if function
> 	type doesn't support it.

tRee-inline.c.

Thanks,

Martin