[RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option
Andrew Hughes
gnu.andrew at redhat.com
Wed Jul 6 05:23:03 UTC 2016
----- Original Message -----
> > On Jul 5, 2016, at 1:33 PM, Andrew Hughes <gnu.andrew at redhat.com> wrote:
> >
> > ----- Original Message -----
> >>> On Jul 5, 2016, at 11:22 AM, Andrew Hughes <gnu.andrew at redhat.com> wrote:
> >> common/autoconf/flags.m4
> >> 716 $2JVM_CFLAGS="${$2JVM_CFLAGS} ${$2CXXSTD_CXXFLAG}"
> >>
> >> There is a pre-existing bug in the setup of ${$2CXXSTD_CXXFLAG}. It
> >> is using FLAGS_CXX_COMPILER_CHECK_ARGUMENTS, which doesn't know about
> >> the BUILD/TARGET distinction.
> >
> > This seems to be a problem with both FLAGS_C_COMPILER_CHECK_ARGUMENTS
> > and FLAGS_CXX_COMPILER_CHECK_ARGUMENTS, and thus with the
> > FLAGS_COMPILER_CHECK_ARGUMENTS macro that calls them both, which are used
> > in several places. I think this is outside the scope of this patch and
> > something which should be fixed by completing the current half-hearted
> > cross-compilation support. My aim here is just to fix the regression
> > which breaks the GCC 6 support on build==target builds, and I'd rather
> > whoever was working on the cross-compilation build continued that work.
> > There is a solution already in the handling of the warning argument
> > check, but it needs to be generalised to the other cases.
>
> I totally agree that fixing the xxx_CHECK_ARGUMENTS is out of scope
> for this patch.
>
> What I'm worried about is that by keeping those checks we might get
> and use the wrong answer in some cases where the BUILD and TARGET
> compilers are of different vintage. Maybe that will just encourage
> someone to fix them...
Thanks. I agree it's an issue. I just don't think I'm the right person
to undertake rewiring all that, as I've never even used the cross-compilation
support so far.
Do you know if there's already a bug for this? If not, I'll file one.
>
> In the specific case of -std=gnu++98, it seems unlikely we'll see such
> a misconfiguration any time soon. That option was introduced in
> gcc3.3, and it seems unlikely to me that anyone is building the JDK
> with such an old compiler (though it wouldn't be the first time I've
> been surprised in that way). OTOH, if the compiler is very new and has
> dropped support for that standard (e.g. some as yet not even announced
> version of gcc), we actually want a build failure, since our code was
> written for that standard and not some later one. So we're unlikely to
> be hurt by the use of xxx_CHECK_ARGUMENTS here.
>
I agree it's more likely that gnu++98 is going to be dropped at some point,
than we had a compiler that doesn't support the -std option. Hopefully,
making what was an implicit default before now explicit will encourage
developers to look at moving the code forward to a later version of the
standard. That's probably something for JDK 10+ though.
> For the gcc6-specific options, see below.
>
> >> Note that this also doesn't seem to affect some non-VM chunks of code,
> >> such as adlc (in hotspot/src/share/vm/adlc). I don't know whether
> >> that matters.
> >
> > Ugh, yes, so I see. It seems a couple of parts of the HotSpot build
> > just blithely ignore all this and set their own flags unconditionally.
> > For example, they also set -m64 regardless of whether it was successfully
> > tested for or not.
> >
> > I've added a patch to pass the std setting down to these parts of the
> > HotSpot build again, but they more generally need to be brought in line
> > with everything else and respect the configure checks.
>
> I think there are some more that are outside of HotSpot.
>
> Searching the build directory for *.o.cmdline files that don’t contain
> -std=gnu++98, e.g.
>
> find . -name "*.o.cmdline" ! -exec egrep -q -e "-std=gnu\+\+98" {} \; -print
> | xargs dirname | uniq
>
> produces a lot of stuff in ./support/native, the afore mentioned adlc, and a
> smattering of others.
>
> I think those might be better addressed by more followups, to get what we’ve
> got so far in.
Thanks for the .cmdline trick. I wasn't aware of that.
The -std=gnu++98 switch is only appropriate for the C++ compiler. Most of the
support/native object files are C files.
C++ code is used in the following:
hotspot/variant-server/tools/adlc/objs
hotspot/variant-server/libjvm/gtest/objs
hotspot/variant-server/libjvm/gtest/launcher-objs
hotspot/variant-server/libjvm/objs
support/native/java.base/libjimage
support/native/jdk.crypto.ec/libsunec
support/native/java.desktop/libfontmanager
support/native/jdk.pack200/unpackexe
support/native/jdk.pack200/libunpack
support/demos/native/jvmti/waiters
Using find . -name "*.o.cmdline" -exec egrep -q -e "g\+\+" {} \; ! -exec egrep -q -e "-std=gnu\+\+98" {} \; -print
I don't see any remaining files with the latest patch.
snip...
> >> common/autoconf/flags.m4
> >> 771 if test "x$1" = xTARGET; then
> >> 772 TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS
> >> 773 else
> >> 774 TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS(BUILD_,$2)
> >> 775 fi
> >>
> >> This is a fix for
> >> https://bugs.openjdk.java.net/browse/JDK-8157358
> >> Syntax error in TOOLCHAIN_CHECK_COMPILER_VERSION
> >>
> >> I think the change I suggested in that CR is better.
> >>
> >
> > I agree. I've reverted this part of my patch. I didn't realise it
> > was already being worked on and it was getting in the way of working on
> > this fix. I just went with the most local fix that got rid of the broken
> > test invocations.
>
> I wasn't planning to work on any of this, but was feeling annoyed
> about the situation, and then events conspired to leave me at loose
> ends for a bit this weekend. Though if I'd remembered how much I
> dislike autoconf, I might have looked harder for alternatives.
>
> But you will need that fix for JDK-8157358. I guess I should post an
> RFR for it... or you can just incorporate it into this patch if I don’t get
> that RFR done soon.
I wasn't planning to either, but it was likewise bugging me when I
was trying to fix the GCC 6 issue. Ironically, it took much longer to
work out what was going on there than it did to realise I needed to
add the flags to JVM_CFLAGS :(
I've included the fix in this patch, assuming that's ok with you.
>
> >> common/autoconf/flags.m4
> >> 1470 $1CFLAGS_JDK="[$]$1CFLAGS_JDK ${NO_NULL_POINTER_CHECK_CFLAG}
> >> ${NO_LIFETIME_DSE_CFLAG}"
> >> 1471 $1JVM_CFLAGS="[$]$1JVM_CFLAGS ${NO_NULL_POINTER_CHECK_CFLAG}
> >> ${NO_LIFETIME_DSE_CFLAG}"
> >>
> >> Adding the prefix to the CFLAGS variables is good.
> >>
> >> Since we've already determined we're using gcc6+ to get here, I don't
> >> think there's any benefit to the pre-existing argument checks.
> >>
> >> More importantly, FLAGS_COMPILER_CHECK_ARGUMENTS doesn't account for
> >> the BUILD / TARGET distinction, so may produce the wrong answer if the
> >> build and target compilers are different versions of gcc.
> >>
> >> So I think those checks should be eliminated.
> >
> > As you may recall, these checks were originally called in all
> > instances and we decided to limit it to GCC 6 or later in review.
> > I still think keeping the checks is wise, as these options may
> > not work with future compilers. If I was to eliminate anything,
> > it would be the version check, but I realise HotSpot developers
> > are pretty conservative about turning off these optimisations
> > on existing builds.
>
> I'm much more worried about getting and using the wrong answer for
> these options. Since we're already in a context where we're known to
> be using gcc6+, we know we're not using a too-old compiler that
> doesn't support them for that reason.
>
> -f[no-]delete-null-pointer-checks has been around since gcc 3.0, and
> seems unlikely to go away, since it has a real documented purpose
> (address zero isn't so special on some platforms).
>
> -f[no-]lifetime-dse is fairly new (introduced in gcc 4.9), so seems
> unlikely to go away soon, and if it did we would want a build failure
> anyway, at least until we've fixed the code that gets broken by that
> optimization.
>
I see your concern; potentially, we could check that one compiler is >= GCC 6,
but then run the argument checks on an older compiler.
As a compromise, I've commented out the argument checks rather than removing them.
I'm generally wary about trusting version checks alone rather than behaviour checks,
and so I would prefer these were re-enabled when the macro is fixed.
We've actually been quite lucky that the macro is only used for options which
aren't enabled in every build at present (stack-protector for slowdebug,
options for the Zero assembler port and the GCC 6 options). The Zero ones
are particularly troublesome, because cross-compiling to a target using
the Zero assembler port seems one of the most likely scenarios, as it allows
a JDK to be built for a new target that doesn't yet have a boot JDK.
Incidentally, both these flags are mentioned in the GCC 6 release notes [0].
snip...
>
> > I've had to bring back CXXSTD_CXXFLAG=@CXXSTD_CXXFLAG@ for the renegade
> > parts
> > of the HotSpot build.
> >
> > New webrevs:
> >
> > http://cr.openjdk.java.net/~andrew/8156980/webrev.03/root/
> > http://cr.openjdk.java.net/~andrew/8156980/webrev.03/hotspot/
>
> ------------------------------------------------------------------------------
> hotspot/make/lib/CompileDtracePostJvm.gmk
> 90 JVM_OFFSETS_CFLAGS := $(CXXSTD_CXXFLAG) -m64
>
> This appears to be in a solaris-specific block, so I don't think this
> change should be made.
Ah, good catch. Removed.
I was looking for CFLAGS that weren't using JVM_CFLAGS or CFLAGS_JDK
and didn't check the full context on that one.
>
> ------------------------------------------------------------------------------
> hotspot/make/gensrc/GensrcAdlc.gmk
> 54 # Set the C++ standard if supported
> 55 ADLC_CFLAGS += $(CXXSTD_CXXFLAG)
>
> This appears to be in a platform-independent block; I think it needs
> to be moved to a different location, probably the linux block starting
> at line 37.
I had it in the Linux block to begin with, but GCC is not specific to Linux.
This distinction between toolchains and platforms is held in the main autotools
code, but not in these Makefiles. From toolchain.gmk, it can be seen that
gcc on MacOS X is also supported:
# These toolchains are valid on different platforms
VALID_TOOLCHAINS_linux="gcc clang"
VALID_TOOLCHAINS_solaris="solstudio"
VALID_TOOLCHAINS_macosx="gcc clang"
VALID_TOOLCHAINS_aix="xlc"
VALID_TOOLCHAINS_windows="microsoft"
and there is the potential for gcc to be used on other platforms too (*BSD springs
to mind).
If gcc is not being used, CXXSTD_CXXFLAG won't be set so this should be a safe no-op.
>
>
>
Revised webrevs:
http://cr.openjdk.java.net/~andrew/8156980/webrev.04/root
http://cr.openjdk.java.net/~andrew/8156980/webrev.04/hotspot
I'm also now seeing a problem with GCC 6 only that is unique to the latest OpenJDK 9
and what looks like the gtest code. It seems to be the result of the header changes
also documented in [0] which were introduced in January [1] (and so probably weren't
in earlier test versions of GCC 6). I'm able to work around it and get a completed
build by setting -D_GLIBCXX_INCLUDE_NEXT_C_HEADERS, so it seems to be something
to do with the changes to stdlib.h in GCC 6. Something for a separate bug.
[0] https://gcc.gnu.org/gcc-6/porting_to.html
[1] https://github.com/gcc-mirror/gcc/commit/6c8ced3f4f867b72a623fe2f23efa204c5786a28
--
Andrew :)
Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
PGP Key: ed25519/35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04 C5A0 CFDA 0F9B 3596 4222
More information about the build-dev
mailing list