From patchwork Wed Sep 2 23:32:21 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sriraman Tallam X-Patchwork-Id: 513693 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 A92DD14029C for ; Thu, 3 Sep 2015 09:32:36 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=PHSw/zbD; 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=V8rq6w37KFAB8E56DU pt/L5Q1h0tJRfYQX0DtGXDJQnUSqyD6xarsuVXDDVkgQSD+86FjCGm8Lj59/679G 4mya7x3aRk9ufim76wj0omNKFmmUvPIyf67+8O2FBMfP1RYdoXRBVV9QeYQir206 Kn9Vy6gIuJPbnIajG1ho+RYI8= 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=woOrjycZyuQGnd3VmpICwvSk cYI=; b=PHSw/zbDQn1rMyGRPMEaDYBeveXzwZKTgceBOkgM4QXLRnnVtg231EyV K8NEXQjhAMaecMPGMuS8EV+ZNDzfTZVGr4ipN5s+Lt5lt3mK3jZ2JVXOtvewCZxL O5QqvcTcY6a2K32ebpjT876td3Pd9Gkf3dgRoezEXIjTfGc1tps= Received: (qmail 36225 invoked by alias); 2 Sep 2015 23:32:28 -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 36197 invoked by uid 89); 2 Sep 2015 23:32:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.2 required=5.0 tests=AWL, BAYES_50, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_LOW, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mail-wi0-f169.google.com Received: from mail-wi0-f169.google.com (HELO mail-wi0-f169.google.com) (209.85.212.169) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 02 Sep 2015 23:32:25 +0000 Received: by wicfx3 with SMTP id fx3so34341717wic.0 for ; Wed, 02 Sep 2015 16:32:22 -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=vyajn9aJkylUs+ZeXICcdRWjaWrPWuz8c2msU6mhc/4=; b=A7R9bxWWvRcr/LTRumuXtU7ACoTOQOF877TKpbg9gSTgZF4TdulQxph9ihhKEOt/uI hU6B14lYHdwR69n6a2b5G0p7tcnEBiLlrYOlf/++2jWHB9TuR/PmQSrpx3EPHOZLJU+A GqE1ei/l7l8rmRnJtlqX0yxx1+Q15y2XsyYqSj/Nxieorg/z4JoNa2w1aZ3P0V70RVSo ZIU1zuVYRr7gSQEyxXkex4TfqSS5ju2EuKTAUP+DycDI8S/kerJ7Bghw85SjYE79xv4l wAOPsNBTHLiD5G1kVKF9Il97D+9uZPHATWvbJ13Q5E0HlxvwEl7SU2ggBl7Xmp1/n7UU b9uA== X-Gm-Message-State: ALoCoQnX7oKq/zgxd6ZNjnb33MMFCinBGIaNVzFXZltqxg4mWcGIL3F8tYmbwp1ioht4UTMiFqTJ MIME-Version: 1.0 X-Received: by 10.194.71.134 with SMTP id v6mr38834193wju.134.1441236742242; Wed, 02 Sep 2015 16:32:22 -0700 (PDT) Received: by 10.28.27.67 with HTTP; Wed, 2 Sep 2015 16:32:21 -0700 (PDT) In-Reply-To: References: Date: Wed, 2 Sep 2015 16:32:21 -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 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. * 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" } } */