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

["Kinney, Michael D" <michael.d.kinney@intel.com>]

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\secpeidebugagent\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\MdeModulePkg\Core\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

Best regards,

Mike

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, May 25, 2017 11:07 AM
> 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 19:38, Kinney, Michael D wrote:
> > Laszlo,
> >
> > I think the equivalent flag for GCC builds is --whole-archive.
> >
> > I tried adding that flag to DLINK_FLAGS in GCC5, and I get the
> > following error building OVMF from edk2/master with
> > -D SOURCE_DEBUG_ENABLE set.
> >
> > DxeLoad.obj (symbol from plugin): In function `InstallIplPermanentMemoryPpis':
> > (.text+0x0): multiple definition of `mMemoryDiscoveredNotifyList'
> > SecPeiDebugAgentLib.obj (symbol from plugin):(.text+0x0): first defined here
> > collect2: error: ld returned 1 exit status
> 
> Great find! That's the error message we should get.
> 
> Unfortunately, after reading the "ld" manual on "--whole-archive", it
> seems that the complete object files will actually be copied into the
> resultant binary, even if several of their symbols will remain unused. I
> think that's quite sub-optimal. (I haven't verified this though.) What
> we'd like to get is (a) the full verification at link time, and (b)
> inclusion of *only* those symbols that are actually necessary.
> 
> In your testing, when you build OVMF with and without "--whole-archive",
> do you see a difference in, say, the DXEFV footprint, when the build
> completes?
> 
> (If so, then I wonder if we should add "--whole-archive" only to the
> NOOPT build... Not sure.)
> 
> > Visual Studio 2015 Update 2 has also added a new linker flag called
> > /WHOLEARCHIVE.  I am working on evaluating that flag to see if it
> > catches the same issue.
> 
> Thanks!
> Laszlo
> 
> >> -----Original Message-----
> >> From: Laszlo Ersek [mailto:lersek@redhat.com]
> >> Sent: Thursday, May 25, 2017 9:09 AM
> >> 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 03:47, Kinney, Michael D wrote:
> >>> Andrew,
> >>>
> >>> I think I have found an alternate fix for this XCODE5 specific
> >>> build failure.  Since there appears to be a difference in the
> >>> linker behavior between MSFT/GCC/XCODE tool chains, I reviewed
> >>> the 'ld' command line options used in XCODE5 tool chain in
> >>> tools_def.txt.
> >>>
> >>> There is a flag set call '-all_load'.  The description of this
> >>> flag is 'Loads all members of static archive libraries.'.
> >>>
> >>> I tried removing this flag from the XCODE5 specific SLINK_FLAGS
> >>> and DLINK_FLAGS statements in tools_def.txt, and the duplicate
> >>> symbol build failure is no longer present.  I am able to build
> >>> and boot OVMF with XCODE5 with -D SOURCE_DEBUG_ENABLE flag set.
> >>>
> >>> This seems to make XCODE5 linker behavior match the MSFT and GCC
> >>> linker behavior.
> >>>
> >>> Do you know why '-all_load' is used in XCODE5 and what impacts
> >>> there may be from removing it?
> >>
> >> Please don't remove -all_load from there; instead we should figure out
> >> if the same can be brought to MSFT and GCC.
> >>
> >> The error message that XCODE5 emitted caught a real bug (undefined
> >> behavior according to ISO C, see my previous email), and so we should
> >> keep that detection enabled (we should even extend it to other
> >> toolchains, if that's possible).
> >>
> >> As for docs, I found this:
> >>
> >> http://www.manpages.info/macosx/ld.1.html
> >>
> >>> -all_load
> >>>     Loads all members of static archive libraries. This option does
> >>>     not apply to dynamic shared libraries.
> >>
> >>
> >> Thanks
> >> Laszlo