From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-in4.apple.com (mail-out4.apple.com [17.151.62.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 3A7E521A02915 for ; Thu, 25 May 2017 14:25:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; d=apple.com; s=mailout2048s; c=relaxed/simple; q=dns/txt; i=@apple.com; t=1495747535; h=From:Sender:Reply-To:Subject:Date:Message-id:To:Cc:MIME-version:Content-type: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-reply-to:References:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=aH6wA48tMB6aU00kSHbKWFmMwP6LxR6PGO2Ou6Ir1wk=; b=IDxcNht7iTOyRkZ4/OU+0RoKsFcyNw8tHu2knQyrxXtvV4OPxme6y1m0CUmUiEP7 FI9Bexgh/cysI6Y+wrSjzDed8J1M8ZfzRWfpqbl75LGRh1n0P8AeXaPZn2TKwTJB GkXWD9aaRgEbyuY9l1yOsqdP0RQoL7TcLhScJmQDRI1Pftcvu/ZlP1jChYMnUIHr C580LCLkgjhDE9Khxt3zUwjHrLRAvfyhNlLjO/NhFeoWpqhpYLCjsdn6aNKFEETl c7DH7I5AAzSi99x07yX1jBc5kirIbHq140j1weNiLZtWo99C+NTEPlo1i+l1bEfI d0kKNxIQp7L374eBxOmEBA==; Received: from relay2.apple.com (relay2.apple.com [17.128.113.67]) (using TLS with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mail-in4.apple.com (Apple Secure Mail Relay) with SMTP id FA.EC.01052.FCB47295; Thu, 25 May 2017 14:25:35 -0700 (PDT) X-AuditID: 11973e12-7285e9a00000041c-6f-59274bcf221d Received: from nwk-mmpp-sz12.apple.com (nwk-mmpp-sz12.apple.com [17.128.115.204]) by relay2.apple.com (Apple SCV relay) with SMTP id 8D.F6.07829.FCB47295; Thu, 25 May 2017 14:25:35 -0700 (PDT) MIME-version: 1.0 Received: from [17.153.17.124] by nwk-mmpp-sz12.apple.com (Oracle Communications Messaging Server 8.0.1.2.20170210 64bit (built Feb 10 2017)) with ESMTPSA id <0OQJ001RW26MG450@nwk-mmpp-sz12.apple.com>; Thu, 25 May 2017 14:25:35 -0700 (PDT) Sender: afish@apple.com From: Andrew Fish Message-id: <340C28EC-60CF-4390-A8AD-25F9DA22538F@apple.com> Date: Thu, 25 May 2017 14:25:33 -0700 In-reply-to: Cc: Ard Biesheuvel , "Wu, Hao A" , "edk2-devel@lists.01.org" , Felix Poludov , Mike Kinney , "Fan, Jeff" To: Laszlo Ersek References: <1495581673-10788-1-git-send-email-michael.d.kinney@intel.com> <542CF652F8836A4AB8DBFAAD40ED192A4C5E94B8@shsmsx102.ccr.corp.intel.com> <56801ADE-446F-43C2-9C99-5500E8EE5881@apple.com> X-Mailer: Apple Mail (2.3273) X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrLLMWRmVeSWpSXmKPExsUi2FDorHveWz3SYN5+Q4v/H3YzWuw5dJTZ Yt2Xc6wWV2/9YrI4uX4Jo8WyYztYLDo6/jE5sHtMv/KL3WPxnpdMHneu7WHz6J79j8Xj/b6r bAGsUVw2Kak5mWWpRfp2CVwZWxZuYStYeYuxYt+qUywNjNv3MnYxcnJICJhIfNh7kb2LkYtD SGA1k8Si/U2sMInO5oeMEIlDjBKn5lwHS/AKCEr8mHyPBcRmFgiTeL7hHDuILSTwlVFi220F EFtYQFzi3ZlNzCA2m4CyxIr5H9ghem0k/hxawwhREy1x9dorMJtFQFXi0LuFQDUcHJwCdhK7 78WC7GUW+MkosejTKbBdIgIqErMnPGCC2HWAWWLypmCIQ2Ulbs2+xAzSICHQzC6xZO5f1gmM QrOQ3DoLya0QtpbE90etQHEOIFte4uB5WYiwpsSze5+gSrQlnry7wLqAkW0Vo1BuYmaObmae iV5iQUFOql5yfu4mRlB8TbcT2sF4apXVIUYBDkYlHt4N99QihVgTy4orcw8xSnOwKInzVsWr RAoJpCeWpGanphakFsUXleakFh9iZOLglGpgZLleKvRD+cgtsZaeSwYnLIxSJCIV489VRRxk 8bu9MrOaoy/v5hz/WxuSa36zVX7JOWJ5cRvjXmHx+BPHFqb67wpYuyJ58sHlXWKiaRvkTh+V EHJ5IhUX2querPHm+u0tvQlnwrJCTjPWnMve+2KpmOvmaVPZF0cVqr/KcFrQ257BMNO0V2eO EktxRqKhFnNRcSIAu+UpY5ACAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrOIsWRmVeSWpSXmKPExsUi2FB8Rve8t3qkQeNKRYv/H3YzWuw5dJTZ Yt2Xc6wWV2/9YrI4uX4Jo8WyYztYLDo6/jE5sHtMv/KL3WPxnpdMHneu7WHz6J79j8Xj/b6r bAGsUVw2Kak5mWWpRfp2CVwZWxZuYStYeYuxYt+qUywNjNv3MnYxcnJICJhIdDY/BLK5OIQE DjFKnJpznRUkwSsgKPFj8j0WEJtZIEzi+YZz7CC2kMBXRolttxVAbGEBcYl3ZzYxg9hsAsoS K+Z/YIfotZH4c2gNI0RNtMTVa6/AbBYBVYlD7xYC1XBwcArYSey+Fwuyl1ngJ6PEok+nwHaJ CKhIzJ7wgAli1wFmicmbgiEOlZW4NfsS8wRG/llIzpuF5DwIW0vi+6NWoDgHkC0vcfC8LERY U+LZvU9QJdoST95dYF3AyLaKUaAoNSex0kgvsaAgJ1UvOT93EyM4HgqddzAeW2Z1iFGAg1GJ h3fDPbVIIdbEsuLKXGAYcTArifA+sFCPFOJNSaysSi3Kjy8qzUktPsQ4kRHoyYnMUqLJ+cBo zSuJNzQxMTAxNjYzNjY3MaelsJI4b1aCSqSQQHpiSWp2ampBahHMUUwcnFINjHGC6ueCJec+ X1jDmm2jEXHXIDZhQo+ruaSwoh9b9pHDbsJ7OC5uOdl85uPG65XFn1d8f/Pa6sv/3U+6vV5F PzE4LsP6J1Xj9YWHXFmxLp9/a0Rtkz/yYnv5XVfZeJWZ098qXahSri9Ur58ze8IXY3mp+Rt6 8i67XKhuU/SdoHzxpRDr/Jdb3JVYijMSDbWYi4oTAWusMc76AgAA X-Content-Filtered-By: Mailman/MimeDel 2.1.22 Subject: Re: [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate symbol X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 May 2017 21:25:37 -0000 Content-Type: text/plain; CHARSET=US-ASCII Content-Transfer-Encoding: 7BIT > On May 25, 2017, at 2:02 PM, Laszlo Ersek wrote: > > On 05/25/17 22:37, Andrew Fish wrote: >> >>> On May 25, 2017, at 1:28 PM, Laszlo Ersek wrote: >>> >>> On 05/25/17 22:11, Ard Biesheuvel wrote: >>>> On 25 May 2017 at 13:06, Kinney, Michael D wrote: >>>>> Laszlo and Andrew, >>>>> >>>>> With the information that has been collected on this thread, I >>>>> still think this patch in its original form is a good change >>>>> to resolve the this one specific duplicate symbol issue for all >>>>> tool chains. 'static' can not be mixed with >>>>> GLOBAL_REMOVE_IF_UNREFERENCED for MSFT tool chains, so renaming >>>>> the global variable is the easiest way to remove the duplicate. >>>>> >>>> >>>> GLOBAL_REMOVE_IF_UNREFERENCED itself is problematic imo. I think it >>>> was Felix who reported on this recently? >>>> >>>> STATIC is really the only sensible way to deal with this for symbols >>>> that are only referenced by a single compilation unit. >>>> >>>>> I will continue to work on ways to detect duplicate symbols for >>>>> all tool chains and will enter a Bugzilla issue to for that >>>>> feature. >>>>> >>>>> In addition, the idea of detecting if a library is exporting more >>>>> than the library class defines is another good feature to consider >>>>> and I will enter a Bugzilla issue for that one as well. >>>>> >>>>> If we can find ways to both restrict the symbols exported by a >>>>> library and strip all symbols that are unused, then we can have >>>>> additional Bugzilla issues to perform that clean up on each >>>>> library instance that is exporting more than the library class. >>>>> >>>> >>>> A static library is nothing more than an archive containing a >>>> collection of object files. Sadly, that implies that we cannot >>>> distinguish between symbols that may only be referenced by other >>>> objects in the same static library and symbols that are exported to >>>> the library client. >>> >>> Do we know for a fact that, with /OPT:REF, VS does not strip unused >>> *static* variables and functions? >>> >>> I mean, is it certain that *replacing* GLOBAL_REMOVE_IF_UNREFERENCED >>> with STATIC in this case would lead to a size increase? >>> >>> If that's the case, then I'm fine if we go ahead with this patch, I'd >>> just like to request that Mike please file some of those BZs, and please >>> reference them from the commit message (as the longer term solution), >>> before committing the patch. >>> >> >> Clang will warn if you have unused static variables when warnings are cranked up. >> >> ~/work/Compiler>cat static.c >> static unsigned char gTest[] = { 42 }; >> >> static int test () >> { >> return 1; >> } >> >> int main () >> { >> return 0; >> } >> ~/work/Compiler>clang -Os static.c -Wall >> static.c:1:22: warning: unused variable 'gTest' [-Wunused-variable] >> static unsigned char gTest[] = { 42 }; >> ^ >> static.c:3:12: warning: unused function 'test' [-Wunused-function] >> static int test () >> ^ >> 2 warnings generated. > > Sorry, my question was imprecise. > > Assume there is a public library function ("external linkage") that > calls a static function in the same library instance and uses a static > variable in the same library instance. Then this library instance is > linked into a driver, but the driver never actually calls the extern > function -- so the static variable and the static function too become > useless. > > In this case, will /OPT:REF remove the static variable and the static > function too? > > It seems counter-intuitive to me that an internal-only function or an > internal-only variable has to be declared extern (via > GLOBAL_REMOVE_IF_UNREFERENCED) just so it can be eliminated at link > time, if it is never referenced (transitively). > Laszlo, I agree. The LLVM LTO does not have an issue "doing the right thing". Seems like static is also more of a compile time concept vs a link time (global optimization) kind of thing? Given on VC++ GLOBAL_REMOVE_IF_UNREFERENCED maps to __declspec( selectany ) I would guess maybe it has more to due with supporting old non standard header files that can't change without breaking compatibility. MSDN on __declspec( selectany ) : A global data item can normally be initialized only once in an EXE or DLL project. selectany can be used in initializing global data defined by headers, when the same header appears in more than one source file. selectany is available in both the C and C++ compilers. Thanks, Andrew Fish > Thanks > Laszlo > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel