Hi,
this is the updated version of patch with testsuite compensation and
some changes:
- As discussed with Richard on IRC, I added -Wlto-type-mismatch to control
the warnings (enabled by default) and split ODR warnings from warnings
about incompatible types (in a sense of weak structural equality tested by
types_compatible_p at LTO time).
Both -Wlto-type-mismatch and -Wodr are on by default. -Wodr points will
output warnings only on conflicts between C++ programs that are positively
undefined by the standard (modulo implementation bugs) and
-Wlto-type-mismatch positives points out likely wrong code since the
declarations are not compatible to -fstrit-aliasing.
The testuiste fallout of Fortran was not that hard to fix, so I hope
-Wlto-type-mismatch is weak enough to make sense for mixed language
units. In fact I can use the ODR type matching tocde to implement
more strict checks for separate flag (-Wlto-strict-type-mismatch)
that would be off by default or perhaps just warn between C and C++
units.
- I got Firefox building and noticed a false positives on functions
when forward declaration and prototype was mixed. THis is because
useless_type_conversion compares function types but requires the outer
type to be the one more specified (prototype).
types_compatible_p checks that the types are convertible both directions
so it return false.
This whole code does not make much sense to be. I do not see why
useless_type_conversion needs to care about funcition types - we never
convert these. I also do not see why it match TYPE_METHOD_BASETYPE
when this field is never used for codegen. It is used by ubsan
and dbxout only.
I think useless_type_conversion can jsut ICE on function/method types
and types_compatible_p can be extended by same code as I have
in warn_types_mismatch minus the TYPE_METHOD_BASETYPE matching.
On a positive note, the patch also finds some real bugs in Firefox :)
- I also noticed that we may miss some ODR warnings when the declaration
is of compound type, not named type. (i.e. odr_type a[4];).
To make these working I added odr_or_derived_type_p and exported
the functionality to make structural compare from ipa-devirt.
The way of walking compount types was discussed with Jason here:
https://gcc.gnu.org/ml/gcc-patches/2014-07/msg00382.html
Bootstrapped/regtested x86_64-linux, ppc64 running. Also tested with
ltobootstrap.
OK?
* ipa-utils.h (warn_types_mismatch, odr_or_derived_type_p,
odr_types_equivalent_p): Declare.
(odr_type_p): Use gcc_checking_assert.
* common.opt (Wlto-type-mismatch): New warning.
* ipa-devirt.c (compound_type_base): New function.
(odr_or_derived_type_p): New function.
(odr_types_equivalent_p): New function.
(add_type_duplicate): Simplify.
* lto-symtab.c (warn_type_compatibility_p): Break out from ...;
compare ODR types (if available) and function types.
(lto_symtab_merge): ... here; output ODR violation warnings
and call warn_types_mismatch.
* gfortran.dg/lto/20091028-2_1.c: Fix return value.
* gfortran.dg/lto/pr41576_1.f90: Add interface.
* gfortran.dg/lto/pr41521_0.f90: Disable lto-type-mismatch
* gfortran.dg/lto/pr60635_0.f90: Disable lto-type-mismatch.
* gfortran.dg/lto/20091028-1_1.c: Fix return type.
* gcc.dg/lto/20120723_0.c: Disbale lto-type-mismatch.
Index: ipa-utils.h
===================================================================
--- ipa-utils.h (revision 222991)
+++ ipa-utils.h (working copy)
@@ -84,6 +84,9 @@ bool types_must_be_same_for_odr (tree, t
bool types_odr_comparable (tree, tree, bool strict = false);
cgraph_node *try_speculative_devirtualization (tree, HOST_WIDE_INT,
ipa_polymorphic_call_context);
+void warn_types_mismatch (tree t1, tree t2);
+bool odr_or_derived_type_p (const_tree t);
+bool odr_types_equivalent_p (tree type1, tree type2);
/* Return vector containing possible targets of polymorphic call E.
If COMPLETEP is non-NULL, store true if the list is complete.
@@ -164,7 +167,7 @@ odr_type_p (const_tree t)
return true;
/* We do not have this information when not in LTO, but we do not need
to care, since it is used only for type merging. */
- gcc_assert (in_lto_p || flag_lto);
+ gcc_checking_assert (in_lto_p || flag_lto);
return (TYPE_NAME (t)
&& (DECL_ASSEMBLER_NAME_SET_P (TYPE_NAME (t))));
Index: common.opt
===================================================================
--- common.opt (revision 222991)
+++ common.opt (working copy)
@@ -607,6 +607,10 @@ Woverflow
Common Var(warn_overflow) Init(1) Warning
Warn about overflow in arithmetic expressions
+Wlto-type-mismatch
+Common Var(warn_lto_type_mismatch) Init(1) Warning
+During link time optimization warn about mismatched types of global
declarations
+
Wpacked
Common Var(warn_packed) Warning
Warn when the packed attribute has no effect on struct layout
Index: lto/lto-symtab.c
===================================================================
--- lto/lto-symtab.c (revision 222991)
+++ lto/lto-symtab.c (working copy)
@@ -56,6 +56,7 @@ along with GCC; see the file COPYING3.
#include "ipa-prop.h"
#include "ipa-inline.h"
#include "builtins.h"
+#include "print-tree.h"
/* Replace the cgraph node NODE with PREVAILING_NODE in the cgraph, merging
all edges and removing the old node. */
@@ -203,45 +204,49 @@ lto_varpool_replace_node (varpool_node *
vnode->remove ();
}
-/* Merge two variable or function symbol table entries PREVAILING and ENTRY.
- Return false if the symbols are not fully compatible and a diagnostic
- should be emitted. */
+/* Return non-zero if we want to output waring about T1 and T2.
+ Return value is a bitmask of reasons of violation:
+ Bit 0 indicates that types are not compatible of memory layout.
+ Bot 1 indicates that types are not compatible because of C++ ODR rule. */
-static bool
-lto_symtab_merge (symtab_node *prevailing, symtab_node *entry)
+static int
+warn_type_compatibility_p (tree prevailing_type, tree type)
{
- tree prevailing_decl = prevailing->decl;
- tree decl = entry->decl;
- tree prevailing_type, type;
-
- if (prevailing_decl == decl)
- return true;
-
- /* Merge decl state in both directions, we may still end up using
- the new decl. */
- TREE_ADDRESSABLE (prevailing_decl) |= TREE_ADDRESSABLE (decl);
- TREE_ADDRESSABLE (decl) |= TREE_ADDRESSABLE (prevailing_decl);
-
- /* The linker may ask us to combine two incompatible symbols.
- Detect this case and notify the caller of required diagnostics. */
-
- if (TREE_CODE (decl) == FUNCTION_DECL)
- {
- if (!types_compatible_p (TREE_TYPE (prevailing_decl),
- TREE_TYPE (decl)))
- /* If we don't have a merged type yet...sigh. The linker
- wouldn't complain if the types were mismatched, so we
- probably shouldn't either. Just use the type from
- whichever decl appears to be associated with the
- definition. If for some odd reason neither decl is, the
- older one wins. */
- (void) 0;
-
- return true;
+ int lev = 0;
+ /* C++ provide a robust way to check for type compatibility via the ODR
+ rule. */
+ if (odr_or_derived_type_p (prevailing_type) && odr_type_p (type)
+ && !odr_types_equivalent_p (prevailing_type, type))
+ lev = 2;
+
+ /* Function types needs special care, because types_compatible_p never
+ thinks prototype is compatible to non-prototype. */
+ if ((TREE_CODE (type) == FUNCTION_TYPE || TREE_CODE (type) == METHOD_TYPE)
+ && TREE_CODE (type) == TREE_CODE (prevailing_type))
+ {
+ lev |= warn_type_compatibility_p (TREE_TYPE (prevailing_type),
+ TREE_TYPE (type));
+ if (TREE_CODE (type) == METHOD_TYPE)
+ lev |= warn_type_compatibility_p (TYPE_METHOD_BASETYPE
(prevailing_type),
+ TYPE_METHOD_BASETYPE (type));
+ if (prototype_p (prevailing_type) && prototype_p (type)
+ && TYPE_ARG_TYPES (prevailing_type) != TYPE_ARG_TYPES (type))
+ {
+ tree parm1, parm2;
+ for (parm1 = TYPE_ARG_TYPES (prevailing_type),
+ parm2 = TYPE_ARG_TYPES (type);
+ parm1 && parm2;
+ parm1 = TREE_CHAIN (prevailing_type),
+ parm2 = TREE_CHAIN (type))
+ lev |= warn_type_compatibility_p (TREE_VALUE (parm1),
+ TREE_VALUE (parm2));
+ if (parm1 || parm2)
+ lev = 3;
+ }
+ if (comp_type_attributes (prevailing_type, type) == 0)
+ lev = 3;
+ return lev;
}
-
- /* Now we exclusively deal with VAR_DECLs. */
-
/* Sharing a global symbol is a strong hint that two types are
compatible. We could use this information to complete
incomplete pointed-to types more aggressively here, ignoring
@@ -254,19 +259,22 @@ lto_symtab_merge (symtab_node *prevailin
??? In principle we might want to only warn for structurally
incompatible types here, but unless we have protective measures
for TBAA in place that would hide useful information. */
- prevailing_type = TYPE_MAIN_VARIANT (TREE_TYPE (prevailing_decl));
- type = TYPE_MAIN_VARIANT (TREE_TYPE (decl));
+ prevailing_type = TYPE_MAIN_VARIANT (prevailing_type);
+ type = TYPE_MAIN_VARIANT (type);
if (!types_compatible_p (prevailing_type, type))
{
- if (COMPLETE_TYPE_P (type))
- return false;
+ if (TREE_CODE (prevailing_type) == FUNCTION_TYPE
+ || TREE_CODE (type) == METHOD_TYPE)
+ return 1 | lev;
+ if (COMPLETE_TYPE_P (type) && COMPLETE_TYPE_P (prevailing_type))
+ return 1 | lev;
/* If type is incomplete then avoid warnings in the cases
that TBAA handles just fine. */
if (TREE_CODE (prevailing_type) != TREE_CODE (type))
- return false;
+ return 1 | lev;
if (TREE_CODE (prevailing_type) == ARRAY_TYPE)
{
@@ -280,10 +288,10 @@ lto_symtab_merge (symtab_node *prevailin
}
if (TREE_CODE (tem1) != TREE_CODE (tem2))
- return false;
+ return 1 | lev;
if (!types_compatible_p (tem1, tem2))
- return false;
+ return 1 | lev;
}
/* Fallthru. Compatible enough. */
@@ -292,6 +300,43 @@ lto_symtab_merge (symtab_node *prevailin
/* ??? We might want to emit a warning here if type qualification
differences were spotted. Do not do this unconditionally though. */
+ return lev;
+}
+
+/* Merge two variable or function symbol table entries PREVAILING and ENTRY.
+ Return false if the symbols are not fully compatible and a diagnostic
+ should be emitted. */
+
+static bool
+lto_symtab_merge (symtab_node *prevailing, symtab_node *entry)
+{
+ tree prevailing_decl = prevailing->decl;
+ tree decl = entry->decl;
+
+ if (prevailing_decl == decl)
+ return true;
+
+ /* Merge decl state in both directions, we may still end up using
+ the new decl. */
+ TREE_ADDRESSABLE (prevailing_decl) |= TREE_ADDRESSABLE (decl);
+ TREE_ADDRESSABLE (decl) |= TREE_ADDRESSABLE (prevailing_decl);
+
+ /* The linker may ask us to combine two incompatible symbols.
+ Detect this case and notify the caller of required diagnostics. */
+
+ if (TREE_CODE (decl) == FUNCTION_DECL)
+ {
+ if (warn_type_compatibility_p (TREE_TYPE (prevailing_decl),
+ TREE_TYPE (decl)))
+ return false;
+
+ return true;
+ }
+
+ if (warn_type_compatibility_p (TREE_TYPE (prevailing_decl),
+ TREE_TYPE (decl)))
+ return false;
+
/* There is no point in comparing too many details of the decls here.
The type compatibility checks or the completing of types has properly
dealt with most issues. */
@@ -483,24 +528,39 @@ lto_symtab_merge_decls_2 (symtab_node *f
/* Diagnose all mismatched re-declarations. */
FOR_EACH_VEC_ELT (mismatches, i, decl)
{
- if (!types_compatible_p (TREE_TYPE (prevailing->decl),
- TREE_TYPE (decl)))
- diagnosed_p |= warning_at (DECL_SOURCE_LOCATION (decl), 0,
- "type of %qD does not match original "
- "declaration", decl);
-
+ int level = warn_type_compatibility_p (TREE_TYPE (prevailing->decl),
+ TREE_TYPE (decl));
+ if (level)
+ {
+ bool diag = false;
+ if (level > 1)
+ diag = warning_at (DECL_SOURCE_LOCATION (decl),
+ OPT_Wodr,
+ "%qD violates the C++ One Definition Rule ",
+ decl);
+ if (!diag && (level & 1))
+ diag = warning_at (DECL_SOURCE_LOCATION (decl),
+ OPT_Wlto_type_mismatch,
+ "type of %qD does not match original "
+ "declaration", decl);
+ if (diag)
+ warn_types_mismatch (TREE_TYPE (prevailing->decl),
+ TREE_TYPE (decl));
+ diagnosed_p |= diag;
+ }
else if ((DECL_USER_ALIGN (prevailing->decl)
&& DECL_USER_ALIGN (decl))
&& DECL_ALIGN (prevailing->decl) < DECL_ALIGN (decl))
{
- diagnosed_p |= warning_at (DECL_SOURCE_LOCATION (decl), 0,
+ diagnosed_p |= warning_at (DECL_SOURCE_LOCATION (decl),
+ OPT_Wlto_type_mismatch,
"alignment of %qD is bigger than "
"original declaration", decl);
}
}
if (diagnosed_p)
inform (DECL_SOURCE_LOCATION (prevailing->decl),
- "previously declared here");
+ "%qD was previously declared here", prevailing->decl);
mismatches.release ();
}
Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c (revision 222991)
+++ ipa-devirt.c (working copy)
@@ -549,6 +549,59 @@ types_must_be_same_for_odr (tree t1, tre
return TYPE_MAIN_VARIANT (t1) == TYPE_MAIN_VARIANT (t2);
}
+/* If T is compound type, return type it is based on. */
+
+static tree
+compound_type_base (const_tree t)
+{
+ if (TREE_CODE (t) == ARRAY_TYPE
+ || POINTER_TYPE_P (t)
+ || TREE_CODE (t) == COMPLEX_TYPE
+ || VECTOR_TYPE_P (t))
+ return TREE_TYPE (t);
+ if (TREE_CODE (t) == METHOD_TYPE)
+ return TYPE_METHOD_BASETYPE (t);
+ if (TREE_CODE (t) == OFFSET_TYPE)
+ return TYPE_OFFSET_BASETYPE (t);
+ return NULL_TREE;
+}
+
+/* Return true if T is either ODR type or compound type based from it.
+ If the function return true, we know that T is a type originating from C++
+ source even at link-time. */
+
+bool
+odr_or_derived_type_p (const_tree t)
+{
+ do
+ {
+ if (odr_type_p (t))
+ return true;
+ /* Function type is a tricky one. Basically we can consider it
+ ODR derived if return type or any of the parameters is.
+ We need to check all parameters because LTO streaming merges
+ common types (such as void) and they are not considered ODR then. */
+ if (TREE_CODE (t) == FUNCTION_TYPE)
+ {
+ if (TYPE_METHOD_BASETYPE (t))
+ t = TYPE_METHOD_BASETYPE (t);
+ else
+ {
+ if (TREE_TYPE (t) && odr_or_derived_type_p (TREE_TYPE (t)))
+ return true;
+ for (t = TYPE_ARG_TYPES (t); t; t = TREE_CHAIN (t))
+ if (odr_or_derived_type_p (TREE_VALUE (t)))
+ return true;
+ return false;
+ }
+ }
+ else
+ t = compound_type_base (t);
+ }
+ while (t);
+ return t;
+}
+
/* Compare types T1 and T2 and return true if they are
equivalent. */
@@ -1577,6 +1642,20 @@ odr_types_equivalent_p (tree t1, tree t2
return true;
}
+/* Return true if TYPE1 and TYPE2 are equivalent for One Definition Rule. */
+
+bool
+odr_types_equivalent_p (tree type1, tree type2)
+{
+ hash_set<type_pair,pair_traits> visited;
+
+#ifdef ENABLE_CHECKING
+ gcc_assert (odr_or_derived_type_p (type1) && odr_or_derived_type_p (type2));
+#endif
+ return odr_types_equivalent_p (type1, type2, false, NULL,
+ &visited);
+}
+
/* TYPE is equivalent to VAL by ODR, but its tree representation differs
from VAL->type. This may happen in LTO where tree merging did not merge
all variants of the same type or due to ODR violation.
@@ -1701,12 +1780,8 @@ add_type_duplicate (odr_type val, tree t
base_mismatch = true;
}
else
- {
- hash_set<type_pair,pair_traits> visited;
- if (!odr_types_equivalent_p (type1, type2, false, NULL,
- &visited))
- base_mismatch = true;
- }
+ if (!odr_types_equivalent_p (type1, type2))
+ base_mismatch = true;
if (base_mismatch)
{
if (!warned && !val->odr_violated)
Index: testsuite/gfortran.dg/lto/20091028-2_1.c
===================================================================
--- testsuite/gfortran.dg/lto/20091028-2_1.c (revision 222991)
+++ testsuite/gfortran.dg/lto/20091028-2_1.c (working copy)
@@ -1,9 +1,9 @@
extern void *memcpy(void *dest, const void *src, __SIZE_TYPE__ n);
char *p;
-int int_gen_ti_header_c_ (char * hdrbuf, int * hdrbufsize,
- int * itypesize, int * typesize,
- int * DataHandle, char * Data,
- int * Count, int * code)
+void int_gen_ti_header_c_ (char * hdrbuf, int * hdrbufsize,
+ int * itypesize, int * typesize,
+ int * DataHandle, char * Data,
+ int * Count, int * code)
{
memcpy (typesize, p, sizeof(int)) ;
memcpy (Data, p, *Count * *typesize) ;
Index: testsuite/gfortran.dg/lto/pr41576_1.f90
===================================================================
--- testsuite/gfortran.dg/lto/pr41576_1.f90 (revision 222991)
+++ testsuite/gfortran.dg/lto/pr41576_1.f90 (working copy)
@@ -5,3 +5,8 @@ program test
if (c/=1 .or. d/=2) call abort
end program test
+interface
+ subroutine foo()
+ end subroutine
+end interface
+
Index: testsuite/gfortran.dg/lto/pr41521_0.f90
===================================================================
--- testsuite/gfortran.dg/lto/pr41521_0.f90 (revision 222991)
+++ testsuite/gfortran.dg/lto/pr41521_0.f90 (working copy)
@@ -1,9 +1,9 @@
! { dg-lto-do link }
-! { dg-lto-options {{-g -flto} {-g -O -flto}} }
+! { dg-lto-options {{-g -flto -Wno-lto-type-mismatch} {-g -O -flto
-Wno-lto-type-mismatch}} }
program species
integer spk(2)
real eval(2)
spk = 2
call atom(1.1,spk,eval)
end program
Index: testsuite/gfortran.dg/lto/pr60635_0.f90
===================================================================
--- testsuite/gfortran.dg/lto/pr60635_0.f90 (revision 222991)
+++ testsuite/gfortran.dg/lto/pr60635_0.f90 (working copy)
@@ -1,4 +1,5 @@
! { dg-lto-do link }
+! { dg-lto-options {{ -Wno-lto-type-mismatch }} }
program test
use iso_fortran_env
Index: testsuite/gfortran.dg/lto/pr41576_0.f90
===================================================================
--- testsuite/gfortran.dg/lto/pr41576_0.f90 (revision 222991)
+++ testsuite/gfortran.dg/lto/pr41576_0.f90 (working copy)
@@ -1,5 +1,5 @@
! { dg-lto-do run }
-! { dg-lto-options { { -O2 -flto -Werror } } }
+! { dg-lto-options { { -O2 -flto -Werror -Wno-lto-type-mismatch } } }
subroutine foo
common /bar/ a, b
Index: testsuite/gfortran.dg/lto/20091028-1_1.c
===================================================================
--- testsuite/gfortran.dg/lto/20091028-1_1.c (revision 222991)
+++ testsuite/gfortran.dg/lto/20091028-1_1.c (working copy)
@@ -1,9 +1,9 @@
extern void bcopy(const void *, void *, __SIZE_TYPE__ n);
char *p;
-int int_gen_ti_header_c_ (char * hdrbuf, int * hdrbufsize,
- int * itypesize, int * typesize,
- int * DataHandle, char * Data,
- int * Count, int * code)
+void int_gen_ti_header_c_ (char * hdrbuf, int * hdrbufsize,
+ int * itypesize, int * typesize,
+ int * DataHandle, char * Data,
+ int * Count, int * code)
{
bcopy (typesize, p, sizeof(int)) ;
bcopy (Data, p, *Count * *typesize) ;
Index: testsuite/gcc.dg/lto/20120723_0.c
===================================================================
--- testsuite/gcc.dg/lto/20120723_0.c (revision 222991)
+++ testsuite/gcc.dg/lto/20120723_0.c (working copy)
@@ -3,7 +3,7 @@
??? This testcase is invalid C and can only pass on specific platforms. */
/* { dg-lto-do run } */
/* { dg-skip-if "" { { sparc*-*-* } && ilp32 } { "*" } { "" } } */
-/* { dg-lto-options { {-O3 -fno-early-inlining -flto}} } */
+/* { dg-lto-options { {-O3 -fno-early-inlining -flto -Wno-lto-type-mismatch}}
} */
extern void abort (void);
