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? Best regards, Mike > -----Original Message----- > From: Kinney, Michael D > Sent: Wednesday, May 24, 2017 5:38 PM > To: Ard Biesheuvel <[email protected]>; Kinney, Michael D > <[email protected]> > Cc: Laszlo Ersek <[email protected]>; Wu, Hao A <[email protected]>; edk2- > [email protected]; [email protected]; Fan, Jeff <[email protected]> > Subject: RE: [edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix > duplicate > symbol > > Ard, > > I agree that it would be good practice for a library instance to > only have the public interfaces(functions/data) that are declared > in the library class .h file. > > I would be useful to have a report that showed the extra interfaces. > > One additional constraint I just found with the MSFT tool chains > involves the GLOBAL_REMOVE_IF_UNREFERENCED. This macro can not be > combined with static. So the recommendation in the thread to add > static to mMemoryDiscoveredNotifyList in SecPeiDebugAgentLib > breaks the build. > > d:\work\github\tianocore\edk2\SourceLevelDebugPkg\Library\DebugAgent\SecPeiDebugAgent\ > SecPeiDebugAgentLib.c(35): error C2496: > 'mDebugAgentMemoryDiscoveredNotifyList': > 'selectany' can only be applied to data items with external linkage > > The MSFT tool chains depend on the following #define in order for > The optimizer to remove global variables that are not used. This > is a valuable size optimization for libraries that may be sparsely > used by modules. > > #define GLOBAL_REMOVE_IF_UNREFERENCED __declspec(selectany) > > Mike > > > -----Original Message----- > > From: Ard Biesheuvel [mailto:[email protected]] > > Sent: Wednesday, May 24, 2017 2:44 PM > > To: Kinney, Michael D <[email protected]> > > Cc: Laszlo Ersek <[email protected]>; Wu, Hao A <[email protected]>; edk2- > > [email protected]; [email protected]; Fan, Jeff <[email protected]> > > Subject: Re: [edk2] [Patch] SourceLevelDebugPkg/SecPeiDebugAgentLib: Fix > > duplicate > > symbol > > > > On 24 May 2017 at 13:18, Kinney, Michael D <[email protected]> > > wrote: > > > Laszlo, > > > > > > I agree with the request to add 'static' to the variable declaration > > > in the SecPeiDebugAgentLib. The variable name change will be retained > > > because the same symbol name can still be confusing when debugging. > > > > > > The part that is more concerning is why this duplicate symbol reference > > > was not triggered by MSFT or GCC tool chain families, so I did some > > > more investigation from the MSFT side this morning. > > > > > > My first thought was that the optimizer in the compiler/linker was > > > optimizing away the duplicate symbol before the final link stage, > > > so I tried -b NOOPT builds. That also did not trigger a duplicate > > > symbol error and the MAP file contains only the single reference to > > > _mMemoryDiscoveredNotifyList from the DxeIpl. > > > > > > 0002:000001b8 _mMemoryDiscoveredNotifyList 000164f8 > > > DxeIpl:DxeLoad.obj > > > > > > I then evaluated what functions in the DebugAgentLib the DxeIpl > > > is using. It turns out that it is only using a single function > > > SaveAndSetDebugTimerInterrupt() that is implemented in > > > > > > SourceLevelDebugPkg\Library\DebugAgent\DebugAgentCommon\DebugAgent.c > > > > > > The mMemoryDiscoveredNotifyList duplicate symbol is implemented in > > > > > > > > > SourceLevelDebugPkg\Library\DebugAgent\SecPeiDebugAgent\SecPeiDebugAgentLib.c > > > > > > Even with a MSFT -b NOOPT build, when I look at the MAP file, the > > > linker only pulls in symbols from the obj in the DebugAgentLib that > > > contains the one SaveAndSetDebugTimerInterrupt() function. The symbols > > > from the other objs in the DebugAgentLib are not included in the final > > > DxeIpl PE/COFF image. > > > > > > 0001:000081b0 _InitializeDebugTimer 00008410 f > > SecPeiDebugAgentLib:DebugTimer.obj > > > 0001:00008330 _IsDebugTimerTimeout 00008590 f > > SecPeiDebugAgentLib:DebugTimer.obj > > > 0001:000083d0 _SaveAndSetDebugTimerInterrupt 00008630 f > > SecPeiDebugAgentLib:DebugTimer.obj > > > > > > So it appears that even with -b NOOPT builds, the linker is > > > filtering out unreferenced objs which explains why the duplicate > > > symbol error is not triggered. > > > > > > > It does not need to filter out what it never considered for inclusion > > in the first place. That is why it is called a library: only objects > > that satisfy some as yet unresolved symbol reference will get pulled > > in. > > > > > The XCODE5 tool chain appears to be evaluating symbols > > > across all objs in a lib and detects a duplicate symbol. > > > > > > The question is what is the required linker behavior for EDK II > > > builds? Require library filtering at obj level, or require no > > > duplicates across all objs in all libs? > > > > > > > Require no *externally visible* duplicates at all. That is really the > > only scaleable solution. Libraries declare their externally visible > > symbols in their .h file, and ideally, nothing else should be exposed. > > I know this is infeasible in the general case, due to the fact that > > libraries typically consist of several objects, but it should be the > > goal not to export anything if we can avoid it. Note that this applies > > to functions as well as variables _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
