Re: [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix duplicate symbol

[Laszlo Ersek <lersek@redhat.com>]

On 05/26/17 07:29, Gao, Liming wrote:
> Mike:
> I think build performance is also a key point. I prefer to add this
> option in NOOPT target. After add this option, we can build edk2
> packages to detect those duplicated issues.
OK, how about this:

- MSFT toolchains: do the two step linking that Mike described, but only
  for NOOPT (assuming that is possible). The user would have to pay for
  the collision detection with CPU time, but not with an exorbitant size
  increase. I think that's a better trade-off than finishing quickly but
  potentially not fitting into the firmware image. (You generally pick
  NOOPT for debugging, so you certainly wish to *run* the image.) DEBUG
  and RELEASE targets wouldn't be changed.

- GCC toolchains: I think I'd like --whole-archive to become the default
  (regardless of build target), since there don't seem to be any
  relevant size costs to it.

Thanks,
Laszlo

> 
> Thanks
> Liming
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>> Kinney, Michael D
>> Sent: Friday, May 26, 2017 6:48 AM
>> To: Laszlo Ersek <lersek@redhat.com>; Ard Biesheuvel
>> <ard.biesheuvel@linaro.org>; Andrew Fish (afish@apple.com)
>> <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>
>> Cc: Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org; Fan, Jeff
>> <jeff.fan@intel.com>
>> Subject: Re: [edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix
>> duplicate symbol
>>
>> Laszlo,
>>
>> The other idea I have is for MSFT tool chains to do the DLINK step twice.  Once
>> with /WHOLEARCHIVE set to a .dll that is not used in later steps, but the doing
>> the DLINK action detects duplicate symbols.
>>
>> If the first DLINK step passes, then so a second DLINK step to a .dll without
>> /WHOLEARCHIVE set and use this .dll to produce the .efi file that goes into the
>> FW image.
>>
>> This 2 step link process would have the side effect of potentially increasing
>> build times, but could be done for specific tool chain families in build_rules.txt.
>>
>> The first DLINK step could also disable a lot of the optimizations that take
>> longer since the goal of this step is only to detect a duplicate symbol.
>>
>> Best regards,
>>
>> Mike
>>
>>> -----Original Message-----
>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>> Laszlo
>>> Ersek
>>> Sent: Thursday, May 25, 2017 1:11 PM
>>> To: Kinney, Michael D <michael.d.kinney@intel.com>; Ard Biesheuvel
>>> <ard.biesheuvel@linaro.org>; Andrew Fish (afish@apple.com)
>> <afish@apple.com>
>>> Cc: Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org; Fan, Jeff
>>> <jeff.fan@intel.com>
>>> Subject: Re: [edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib:
>> Fix
>>> duplicate symbol
>>>
>>> On 05/25/17 21:57, Kinney, Michael D wrote:
>>>> Laszlo,
>>>>
>>>> I have the same concern on final image sizes.  I have done some
>>>> evaluation:
>>>>
>>>> GCC5 OVMF X64 DEBUG without -whole-archive
>>>> ==========================================
>>>> FV Space Information
>>>> SECFV [19%Full] 212992 total, 42000 used, 170992 free
>>>> FVMAIN_COMPACT [33%Full] 3440640 total, 1162760 used, 2277880 free
>>>> DXEFV [38%Full] 10485760 total, 4024024 used, 6461736 free
>>>> PEIFV [19%Full] 917504 total, 180648 used, 736856 free
>>>> Total used = 5409432
>>>>
>>>> GCC5 OVMF X64 DEBUG with -whole-archive
>>>> =======================================
>>>> FV Space Information
>>>> SECFV [19%Full] 212992 total, 41936 used, 171056 free
>>>> FVMAIN_COMPACT [33%Full] 3440640 total, 1158304 used, 2282336 free
>>>> DXEFV [38%Full] 10485760 total, 4029656 used, 6456104 free
>>>> PEIFV [19%Full] 917504 total, 181352 used, 736152 free
>>>> Total used = 5411248
>>>>
>>>> Total used difference = 1816 bytes larger with -whole-archive
>>>>
>>>> I was also able to do a MSFT VS2015 build with /WHOLEARCHIVE set
>>>> and it also catches the same duplicate symbol error now.
>>>>
>>>> error C2220: warning treated as error - no 'executable' file generated
>>>> warning C4744: 'mMemoryDiscoveredNotifyList' has different type in
>>>
>> 'd:\work\github\tianocore\edk2\mdemodulepkg\core\dxeiplpeim\dxeload.c'
>> and
>>>
>> 'd:\work\github\tianocore\edk2\sourceleveldebugpkg\library\debugagent\s
>> ecpeidebug
>>> agent\secpeidebugagentlib.c': 'struct (12 bytes)' and 'array (12 bytes)'
>>>> DxeIpl.lib(DxeLoad.obj) : error LNK2005: _mMemoryDiscoveredNotifyList
>> already
>>> defined in SecPeiDebugAgentLib.lib(SecPeiDebugAgentLib.obj)
>>>>
>>>
>> d:\work\github\tianocore\edk2\Build\OvmfIa32\DEBUG_VS2015x86\IA32\M
>> deModulePkg\Co
>>> re\DxeIplPeim\DxeIpl\DEBUG\DxeIpl.dll : fatal error LNK1169: one or more
>> multiply
>>> defined symbols found
>>>>
>>>> VS2015 OVMF X64 DEBUG without /WHOLEARCHIVE
>>>> ===========================================
>>>> FV Space Information
>>>> SECFV [22%Full] 212992 total, 48560 used, 164432 free
>>>> FVMAIN_COMPACT [33%Full] 3440640 total, 1147464 used, 2293176 free
>>>> DXEFV [39%Full] 10485760 total, 4163888 used, 6321872 free
>>>> PEIFV [22%Full] 917504 total, 204840 used, 712664 free
>>>> Total used = 5564752
>>>>
>>>>
>>>> VS2015 OVMF X64 DEBUG with /WHOLEARCHIVE
>>>> ===========================================
>>>> FV Space Information
>>>> SECFV [23%Full] 212992 total, 50384 used, 162608 free
>>>> FVMAIN_COMPACT [33%Full] 3440640 total, 1147424 used, 2293216 free
>>>> DXEFV [42%Full] 10485760 total, 4422992 used, 6062768 free
>>>> PEIFV [27%Full] 917504 total, 255528 used, 661976 free
>>>> Total used = 5875338
>>>>
>>>> Total used difference = 310586 bytes larger with /WHOLEARCHIVE
>>>>
>>>> For tool chains that do have size impacts, one option is to
>>>> have a "test" build that enables the linker flags to detect
>>>> duplicate symbols.  For example the following could be added
>>>> to a DSC file.  May want to disable GenFds stage when doing
>>>> this type of build.
>>>>
>>>> [BuildOptions]
>>>> !ifdef $(DETECT_DUPLICATE_SYMBOLS)
>>>>   MSFT:*_VS2015_*_DLINK_FLAGS = /WHOLEARCHIVE
>>>> !endif
>>>
>>> Thank you (again) for the research! Looks like the gcc size impact is
>>> friendly enough to keep --whole-archive enabled at all times (possibly
>>> due to the --gc-sections flag mentioned by Ard, which we already have
>>> enabled).
>>>
>>> The VS2015 impact is really large however.
>>>
>>> I was hoping we could add these flags to
>>> "BaseTools/Conf/tools_def.template", regardless of platform DSC. (If the
>>> flag is non-default, and/or it's platform-dependent, then it will almost
>>> never be used, most likely.) But the VS2015 size increase really
>>> precludes /WHOLEARCHIVE (for the MSFT family) from
>> "tools_def.template".
>>>
>>> Would it be acceptable to add --whole-archive to "tools_def.template"
>>> only for GCC? After all, at the moment only XCODE*/XCLANG have "-
>> all_load".
>>>
>>> Thanks,
>>> Laszlo
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel