From: Laszlo Ersek <lersek@redhat.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
"Kinney, Michael D" <michael.d.kinney@intel.com>,
Felix Poludov <Felixp@ami.com>
Cc: "Fan, Jeff" <jeff.fan@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
"Wu, Hao A" <hao.a.wu@intel.com>, Andrew Fish <afish@apple.com>
Subject: Re: [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate symbol
Date: Thu, 25 May 2017 22:28:13 +0200 [thread overview]
Message-ID: <e55180fe-4baf-e84f-3acb-0b075d20c5f4@redhat.com> (raw)
In-Reply-To: <CAKv+Gu8S=a5RgNWz3SrhP208vrX-0x3gw7cp6Tc=Ogf_V+AO8A@mail.gmail.com>
On 05/25/17 22:11, Ard Biesheuvel wrote:
> On 25 May 2017 at 13:06, Kinney, Michael D <michael.d.kinney@intel.com> 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.
Thanks
Laszlo
next prev parent reply other threads:[~2017-05-25 20:28 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-23 23:21 [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate symbol Michael Kinney
2017-05-23 23:25 ` Andrew Fish
2017-05-24 0:27 ` Kinney, Michael D
2017-05-24 8:48 ` Laszlo Ersek
2017-05-24 11:52 ` Ard Biesheuvel
2017-05-24 20:18 ` Kinney, Michael D
2017-05-24 21:44 ` Ard Biesheuvel
2017-05-25 0:38 ` Kinney, Michael D
2017-05-25 1:47 ` Kinney, Michael D
2017-05-25 16:08 ` Laszlo Ersek
2017-05-25 16:14 ` Andrew Fish
2017-05-25 17:38 ` Kinney, Michael D
2017-05-25 18:06 ` Laszlo Ersek
2017-05-25 19:55 ` Ard Biesheuvel
2017-05-25 20:01 ` Laszlo Ersek
2017-05-25 19:57 ` Kinney, Michael D
2017-05-25 20:10 ` Laszlo Ersek
2017-05-25 22:47 ` Kinney, Michael D
2017-05-26 5:29 ` Gao, Liming
2017-05-26 9:05 ` Laszlo Ersek
2017-05-25 16:01 ` Laszlo Ersek
2017-05-24 2:47 ` Fan, Jeff
2017-05-25 20:06 ` Kinney, Michael D
2017-05-25 20:11 ` Ard Biesheuvel
2017-05-25 20:28 ` Laszlo Ersek [this message]
2017-05-25 20:37 ` Andrew Fish
2017-05-25 21:02 ` Laszlo Ersek
2017-05-25 21:25 ` Andrew Fish
2017-05-25 22:42 ` Kinney, Michael D
2017-05-26 5:21 ` Gao, Liming
2017-05-26 6:20 ` Kinney, Michael D
2017-05-26 8:41 ` Gao, Liming
2017-05-26 22:11 ` Felix Poludov
2017-05-26 23:06 ` Kinney, Michael D
2017-05-27 12:27 ` Ard Biesheuvel
2017-05-29 10:21 ` Laszlo Ersek
-- strict thread matches above, loose matches on Subject: below --
2017-12-07 7:48 Liming Gao
2017-12-07 7:55 ` Ni, Ruiyu
2017-12-07 7:55 ` Ni, Ruiyu
2017-12-07 8:24 ` Wu, Hao A
2017-12-07 8:46 ` Ard Biesheuvel
2017-12-07 11:18 ` Laszlo Ersek
2017-12-07 11:41 ` Ard Biesheuvel
2017-12-07 14:19 ` Gao, Liming
2017-12-07 14:21 ` Ard Biesheuvel
2017-12-07 16:52 ` Kinney, Michael D
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e55180fe-4baf-e84f-3acb-0b075d20c5f4@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox