On 25 March 2016 at 00:19, Jordan Justen <jordan.l.jus...@intel.com> wrote: > On 2016-03-24 03:37:24, Laszlo Ersek wrote: >> From: Ard Biesheuvel <ard.biesheu...@linaro.org> >> >> This aligns the GCC definitions for 4.6 and up to align with the ARM and >> AARCH64 definitions, which is to ignore unused but set variables only on >> RELEASE builds. This allows us to find instances of unused variables that >> are left behind after refactoring. It also allows us to find bad new code, >> which, due to the EDK2 coding style which disallows initialized automatic >> variables, may contain such variables without having been noticed by other >> toolchains. >> > > I guess that RELEASE builds must keep the flag because debug code > (such as assertions) will collapse to nothing and therefore trigger > the warning? Maybe that should be noted in the commit message? >
Good point. That was implied in the reference I made to Laszlo's 33 patches in the cover letter, since he brought up the issue about DEBUG() statements being the sole references to some variables. But making note of it here seems appropriate. > I agree with Ard that it is probably better to add ASSERT_EFI_STATUS > in most cases. But, your stance of leaving that to the maintainer > seems fine as well. > > Series Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com> > >> 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> >> Cc: Yonghong Zhu <yonghong....@intel.com> >> Signed-off-by: Laszlo Ersek <ler...@redhat.com> >> --- >> BaseTools/Conf/tools_def.template | 36 ++++++++++++++------ >> 1 file changed, 26 insertions(+), 10 deletions(-) >> >> diff --git a/BaseTools/Conf/tools_def.template >> b/BaseTools/Conf/tools_def.template >> index b1577af8c897..fc06ffa6e6be 100644 >> --- a/BaseTools/Conf/tools_def.template >> +++ b/BaseTools/Conf/tools_def.template >> @@ -4371,8 +4371,8 @@ DEFINE GCC45_X64_DLINK_FLAGS = >> DEF(GCC44_X64_DLINK_FLAGS) >> DEFINE GCC45_X64_DLINK2_FLAGS = DEF(GCC44_X64_DLINK2_FLAGS) >> DEFINE GCC45_ASM_FLAGS = DEF(GCC44_ASM_FLAGS) >> >> -DEFINE GCC46_IA32_CC_FLAGS = DEF(GCC45_IA32_CC_FLAGS) >> -Wno-address -Wno-unused-but-set-variable >> -DEFINE GCC46_X64_CC_FLAGS = DEF(GCC45_X64_CC_FLAGS) -Wno-address >> -Wno-unused-but-set-variable >> +DEFINE GCC46_IA32_CC_FLAGS = DEF(GCC45_IA32_CC_FLAGS) -Wno-address >> +DEFINE GCC46_X64_CC_FLAGS = DEF(GCC45_X64_CC_FLAGS) -Wno-address >> DEFINE GCC46_IA32_X64_DLINK_COMMON = DEF(GCC45_IA32_X64_DLINK_COMMON) >> DEFINE GCC46_IA32_X64_ASLDLINK_FLAGS = DEF(GCC45_IA32_X64_ASLDLINK_FLAGS) >> DEFINE GCC46_IA32_X64_DLINK_FLAGS = DEF(GCC45_IA32_X64_DLINK_FLAGS) >> @@ -4715,13 +4715,15 @@ DEFINE GCC49_AARCH64_ASLDLINK_FLAGS = >> DEF(GCC48_AARCH64_ASLDLINK_FLAGS) >> *_GCC46_IA32_ASLCC_FLAGS = DEF(GCC_ASLCC_FLAGS) -m32 >> *_GCC46_IA32_ASLDLINK_FLAGS = DEF(GCC46_IA32_X64_ASLDLINK_FLAGS) -m >> elf_i386 >> *_GCC46_IA32_ASM_FLAGS = DEF(GCC46_ASM_FLAGS) -m32 -march=i386 >> -*_GCC46_IA32_CC_FLAGS = DEF(GCC46_IA32_CC_FLAGS) -Os >> *_GCC46_IA32_DLINK_FLAGS = DEF(GCC46_IA32_X64_DLINK_FLAGS) -m >> elf_i386 --oformat=elf32-i386 >> *_GCC46_IA32_DLINK2_FLAGS = DEF(GCC46_IA32_DLINK2_FLAGS) >> *_GCC46_IA32_RC_FLAGS = DEF(GCC_IA32_RC_FLAGS) >> *_GCC46_IA32_OBJCOPY_FLAGS = >> *_GCC46_IA32_NASM_FLAGS = -f elf32 >> >> + DEBUG_GCC46_IA32_CC_FLAGS = DEF(GCC46_IA32_CC_FLAGS) -Os >> +RELEASE_GCC46_IA32_CC_FLAGS = DEF(GCC46_IA32_CC_FLAGS) -Os >> -Wno-unused-but-set-variable >> + >> ################## >> # GCC46 X64 definitions >> ################## >> @@ -4740,13 +4742,15 @@ DEFINE GCC49_AARCH64_ASLDLINK_FLAGS = >> DEF(GCC48_AARCH64_ASLDLINK_FLAGS) >> *_GCC46_X64_ASLCC_FLAGS = DEF(GCC_ASLCC_FLAGS) -m64 >> *_GCC46_X64_ASLDLINK_FLAGS = DEF(GCC46_IA32_X64_ASLDLINK_FLAGS) -m >> elf_x86_64 >> *_GCC46_X64_ASM_FLAGS = DEF(GCC46_ASM_FLAGS) -m64 -melf_x86_64 >> -*_GCC46_X64_CC_FLAGS = DEF(GCC46_X64_CC_FLAGS) >> *_GCC46_X64_DLINK_FLAGS = DEF(GCC46_X64_DLINK_FLAGS) >> *_GCC46_X64_DLINK2_FLAGS = DEF(GCC46_X64_DLINK2_FLAGS) >> *_GCC46_X64_RC_FLAGS = DEF(GCC_X64_RC_FLAGS) >> *_GCC46_X64_OBJCOPY_FLAGS = >> *_GCC46_X64_NASM_FLAGS = -f elf64 >> >> + DEBUG_GCC46_X64_CC_FLAGS = DEF(GCC46_X64_CC_FLAGS) >> +RELEASE_GCC46_X64_CC_FLAGS = DEF(GCC46_X64_CC_FLAGS) >> -Wno-unused-but-set-variable >> + >> ################## >> # GCC46 ARM definitions >> ################## >> @@ -4816,13 +4820,15 @@ RELEASE_GCC46_ARM_CC_FLAGS = >> DEF(GCC46_ARM_CC_FLAGS) -D__ARM_FEATURE_UNALI >> *_GCC47_IA32_ASLCC_FLAGS = DEF(GCC_ASLCC_FLAGS) -m32 >> *_GCC47_IA32_ASLDLINK_FLAGS = DEF(GCC47_IA32_X64_ASLDLINK_FLAGS) -m >> elf_i386 >> *_GCC47_IA32_ASM_FLAGS = DEF(GCC47_ASM_FLAGS) -m32 -march=i386 >> -*_GCC47_IA32_CC_FLAGS = DEF(GCC47_IA32_CC_FLAGS) -Os >> *_GCC47_IA32_DLINK_FLAGS = DEF(GCC47_IA32_X64_DLINK_FLAGS) -m >> elf_i386 --oformat=elf32-i386 >> *_GCC47_IA32_DLINK2_FLAGS = DEF(GCC47_IA32_DLINK2_FLAGS) >> *_GCC47_IA32_RC_FLAGS = DEF(GCC_IA32_RC_FLAGS) >> *_GCC47_IA32_OBJCOPY_FLAGS = >> *_GCC47_IA32_NASM_FLAGS = -f elf32 >> >> + DEBUG_GCC47_IA32_CC_FLAGS = DEF(GCC47_IA32_CC_FLAGS) -Os >> +RELEASE_GCC47_IA32_CC_FLAGS = DEF(GCC47_IA32_CC_FLAGS) -Os >> -Wno-unused-but-set-variable >> + >> ################## >> # GCC47 X64 definitions >> ################## >> @@ -4841,13 +4847,15 @@ RELEASE_GCC46_ARM_CC_FLAGS = >> DEF(GCC46_ARM_CC_FLAGS) -D__ARM_FEATURE_UNALI >> *_GCC47_X64_ASLCC_FLAGS = DEF(GCC_ASLCC_FLAGS) -m64 >> *_GCC47_X64_ASLDLINK_FLAGS = DEF(GCC47_IA32_X64_ASLDLINK_FLAGS) -m >> elf_x86_64 >> *_GCC47_X64_ASM_FLAGS = DEF(GCC47_ASM_FLAGS) -m64 >> -*_GCC47_X64_CC_FLAGS = DEF(GCC47_X64_CC_FLAGS) >> *_GCC47_X64_DLINK_FLAGS = DEF(GCC47_X64_DLINK_FLAGS) >> *_GCC47_X64_DLINK2_FLAGS = DEF(GCC47_X64_DLINK2_FLAGS) >> *_GCC47_X64_RC_FLAGS = DEF(GCC_X64_RC_FLAGS) >> *_GCC47_X64_OBJCOPY_FLAGS = >> *_GCC47_X64_NASM_FLAGS = -f elf64 >> >> + DEBUG_GCC47_X64_CC_FLAGS = DEF(GCC47_X64_CC_FLAGS) >> +RELEASE_GCC47_X64_CC_FLAGS = DEF(GCC47_X64_CC_FLAGS) >> -Wno-unused-but-set-variable >> + >> ################## >> # GCC47 ARM definitions >> ################## >> @@ -4944,13 +4952,15 @@ RELEASE_GCC47_AARCH64_CC_FLAGS = >> DEF(GCC47_AARCH64_CC_FLAGS) -Wno-unused-but-s >> *_GCC48_IA32_ASLCC_FLAGS = DEF(GCC_ASLCC_FLAGS) -m32 >> *_GCC48_IA32_ASLDLINK_FLAGS = DEF(GCC48_IA32_X64_ASLDLINK_FLAGS) -m >> elf_i386 >> *_GCC48_IA32_ASM_FLAGS = DEF(GCC48_ASM_FLAGS) -m32 -march=i386 >> -*_GCC48_IA32_CC_FLAGS = DEF(GCC48_IA32_CC_FLAGS) -Os >> *_GCC48_IA32_DLINK_FLAGS = DEF(GCC48_IA32_X64_DLINK_FLAGS) -m >> elf_i386 --oformat=elf32-i386 >> *_GCC48_IA32_DLINK2_FLAGS = DEF(GCC48_IA32_DLINK2_FLAGS) >> *_GCC48_IA32_RC_FLAGS = DEF(GCC_IA32_RC_FLAGS) >> *_GCC48_IA32_OBJCOPY_FLAGS = >> *_GCC48_IA32_NASM_FLAGS = -f elf32 >> >> + DEBUG_GCC48_IA32_CC_FLAGS = DEF(GCC48_IA32_CC_FLAGS) -Os >> +RELEASE_GCC48_IA32_CC_FLAGS = DEF(GCC48_IA32_CC_FLAGS) -Os >> -Wno-unused-but-set-variable >> + >> ################## >> # GCC48 X64 definitions >> ################## >> @@ -4969,13 +4979,15 @@ RELEASE_GCC47_AARCH64_CC_FLAGS = >> DEF(GCC47_AARCH64_CC_FLAGS) -Wno-unused-but-s >> *_GCC48_X64_ASLCC_FLAGS = DEF(GCC_ASLCC_FLAGS) -m64 >> *_GCC48_X64_ASLDLINK_FLAGS = DEF(GCC48_IA32_X64_ASLDLINK_FLAGS) -m >> elf_x86_64 >> *_GCC48_X64_ASM_FLAGS = DEF(GCC48_ASM_FLAGS) -m64 >> -*_GCC48_X64_CC_FLAGS = DEF(GCC48_X64_CC_FLAGS) >> *_GCC48_X64_DLINK_FLAGS = DEF(GCC48_X64_DLINK_FLAGS) >> *_GCC48_X64_DLINK2_FLAGS = DEF(GCC48_X64_DLINK2_FLAGS) >> *_GCC48_X64_RC_FLAGS = DEF(GCC_X64_RC_FLAGS) >> *_GCC48_X64_OBJCOPY_FLAGS = >> *_GCC48_X64_NASM_FLAGS = -f elf64 >> >> + DEBUG_GCC48_X64_CC_FLAGS = DEF(GCC48_X64_CC_FLAGS) >> +RELEASE_GCC48_X64_CC_FLAGS = DEF(GCC48_X64_CC_FLAGS) >> -Wno-unused-but-set-variable >> + >> ################## >> # GCC48 ARM definitions >> ################## >> @@ -5072,13 +5084,15 @@ RELEASE_GCC48_AARCH64_CC_FLAGS = >> DEF(GCC48_AARCH64_CC_FLAGS) -Wno-unused-but-s >> *_GCC49_IA32_ASLCC_FLAGS = DEF(GCC_ASLCC_FLAGS) -m32 >> *_GCC49_IA32_ASLDLINK_FLAGS = DEF(GCC49_IA32_X64_ASLDLINK_FLAGS) -m >> elf_i386 >> *_GCC49_IA32_ASM_FLAGS = DEF(GCC49_ASM_FLAGS) -m32 -march=i386 >> -*_GCC49_IA32_CC_FLAGS = DEF(GCC49_IA32_CC_FLAGS) -Os >> *_GCC49_IA32_DLINK_FLAGS = DEF(GCC49_IA32_X64_DLINK_FLAGS) -m >> elf_i386 --oformat=elf32-i386 >> *_GCC49_IA32_DLINK2_FLAGS = DEF(GCC49_IA32_DLINK2_FLAGS) >> *_GCC49_IA32_RC_FLAGS = DEF(GCC_IA32_RC_FLAGS) >> *_GCC49_IA32_OBJCOPY_FLAGS = >> *_GCC49_IA32_NASM_FLAGS = -f elf32 >> >> + DEBUG_GCC49_IA32_CC_FLAGS = DEF(GCC49_IA32_CC_FLAGS) -Os >> +RELEASE_GCC49_IA32_CC_FLAGS = DEF(GCC49_IA32_CC_FLAGS) -Os >> -Wno-unused-but-set-variable >> + >> ################## >> # GCC49 X64 definitions >> ################## >> @@ -5097,13 +5111,15 @@ RELEASE_GCC48_AARCH64_CC_FLAGS = >> DEF(GCC48_AARCH64_CC_FLAGS) -Wno-unused-but-s >> *_GCC49_X64_ASLCC_FLAGS = DEF(GCC_ASLCC_FLAGS) -m64 >> *_GCC49_X64_ASLDLINK_FLAGS = DEF(GCC49_IA32_X64_ASLDLINK_FLAGS) -m >> elf_x86_64 >> *_GCC49_X64_ASM_FLAGS = DEF(GCC49_ASM_FLAGS) -m64 >> -*_GCC49_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) >> *_GCC49_X64_DLINK_FLAGS = DEF(GCC49_X64_DLINK_FLAGS) >> *_GCC49_X64_DLINK2_FLAGS = DEF(GCC49_X64_DLINK2_FLAGS) >> *_GCC49_X64_RC_FLAGS = DEF(GCC_X64_RC_FLAGS) >> *_GCC49_X64_OBJCOPY_FLAGS = >> *_GCC49_X64_NASM_FLAGS = -f elf64 >> >> + DEBUG_GCC49_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) >> +RELEASE_GCC49_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) >> -Wno-unused-but-set-variable >> + >> ################## >> # GCC49 ARM definitions >> ################## >> -- >> 1.8.3.1 >> >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
