On 23.03.2014, at 6:19, Scott Duplichan wrote: > Laszlo Ersek [mailto:ler...@redhat.com] wrote: > > ]Hi, > ] > ]I'm interested in feedback for the following BuildTools-related idea. > ] > ]Over the past months working with OVMF I have distilled the following > ]conviction about invalid compiler warnings (false positives emitted for > ]intended and valid C constructs): > ] > ](1) gcc likes to warn about uninitialized local variables that it finds > ]might be read without an intervening assignment. > ] > ](2) MSVC like to warn about implicit type conversions in assignments and > ]argument passing. > > I agree the MSVC 'possible loss of data' warning is not suitable > for use with EDK2. Developers often have little choice other than > to silence the warning with a type cast, and that has the > undesirable side-effect of hard coding the conversion. If instead > the warning is disabled, the compiler can continue to choose the > best conversion. The attached patch does this and is tested by > adding code to OvmfPkg to trigger MSVC warning C4244. Building > with DDK3790, VS2005, VS2008, VS2010, VS2012 then succeeds. > > ]Experience has shown that, for OVMF code specifically, these warnings > ]are *always* wrong. They force us to mangle the code in order to shut > ]them up, and they tend to throw a wrench in the gears of development > ]process. > ] > ]The explanation for the latter fact is that the set of constructs that > ]the compilers warn about is a moving target. It varies with compiler > ]version *and* with target architecture (Ia32 vs. X64). > ] > ]In effect, requiring that these two classes of warnings be treated as > ]errors under all circumstances *guarantees* that our *valid* code will > ]be unjustifiedly rejected by *some* (compiler, architecture) > ]combination. We cannot prepare ourselves in advance for all possible > ](compiler, architecture) pairs, hence the horrible disruption of > ]workflow that I mention above. > ] > ]The most recent example is: > ] > ] for Ia32 for X64 > ] --------- ----------------------- > ]gcc-4.4 breaks[1] builds without a squeak > ]gcc-4.8 breaks[2] builds without a squeak > ] > ]Note that [1] and [2] are *completely distinct* warnings. As in, when > ]building for Ia32, gcc-4.4 and gcc-4.8 warn about uninitialized > ]variables in *distinct* source files. The intersection of the warnings > ]emitted is the empty set. (I know this because I completed both builds > ]afterwards.) > ] > ]My proposal is the following. > ] > ](1) For GCC, introduce the following additional compiler flags: > ] > ]GCC:*_GCC44_*_CC_FLAGS = -Wno-error=uninitialized > ]GCC:*_GCC45_*_CC_FLAGS = -Wno-error=uninitialized > ]GCC:*_GCC46_*_CC_FLAGS = -Wno-error=uninitialized > ]GCC:*_GCC47_*_CC_FLAGS = -Wno-error=maybe-uninitialized > ]GCC:*_GCC48_*_CC_FLAGS = -Wno-error=maybe-uninitialized > ] > ]Gcc-4.4 to gcc-4.6 cannot distinguish the following two cases: > ]- "I can *prove* that this local variable is read while uninitialized" > ]- "I can see some way, a possibly unreachable way, to read an > ]uninitialized local variable". > ] > ]Hence for gcc-4.4 to gcc-4.6, we should degrade -Wuninitialized from > ]error to warning. > > Has anyone proposed reducing the number of gcc versions supported? > Would dropping support for 4.4, 4,5 and 4.6 create objections?
My vote is for dropping support for old compilers. gcc-4.8 is the best and free. There is no reason to keep older version. Who interesting in compiling some UEFI project is able to update his toolchain. Not a big deal. > > ]http://gcc.gnu.org/onlinedocs/gcc-4.4.7/gcc/Warning-Options.html#Warning-Options > ]http://gcc.gnu.org/onlinedocs/gcc-4.5.4/gcc/Warning-Options.html#Warning-Options > ]http://gcc.gnu.org/onlinedocs/gcc-4.6.4/gcc/Warning-Options.html#Warning-Options > ] > ]Gcc-4.7 and later *can* distinguish the above two cases (proven lack of > ]initialization vs. possible). Hence we should degrade only > ]-Wmaybe-uninitialized. > ] > ]http://gcc.gnu.org/onlinedocs/gcc-4.7.3/gcc/Warning-Options.html#Warning-Options > ]http://gcc.gnu.org/onlinedocs/gcc-4.8.2/gcc/Warning-Options.html#Warning-Options > ] > ](2) For MSVC, come up with a similar solution that degrades errors about > ]*valid* implicit type conversions to warnings. > > The Microsoft compilers do not have the capability to selectively > control which warnings stop the compile, so any unwanted warning > must be disabled. > > ]Again, experience shows that, for OVMF specifically, treating these > ]warnings as errors produces *net negative* value. There has not been > ]*one* instance where the warning was right. > ] > ]The warnings should stay, but they must not stop the build. We will > ]continue to address warnings *that we see* while developing code. We > ]shall not allow the build to break for users who happen to use compilers > ]/ arches different from ours. > ] > ]Thanks, > ]Laszlo > > <customize-msvc-warnings.patch>------------------------------------------------------------------------------ > Learn Graph Databases - Download FREE O'Reilly Book > "Graph Databases" is the definitive new guide to graph databases and their > applications. Written by three acclaimed leaders in the field, > this first edition is now available. Download your free book today! > http://p.sf.net/sfu/13534_NeoTech_______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ Learn Graph Databases - Download FREE O'Reilly Book "Graph Databases" is the definitive new guide to graph databases and their applications. Written by three acclaimed leaders in the field, this first edition is now available. Download your free book today! http://p.sf.net/sfu/13534_NeoTech _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
