RFR: 8151841: Build needs additional flags to compile with GCC 6

[Kim Barrett]

> On Mar 14, 2016, at 3:17 PM, Andrew Hughes <gnu.andrew at redhat.com> wrote:
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8151841
> Webrev: http://cr.openjdk.java.net/~andrew/8151841/webrev.01/
> 
> A number of additional flags need to be passed to the C and C++ compiler
> for OpenJDK to compile with GCC 6:

This might not be the best time to be making interesting changes to
the Hotspot build system in order to support a not yet released
compiler, since there is a new Hotspot build system coming soon:
https://bugs.openjdk.java.net/browse/JDK-8076052,
http://openjdk.java.net/jeps/284.  But I'll leave that up to the folks
in charge of the build infrastructure.

That said, here are some specific comments on the webrev.

------------------------------------------------------------------------------ 
common/autoconf/hotspot-spec.gmk.in
Needs copyright update.

------------------------------------------------------------------------------ 
common/autoconf/flags.m4 
 631     # These flags are required for GCC 6 builds but may not be available on earlier versions
 632     NO_NULL_POINTER_CHECK_CFLAG="-fno-delete-null-pointer-checks"

According to gcc documentation, this option goes back at least into
the 3.x series.  So this seems somewhat overkill.

------------------------------------------------------------------------------  
common/autoconf/flags.m4 
 631     # These flags are required for GCC 6 builds but may not be available on earlier versions
...
 636     NO_LIFETIME_DSE_CFLAG="-fno-lifetime-dse"

This one does seem to be relatively new; I think it's introduced in
gcc4.9. However, there are other places where version
conditionalization of options is performed, such as
hotspot/make/linux/makefiles/gcc.make, where the addition of some
options is dependent on the version.  Here it's done based on behavior
rather than simply acceptance by the compiler being used.  So, for
example, -Wuninitialized isn't turned on until gcc4.8, because earlier
versions apparently report false positives.

Of course, there's the annoying fact that there are multiple gcc.make
files that might need to be modified for this.  But I'm not sure the
simple "the compiler accepts this option" is the right test here.

------------------------------------------------------------------------------  
common/autoconf/flags.m4 
 588   elif test "x$TOOLCHAIN_TYPE" = xgcc; then
 589     CXXSTD_CXXFLAG="-std=gnu++98"
 590     FLAGS_CXX_COMPILER_CHECK_ARGUMENTS(ARGUMENT: [$CXXSTD_CXXFLAG -Werror],
 591                                                  IF_FALSE: [CXXSTD_CXXFLAG=""])

Checking for acceptance of this option seems like overkill.

------------------------------------------------------------------------------  
common/autoconf/flags.m4 

If the new option checks weren't being made (as discussed above), the
changes to the checker wouldn't be needed.

------------------------------------------------------------------------------  
common/autoconf/hotspot-spec.gmk.in

 113 EXTRA_CFLAGS=@LEGACY_EXTRA_CFLAGS@ $(CFLAGS_CCACHE) $(NO_NULL_POINTER_CHECK_FLAG) $(NO_LIFETIME_DSE_CFLAG) $(CXXSTD_CXXFLAG)

It seems strange to only include $(NO_NULL_POINTER_CHECK_FLAG) and
$(NO_LIFETIME_DSE_FLAG) in EXTRA_CFLAGS, but not EXTRA_CXXFLAGS.

And it seems strange to include $(CXXSTD_CXXFLAG) in EXTRA_CFLAGS at
all, rather than in EXTRA_CXXFLAGS.

------------------------------------------------------------------------------ 

> 2. A number of optimisations in GCC 6 lead to a broken JVM. We need to
> add -fno-delete-null-pointer-checks and -fno-lifetime-dse to get a
> working JVM.

That's very disturbing!

-fdelete-null-pointer-checks is the default in much earlier versions
than gcc6 (since 4.5?), and much longer than that at higher
optimization levels (since somewhere in the 3.x series?).  But maybe
gcc6 is doing a better job of recognizing the relevant situations.  Or
maybe there's a bug in gcc6, which is not a released version yet.

One specific gcc6 change that could be relevant is:
  Value range propagation now assumes that the this pointer of C++
  member functions is non-null. This eliminates common null pointer
  checks but also breaks some non-conforming code-bases (such as Qt-5,
  Chromium, KDevelop). As a temporary work-around
  -fno-delete-null-pointer-checks can be used. Wrong code can be
  identified by using -fsanitize=undefined.

There's also a new warning option in gcc6 that might help find
places where -fdelete-null-pointer-checks is leading to problems:
-Wnull-dereference.

Do you have any information as to where things are being broken by
this option?  I think I would prefer an attempt to track down this
class of problem rather than hiding it via
-fno-delete-null-pointer-checks.

I don't have any suggestions for why gcc6 might be having problems
because of -flifetime-dse, or how to find them.  Do you?  This seems
to be a relatively new option, having been introduced in gcc4.9(?),
and seems to have always been on by default since being introduced.
Again, this could be a matter of gcc6 doing a better job of
recognizing relevant situations, or a bug in that not-yet-released
version.