From patchwork Wed Feb 12 06:27:45 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jan Hubicka X-Patchwork-Id: 319523 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id A28EE2C00B3 for ; Wed, 12 Feb 2014 17:27:58 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:content-transfer-encoding:in-reply-to; q=dns; s= default; b=k0RZQ70+emhPEtSj92MojFpZvdBJBP/SRWSJfRi1/MxDpvPKsxUK/ FaR59pU0HCHm/j2Xnp3eDiQ8Kqokfug17ZcxkzrzwM74IgWLm50jiKE1E9R+EVz2 r0QdhpMmechDkQLG4ko8DL72bN0ZSetCHOoYCm2tG5RDTV6isvOZD0= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:content-transfer-encoding:in-reply-to; s=default; bh=wVD7ayhQXE0iuYknuvixh8x2AOk=; b=wrAPSZ/F0Xu8sSHBagKL6e1gLrim 8z6d/8gCzYvJbWcHF6F49sXU+KxC8mp+NwbbSTKTquiW/sweW0ro+AvdqVKE4A3G glwGzHBfqNViksk16G0xzcwXtuldUlO5M2qPj4w4Au+jZpW3xizVQFqV0cHz3Wv5 s/X4vCt7jcS4tVI= Received: (qmail 23635 invoked by alias); 12 Feb 2014 06:27:51 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 23618 invoked by uid 89); 12 Feb 2014 06:27:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: nikam.ms.mff.cuni.cz Received: from nikam.ms.mff.cuni.cz (HELO nikam.ms.mff.cuni.cz) (195.113.20.16) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 12 Feb 2014 06:27:48 +0000 Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 52DD8543DB2; Wed, 12 Feb 2014 07:27:45 +0100 (CET) Date: Wed, 12 Feb 2014 07:27:45 +0100 From: Jan Hubicka To: Jason Merrill Cc: Jan Hubicka , gcc-patches@gcc.gnu.org Subject: Re: Warn about virtual table mismatches Message-ID: <20140212062745.GA11693@kam.mff.cuni.cz> References: <20140212035443.GE18400@kam.mff.cuni.cz> <52FAF715.2020100@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <52FAF715.2020100@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) > 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 + readonly constant + arg 0 + readonly + arg 0 + arg 0 > arg 1 >> */ + + 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.