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 <mjam...@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