* Warn about virtual table mismatches
@ 2014-02-12 3:54 Jan Hubicka
2014-02-12 4:22 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Jan Hubicka @ 2014-02-12 3:54 UTC (permalink / raw)
To: gcc-patches, jason
Hi,
this patch implements warning when we merge two virtual tables that do not
match. It compares the size and also go into fields. I had to implement my
own comparsion code, since the vtables may subtly differ - in RTTI and also I
need to do so during DECL merging (since we will forget about the decl then)
Here is an example of real ODR violation in bugzilla it finds.
../../dist/include/mozilla/a11y/DocAccessible.h:40:0: warning: type �struct DocAccessible� violates one definition rule [enabled by default]
class DocAccessible : public HyperTextAccessibleWrap,
^
/aux/hubicka/firefox/accessible/src/generic/DocAccessible.h:40:0: note: a type with the same name but different virtual table is defined in another translation unit
class DocAccessible : public HyperTextAccessibleWrap,
^
/aux/hubicka/firefox/accessible/src/generic/DocAccessible.cpp:1232:0: note: the first different method is �HandleAccEvent�
DocAccessible::HandleAccEvent(AccEvent* aEvent)
^
/aux/hubicka/firefox/accessible/src/atk/AccessibleWrap.cpp:956:0: note: a conflicting method is �HandleAccEvent�
AccessibleWrap::HandleAccEvent(AccEvent* aEvent)
^
The patch is not terribly pretty, but does the job. I will wait for few
days for comments/suggestins and then plan to commit it.
PR lto/59468
* lto/lto-symtab.c (lto_symtab_prevailing_decl): Check
virtual tables for ODR violations.
* ipa-devirt.c (compare_virtual_tables): New function.
* ipa-utils.h (compare_virtual_tables): Declare.
Index: lto/lto-symtab.c
===================================================================
--- lto/lto-symtab.c (revision 207702)
+++ lto/lto-symtab.c (working copy)
@@ -679,5 +679,13 @@ lto_symtab_prevailing_decl (tree decl)
if (!ret)
return decl;
+ /* Check and report ODR violations on virtual tables. */
+ if (decl != ret->decl && DECL_VIRTUAL_P (decl))
+ {
+ compare_virtual_tables (ret->decl, decl);
+ /* We are done with checking and DECL will die after merigng. */
+ DECL_VIRTUAL_P (decl) = 0;
+ }
+
return ret->decl;
}
Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c (revision 207702)
+++ ipa-devirt.c (working copy)
@@ -1675,6 +1673,101 @@ update_type_inheritance_graph (void)
}
+/* Compare two virtual tables, PREVAILING and VTABLE and output ODR
+ violation warings. */
+
+void
+compare_virtual_tables (tree prevailing, tree vtable)
+{
+ tree init1 = DECL_INITIAL (prevailing), init2 = DECL_INITIAL (vtable);
+ if (init1 == init2)
+ return;
+ if (init2 == error_mark_node)
+ return;
+ /* Be sure to keep virtual table contents even for external
+ vtables when they are available. */
+ if (init1 == error_mark_node || !init1)
+ {
+ DECL_INITIAL (prevailing) = DECL_INITIAL (vtable);
+ return;
+ }
+ if (!init2 && DECL_EXTERNAL (vtable))
+ return;
+ if (DECL_VIRTUAL_P (prevailing) && init1 && init2
+ && CONSTRUCTOR_NELTS (init1) == CONSTRUCTOR_NELTS (init2))
+ {
+ unsigned i;
+ tree field1, field2;
+ tree val1, val2;
+ bool matched = true;
+
+ FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (init1),
+ i, field1, val1)
+ {
+ gcc_assert (!field1);
+ field2 = CONSTRUCTOR_ELT (init2, i)->index;
+ val2 = CONSTRUCTOR_ELT (init2, i)->value;
+ gcc_assert (!field2);
+ if (val2 == val1)
+ continue;
+ STRIP_NOPS (val1);
+ STRIP_NOPS (val2);
+ /* Unwind
+ val <addr_expr type <pointer_type>
+ readonly constant
+ arg 0 <mem_ref type <pointer_type __vtbl_ptr_type>
+ readonly
+ arg 0 <addr_expr type <pointer_type>
+ arg 0 <var_decl _ZTCSd0_Si>> arg 1 <integer_cst 24>>> */
+
+ while ((TREE_CODE (val1) == MEM_REF
+ && TREE_CODE (val2) == MEM_REF
+ && (TREE_OPERAND (val1, 1)
+ == TREE_OPERAND (val2, 1)))
+ || (TREE_CODE (val1) == ADDR_EXPR
+ && TREE_CODE (val2) == ADDR_EXPR))
+ {
+ val1 = TREE_OPERAND (val1, 0);
+ val2 = TREE_OPERAND (val2, 0);
+ }
+ if (val1 == val2)
+ continue;
+ if (TREE_CODE (val2) == ADDR_EXPR)
+ {
+ tree tmp = val1;
+ val1 = val2;
+ val2 = tmp;
+ }
+ /* Allow combining RTTI and non-RTTI is OK. */
+ if (TREE_CODE (val1) == ADDR_EXPR
+ && TREE_CODE (TREE_OPERAND (val1, 0)) == VAR_DECL
+ && !DECL_VIRTUAL_P (TREE_OPERAND (val1, 0))
+ && TREE_CODE (val2) == NOP_EXPR
+ && integer_zerop (TREE_OPERAND (val2, 0)))
+ continue;
+ if (TREE_CODE (val1) == NOP_EXPR
+ && TREE_CODE (val2) == NOP_EXPR
+ && integer_zerop (TREE_OPERAND (val1, 0))
+ && integer_zerop (TREE_OPERAND (val2, 0)))
+ continue;
+ if (DECL_P (val1) && DECL_P (val2)
+ && DECL_ASSEMBLER_NAME (val1) == DECL_ASSEMBLER_NAME (val2))
+ continue;
+ matched = false;
+ break;
+ }
+ if (matched)
+ return;
+ }
+ odr_violation_reported = true;
+ if (warning_at (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (vtable))), 0,
+ "type %qD violates one definition rule ",
+ DECL_CONTEXT (vtable)))
+ inform (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (prevailing))),
+ "a type with the same name but different virtual table is "
+ "defined in another translation unit");
+}
+
/* Return true if N looks like likely target of a polymorphic call.
Rule out cxa_pure_virtual, noreturns, function declared cold and
other obvious cases. */
Index: ipa-utils.h
===================================================================
--- ipa-utils.h (revision 207702)
+++ ipa-utils.h (working copy)
@@ -92,6 +92,7 @@ bool get_polymorphic_call_info_from_inva
tree, tree, HOST_WIDE_INT);
tree vtable_pointer_value_to_binfo (tree t);
bool vtable_pointer_value_to_vtable (tree, tree *, unsigned HOST_WIDE_INT *);
+void compare_virtual_tables (tree, tree);
/* Return vector containing possible targets of polymorphic call E.
If FINALP is non-NULL, store true if the list is complette.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Warn about virtual table mismatches
2014-02-12 3:54 Warn about virtual table mismatches Jan Hubicka
@ 2014-02-12 4:22 ` Jason Merrill
2014-02-12 6:27 ` Jan Hubicka
0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2014-02-12 4:22 UTC (permalink / raw)
To: Jan Hubicka, gcc-patches
On 02/11/2014 07:54 PM, Jan Hubicka wrote:
> + /* Allow combining RTTI and non-RTTI is OK. */
You mean combining -frtti and -fno-rtti compiles? Yes, that's fine,
though you need to prefer the -frtti version in case code from that
translation unit uses the RTTI info.
> /aux/hubicka/firefox/accessible/src/generic/DocAccessible.cpp:1232:0: note: the first different method is �HandleAccEvent�
I don't see where this note would come from in the patch.
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Warn about virtual table mismatches
2014-02-12 4:22 ` Jason Merrill
@ 2014-02-12 6:27 ` Jan Hubicka
2014-02-12 8:01 ` Jason Merrill
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jan Hubicka @ 2014-02-12 6:27 UTC (permalink / raw)
To: Jason Merrill; +Cc: Jan Hubicka, gcc-patches
> On 02/11/2014 07:54 PM, Jan Hubicka wrote:
> >+ /* Allow combining RTTI and non-RTTI is OK. */
>
> You mean combining -frtti and -fno-rtti compiles? Yes, that's fine,
> though you need to prefer the -frtti version in case code from that
> translation unit uses the RTTI info.
Is there some mechanism that linker will do so? At the moment we just chose variant
that would be selected by linker. I can make the choice, but what happens with non-LTO?
>
> >/aux/hubicka/firefox/accessible/src/generic/DocAccessible.cpp:1232:0: note: the first different method is �HandleAccEvent�
>
> I don't see where this note would come from in the patch.
>
Sorry, diffed old tree
Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c (revision 207702)
+++ ipa-devirt.c (working copy)
@@ -1675,6 +1675,132 @@
}
+/* Compare two virtual tables, PREVAILING and VTABLE and output ODR
+ violation warings. */
+
+void
+compare_virtual_tables (tree prevailing, tree vtable)
+{
+ tree init1 = DECL_INITIAL (prevailing), init2 = DECL_INITIAL (vtable);
+ tree val1 = NULL, val2 = NULL;
+ if (!DECL_VIRTUAL_P (prevailing))
+ {
+ odr_violation_reported = true;
+ if (warning_at (DECL_SOURCE_LOCATION (prevailing), 0,
+ "declaration %D conflict with a virtual table",
+ prevailing))
+ inform (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (vtable))),
+ "a type defining the virtual table in another translation unit");
+ return;
+ }
+ if (init1 == init2 || init2 == error_mark_node)
+ return;
+ /* Be sure to keep virtual table contents even for external
+ vtables when they are available. */
+ gcc_assert (init1 && init1 != error_mark_node);
+ if (!init2 && DECL_EXTERNAL (vtable))
+ return;
+ if (init1 && init2
+ && CONSTRUCTOR_NELTS (init1) == CONSTRUCTOR_NELTS (init2))
+ {
+ unsigned i;
+ tree field1, field2;
+ bool matched = true;
+
+ FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (init1),
+ i, field1, val1)
+ {
+ gcc_assert (!field1);
+ field2 = CONSTRUCTOR_ELT (init2, i)->index;
+ val2 = CONSTRUCTOR_ELT (init2, i)->value;
+ gcc_assert (!field2);
+ if (val2 == val1)
+ continue;
+ if (TREE_CODE (val1) == NOP_EXPR)
+ val1 = TREE_OPERAND (val1, 0);
+ if (TREE_CODE (val2) == NOP_EXPR)
+ val2 = TREE_OPERAND (val2, 0);
+ /* Unwind
+ val <addr_expr type <pointer_type>
+ readonly constant
+ arg 0 <mem_ref type <pointer_type __vtbl_ptr_type>
+ readonly
+ arg 0 <addr_expr type <pointer_type>
+ arg 0 <var_decl _ZTCSd0_Si>> arg 1 <integer_cst 24>>> */
+
+ while (TREE_CODE (val1) == TREE_CODE (val2)
+ && (((TREE_CODE (val1) == MEM_REF
+ || TREE_CODE (val1) == POINTER_PLUS_EXPR)
+ && (TREE_OPERAND (val1, 1))
+ == TREE_OPERAND (val2, 1))
+ || TREE_CODE (val1) == ADDR_EXPR))
+ {
+ val1 = TREE_OPERAND (val1, 0);
+ val2 = TREE_OPERAND (val2, 0);
+ if (TREE_CODE (val1) == NOP_EXPR)
+ val1 = TREE_OPERAND (val1, 0);
+ if (TREE_CODE (val2) == NOP_EXPR)
+ val2 = TREE_OPERAND (val2, 0);
+ }
+ if (val1 == val2)
+ continue;
+ if (TREE_CODE (val2) == ADDR_EXPR)
+ {
+ tree tmp = val1;
+ val1 = val2;
+ val2 = tmp;
+ }
+ /* Combining RTTI and non-RTTI vtables is OK.
+ ??? Perhaps we can alsa (optionally) warn here? */
+ if (TREE_CODE (val1) == ADDR_EXPR
+ && TREE_CODE (TREE_OPERAND (val1, 0)) == VAR_DECL
+ && !DECL_VIRTUAL_P (TREE_OPERAND (val1, 0))
+ && integer_zerop (val2))
+ continue;
+ /* For some reason zeros gets NOP_EXPR wrappers. */
+ if (integer_zerop (val1)
+ && integer_zerop (val2))
+ continue;
+ /* Compare assembler names; this function is run during
+ declaration merging. */
+ if (DECL_P (val1) && DECL_P (val2)
+ && DECL_ASSEMBLER_NAME (val1) == DECL_ASSEMBLER_NAME (val2))
+ continue;
+ matched = false;
+ break;
+ }
+ if (!matched)
+ {
+ odr_violation_reported = true;
+ if (warning_at (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (vtable))), 0,
+ "type %qD violates one definition rule",
+ DECL_CONTEXT (vtable)))
+ {
+ inform (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (prevailing))),
+ "a type with the same name but different virtual table is "
+ "defined in another translation unit");
+ /* See if we can be more informative. */
+ if (val1 && val2 && TREE_CODE (val1) == FUNCTION_DECL
+ && TREE_CODE (val1) == FUNCTION_DECL
+ && !DECL_ARTIFICIAL (val1) && !DECL_ARTIFICIAL (val2))
+ {
+ inform (DECL_SOURCE_LOCATION (val1),
+ "the first different method is %qD", val1);
+ inform (DECL_SOURCE_LOCATION (val2),
+ "a conflicting method is %qD", val2);
+ }
+ }
+ }
+ return;
+ }
+ if (warning_at (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (vtable))), 0,
+ "type %qD violates one definition rule",
+ DECL_CONTEXT (vtable)))
+ inform (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (prevailing))),
+ "a type with the same name but number of virtual methods is "
+ "defined in another translation unit");
+}
+
/* Return true if N looks like likely target of a polymorphic call.
Rule out cxa_pure_virtual, noreturns, function declared cold and
other obvious cases. */
Index: lto/lto-symtab.c
===================================================================
--- lto/lto-symtab.c (revision 207701)
+++ lto/lto-symtab.c (working copy)
@@ -679,5 +679,14 @@
if (!ret)
return decl;
+ /* Check and report ODR violations on virtual tables. */
+ if (decl != ret->decl
+ && TREE_CODE (decl) == VAR_DECL && DECL_VIRTUAL_P (decl))
+ {
+ compare_virtual_tables (ret->decl, decl);
+ /* We are done with checking and DECL will die after merigng. */
+ DECL_VIRTUAL_P (decl) = 0;
+ }
+
return ret->decl;
}
Index: ipa-utils.h
===================================================================
--- ipa-utils.h (revision 207702)
+++ ipa-utils.h (working copy)
@@ -92,6 +92,7 @@
tree, tree, HOST_WIDE_INT);
tree vtable_pointer_value_to_binfo (tree t);
bool vtable_pointer_value_to_vtable (tree, tree *, unsigned HOST_WIDE_INT *);
+void compare_virtual_tables (tree, tree);
/* Return vector containing possible targets of polymorphic call E.
If FINALP is non-NULL, store true if the list is complette.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Warn about virtual table mismatches
2014-02-12 6:27 ` Jan Hubicka
@ 2014-02-12 8:01 ` Jason Merrill
2014-02-12 11:21 ` Richard Biener
2014-02-12 18:09 ` Bernhard Reutner-Fischer
2 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2014-02-12 8:01 UTC (permalink / raw)
To: Jan Hubicka; +Cc: gcc-patches
On 02/11/2014 10:27 PM, Jan Hubicka wrote:
>> On 02/11/2014 07:54 PM, Jan Hubicka wrote:
>>> + /* Allow combining RTTI and non-RTTI is OK. */
>>
>> You mean combining -frtti and -fno-rtti compiles? Yes, that's fine,
>> though you need to prefer the -frtti version in case code from that
>> translation unit uses the RTTI info.
>
> Is there some mechanism that linker will do so? At the moment we just chose variant
> that would be selected by linker. I can make the choice, but what happens with non-LTO?
Hmm, the linker might well make the wrong choice. Might be worth
warning about this as well.
> + "a type with the same name but number of virtual methods is "
"but different number"
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Warn about virtual table mismatches
2014-02-12 6:27 ` Jan Hubicka
2014-02-12 8:01 ` Jason Merrill
@ 2014-02-12 11:21 ` Richard Biener
2014-02-12 18:09 ` Bernhard Reutner-Fischer
2 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2014-02-12 11:21 UTC (permalink / raw)
To: Jan Hubicka; +Cc: Jason Merrill, GCC Patches
On Wed, Feb 12, 2014 at 7:27 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On 02/11/2014 07:54 PM, Jan Hubicka wrote:
>> >+ /* Allow combining RTTI and non-RTTI is OK. */
>>
>> You mean combining -frtti and -fno-rtti compiles? Yes, that's fine,
>> though you need to prefer the -frtti version in case code from that
>> translation unit uses the RTTI info.
>
> Is there some mechanism that linker will do so? At the moment we just chose variant
> that would be selected by linker. I can make the choice, but what happens with non-LTO?
>>
>> >/aux/hubicka/firefox/accessible/src/generic/DocAccessible.cpp:1232:0: note: the first different method is �HandleAccEvent�
>>
>> I don't see where this note would come from in the patch.
>>
> Sorry, diffed old tree
>
> Index: ipa-devirt.c
> ===================================================================
> --- ipa-devirt.c (revision 207702)
> +++ ipa-devirt.c (working copy)
> @@ -1675,6 +1675,132 @@
> }
>
>
> +/* Compare two virtual tables, PREVAILING and VTABLE and output ODR
> + violation warings. */
> +
> +void
> +compare_virtual_tables (tree prevailing, tree vtable)
> +{
> + tree init1 = DECL_INITIAL (prevailing), init2 = DECL_INITIAL (vtable);
> + tree val1 = NULL, val2 = NULL;
> + if (!DECL_VIRTUAL_P (prevailing))
> + {
> + odr_violation_reported = true;
> + if (warning_at (DECL_SOURCE_LOCATION (prevailing), 0,
> + "declaration %D conflict with a virtual table",
> + prevailing))
> + inform (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (vtable))),
> + "a type defining the virtual table in another translation unit");
> + return;
> + }
> + if (init1 == init2 || init2 == error_mark_node)
> + return;
> + /* Be sure to keep virtual table contents even for external
> + vtables when they are available. */
> + gcc_assert (init1 && init1 != error_mark_node);
> + if (!init2 && DECL_EXTERNAL (vtable))
> + return;
> + if (init1 && init2
> + && CONSTRUCTOR_NELTS (init1) == CONSTRUCTOR_NELTS (init2))
> + {
> + unsigned i;
> + tree field1, field2;
> + bool matched = true;
> +
> + FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (init1),
> + i, field1, val1)
> + {
> + gcc_assert (!field1);
> + field2 = CONSTRUCTOR_ELT (init2, i)->index;
> + val2 = CONSTRUCTOR_ELT (init2, i)->value;
> + gcc_assert (!field2);
> + if (val2 == val1)
> + continue;
> + if (TREE_CODE (val1) == NOP_EXPR)
> + val1 = TREE_OPERAND (val1, 0);
> + if (TREE_CODE (val2) == NOP_EXPR)
> + val2 = TREE_OPERAND (val2, 0);
> + /* Unwind
> + val <addr_expr type <pointer_type>
> + readonly constant
> + arg 0 <mem_ref type <pointer_type __vtbl_ptr_type>
> + readonly
> + arg 0 <addr_expr type <pointer_type>
> + arg 0 <var_decl _ZTCSd0_Si>> arg 1 <integer_cst 24>>> */
> +
> + while (TREE_CODE (val1) == TREE_CODE (val2)
> + && (((TREE_CODE (val1) == MEM_REF
> + || TREE_CODE (val1) == POINTER_PLUS_EXPR)
> + && (TREE_OPERAND (val1, 1))
> + == TREE_OPERAND (val2, 1))
> + || TREE_CODE (val1) == ADDR_EXPR))
> + {
> + val1 = TREE_OPERAND (val1, 0);
> + val2 = TREE_OPERAND (val2, 0);
> + if (TREE_CODE (val1) == NOP_EXPR)
> + val1 = TREE_OPERAND (val1, 0);
> + if (TREE_CODE (val2) == NOP_EXPR)
> + val2 = TREE_OPERAND (val2, 0);
> + }
> + if (val1 == val2)
> + continue;
> + if (TREE_CODE (val2) == ADDR_EXPR)
> + {
> + tree tmp = val1;
> + val1 = val2;
> + val2 = tmp;
> + }
> + /* Combining RTTI and non-RTTI vtables is OK.
> + ??? Perhaps we can alsa (optionally) warn here? */
> + if (TREE_CODE (val1) == ADDR_EXPR
> + && TREE_CODE (TREE_OPERAND (val1, 0)) == VAR_DECL
> + && !DECL_VIRTUAL_P (TREE_OPERAND (val1, 0))
> + && integer_zerop (val2))
> + continue;
> + /* For some reason zeros gets NOP_EXPR wrappers. */
> + if (integer_zerop (val1)
> + && integer_zerop (val2))
> + continue;
> + /* Compare assembler names; this function is run during
> + declaration merging. */
> + if (DECL_P (val1) && DECL_P (val2)
> + && DECL_ASSEMBLER_NAME (val1) == DECL_ASSEMBLER_NAME (val2))
> + continue;
> + matched = false;
> + break;
> + }
> + if (!matched)
> + {
> + odr_violation_reported = true;
> + if (warning_at (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (vtable))), 0,
Please don't add new warnings that cannot be turned off. Maybe
add -Wodr? (also use that on the two warnings emitted in lto-symtab.c).
Thanks,
Richard.
> + "type %qD violates one definition rule",
> + DECL_CONTEXT (vtable)))
> + {
> + inform (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (prevailing))),
> + "a type with the same name but different virtual table is "
> + "defined in another translation unit");
> + /* See if we can be more informative. */
> + if (val1 && val2 && TREE_CODE (val1) == FUNCTION_DECL
> + && TREE_CODE (val1) == FUNCTION_DECL
> + && !DECL_ARTIFICIAL (val1) && !DECL_ARTIFICIAL (val2))
> + {
> + inform (DECL_SOURCE_LOCATION (val1),
> + "the first different method is %qD", val1);
> + inform (DECL_SOURCE_LOCATION (val2),
> + "a conflicting method is %qD", val2);
> + }
> + }
> + }
> + return;
> + }
> + if (warning_at (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (vtable))), 0,
> + "type %qD violates one definition rule",
> + DECL_CONTEXT (vtable)))
> + inform (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (prevailing))),
> + "a type with the same name but number of virtual methods is "
> + "defined in another translation unit");
> +}
> +
> /* Return true if N looks like likely target of a polymorphic call.
> Rule out cxa_pure_virtual, noreturns, function declared cold and
> other obvious cases. */
> Index: lto/lto-symtab.c
> ===================================================================
> --- lto/lto-symtab.c (revision 207701)
> +++ lto/lto-symtab.c (working copy)
> @@ -679,5 +679,14 @@
> if (!ret)
> return decl;
>
> + /* Check and report ODR violations on virtual tables. */
> + if (decl != ret->decl
> + && TREE_CODE (decl) == VAR_DECL && DECL_VIRTUAL_P (decl))
> + {
> + compare_virtual_tables (ret->decl, decl);
> + /* We are done with checking and DECL will die after merigng. */
> + DECL_VIRTUAL_P (decl) = 0;
> + }
> +
> return ret->decl;
> }
> Index: ipa-utils.h
> ===================================================================
> --- ipa-utils.h (revision 207702)
> +++ ipa-utils.h (working copy)
> @@ -92,6 +92,7 @@
> tree, tree, HOST_WIDE_INT);
> tree vtable_pointer_value_to_binfo (tree t);
> bool vtable_pointer_value_to_vtable (tree, tree *, unsigned HOST_WIDE_INT *);
> +void compare_virtual_tables (tree, tree);
>
> /* Return vector containing possible targets of polymorphic call E.
> If FINALP is non-NULL, store true if the list is complette.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Warn about virtual table mismatches
2014-02-12 6:27 ` Jan Hubicka
2014-02-12 8:01 ` Jason Merrill
2014-02-12 11:21 ` Richard Biener
@ 2014-02-12 18:09 ` Bernhard Reutner-Fischer
2 siblings, 0 replies; 6+ messages in thread
From: Bernhard Reutner-Fischer @ 2014-02-12 18:09 UTC (permalink / raw)
To: Jan Hubicka, Jason Merrill; +Cc: gcc-patches
On 12 February 2014 07:27:59 Jan Hubicka <hubicka@ucw.cz> wrote:
> > On 02/11/2014 07:54 PM, Jan Hubicka wrote:
> > >+ /* Allow combining RTTI and non-RTTI is OK. */
> > You mean combining -frtti and -fno-rtti compiles? Yes, that's fine,
> > though you need to prefer the -frtti version in case code from that
> > translation unit uses the RTTI info.
>
> Is there some mechanism that linker will do so? At the moment we just chose
> variant
> that would be selected by linker. I can make the choice, but what happens
> with non-LTO?
> > >/aux/hubicka/firefox/accessible/src/generic/DocAccessible.cpp:1232:0:
> note: the first different method is �HandleAccEvent�
> > I don't see where this note would come from in the patch.
> > Sorry, diffed old tree
>
> Index: ipa-devirt.c
> ===================================================================
> --- ipa-devirt.c (revision 207702)
> +++ ipa-devirt.c (working copy)
> @@ -1675,6 +1675,132 @@
> }
>
>
> +/* Compare two virtual tables, PREVAILING and VTABLE and output ODR
> + violation warings. */
> +
> +void
> +compare_virtual_tables (tree prevailing, tree vtable)
> +{
> + tree init1 = DECL_INITIAL (prevailing), init2 = DECL_INITIAL (vtable);
> + tree val1 = NULL, val2 = NULL;
> + if (!DECL_VIRTUAL_P (prevailing))
> + {
> + odr_violation_reported = true;
> + if (warning_at (DECL_SOURCE_LOCATION (prevailing), 0,
> + "declaration %D conflict with a virtual table",
> + prevailing))
> + inform (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (vtable))),
> + "a type defining the virtual table in another translation unit");
> + return;
> + }
> + if (init1 == init2 || init2 == error_mark_node)
> + return;
> + /* Be sure to keep virtual table contents even for external
> + vtables when they are available. */
> + gcc_assert (init1 && init1 != error_mark_node);
> + if (!init2 && DECL_EXTERNAL (vtable))
> + return;
> + if (init1 && init2
> + && CONSTRUCTOR_NELTS (init1) == CONSTRUCTOR_NELTS (init2))
> + {
> + unsigned i;
> + tree field1, field2;
> + bool matched = true;
> +
> + FOR_EACH_CONSTRUCTOR_ELT (CONSTRUCTOR_ELTS (init1),
> + i, field1, val1)
> + {
> + gcc_assert (!field1);
> + field2 = CONSTRUCTOR_ELT (init2, i)->index;
> + val2 = CONSTRUCTOR_ELT (init2, i)->value;
> + gcc_assert (!field2);
> + if (val2 == val1)
> + continue;
> + if (TREE_CODE (val1) == NOP_EXPR)
> + val1 = TREE_OPERAND (val1, 0);
> + if (TREE_CODE (val2) == NOP_EXPR)
> + val2 = TREE_OPERAND (val2, 0);
> + /* Unwind
> + val <addr_expr type <pointer_type>
> + readonly constant
> + arg 0 <mem_ref type <pointer_type __vtbl_ptr_type>
> + readonly
> + arg 0 <addr_expr type <pointer_type>
> + arg 0 <var_decl _ZTCSd0_Si>> arg 1 <integer_cst 24>>> */
> +
> + while (TREE_CODE (val1) == TREE_CODE (val2)
> + && (((TREE_CODE (val1) == MEM_REF
> + || TREE_CODE (val1) == POINTER_PLUS_EXPR)
> + && (TREE_OPERAND (val1, 1))
> + == TREE_OPERAND (val2, 1))
> + || TREE_CODE (val1) == ADDR_EXPR))
> + {
> + val1 = TREE_OPERAND (val1, 0);
> + val2 = TREE_OPERAND (val2, 0);
> + if (TREE_CODE (val1) == NOP_EXPR)
> + val1 = TREE_OPERAND (val1, 0);
> + if (TREE_CODE (val2) == NOP_EXPR)
> + val2 = TREE_OPERAND (val2, 0);
> + }
> + if (val1 == val2)
> + continue;
> + if (TREE_CODE (val2) == ADDR_EXPR)
> + {
> + tree tmp = val1;
> + val1 = val2;
> + val2 = tmp;
> + }
> + /* Combining RTTI and non-RTTI vtables is OK.
> + ??? Perhaps we can alsa (optionally) warn here? */
> + if (TREE_CODE (val1) == ADDR_EXPR
> + && TREE_CODE (TREE_OPERAND (val1, 0)) == VAR_DECL
> + && !DECL_VIRTUAL_P (TREE_OPERAND (val1, 0))
> + && integer_zerop (val2))
> + continue;
> + /* For some reason zeros gets NOP_EXPR wrappers. */
> + if (integer_zerop (val1)
> + && integer_zerop (val2))
> + continue;
> + /* Compare assembler names; this function is run during
> + declaration merging. */
> + if (DECL_P (val1) && DECL_P (val2)
> + && DECL_ASSEMBLER_NAME (val1) == DECL_ASSEMBLER_NAME (val2))
> + continue;
> + matched = false;
> + break;
> + }
> + if (!matched)
> + {
> + odr_violation_reported = true;
> + if (warning_at (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT
> (vtable))), 0,
> + "type %qD violates one definition rule",
> + DECL_CONTEXT (vtable)))
> + {
> + inform (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (prevailing))),
> + "a type with the same name but different virtual table is "
> + "defined in another translation unit");
> + /* See if we can be more informative. */
> + if (val1 && val2 && TREE_CODE (val1) == FUNCTION_DECL
> + && TREE_CODE (val1) == FUNCTION_DECL
I suppose this second val1 ought to be val2.
> + && !DECL_ARTIFICIAL (val1) && !DECL_ARTIFICIAL (val2))
> + {
> + inform (DECL_SOURCE_LOCATION (val1),
> + "the first different method is %qD", val1);
> + inform (DECL_SOURCE_LOCATION (val2),
> + "a conflicting method is %qD", val2);
> + }
> + }
> + }
> + return;
> + }
> + if (warning_at (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (vtable))), 0,
> + "type %qD violates one definition rule",
> + DECL_CONTEXT (vtable)))
> + inform (DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (prevailing))),
> + "a type with the same name but number of virtual methods is "
> + "defined in another translation unit");
> +}
> +
> /* Return true if N looks like likely target of a polymorphic call.
> Rule out cxa_pure_virtual, noreturns, function declared cold and
> other obvious cases. */
> Index: lto/lto-symtab.c
> ===================================================================
> --- lto/lto-symtab.c (revision 207701)
> +++ lto/lto-symtab.c (working copy)
> @@ -679,5 +679,14 @@
> if (!ret)
> return decl;
>
> + /* Check and report ODR violations on virtual tables. */
> + if (decl != ret->decl
> + && TREE_CODE (decl) == VAR_DECL && DECL_VIRTUAL_P (decl))
> + {
> + compare_virtual_tables (ret->decl, decl);
> + /* We are done with checking and DECL will die after merigng. */
merging
Thanks,
> + DECL_VIRTUAL_P (decl) = 0;
> + }
> +
> return ret->decl;
> }
> Index: ipa-utils.h
> ===================================================================
> --- ipa-utils.h (revision 207702)
> +++ ipa-utils.h (working copy)
> @@ -92,6 +92,7 @@
> tree, tree, HOST_WIDE_INT);
> tree vtable_pointer_value_to_binfo (tree t);
> bool vtable_pointer_value_to_vtable (tree, tree *, unsigned HOST_WIDE_INT *);
> +void compare_virtual_tables (tree, tree);
>
> /* Return vector containing possible targets of polymorphic call E.
> If FINALP is non-NULL, store true if the list is complette.
Sent with AquaMail for Android
http://www.aqua-mail.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-12 18:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-12 3:54 Warn about virtual table mismatches Jan Hubicka
2014-02-12 4:22 ` Jason Merrill
2014-02-12 6:27 ` Jan Hubicka
2014-02-12 8:01 ` Jason Merrill
2014-02-12 11:21 ` Richard Biener
2014-02-12 18:09 ` Bernhard Reutner-Fischer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).