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 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.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
Reviewed-by: Liming Gao <liming....@intel.com>
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
---
v2: added the -fno-common argument to GCC44_ALL_CC_FLAGS as well, since GCC4x
    for X64 and IA32 does not include GCC_ALL_CC_FLAGS

 BaseTools/Conf/tools_def.template | 4 ++--
 BaseTools/Scripts/GccBase.lds     | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index d6b0af43d772..1a44fbd1d3eb 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 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

Reply via email to