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

Erik Joelsson erik.joelsson at oracle.com
Tue Mar 15 09:18:59 UTC 2016


As representative for the build-infra group creating the new Hotspot 
build, I appreciate that the changes are made in configure. That will at 
least automatically force me to merge them correctly when they hit the 
build-infra forest and will make the merge easier. The idea is to start 
getting the new Hotspot build in soon after Jigsaw M3 is integrated. If 
the GCC 6 changes get in soon enough in jdk9/dev I will be able to merge 
and convert in time.

I like the refactoring of the FLAGS_COMPILER_CHECK_ARGUMENTS. Disabling 
optimizations that have obviously worked fine for a long time doesn't 
seem like a good idea though. I would prefer putting a conditional on 
the GCC version in those cases, but still keep the proposed flag check 
as well. There should be a toolchain version variable to compare 
against. I would also prefer if you could break the rather long line in 

If you would like to try out the new build in its current state, feel 
free to clone build-infra/jdk9.


On 2016-03-15 05:18, Andrew Hughes wrote:
> ----- Original Message -----
>>> 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.
> I'm not sure I'd refer to this as 'interesting changes'. GCC 6 is in final
> regression testing at the moment and will be appearing in the upcoming releases
> of a number of GNU/Linux distributions. It's pretty necessary to get this fix
> in now, so that it can also be fixed in 8u and get out into released versions
> which are being packaged for these distributions. It may not be released right
> now, but, by the time it's worked its way through the system, people will already
> be experiencing build failures with GCC 6.
> I briefly saw a post about these HotSpot changes when I was about to post this,
> and I was wondering when this was going to be finally cleaned up when I was working
> on the patch. It's good to see it being done. I don't think it changes the need
> for this patch though, as these flags also need to be added to the JDK build.
> At best, it will simplify the HotSpot part of this patch, which at the moment,
> is pretty ugly. Hopefully, this will mean that HotSpot is then actually using
> the C++ flags rather than the C ones! If you'd like some further testing of
> this new build, I'd be happy to take a look.
> My primary target for this though is 8u, so I'd like to see this in sooner rather
> than later. If we wait for the HotSpot build system to change, we're going to have
> a lot of broken 8u reports.
>> That said, here are some specific comments on the webrev.
>> ------------------------------------------------------------------------------
>> common/autoconf/hotspot-spec.gmk.in
>> Needs copyright update.
> It looks like it did years ago. My feelings on these are that they are best
> done in bulk in their own changeset. Including them with other changes makes
> it hard to then backport the patch cleanly. But I can add that if it's really
> necessary.
>> ------------------------------------------------------------------------------
>> 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.
> Possibly. I've tended to err on the side of caution; the option could equally be
> removed or renamed in some future release.
>> ------------------------------------------------------------------------------
>> 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.
> Yes, there seems to be a lot of logic duplicated in various places. The
> reason we're doing this at the root level is these options are also
> being applied to the JDK build. It seems like this is also more likely
> to survive the HotSpot build refactoring this way.
> Do you have an idea as to what the right test might be? I tend to
> find that checking an option is accepted is better than assuming
> it's accepted because we're using version x; changes can occasionally
> be backported to older versions at a later date. I'm not sure
> if that's what you were getting at or not. As this is an optimisation
> being disabled, rather than a warning, the behaviour is not as easily
> testable.
> I did consider restricting the additions to GCC >= 6, but my understanding
> is that these optimisations are problematic because of some of the OpenJDK
> code itself, and it's more simple luck that they haven't caused crashes before.
> I'm willing to defer to someone else more familiar with the code on this one
> though, and we can restrict it if it's known to be safe on earlier versions.
>> ------------------------------------------------------------------------------
>> common/autoconf/flags.m4
>>   588   elif test "x$TOOLCHAIN_TYPE" = xgcc; then
>>   589     CXXSTD_CXXFLAG="-std=gnu++98"
>>   -Werror],
>>   591                                                  IF_FALSE:
>> Checking for acceptance of this option seems like overkill.
> Again, possibly. I know it fails with the C compiler. configure is not
> particularly performance critical and it's better to fail early there than
> halfway through the build.
>> ------------------------------------------------------------------------------
>> 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.
> Agreed, it could be much simpler. I think the changes are generally beneficial
> though.
>> ------------------------------------------------------------------------------
>> common/autoconf/hotspot-spec.gmk.in
>> It seems strange to only include $(NO_NULL_POINTER_CHECK_FLAG) and
>> And it seems strange to include $(CXXSTD_CXXFLAG) in EXTRA_CFLAGS at
>> all, rather than in EXTRA_CXXFLAGS.
> It is strange, but it's because HotSpot currently completely ignores EXTRA_CXXFLAGS.
> I found this out the hard way :-)
> When we were testing this by passing --with-extra-cflags and --with-extra-cxxflags
> to the build, just putting the -std option in cxxflags didn't fix the HotSpot build.
> There's actually no way of doing it via the command-line options such that
> that the JDK C compiler doesn't get given the -std option and issue a warning
> as a result.
> hotspot/make/linux/makefiles/rules.make:CC_COMPILE       = $(CC) $(CXXFLAGS) $(CFLAGS)
> hotspot/make/linux/makefiles/rules.make:CXX_COMPILE      = $(CXX) $(CXXFLAGS) $(CFLAGS)
> EXTRA_CFLAGS are added to CFLAGS, but CXXFLAGS is only ever given -D and -I options.
> Essentially it's being used to mean the pre-processor, not the C++ compiler in the
> HotSpot build. The C compiler is hardly used at all in the HotSpot build.
>> ------------------------------------------------------------------------------
>>> 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.
> Andrew Haley (CCed) has more details on the need for these options,
> as he diagnosed the reason for the crash, with the help of the GCC
> developers. From what I understand of it, it is a case of more
> aggressive optimisations in the new GCC running into problems with
> code that doesn't strictly conform to the specification and exhibit
> undefined behaviour. The need for -flifetime-dse is covered in
> comment #47 of the bug for this [0]; "an object field [is] being
> accessed before the object is constructed, in breach of
> C++98 [class.cdtor]".
> I concur with you that the real solution here is to fix this
> undefined behaviour, but that's quite an involved job (as is
> converting the code to work with C++14) and one which may not be
> able to be backported when done. What I'm
> proposing here is a workaround to keep OpenJDK building in
> the mean-time. Getting a stable OpenJDK build on GCC 6 with
> these optimisations is a more involved task.
> We could limit disabling these options to GCC 6 and above.
> With this initial version of the patch, I've generally
> taken the safest route; although builds with earlier versions
> may not exhibit crashes, they still perform optimisations
> where the basis for these optimisations is being broken
> by some of the code in OpenJDK.
> [0] https://bugzilla.redhat.com/show_bug.cgi?id=1306558#c47
> Thanks for the extensive feedback,

More information about the hotspot-dev mailing list