On Wed, Feb 12, 2014 at 7:27 AM, Jan Hubicka <hubi...@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.
