From patchwork Tue Jun 16 23:22:05 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sriraman Tallam X-Patchwork-Id: 485225 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 608651401F6 for ; Wed, 17 Jun 2015 09:22:18 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=VCqGREyQ; 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=Gp+rmgTZ9d+9Gl8104 wrVzKlH37IbA0h1zMYlRZdtzz5DmbY1hWmVYq0Tqj7mcqH2awbHzymwoIqYpInia TZSZlFpZt4QdiETali+TRh+8N7fHX7A5azzz9VigwJCdrD8zuATp0VPcQRtWJHdX dCLBfAxSYSyJpDh05Yj2NuIQU= 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=sLr3HHDJefO7IkIumSu41jCh /JA=; b=VCqGREyQxKRRaKP0e7wEjhLLqef9TFWm5GXplkDuTrnBXOqxPTtNJUwh C8+TDBtWL/g0VqkHBEzTbrvb3D2H4FXR18xvMAH58zCSBDjjLGdFhvLSV74WrybF Qnjp1aYZc+0tw6N0W1e6i1BRY06DplW8jEBRoVZG5qw39vWvjyE= Received: (qmail 24820 invoked by alias); 16 Jun 2015 23:22:11 -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 24792 invoked by uid 89); 16 Jun 2015 23:22:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.7 required=5.0 tests=AWL, BAYES_05, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS autolearn=unavailable version=3.3.2 X-HELO: mail-yk0-f172.google.com Received: from mail-yk0-f172.google.com (HELO mail-yk0-f172.google.com) (209.85.160.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 16 Jun 2015 23:22:08 +0000 Received: by ykfl8 with SMTP id l8so26588632ykf.1 for ; Tue, 16 Jun 2015 16:22:06 -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=TVzC2WoAE79nahICj7yIS3j9cWWIz/UWdnNgTg432tI=; b=WZSHWP4F3LIDAebq2kSdQhydwqGbFJ2/wd+x2+PrPtVUDQLzpP1jeZvRSd5qNH6W8W fOVdr5UZ/O5BZVruEhjhIjYLMOxGmU/C33/Hd0t4jyZPdlI8Ppjx0KzifLebt8BUrXDZ Wjd9nZtiRjs/8ad7MVRSq/pKRkRNeiqOmDGt2MluKHRvKUgdsaYXv3AuOduzEEEancrn sx8Ew+8PAk48m0M4rpXQGQnzzNV2DQe5s1NIfjrtu5xMr/YrK94YPqLel5YxCYQKgB0y mp9c0zU0UKUffAKShdZACSmp1EGdN4ai0+GU+ESCTq/hg9DAeOq5RWHRjOtNO76sbbxG LqPA== X-Gm-Message-State: ALoCoQmY98TU0HF8N1COnyLccm44OggLWHPbwrV//QutmFYSAGMzKpDfmM6sApx5KDhLGNIN5IPu MIME-Version: 1.0 X-Received: by 10.52.138.11 with SMTP id qm11mr2372386vdb.40.1434496925973; Tue, 16 Jun 2015 16:22:05 -0700 (PDT) Received: by 10.52.135.162 with HTTP; Tue, 16 Jun 2015 16:22:05 -0700 (PDT) In-Reply-To: References: Date: Tue, 16 Jun 2015 16:22:05 -0700 Message-ID: Subject: Re: [RFC] COMDAT Safe Module Level Multi versioning From: Sriraman Tallam To: Xinliang David Li Cc: Richard Biener , "H.J. Lu" , GCC Patches , binutils , Cary Coutant , Yury Gribov X-IsSubscribed: yes On Tue, May 19, 2015 at 9:11 AM, Xinliang David Li wrote: >> >> Hm. But which options are unsafe? Also wouldn't it be better to simply >> _not_ have unsafe options produce comdats but always make local clones >> for them (thus emit the comdat with "unsafe" flags dropped)? > > Always localize comdat functions may lead to text size increase. It > does not work if the comdat function is a virtual function for > instance. Based on Richard's suggestion, I have a patch to localize comdat functions which seems like a very effective solution to this problem. The text size increase is limited to the extra comdat copies generated for the specialized modules (modules with unsafe options) which is usually only a few. Since -fweak does something similar for functions, I have called the new option -fweak-comdat-functions. This does not apply to virtual comdat functions as their addresses can always be leaked via the vtable. Using this flag with virtual functions generates a warning. To summarize, this is the intended usage of this option. Modules which use unsafe code options, like -m for multiversioning, to generate code that is meant to run only on a subset of CPUs can generate comdats with specialized instructions which when picked by the linker can get run unconditionally causing SIGILL on unsupported platforms. This flag hides these comdats to be local to these modules and not make them available publicly, with the caveat that it does not apply to virtual comdats. Could you please review? * c-family/c.opt (fweak-comdat-functions): New option. * cp/decl2.c (comdat_linkage): Implement new option. Warn when virtual comdat functions are seen. * doc/invoke.texi: Document new option. * testsuite/g++.dg/no-weak-comdat-functions-1.C: New test. Thanks Sri > > David > > >> >> Richard. >> >>> >>> Thanks >>> Sri * c-family/c.opt (fweak-comdat-functions): New option. * cp/decl2.c (comdat_linkage): Implement new option. Warn when virtual comdat functions are seen. * doc/invoke.texi: Document new option. * testsuite/g++.dg/no-weak-comdat-functions-1.C: New test. Index: c-family/c.opt =================================================================== --- c-family/c.opt (revision 224486) +++ c-family/c.opt (working copy) @@ -1236,6 +1236,14 @@ fweak C++ ObjC++ Var(flag_weak) Init(1) Emit common-like symbols as weak symbols +fweak-comdat-functions +C++ Var(flag_weak_comdat_functions) Init(1) +Specific to comdat functions(-fno-weak-comdat-functions : Localize Comdat Functions). +With -fno-weak-comdat-functions, virtual comdat functions are still linked as +weak functions. With -fno-weak-comdat-functions, the address of the comdat +functions that are localized will be unique and this can cause unintended +behavior when addresses of comdat functions are used. + 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 224486) +++ cp/decl2.c (working copy) @@ -1702,8 +1702,19 @@ 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_weak_comdat_functions + || TREE_CODE (decl) != FUNCTION_DECL + || DECL_VIRTUAL_P (decl))) + { + make_decl_one_only (decl, cxx_comdat_group (decl)); + if (TREE_CODE (decl) == FUNCTION_DECL + && DECL_VIRTUAL_P (decl) + && !flag_weak_comdat_functions) + warning_at (DECL_SOURCE_LOCATION (decl), 0, + "fno-weak-comdat-functions: Comdat linkage of virtual " + "function %q#D preserved."); + } 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 224486) +++ 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-weak-comdat-functions -nostdinc++ @gol -fvisibility-inlines-hidden @gol -fvtable-verify=@var{std|preinit|none} @gol -fvtv-counts -fvtv-debug @gol @@ -2445,6 +2445,16 @@ 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-weak-comdat-functions +@opindex fno-weak-comdat-functions +Do not use weak symbol support for comdat non-virtual functions, even if it +is provided by the linker. By default, G++ uses weak symbols if they are +available. 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 comdat functions are used. + @item -nostdinc++ @opindex nostdinc++ Do not search for header files in the standard directories specific to Index: testsuite/g++.dg/no-weak-comdat-functions-1.C =================================================================== --- testsuite/g++.dg/no-weak-comdat-functions-1.C (revision 0) +++ testsuite/g++.dg/no-weak-comdat-functions-1.C (working copy) @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-fno-weak-comdat-functions -fkeep-inline-functions -ffunction-sections" } */ + +inline int foo () { + return 0; +} + +/* { dg-final { scan-assembler "_Z3foov" } } */ +/* { dg-final { scan-assembler-not "_Z3foov.*comdat" } } */