From patchwork Mon Aug 26 15:21:41 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Hubicka X-Patchwork-Id: 269907 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 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "www.sourceware.org", Issuer "StartCom Class 1 Primary Intermediate Server CA" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 258CF2C00B1 for ; Tue, 27 Aug 2013 01:21:52 +1000 (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:subject:message-id:mime-version:content-type; q=dns; s= default; b=DJRM+yufqHtsTs/5dcnjt7cvfozCaJH79W8JMqXt3desYr9lRY5Eb ZSkV72Ql202S1MfObpBx1+48+GMnTIQZ2DpKktbRzoUVSOeLw7a1H9k28a0tY0wn jZiN47xTRVEfFOH5aKjUL5ENcG5Szs3jeR2QzvLmX6vza5MwR/Y7ZQ= 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:subject:message-id:mime-version:content-type; s= default; bh=Wqmu333PojRPmNB2lQjF90oimk8=; b=oPepDKQu9jCFXSMotCmb cKaJsShuUEbe2BdNdNJc0FpLWePksY5amGvTHJ3p35Gt9Vq7Zu5X4tb7TQp/fpD8 wp0sY4pv2k6dsmQBEga3CkljJwN32pexkvxZnYYONtspC3XFa/ajE1h+PZsQNTJi rmjgWsXxvd6b65DijYpSBow= Received: (qmail 27616 invoked by alias); 26 Aug 2013 15:21:46 -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 27607 invoked by uid 89); 26 Aug 2013 15:21:46 -0000 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-SHA encrypted) ESMTPS; Mon, 26 Aug 2013 15:21:46 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL, BAYES_00, NO_RELAYS, TO_NO_BRKTS_PCNT autolearn=no version=3.3.2 X-HELO: nikam.ms.mff.cuni.cz Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 6675F543B2B; Mon, 26 Aug 2013 17:21:41 +0200 (CEST) Date: Mon, 26 Aug 2013 17:21:41 +0200 From: Jan Hubicka To: gcc-patches@gcc.gnu.org, jason@redhat.com, nathan@codesourcery.com, gdr@integrable-solutions.net Subject: ELF interposition and One Definition Rule Message-ID: <20130826152141.GA19918@kam.mff.cuni.cz> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-06-14) Hi, cgraph_function_body_availability determine if body of a given function is available in current compilation unit and if so, if it may be overwritten by completely different semantic by (dynamic)linker. Function that is AVAIL_AVAILABLE can still be repleaced by a function fron different unit, but its effects must be the same. Our default behaviour special case inline functions that are always AVAIL_AVAILABLE and, via decl_replaceable_p, also any COMDAT (that may be for functions since all COMDATs are also inlines, but makes difference for variables I think). In the variable initializer case we also special case DECL_VIRTUAL and assume that there is only one possible initializer for every virtual variable. The performance implications of cgraph_function_body_availability are important; -fPIC costs over 10% of performance but making everything AVAIL_AVAILABLE cuts the costs well under 1%. My understanding of C++ One Definition Rule, in a strict sense, does not a allow in to define two functions of the same name and different semantics in a valid program . I also think that all DSOs eventually linked together or dlopenned are part of the same program. So theoretically, for C++ produced code, we may go with AVAIL_AVAILABLE everywhere. After some discussion on mainline list Michael Matz pointed out that one can understand ODR in a sense that the interposed function was never part of the program, since it is completely replaced. On IRC we got into an agreement that we may disallow interposition for virtuals, since replacing these seems fishy - they don't even have address well defined and compiler can freely duplicate and/or unify them. I think same apply for C++ constructors and destructors now. Does the following patch seems sane? If there will be no opposition till end of week, I will go ahead with it. Of course I would be happier with a stronger rule - for instance allowing interposition only on plain functions not on methods. Main benefits of AVAILABLE is inlining, but it also helps to avoid expensive dynamic relocations. Making virtual functions AVAILABLE will allow us to hide them fron interaces of shared libraries. This should turn good part of non-local dynamic relocations into local dynamic relocations in practice. Bootstrapped/regtested ppc64-linux. Honza * cgraph.c (cgraph_function_body_availability): Also return AVAIL_AVAILABLE for virtual functions, constructors and destructors. Index: cgraph.c =================================================================== --- cgraph.c (revision 201996) +++ cgraph.c (working copy) @@ -2035,6 +2052,29 @@ cgraph_function_body_availability (struc behaviour on replacing inline function by different body. */ else if (DECL_DECLARED_INLINE_P (node->symbol.decl)) avail = AVAIL_AVAILABLE; + /* C++ One Deifnition Rule states that in the entire program, an object or + non-inline function cannot have more than one definition; if an object or + function is used, it must have exactly one definition. You can declare an + object or function that is never used, in which case you don't have to + provide a definition. In no event can there be more than one definition. + + In the interposition done by linking or dynamic linking one function + is replaced by another. Direct interpretation of C++ ODR would imply + that both functions must origin from the same definition and thus be + semantically equivalent and we should always return AVAIL_AVAILABLE. + + We however opted to be more conservative and allow interposition + for common (noninline) functions and methods based on interpretation + of ODR that simply consider the replaced function definition to not + be part of the program. + + We however do allow interposition of virtual functions on the basis that + their address is not well defined and compiler is free to duplicate their + bodies same way as it does with inline functions. */ + else if (DECL_VIRTUAL_P (node->symbol.decl) + || DECL_CXX_CONSTRUCTOR_P (node->symbol.decl) + || DECL_CXX_DESTRUCTOR_P (node->symbol.decl)) + avail = AVAIL_AVAILABLE; /* If the function can be overwritten, return OVERWRITABLE. Take care at least of two notable extensions - the COMDAT functions