From patchwork Mon Oct 5 21:58:48 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sriraman Tallam X-Patchwork-Id: 526556 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 4FFE11402A8 for ; Tue, 6 Oct 2015 08:59:15 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=NrzAvjWQ; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=o5UHbpXEYdvUlPZzT2 k6P0QqA8ue1mFKaCYOrwvIdzTa09tcJWWOekZTIFDIWncu9iVr7v4XNltTd5J6b4 eqD5k1oxjj+GQ+dXWWzfWlOlHsy0VB67ew1EGPl6hZRHGXm1N9u9q6BUPiYZ7Wob 7RAylRFM7X6OP2eyf0CJbgjGc= 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 :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=Oiw/D3BlRc26Q/nf+IxK7Gz/ go0=; b=NrzAvjWQ44ToZvrzmD3Y+2Yg3dC0I/Rx8xF8ZpqK5scBsqo67lG3TBbx RO7wuUeQmyvNS18WEQGHWcvIF6A+5AeS/yKQ9GtF4IrP2D82tzAsl7UqqpMtGEZg Uf7saMVQVDqAmwTo4b58D1FPNbxFwBOWzMmDy6dlUXDLEYveb5Y= Received: (qmail 122089 invoked by alias); 5 Oct 2015 21:58:55 -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 122070 invoked by uid 89); 5 Oct 2015 21:58:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL, BAYES_00, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_LOW, SPF_PASS, T_RP_MATCHES_RCVD autolearn=unavailable version=3.3.2 X-HELO: mail-wi0-f172.google.com Received: from mail-wi0-f172.google.com (HELO mail-wi0-f172.google.com) (209.85.212.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 05 Oct 2015 21:58:51 +0000 Received: by wicfx3 with SMTP id fx3so140027056wic.1 for ; Mon, 05 Oct 2015 14:58:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=eM2so/WacoRl8DcURr601m9v675bDzrZ/Ruk5M/ijzk=; b=brmod1Ya8mJg8mZH69H7i28mdIwp61y6oKwfz2tdiGW6kNoCGcQl10TUNB/4N1khCt g8KOQLLUuhXPocmX5TsUV9lgfY6n3Q7hxKLraN9/8v6f/gjPbDgiDMPNOAu73i1FHGIr uEMPmat+NbhPoMdJXGaDk1hvhfEj8C2b3dE/EKp++kYXZEOZGFZePq69PZoIAd7dTKYx DX/lNIpEU55bq7+1zf7rsXMCXb+HMZSjoc+Me9TJyQWrbbsAApvFKT/Uj+/OfINdDZGl YwzCK1gqkmSr458e4VnOaRZ9vDan07KDdbkJbFQdkMM/ACMOD3I4NPshhBBW2rbID/pK oQuQ== X-Gm-Message-State: ALoCoQmHid9sQmKY5dOzp1MHwrnqqx3GbnUFyQ+D8jvI33nvjGDvazm2172c6a9O7znA+CwDQohm MIME-Version: 1.0 X-Received: by 10.180.100.161 with SMTP id ez1mr14830610wib.60.1444082328544; Mon, 05 Oct 2015 14:58:48 -0700 (PDT) Received: by 10.28.27.67 with HTTP; Mon, 5 Oct 2015 14:58:48 -0700 (PDT) In-Reply-To: References: Date: Mon, 5 Oct 2015 14:58:48 -0700 Message-ID: Subject: Re: [RFC] COMDAT Safe Module Level Multi versioning From: Sriraman Tallam To: Cary Coutant Cc: Xinliang David Li , Richard Biener , "H.J. Lu" , GCC Patches , binutils , Yury Gribov X-IsSubscribed: yes On Wed, Sep 9, 2015 at 4:01 PM, Sriraman Tallam wrote: > On Wed, Sep 2, 2015 at 4:32 PM, Sriraman Tallam wrote: >> >> On Tue, Aug 18, 2015 at 9:46 PM, Cary Coutant wrote: >> >> Thanks, will make those changes. Do you recommend a different name >> >> for this flag like -fmake-comdat-functions-static? >> > >> > Well, the C++ ABI refers to this as "vague linkage." It may be a bit >> > too long or too ABI-specific, but maybe something like >> > -f[no-]use-vague-linkage-for-functions or >> > -f[no-]functions-vague-linkage? >> >> Done and patch attached. > > > > Ping. Ping. > > * c-family/c.opt (fvague-linkage-functions): New option. > * cp/decl2.c (comdat_linkage): Implement new option. Warn when > virtual comdat functions are seen. > * ipa.c (function_and_variable_visibility): Check for no vague > linkage. > * doc/invoke.texi: Document new option. > * testsuite/g++.dg/no-vague-linkage-functions-1.C: New test. > >> >> >> * c-family/c.opt (fvague-linkage-functions): New option. >> * cp/decl2.c (comdat_linkage): Implement new option. Warn when >> virtual comdat functions are seen. >> * ipa.c (function_and_variable_visibility): Check for no vague >> linkage. >> * doc/invoke.texi: Document new option. >> * testsuite/g++.dg/no-vague-linkage-functions-1.C: New test. >> >> >> >> >> > >> > And perhaps note in the doc that using this option may technically >> > break the C++ ODR, so it should be used only when you know what you're >> > doing. >> >> Done. >> >> Thanks >> Sri >> >> > >> > -cary * c-family/c.opt (fvague-linkage-functions): New option. * cp/decl2.c (comdat_linkage): Implement new option. Warn when virtual comdat functions are seen. * ipa.c (function_and_variable_visibility): Check for no vague linkage. * doc/invoke.texi: Document new option. * testsuite/g++.dg/no-vague-linkage-functions-1.C: New test. Index: c-family/c.opt =================================================================== --- c-family/c.opt (revision 227383) +++ c-family/c.opt (working copy) @@ -1236,6 +1236,16 @@ fweak C++ ObjC++ Var(flag_weak) Init(1) Emit common-like symbols as weak symbols +fvague-linkage-functions +C++ Var(flag_vague_linkage_functions) Init(1) +Option -fno-vague-linkage-functions makes comdat functions static and local +to the module. With -fno-vague-linkage-functions, virtual comdat functions +still use vague linkage. With -fno-vague-linkage-functions, the address of +the comdat functions that are made local will be unique and this can cause +unintended behavior when addresses of these comdat functions are used in +comparisons. This option may technically break the C++ ODR and users of +this flag should know what they are doing. + fwide-exec-charset= C ObjC C++ ObjC++ Joined RejectNegative -fwide-exec-charset= Convert all wide strings and character constants to character set Index: cp/decl2.c =================================================================== --- cp/decl2.c (revision 227383) +++ cp/decl2.c (working copy) @@ -1702,8 +1702,22 @@ mark_vtable_entries (tree decl) void comdat_linkage (tree decl) { - if (flag_weak - make_decl_one_only (decl, cxx_comdat_group (decl)); + if (flag_weak + && (flag_vague_linkage_functions + || TREE_CODE (decl) != FUNCTION_DECL + || DECL_VIRTUAL_P (decl))) + { + make_decl_one_only (decl, cxx_comdat_group (decl)); + /* Warn when -fno-vague-linkage-functions is used and we found virtual + comdat functions. Virtual comdat functions must still use vague + linkage. */ + if (TREE_CODE (decl) == FUNCTION_DECL + && DECL_VIRTUAL_P (decl) + && !flag_vague_linkage_functions) + warning_at (DECL_SOURCE_LOCATION (decl), 0, + "fno-vague-linkage-functions: Comdat linkage of virtual " + "function %q#D preserved.", decl); + } else if (TREE_CODE (decl) == FUNCTION_DECL || (VAR_P (decl) && DECL_ARTIFICIAL (decl))) /* We can just emit function and compiler-generated variables Index: doc/invoke.texi =================================================================== --- doc/invoke.texi (revision 227383) +++ doc/invoke.texi (working copy) @@ -189,7 +189,7 @@ in the following sections. -fno-pretty-templates @gol -frepo -fno-rtti -fstats -ftemplate-backtrace-limit=@var{n} @gol -ftemplate-depth=@var{n} @gol --fno-threadsafe-statics -fuse-cxa-atexit -fno-weak -nostdinc++ @gol +-fno-threadsafe-statics -fuse-cxa-atexit -fno-weak -fno-vague-linkage-functions -nostdinc++ @gol -fvisibility-inlines-hidden @gol -fvtable-verify=@var{std|preinit|none} @gol -fvtv-counts -fvtv-debug @gol @@ -2448,6 +2448,18 @@ option exists only for testing, and should not be it results in inferior code and has no benefits. This option may be removed in a future release of G++. +@item -fno-vague-linkage-functions +@opindex fno-vague-linkage-functions +Do not use vague linkage for comdat non-virtual functions, even if it +is provided by the linker. This option is useful when comdat functions +generated in certain compilation units need to be kept local to the +respective units and not exposed globally. This does not apply to virtual +comdat functions as their pointers may be taken via virtual tables. +This can cause unintended behavior if the addresses of the comdat functions +that are made local are used in comparisons, which are not warned about. +This option may technically break the C++ ODR and users of this flag should +know what they are doing. + @item -nostdinc++ @opindex nostdinc++ Do not search for header files in the standard directories specific to Index: ipa.c =================================================================== --- ipa.c (revision 227383) +++ ipa.c (working copy) @@ -1037,6 +1037,7 @@ function_and_variable_visibility (bool whole_progr } gcc_assert ((!DECL_WEAK (node->decl) && !DECL_COMDAT (node->decl)) + || !flag_vague_linkage_functions || TREE_PUBLIC (node->decl) || node->weakref || DECL_EXTERNAL (node->decl)); Index: testsuite/g++.dg/no-vague-linkage-functions-1.C =================================================================== --- testsuite/g++.dg/no-vague-linkage-functions-1.C (revision 0) +++ testsuite/g++.dg/no-vague-linkage-functions-1.C (working copy) @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-fno-vague-linkage-functions -fkeep-inline-functions -ffunction-sections" } */ + +inline int foo () { + return 0; +} + +/* { dg-final { scan-assembler "_Z3foov" } } */ +/* { dg-final { scan-assembler-not "_Z3foov.*comdat" } } */