From patchwork Wed Sep 9 23:01:44 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sriraman Tallam X-Patchwork-Id: 516061 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 035C8140271 for ; Thu, 10 Sep 2015 09:01:58 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=IYEwKC6K; 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=pL9NRJrSKMwAON/emf 4TfGAgxiq3ZetBt9MFf5HIeFOycgt4l3ZIwkrcJtkeq1+IBnRVWncsQAKVXWyj/1 7JFXuHgtmWoz0+TOfkYFi+UX+KJwHQphAfxJsXRF8Qd19A50R20bmwHQLdxIlXfw ATUAiwemdKaG0nL5t8LbvIFOE= 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=VCInvph45SS7vzXVX9AOnVai u2U=; b=IYEwKC6KvDuFQ8diYgPFD7AumTNT/5xMr2iHLVW+pY1sTiUKTHUXbEgT /FEleD86xKoI1ci7ql6jPpIPr0YxCAFMnrXzGCwVeyibSBJdYPTNirX6s5jbB34B 2iI/DN3C2XN1qYrLFV9E3Ovc8JxeZRQZFVdKvsmDdJfCVY3QGb4= Received: (qmail 59683 invoked by alias); 9 Sep 2015 23:01:50 -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 59661 invoked by uid 89); 9 Sep 2015 23:01:49 -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=ham version=3.3.2 X-HELO: mail-wi0-f170.google.com Received: from mail-wi0-f170.google.com (HELO mail-wi0-f170.google.com) (209.85.212.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 09 Sep 2015 23:01:47 +0000 Received: by wicgb1 with SMTP id gb1so3601755wic.1 for ; Wed, 09 Sep 2015 16:01:45 -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=zJfYFMYnZZPbs4mKJhXp9GBiSEiixfeV7TXdEdwvohw=; b=F68XZfHXQdj6590f0qzZ50gn9Ue/mQMGUI3ntCGsTN4nyiO1Zpv0Uct12KtZ0EWgep jl+aIARmmMX4BJkt99SDG6xbNffsVukNqsEu0vTe0PfRXQvJoyZtaSeeiaEE2QRK3ohg qUALjJKldmVhvzUJyWNknXF+vgn2xRdUKZ+b2+wXxbwxbI3urMPyYWMff/uMwXx7yT7v tOH2ZbfD8ZSKgIbgI3yE0WSK7npuPHpV47pqONiLfWqOpVWcAPn4S7N5WN8D9O+7ef27 19HeqzMbn7DT0fznP4mx8EThgPwpaf50fnM9NqlhB9t3PoU6zAoYHiL3h64uRMeQ+99Z 2eJQ== X-Gm-Message-State: ALoCoQmfddVoCHgRydQhsNnnwuE6EVc4dld5EVUrY0Mn1Jn1JGz4dOFeIreZ0WcFJToGouJaZuLX MIME-Version: 1.0 X-Received: by 10.180.108.175 with SMTP id hl15mr60778762wib.1.1441839704853; Wed, 09 Sep 2015 16:01:44 -0700 (PDT) Received: by 10.28.27.67 with HTTP; Wed, 9 Sep 2015 16:01:44 -0700 (PDT) In-Reply-To: References: Date: Wed, 9 Sep 2015 16:01:44 -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 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. * 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" } } */