On 3/6/20 2:11 AM, Richard Biener wrote:
On Fri, Mar 6, 2020 at 2:04 AM Martin Sebor <mse...@gmail.com> wrote:
Treating incompatible redeclarations of built-in functions as built-ins
is a problem not just for the middle-end but even for the C front-end
itself, when different parts of it make different assumptions about
what is and isn't valid. The test case that is the subject of this
bug report (a GCC 9 and 10 regression) is one such example: it shows
that the attribute format validation assumes the function declaration
the attribute applies to has passed the prerequisite validation. But
that's not the case when the function is an incompatibly redeclared
built-in where a format attribute's positional argument refers to
parameter of an invalid/nonsensical type.
The attached patch further adjusts the front-end to consider even more
incompatible redeclarations as built-ins: in particular, redeclarations
whose pointer arguments point to incompatible variants of unqualified
types (e.g., char* vs int*, though not char* vs const char*).
Besides avoiding the front-end and some middle-end ICEs, the effect
of the patch is also to diagnose more incompatible redeclarations
of built-ins than before, but fewer invalid calls to such functions
(since they're no longer considered built-ins). That seems like
an unavoidable trade-off.
Tested on x86_64-linux. Is this acceptable for GCC 10? How about 9?
The actual patch needs reviewing from a FE maintainer but I'd support
putting this in for GCC 10.
It would be nice if we can put in code like the following to catch "bogus"
built-in declarations. I've put it in call verification because there it's
where it matters most, free-lang-data would be another canonical place
which would then process all "reachable" function declarations but we
don't yet call free-lang-data when not doing LTO.
I committed the patch to trunk as it was but I wasn't brave enough
to also add this. I have tested it though and it fails in pr89998*.c
which declares sprintf to return unsigned int while the built-in has
it return signed in. GCC does diagnose these mismatches but only
with -Wextra and it still accepts such redeclarations as benign.
The patch also fails in c-c++-common/dfp/signbit-2.c which declares
int signbit (double) while GCC declares it as a variadic function.
(There a few more similar failures like this.)
I'm sure these expected mismatches can be dealt with and I'm up for
tightening up the checking even more and requiring exact matches but
I'd rather not do it at this stage.
Martin
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index f7b817d94e6..ae695891bd4 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -3356,6 +3356,17 @@ verify_gimple_call (gcall *stmt)
return true;
}
+ if (fndecl
+ && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)
+ && !types_compatible_p (TREE_TYPE (fndecl),
+ TREE_TYPE (builtin_decl_explicit
+ (DECL_FUNCTION_CODE (fndecl)))))
+ {
+ error ("function declaration declared built-in but does not "
+ "match expected type");
+ return true;
+ }
+
tree lhs = gimple_call_lhs (stmt);
if (lhs
&& (!is_gimple_lvalue (lhs)
Martin
