Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] wrote:
]Sent: Wednesday, July 08, 2015 08:24 AM ]To: edk2-devel@lists.sourceforge.net; ler...@redhat.com; olivier.mar...@arm.com; leif.lindh...@linaro.org; ]jordan.l.jus...@intel.com ]Subject: [edk2] [PATCH] BaseTools/GCC: allow unused but set variables ] ]This fixes a recurring problem where patches that have only been ]tested on MS toolchains break the build on GCC because they use ]variables that are only written but never read. An alternative point of view is that anyone who tests with MS tool chains should also test with gcc tool chains. Windows hosted gcc tool chains of all supported versions are free, small and easy to download and use: http://sourceforge.net/projects/edk2developertoolsforwindows/ ]However, there is a valid pattern where this may happen as well. ]For instance, ] ] Status = SomeFunc (&OutVar); ] ASSERT_EFI_ERROR (Status); ] if (Outvar == ... ) { ... } ] // Status never referenced again ] ]may never access Status again at all in RELEASE builds, since the ]ASSERT_EFI_ERROR () macro evaluates to nothing in that case. To me this gcc warning is a good thing because it is pointing out a coding problem. The coding problem is that error handling is completely missing from the release build. Once the coding problem is fixed, the warning goes away. After years of debate on the role of ASSERT in EDK2 code, there is now a sort of official opinion in the EDK2 Coding document: ASSERT macros are development and debugging aids and shall never be used for error handling. Assertions are used to catch conditions caused by programming errors that are resolved prior to product release. The EDK II PCD, PcdDebugPropertyMask, can be used to enable or disable the generation of code associated with ASSERT usage. Thus, all code must be able to operate, and recover in a reasonable maner [sic] with ASSERTs disabled. If you never plan on using a release build, which I think might be the case with OVMF, the RELEASE option could be removed from the DSC file and -Wno-error=unused-but-set-variable could be added through the DSC file for that project only. Just my opinion anyway. We all know from the old Microsoft tool chains how annoying unwanted warnings are. Thanks, Scott ]So let's allow this pattern, by passing the appropriate GCC command ]line option. ] ]Contributed-under: TianoCore Contribution Agreement 1.0 ]Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org> ]--- ] BaseTools/Conf/tools_def.template | 2 +- ] 1 file changed, 1 insertion(+), 1 deletion(-) ] ]diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template ]index 7edd7590956b..15d8db04232f 100644 ]--- a/BaseTools/Conf/tools_def.template ]+++ b/BaseTools/Conf/tools_def.template ]@@ -3813,7 +3813,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 -Wno-error=unused-but-set-variable ] 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 ]-- ]1.9.1 ------------------------------------------------------------------------------ Don't Limit Your Business. Reach for the Cloud. GigeNET's Cloud Solutions provide you with the tools and support that you need to offload your IT needs and focus on growing your business. Configured For All Businesses. Start Your Cloud Today. https://www.gigenetcloud.com/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
