About the build infrastructure. An option would be to investigate solution like travis-ci (https://travis-ci.com/). The little I know about travis-ci is it is free for open-source project, it can follow your github project and report if it builds or not (as soon as your build does not take more than 10-15min). When you send github 'push-request' it can also say if you break the build. The build script is part of your repository (*.travis.yml), so everyone can contribute to it.
-----Original Message----- From: Laszlo Ersek [mailto:ler...@redhat.com] Sent: 08 July 2015 20:02 To: Bill Paul Cc: edk2-devel@lists.sourceforge.net; Scott Duplichan; Olivier Martin; leif.lindh...@linaro.org; jordan.l.jus...@intel.com Subject: Re: [edk2] [PATCH] BaseTools/GCC: allow unused but set variables On 07/08/15 20:15, Bill Paul wrote: > Of all the gin joints in all the towns in all the world, Laszlo Ersek > had to walk into mine at 09:17:08 on Wednesday 08 July 2015 and say: >> In the above pattern, ASSERT_EFI_ERROR() is not necessarily used for >> error handling. > > You are absolutely right: it's not. It's being used as a panic(). Good point! I guess I've always used ASSERT() in the sense of PANIC_UNLESS(). > [...] There is also DEBUG(). You will find several instances of the > following construction: > > int SomeInterestingValue; > > /* Populate SomeInterestingValue */ > > SomeInterestingValue = GetSomeInterestingStatus (handle); > > /* SomeInterestingValue is never referenced again */ > > DEBUG(EFI_D_INFO, "INTERESTING VALUE IS: %d\n", > SomeInterestingValue); > > DEBUG() is also disabled when MDEPKG_NDEBUG is enabled, which creates > the same problem. What would be the correct way to handle this case? Honestly, I don't know. I only concern myself with the case when even DEBUG_VERBOSE is enabled (PcdDebugPrintErrorLevel == 0x8040004F). The case when I have to look at debug messages and analyze behavior is the *norm*, not the exception (even if OVMF is not at fault -- the user may have passed wrong QEMU options, for example). It's an incredible chore when a user reports behavior he doesn't understand or expect, and I first have to ask him to reproduce on a DEBUG_VERBOSE build, and that he send me *that* log. Usually he has no access to such a build, cannot build one himself, so I get to build it for him. Blech. (I think Gerd's packages do enable DEBUG_VERBOSE, but I'm not sure.) > It should be noted that once in a while I run into this sort of thing > in VxWorks as well, only it's often the other way around. We typically > have DEBUG()-style messages turned off by default, and sometimes there > are debug messages that refer to variables which no longer exist in > the code. (The variables were removed but the debug messages were > never updated to match.) SeaBIOS employs the following trick to avoid this: all CONFIG_XXXX flags are checked in regular if() statements, not in #if macros. That is, any elmination happens during compilation / optimization / linking, not in preprocessing. This allows the compiler to catch such errors even when a CONFIG_XXXX option is predominantly false (or predominantly true). >> Right. I do agree with you that the optimal solution would be a build >> farm where all developers could test-build against all supported >> compilers. I doubt it's ever going to happen. > > Given that UEFI firmware is supposed to be the standard for a large > number of commercially available computer platforms, I find your lack of > faith... > disturbing. My lack of faith is based on "experience" :) We've been complaining about this for ages on the list. I think it is safe to assume that the primary "participant" that has legal access to all supported Microsoft toolchains is Intel. As described (and provided!) by Scott, gcc-for-Windows is almost trivially available (all supported gcc versions). It seems to follow that Intel should operate such a build farm. Based on how long we've been whining about this, I don't think it's going to happen any time soon. > Lastly, yesterday I actually spent some time updating the > mingw-gcc-build.py to support binutils 2.25 and GCC 4.9.3, and I ran > into this exact issue. I had to add -Wno-unused-but-set-variable to > the UNIXGCC instances of CC_FLAGS in tools_def.template in order to > get the OVMF build to work. But along the way, I noticed about a dozen > instances of this anti-pattern. There are at least as many cases that stem > from DEBUG() rather than ASSERT() or ASSERT_EFI_ERROR(). > > However I also found at least one legitimate error case of a useless > "set but never used" variable too (i.e. a case that didn't involve > conditional compilation effects). This means that now that GCC has > been muzzled to disguise intentional abuses of this pattern, some > unintentional abuses are now being hidden too. Optimally, we shouldn't conflate the different (deeper) reasons this warning gets emitted for. Unfortunately, it's not practical even for me to build OVMF in all possible configurations: - DEBUG and RELEASE (2) - gcc 4.4 to 4.9 (6) - Ia32, X64, Ia32X64 for OVMF (3) That's 36 configs (and yes, the data flow analysis is specific to both target architecture and gcc version, so some warnings would only be found in configs that I can't test). Add ARM and AARCH64 for ArmVirtPkg (QEMU and Xen separately)... Secure Boot enabled vs. disabled... ARM BDS vs. Intel BDS... For OVMF, CSM enabled vs. disabled; IPv6 enabled vs. disabled... It's really unmanageable without a build farm. Yes, someone could say "hey Laszlo why don't you create a number of virtual machines, with different gcc versions installed, and implement at least the 'free software' build farm?" -- Because I don't have time for it. (And that's the reason I can't really blame Intel people either, for not setting up the MSVC+gcc build farm. Someone would have to take a *personal* hit.) For now we can say that we'll fix up such errors when we find out about them (and thus we don't patch the warning flags now). Annoying, but it does lead to a better code base. Thanks Laszlo > > -Bill > >> Thanks >> Laszlo >> >>> 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 > -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782 ------------------------------------------------------------------------------ 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
