On 4 December 2015 at 10:17, Gao, Liming <liming....@intel.com> wrote: > Good fix. How do you find this issue? >
I got build failures with RVCT when using the UEFI Secure Boot versions of VariableRuntimeDxe. Could you check how VS deals with this? I.e., if the two copies of mVirtualAddressChangeEvent are mapped onto the same memory location? > Reviewed-by: Liming Gao <liming....@intel.com> > Thanks > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > Sent: Thursday, December 03, 2015 8:54 PM > To: edk2-devel@lists.01.org; Justen, Jordan L; Gao, Liming; Zhu, Yonghong > Cc: Ard Biesheuvel > Subject: [PATCH] BaseTools GCC: avoid the use of COMMON symbols > > The default behavior of the GCC compiler is to emit uninitialized globals > into a COMMON section, where duplicate definitions are merged. > This may result in unexpected behavior, since global variables appearing by > the same name in different C files typically do not refer to the same > conceptual data item. > > For instance, the definitions of EFI_EVENT mVirtualAddressChangeEvent in that > appear in the following files: > > CryptoPkg/Library/BaseCryptLib/SysCall/RuntimeMemAllocation.c > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c > > will be folded into a single instance of the variable when the latter module > includes the former library, which can lead to unexpected results. > > So prevent GCC from emitting variables into the COMMON section, by passing > -fno-common to the compiler, and discarding the COMMON section in the GNU ld > linker script. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org> > --- > > NOTE: this patch will result in build failures until we fix the code to make > at least some instances of mVirtualAddressChangeEvent STATIC > > BaseTools/Conf/tools_def.template | 2 +- > BaseTools/Scripts/GccBase.lds | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/BaseTools/Conf/tools_def.template > b/BaseTools/Conf/tools_def.template > index 564d1336650c..2216e747c02d 100644 > --- a/BaseTools/Conf/tools_def.template > +++ b/BaseTools/Conf/tools_def.template > @@ -4318,7 +4318,7 @@ NOOPT_DDK3790xASL_IPF_DLINK_FLAGS = /NOLOGO > /NODEFAULTLIB /LTCG /DLL /OPT:REF > DEBUG_*_*_OBJCOPY_ADDDEBUGFLAG = > --add-gnu-debuglink=$(DEBUG_DIR)/$(MODULE_NAME).debug > RELEASE_*_*_OBJCOPY_ADDDEBUGFLAG = > > -DEFINE GCC_ALL_CC_FLAGS = -g -Os -fshort-wchar > -fno-strict-aliasing -Wall -Werror -Wno-array-bounds -c -include AutoGen.h > +DEFINE GCC_ALL_CC_FLAGS = -g -Os -fshort-wchar > -fno-strict-aliasing -Wall -Werror -Wno-array-bounds -c -include AutoGen.h > -fno-common > DEFINE GCC_IA32_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -m32 > -malign-double -freorder-blocks -freorder-blocks-and-partition -O2 > -mno-stack-arg-probe > DEFINE GCC_X64_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -mno-red-zone > -Wno-address -mno-stack-arg-probe > DEFINE GCC_IPF_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) > -minline-int-divide-min-latency > diff --git a/BaseTools/Scripts/GccBase.lds b/BaseTools/Scripts/GccBase.lds > index 4ee6d998532c..32310bc75dcc 100644 > --- a/BaseTools/Scripts/GccBase.lds > +++ b/BaseTools/Scripts/GccBase.lds > @@ -46,7 +46,7 @@ SECTIONS { > */ > .data ALIGN(ALIGNOF(.text)) : ALIGN(CONSTANT(COMMONPAGESIZE)) { > *(.data .data.* .gnu.linkonce.d.*) > - *(.bss .bss.* *COM*) > + *(.bss .bss.*) > } > > .eh_frame ALIGN(CONSTANT(COMMONPAGESIZE)) : { @@ -66,5 +66,6 @@ SECTIONS { > *(.dynamic) > *(.hash) > *(.comment) > + *(COMMON) > } > } > -- > 1.9.1 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
