summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorArd Biesheuvel <ard.biesheuvel@linaro.org>2015-12-18 06:51:36 +0000
committervanjeff <vanjeff@Edk2>2015-12-18 06:51:36 +0000
commit033469d9a9ed257afe3073e9d97ef4a85cbcf366 (patch)
treea5ca76c9f312638da61fcac66f33eab1d9054e1f
parentf8ae6c9e4c81bcc2b77e11a79b6bfd45390e0c9f (diff)
downloadedk2-platforms-033469d9a9ed257afe3073e9d97ef4a85cbcf366.tar.xz
BaseTools GCC: avoid the use of COMMON symbols
The default behavior of the GCC compiler is to emit uninitialized globals with external linkage into a COMMON section, where duplicate definitions are merged. This may result in unexpected behavior, since global variables defined under the same name in different C files may not refer to the same logical data item. For instance, the definitions of EFI_EVENT mVirtualAddressChangeEvent that [used to] 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. Even if some may argue that there are legal uses for COMMON allocation, the high modularity of EDK2 combined with the low level of awareness of the intracicies surrounding common allocation and the generally poor EDK2 developer discipline regarding the use of the STATIC keyword* make a strong case for disabling it by default, and re-enabling it explicitly for packages that depend on it. So prevent GCC from emitting variables into the COMMON section, by passing -fno-common to the compiler, and discarding the section in the GNU ld linker script. * Any function or variable that is only referenced from the translation unit that defines it could be made STATIC. This does not only prevent issues like the above, it also allows the compiler to generate better code, e.g., drop out of line function definitions after inlining all invocations or perform constant propagation on variables. (Sync patch r19164 from main trunk.) Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Liming Gao <liming.gao@intel.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Tested-by: Laszlo Ersek <lersek@redhat.com> git-svn-id: https://svn.code.sf.net/p/edk2/code/branches/UDK2015@19381 6f19259b-4bc3-4df7-8a09-765794883524
-rw-r--r--BaseTools/Conf/tools_def.template4
-rw-r--r--BaseTools/Scripts/GccBase.lds3
2 files changed, 4 insertions, 3 deletions
diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
index d6b0af43d7..1a44fbd1d3 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
@@ -4349,7 +4349,7 @@ DEFINE GCC_IPF_RC_FLAGS = -I binary -O elf64-ia64-little -B ia64
DEFINE GCC_ARM_RC_FLAGS = -I binary -O elf32-littlearm -B arm --rename-section .data=.hii
DEFINE GCC_AARCH64_RC_FLAGS = -I binary -O elf64-littleaarch64 -B aarch64 --rename-section .data=.hii
-DEFINE GCC44_ALL_CC_FLAGS = -g -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-array-bounds -ffunction-sections -fdata-sections -c -include AutoGen.h -DSTRING_ARRAY_NAME=$(BASE_NAME)Strings
+DEFINE GCC44_ALL_CC_FLAGS = -g -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-array-bounds -ffunction-sections -fdata-sections -c -include AutoGen.h -fno-common -DSTRING_ARRAY_NAME=$(BASE_NAME)Strings
DEFINE GCC44_IA32_CC_FLAGS = DEF(GCC44_ALL_CC_FLAGS) -m32 -malign-double -fno-stack-protector -D EFI32 -fno-asynchronous-unwind-tables
DEFINE GCC44_X64_CC_FLAGS = DEF(GCC44_ALL_CC_FLAGS) -m64 -fno-stack-protector "-DEFIAPI=__attribute__((ms_abi))" -DNO_BUILTIN_VA_FUNCS -mno-red-zone -Wno-address -mcmodel=large -fno-asynchronous-unwind-tables
DEFINE GCC44_IA32_X64_DLINK_COMMON = -nostdlib -n -q --gc-sections -z common-page-size=0x20
diff --git a/BaseTools/Scripts/GccBase.lds b/BaseTools/Scripts/GccBase.lds
index 4ee6d99853..32310bc75d 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)
}
}