Re: [PATCH] improve detection of incompatible redeclarations (PR 97882)
On Thu, 4 Feb 2021, Martin Sebor via Gcc-patches wrote: > Okay, that's much simpler. Thanks for nudging me in the right > direction! How's the attached patch? Retested on x86_64-linux. OK, minus the addition of gcc/testsuite/c-c++-common/array-lit.s which looks like a mistake. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] improve detection of incompatible redeclarations (PR 97882)
On 2/4/21 9:48 AM, Martin Sebor wrote:
On 2/3/21 5:15 PM, Joseph Myers wrote:
On Wed, 3 Feb 2021, Martin Sebor via Gcc-patches wrote:
+/* Return true of T1 and T2 are matching types for the purposes of
+ redeclaring a variable or a function without a prototype (i.e.,
+ considering just its return type). */
I think this comment is confusing (it suggests it's checking something
looser than the notion of compatibility checked by comptypes, but it's
actually checking something stricter). But I also think it's wrong to do
anything restricted to the return type.
For example, I think the following should be rejected, as enum E and
unsigned int end up incompatible and quite likely ABI-incompatible.
enum E;
void f(enum E);
void f(unsigned int);
enum E { x = 1ULL << 63 };
Maybe the ICE is specific to the case of return types, but I think the
same rules about type compatibility should apply regardless of precisely
where the incomplete enum type appears.
Ah, it's even more wide-spread than I realized.
I'd expect the natural fix for this PR to involve making
comptypes_internal treat an incomplete enum as incompatible with an
integer type. Only if that causes too many problems should we then look
at other approaches.
Let me look into it then.
Okay, that's much simpler. Thanks for nudging me in the right
direction! How's the attached patch? Retested on x86_64-linux.
Martin
PR c/97882 - Segmentation Fault on improper redeclaration of function
gcc/c/ChangeLog:
PR c/97882
* c-decl.c (locate_old_decl): Add type to diagnostic output.
(diagnose_mismatched_decls): Same.
(start_function): Introduce temporaries for better readability.
* c-typeck.c (comptypes_internal): Only consider complete enum
types in comparisons with integers.
gcc/testsuite/ChangeLog:
PR c/97882
* gcc.dg/decl-8.c: Adjust text of expected diagnostic.
* gcc.dg/label-decl-4.c: Same.
* gcc.dg/mismatch-decl-1.c: Same.
* gcc.dg/old-style-then-proto-1.c: Same.
* gcc.dg/parm-mismatch-1.c: Same.
* gcc.dg/pr35445.c: Same.
* gcc.dg/redecl-11.c: Same.
* gcc.dg/redecl-12.c: Same.
* gcc.dg/redecl-13.c: Same.
* gcc.dg/redecl-15.c: Same.
* gcc.dg/tls/thr-init-1.c: Same.
* objc.dg/id-1.m: Same.
* objc.dg/tls/diag-3.m: Same.
* c-c++-common/array-lit.s: New test.
* gcc.dg/pr97882.c: New test.
* gcc.dg/qual-return-7.c: New test.
* gcc.dg/qual-return-8.c: New test.
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index be95643fcf9..a5852a3bae7 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -1910,15 +1910,22 @@ validate_proto_after_old_defn (tree newdecl, tree newtype, tree oldtype)
static void
locate_old_decl (tree decl)
{
- if (TREE_CODE (decl) == FUNCTION_DECL && fndecl_built_in_p (decl)
+ if (TREE_CODE (decl) == FUNCTION_DECL
+ && fndecl_built_in_p (decl)
&& !C_DECL_DECLARED_BUILTIN (decl))
;
else if (DECL_INITIAL (decl))
-inform (input_location, "previous definition of %q+D was here", decl);
+inform (input_location,
+ "previous definition of %q+D with type %qT",
+ decl, TREE_TYPE (decl));
else if (C_DECL_IMPLICIT (decl))
-inform (input_location, "previous implicit declaration of %q+D was here", decl);
+inform (input_location,
+ "previous implicit declaration of %q+D with type %qT",
+ decl, TREE_TYPE (decl));
else
-inform (input_location, "previous declaration of %q+D was here", decl);
+inform (input_location,
+ "previous declaration of %q+D with type %qT",
+ decl, TREE_TYPE (decl));
}
/* Subroutine of duplicate_decls. Compare NEWDECL to OLDDECL.
@@ -2083,7 +2090,8 @@ diagnose_mismatched_decls (tree newdecl, tree olddecl,
&& C_DECL_IMPLICIT (olddecl) && !DECL_INITIAL (olddecl))
{
pedwarned = pedwarn (input_location, 0,
- "conflicting types for %q+D", newdecl);
+ "conflicting types for %q+D; have %qT",
+ newdecl, newtype);
/* Make sure we keep void as the return type. */
TREE_TYPE (olddecl) = *oldtypep = oldtype = newtype;
}
@@ -2119,7 +2127,7 @@ diagnose_mismatched_decls (tree newdecl, tree olddecl,
error ("conflicting type qualifiers for %q+D", newdecl);
}
else
- error ("conflicting types for %q+D", newdecl);
+ error ("conflicting types for %q+D; have %qT", newdecl, newtype);
diagnose_arglist_conflict (newdecl, olddecl, newtype, oldtype);
locate_old_decl (olddecl);
return false;
@@ -9457,27 +9465,29 @@ start_function (struct c_declspecs *declspecs, struct c_declarator *declarator,
current_function_prototype_locus = UNKNOWN_LOCATION;
current_function_prototype_built_in = false;
current_function_prototype_arg_types = NULL_TREE;
- if (!prototype_p (TREE_TYPE (decl1)))
+ tree newtype = TREE_TYPE (decl1);
+ tree oldtype = old_decl ? TREE_TYPE (old_decl) : newtype;
+ if (!prototype_p (newtype))
{
+ tree oldrt = TREE_TYPE (oldtype);
+ tree newrt = TREE_TYPE (newtype);
if (old_decl != NULL_TREE
- && TREE_CODE (TREE_TYPE
Re: [PATCH] improve detection of incompatible redeclarations (PR 97882)
On 2/3/21 5:15 PM, Joseph Myers wrote:
On Wed, 3 Feb 2021, Martin Sebor via Gcc-patches wrote:
+/* Return true of T1 and T2 are matching types for the purposes of
+ redeclaring a variable or a function without a prototype (i.e.,
+ considering just its return type). */
I think this comment is confusing (it suggests it's checking something
looser than the notion of compatibility checked by comptypes, but it's
actually checking something stricter). But I also think it's wrong to do
anything restricted to the return type.
For example, I think the following should be rejected, as enum E and
unsigned int end up incompatible and quite likely ABI-incompatible.
enum E;
void f(enum E);
void f(unsigned int);
enum E { x = 1ULL << 63 };
Maybe the ICE is specific to the case of return types, but I think the
same rules about type compatibility should apply regardless of precisely
where the incomplete enum type appears.
Ah, it's even more wide-spread than I realized.
I'd expect the natural fix for this PR to involve making
comptypes_internal treat an incomplete enum as incompatible with an
integer type. Only if that causes too many problems should we then look
at other approaches.
Let me look into it then.
Martin
Re: [PATCH] improve detection of incompatible redeclarations (PR 97882)
On Wed, 3 Feb 2021, Martin Sebor via Gcc-patches wrote:
> +/* Return true of T1 and T2 are matching types for the purposes of
> + redeclaring a variable or a function without a prototype (i.e.,
> + considering just its return type). */
I think this comment is confusing (it suggests it's checking something
looser than the notion of compatibility checked by comptypes, but it's
actually checking something stricter). But I also think it's wrong to do
anything restricted to the return type.
For example, I think the following should be rejected, as enum E and
unsigned int end up incompatible and quite likely ABI-incompatible.
enum E;
void f(enum E);
void f(unsigned int);
enum E { x = 1ULL << 63 };
Maybe the ICE is specific to the case of return types, but I think the
same rules about type compatibility should apply regardless of precisely
where the incomplete enum type appears.
I'd expect the natural fix for this PR to involve making
comptypes_internal treat an incomplete enum as incompatible with an
integer type. Only if that causes too many problems should we then look
at other approaches.
--
Joseph S. Myers
jos...@codesourcery.com
[PATCH] improve detection of incompatible redeclarations (PR 97882)
The test case in the bug report shows that the C front end is
too permissive in accepting invalid redeclarations that involve
an incomplete enum and an otherwise compatible integer type as
return types of a function without a prototype, or as types of
an ordinary variable. For example, the redeclaration below is
accepted:
extern enum E { e0 } e;
extern unsigned e;
In the case of a function the redeclaration can lead to a back
end ICE when one of the declarations is a function definition:
extern enum F f ();
extern unsigned f () { }
The attached patch tightens up the front end to reject even these
invalid redeclarations, thus avoiding the ICE.
Tested on x86_64-linux.
The bug is a P2 GCC 7-11 regression but in his comment Joseph
suggests to avoid backporting the fix to release branches due to
the potential to invalidate otherwise presumably benign code that's
accepted there, so I'm looking for approval only for GCC 11.
Martin
PR c/97882 - Segmentation Fault on improper redeclaration of function
gcc/c/ChangeLog:
PR c/97882
* c-decl.c (matching_types_p): New function.
(diagnose_mismatched_decls): Call it. Add detail to warning message.
(start_function): Call matching_types_p instead of comptypes.
gcc/testsuite/ChangeLog:
PR c/97882
* gcc.dg/decl-8.c: Adjust text of expected diagnostic.
* gcc.dg/label-decl-4.c: Same.
* gcc.dg/mismatch-decl-1.c: Same.
* gcc.dg/old-style-then-proto-1.c: Same.
* gcc.dg/parm-mismatch-1.c: Same.
* gcc.dg/pr35445.c: Same.
* gcc.dg/redecl-11.c: Same.
* gcc.dg/redecl-12.c: Same.
* gcc.dg/redecl-13.c: Same.
* gcc.dg/redecl-15.c: Same.
* gcc.dg/tls/thr-init-1.c: Same.
* objc.dg/tls/diag-3.m: Same.
* c-c++-common/array-lit.s: New test.
* gcc.dg/pr97882.c: New test.
* gcc.dg/qual-return-7.c: New test.
* gcc.dg/qual-return-8.c: New test.
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index be95643fcf9..d8102d71766 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -1910,15 +1910,79 @@ validate_proto_after_old_defn (tree newdecl, tree newtype, tree oldtype)
static void
locate_old_decl (tree decl)
{
- if (TREE_CODE (decl) == FUNCTION_DECL && fndecl_built_in_p (decl)
+ if (TREE_CODE (decl) == FUNCTION_DECL
+ && fndecl_built_in_p (decl)
&& !C_DECL_DECLARED_BUILTIN (decl))
;
else if (DECL_INITIAL (decl))
-inform (input_location, "previous definition of %q+D was here", decl);
+inform (input_location,
+ "previous definition of %q+D with type %qT",
+ decl, TREE_TYPE (decl));
else if (C_DECL_IMPLICIT (decl))
-inform (input_location, "previous implicit declaration of %q+D was here", decl);
+inform (input_location,
+ "previous implicit declaration of %q+D with type %qT",
+ decl, TREE_TYPE (decl));
else
-inform (input_location, "previous declaration of %q+D was here", decl);
+inform (input_location,
+ "previous declaration of %q+D with type %qT",
+ decl, TREE_TYPE (decl));
+}
+
+/* Return true of T1 and T2 are matching types for the purposes of
+ redeclaring a variable or a function without a prototype (i.e.,
+ considering just its return type). */
+
+static bool
+matching_types_p (tree_code which, tree t0, tree t1)
+{
+ bool types_differ = false;
+ if (!comptypes_check_different_types (t0, t1, _differ))
+return false;
+
+ if (!types_differ)
+return true;
+
+ if (which == FUNCTION_DECL)
+{
+ /* When WHICH denotes a function T1 and T2 may be either function
+ types or return types. */
+ if (TREE_CODE (t0) == FUNCTION_TYPE)
+ {
+ if (prototype_p (t0))
+ return true;
+
+ t0 = TREE_TYPE (t0);
+ t1 = TREE_TYPE (t1);
+ }
+
+ if (flag_isoc11)
+ {
+ t0 = TYPE_MAIN_VARIANT (t0);
+ t1 = TYPE_MAIN_VARIANT (t1);
+ }
+}
+ else if (which != VAR_DECL)
+return true;
+
+ /* Check the return type by itself and detect mismatches in non-pointer
+ types only. Pointers to arrays may be different when thee latter
+ specifies a bound that the former doesn't. */
+
+ while (TREE_CODE (t0) == ARRAY_TYPE || POINTER_TYPE_P (t0))
+t0 = TREE_TYPE (t0);
+ while (TREE_CODE (t1) == ARRAY_TYPE || POINTER_TYPE_P (t1))
+t1 = TREE_TYPE (t1);
+
+ if (TREE_CODE (t0) == FUNCTION_TYPE)
+/* For pointers to functions be sure to apply the rules above. */
+return matching_types_p (FUNCTION_DECL, t0, t1);
+
+ types_differ = false;
+ if (!comptypes_check_different_types (t0, t1, _differ)
+ || types_differ)
+return false;
+
+ return true;
}
/* Subroutine of duplicate_decls. Compare NEWDECL to OLDDECL.
@@ -1983,8 +2047,7 @@ diagnose_mismatched_decls (tree newdecl, tree olddecl,
bool pedwarned = false;
bool warned = false;
auto_diagnostic_group d;
-
- if (!comptypes (oldtype, newtype))
+ if (!matching_types_p (TREE_CODE (olddecl), oldtype, newtype))
{
if (TREE_CODE (olddecl) == FUNCTION_DECL
&& fndecl_built_in_p (olddecl, BUILT_IN_NORMAL)
@@ -2083,7 +2146,8 @@ diagnose_mismatched_decls (tree
