[PR 70929] IPA call type compatibility fix/workaround

[Martin Jambor <mjambor at suse dot cz>]

Hi,

I've been looking at PR 70929 and at the newly reported duplicate PR
91988 and would like to propose the following patch to address them.
Basically, the idea is that if the types or arguments in TYPE_ARG_TYPES
(as opposed to DECL_ARGUMENTS) from both the type from the target fndecl
and from call statement fntype match, then we can assume that whatever
promotions happened to the arguments they are the same in all
compilation units and the calls will be compatible.  I inserted this
test in the middle of gimple_check_call_args and it works for testcase
from both bugs.

The new test of course can be fooled with programs with clearly
undefined behavior, for example by having an indirect call which early
optimizations would discover to call an incompatible functions - but the
change considered in comment #5 of the bug would be too.  Moreover,
unless we stream argument types one will always be able to fool the
compiler and I find being careful about those and not inlining valid
calls with references to TREE_ADDRESSABLE classes a bad tradeoff.

I verified that - at least on gcc/libstdc++ testsuites - the new
gimple_check_call_args never returns false when the old one would return
true.  I can imagine us not doing the

  fold_convertible_p (TREE_VALUE (f), arg)

bit or returning false whenever reach the line

  tree p;

and the function has any parameters at all.  This would make the
function return false for on un-prototyped and/or K&R function
declarations, but perhaps we don't care too much?

In any event, I have bootstrapped and tested this on x86_64-linux, is it
perhaps OK for trunk?

Martin


2019-10-03  Martin Jambor  <mjambor@suse.cz>

	PR lto/70929
	* cgraph.c (gimple_check_call_args): Also compare types of argumen
	types and call statement fntype types.

	testsuite/
	* g++.dg/lto/pr70929_[01].C: New test.
---
 gcc/cgraph.c                         | 83 ++++++++++++++++++++++------
 gcc/testsuite/g++.dg/lto/pr70929_0.C | 18 ++++++
 gcc/testsuite/g++.dg/lto/pr70929_1.C | 10 ++++
 3 files changed, 95 insertions(+), 16 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/lto/pr70929_0.C
 create mode 100644 gcc/testsuite/g++.dg/lto/pr70929_1.C

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 0c3c6e7cac4..5a4c5253b49 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -3636,26 +3636,19 @@ cgraph_node::get_fun () const
 static bool
 gimple_check_call_args (gimple *stmt, tree fndecl, bool args_count_match)
 {
-  tree parms, p;
-  unsigned int i, nargs;
-
   /* Calls to internal functions always match their signature.  */
   if (gimple_call_internal_p (stmt))
     return true;
 
-  nargs = gimple_call_num_args (stmt);
+  unsigned int nargs = gimple_call_num_args (stmt);
 
-  /* Get argument types for verification.  */
-  if (fndecl)
-    parms = TYPE_ARG_TYPES (TREE_TYPE (fndecl));
-  else
-    parms = TYPE_ARG_TYPES (gimple_call_fntype (stmt));
-
-  /* Verify if the type of the argument matches that of the function
-     declaration.  If we cannot verify this or there is a mismatch,
-     return false.  */
+  /* If we have access to the declarations of formal parameters, match against
+     those.  */
   if (fndecl && DECL_ARGUMENTS (fndecl))
     {
+      unsigned int i;
+      tree p;
+
       for (i = 0, p = DECL_ARGUMENTS (fndecl);
 	   i < nargs;
 	   i++, p = DECL_CHAIN (p))
@@ -3676,17 +3669,75 @@ gimple_check_call_args (gimple *stmt, tree fndecl, bool args_count_match)
 	}
       if (args_count_match && p)
 	return false;
+
+      return true;
     }
-  else if (parms)
+
+  /* If we don't have decls of formal parameters, see if we have both the type
+     of the callee arguments in the fntype of the call statement and compare
+     those.  We rely on the fact that whatever promotions happened to
+     declarations for exactly the same sequence of types of parameters also
+     happened on the callee side.  */
+  if (fndecl
+      && TYPE_ARG_TYPES (TREE_TYPE (fndecl))
+      && TYPE_ARG_TYPES (gimple_call_fntype (stmt)))
     {
-      for (i = 0, p = parms; i < nargs; i++, p = TREE_CHAIN (p))
+      unsigned int arg_idx = 0;
+      for (tree f = TYPE_ARG_TYPES (TREE_TYPE (fndecl)),
+	     s = TYPE_ARG_TYPES (gimple_call_fntype (stmt));
+	   f || s;
+	   f = TREE_CHAIN (f), s = TREE_CHAIN (s), arg_idx++)
 	{
+	  if (!f
+	      || !s
+	      || TREE_VALUE (f) == error_mark_node
+	      || TREE_VALUE (s) == error_mark_node)
+	    return false;
+	  if (TREE_CODE (TREE_VALUE (f)) == VOID_TYPE)
+	    {
+	      if (TREE_CODE (TREE_VALUE (s)) != VOID_TYPE
+		  || arg_idx != nargs)
+		return false;
+	      else
+		break;
+	    }
+
 	  tree arg;
+
+	  if (arg_idx >= nargs
+	      || (arg = gimple_call_arg (stmt, arg_idx)) == error_mark_node)
+	    return false;
+
+	  if (TREE_CODE (TREE_VALUE (s)) == VOID_TYPE
+	      || (!types_compatible_p (TREE_VALUE (f), TREE_VALUE (s))
+		  && !fold_convertible_p (TREE_VALUE (f), arg)))
+	    return false;
+	}
+
+      if (args_count_match && arg_idx != nargs)
+	return false;
+
+      return true;
+    }
+
+  /* If we only have the fntype extracted from the call statement, check it
+     against the type of declarations while being pessimistic about
+     promotions.  */
+  tree p;
+
+  if (fndecl)
+    p = TYPE_ARG_TYPES (TREE_TYPE (fndecl));
+  else
+    p = TYPE_ARG_TYPES (gimple_call_fntype (stmt));
+  if (p)
+    {
+      for (unsigned i = 0; i < nargs; i++, p = TREE_CHAIN (p))
+	{
 	  /* If this is a varargs function defer inlining decision
 	     to callee.  */
 	  if (!p)
 	    break;
-	  arg = gimple_call_arg (stmt, i);
+	  tree arg = gimple_call_arg (stmt, i);
 	  if (TREE_VALUE (p) == error_mark_node
 	      || arg == error_mark_node
 	      || TREE_CODE (TREE_VALUE (p)) == VOID_TYPE
diff --git a/gcc/testsuite/g++.dg/lto/pr70929_0.C b/gcc/testsuite/g++.dg/lto/pr70929_0.C
new file mode 100644
index 00000000000..c96fb1c743a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr70929_0.C
@@ -0,0 +1,18 @@
+// { dg-lto-do run }
+// { dg-lto-options { "-O3 -flto" } }
+
+struct s
+{
+  int a;
+  s() {a=1;}
+  ~s() {}
+};
+int t(struct s s);
+int main()
+{
+  s s;
+  int v=t(s);
+  if (!__builtin_constant_p (v))
+    __builtin_abort ();
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/lto/pr70929_1.C b/gcc/testsuite/g++.dg/lto/pr70929_1.C
new file mode 100644
index 00000000000..b33aa8f35f0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr70929_1.C
@@ -0,0 +1,10 @@
+struct s
+{
+  int a;
+  s() {a=1;}
+  ~s() {}
+};
+int t(struct s s)
+{
+  return s.a;
+}
-- 
2.23.0