Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Leslie Zhai]

Hi Andrew,

Thanks for your kind response!


在 2018年09月11日 01:19, Andrew Hughes 写道:

On Wed, 5 Sep 2018 at 10:52, Leslie Zhai  wrote:

Hi Andrew,

Thanks for your response!

I just quote it from here:

http://mail.openjdk.java.net/pipermail/build-dev/2016-July/017464.html

I spotted that jsig is just a single C file and so doesn't
need the -std flag. In fact, it complains about it:

Compiling jsig.c (for libjsig.so)
( ( /usr/bin/gcc -fPIC -D_GNU_SOURCE -D_REENTRANT -O2 -pipe -march=core2
-std=gnu++98 -m64 -g -DTHIS_FILE='"jsig.c"' -c -MMD -\
MF /home/andrew/builder/dev/hotspot/libjsig/objs/jsig.d -o
/home/andrew/builder/dev/hotspot/libjsig/objs/jsig.o /home/andrew/p\
rojects/openjdk/upstream/dev/hotspot/src/os/linux/vm/jsig.c >
  >(/usr/bin/tee /home/andrew/builder/dev/hotspot/libjsig/objs/jsi\
g.o.log) 2> >(/usr/bin/tee
/home/andrew/builder/dev/hotspot/libjsig/objs/jsig.o.log >&2) || (
exitcode=$? && /bin/cp /home/and\
rew/builder/dev/hotspot/libjsig/objs/jsig.o.log
/home/andrew/builder/dev/make-support/failure-logs/hotspot_libjsig_objs_jsig.o\
.log && exit $exitcode ) ) && wait )
cc1: warning: command line option '-std=gnu++98' is valid for C++/ObjC++
but not for C

It is still able to reproducible the warning for OpenJDK8 mips64el with
GCC and LLVM toolchains.  And gcc treat it as warning,  so it is often
be ignored,  but clang treat it as error.
- 8<  8<  8<  8<  8<  8< ---

Because CXXSTD_CXXFLAG="-std=gnu++98",  such flag might be effect
others,  so workaround for LLVM toolchain:
diff -r 1a87e769fb7f hotspot/make/linux/makefiles/jsig.make
--- a/hotspot/make/linux/makefiles/jsig.makeMon Sep 03 18:02:35 2018
+0800
+++ b/hotspot/make/linux/makefiles/jsig.makeWed Sep 05 15:53:22 2018
+0800
@@ -54,7 +54,7 @@
   $(LIBJSIG): $(JSIGSRCDIR)/jsig.c $(LIBJSIG_MAPFILE)
   @echo Making signal interposition lib...
   $(QUIETLY) $(CC) $(SYMFLAG) $(ARCHFLAG) $(SHARED_FLAG) $(PICFLAG) \
- $(LFLAGS_JSIG) $(JSIG_DEBUG_CFLAGS)
$(EXTRA_CFLAGS) -o $@ $< -ldl
+ $(LFLAGS_JSIG) $(JSIG_DEBUG_CFLAGS)
-fintegrated-as -o $@ $< -ldl
   ifeq ($(ENABLE_FULL_DEBUG_SYMBOLS),1)
   $(QUIETLY) $(OBJCOPY) --only-keep-debug $@ $(LIBJSIG_DEBUGINFO)
   $(QUIETLY) $(OBJCOPY) --add-gnu-debuglink=$(LIBJSIG_DEBUGINFO) $@
diff -r 1a87e769fb7f hotspot/make/linux/makefiles/saproc.make
--- a/hotspot/make/linux/makefiles/saproc.makeMon Sep 03 18:02:35
2018 +0800
+++ b/hotspot/make/linux/makefiles/saproc.makeWed Sep 05 15:53:22
2018 +0800
@@ -118,7 +118,7 @@
  $(SASRCFILES) \
  $(SA_LFLAGS) \
  $(SA_DEBUG_CFLAGS) \
-   $(EXTRA_CFLAGS) \
+ -fintegrated-as  \
  -o $@ \
  -lthread_db
   endif

Please give me some suggestion,  thanks a lot!

--
Regards,
Leslie Zhai



Is this the OpenJDK 8 build? It's quite different to the OpenJDK 9+ build I was
referring to in my e-mail.

Yes, I am compiling OpenJDK 8 with LLVM for mips64el
http://mail.openjdk.java.net/pipermail/build-dev/2018-September/023074.html



There are effectively three different OpenJDK build systems:

1. OpenJDK 6 & 7 - the old legacy make & Ant build throughtout
2. OpenJDK 8 - new configure & make build for most repositories, but invoking
the old legacy build for HotSpot
3. OpenJDK 9 - full  new configure & make build across the board

This leaves OpenJDK 8 as unique and quite confusing. It means when backporting
build changes from 9+, they essentially have to be duplicated to apply
to the HotSpot
part of the build as well.

I'm aware of the build warning due to -std in the backport to 8. It
was unavoidable
as the older build only uses CFLAGS, and primarily for C++ code. There is simply
no variable to add it to instead. CXXFLAGS is used there as a variable for some
preprocessor directives :

hotspot/make/linux/makefiles/rules.make:CC_COMPILE   = $(CC)
$(CXXFLAGS) $(CFLAGS)
hotspot/make/linux/makefiles/rules.make:CXX_COMPILE  = $(CXX)
$(CXXFLAGS) $(CFLAGS)

Trying to alter this at this stage in 8u's development is really too
dangerous, considering
the range of systems relying on being able to build 8u. I'm already a
little worried by the
number of build system changes coming into 8u. I would regard the
build system for
a legacy release as needing the same stability guarantees as the code itself.

Certainly removing EXTRA_CFLAGS is not appropriate as this code will
then be built
without flags passed in by configure and other additions made by
configure checks.
Sorry for my monkey patch, I will rebase the workaround compile with 
llvm patch.



Being able to build without that is one thing. Being able to ensure
there are no regressions
on all builds is quite another and we've already seen an issue where
flags for GCC >= 6
were dropped because of a typo [0].

My suggestion would be that -std is filtered out locally where it is
known that 

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Andrew Hughes]

On Wed, 5 Sep 2018 at 10:52, Leslie Zhai  wrote:
>
> Hi Andrew,
>
> Thanks for your response!
>
> I just quote it from here:
>
> http://mail.openjdk.java.net/pipermail/build-dev/2016-July/017464.html
>
> I spotted that jsig is just a single C file and so doesn't
> need the -std flag. In fact, it complains about it:
>
> Compiling jsig.c (for libjsig.so)
> ( ( /usr/bin/gcc -fPIC -D_GNU_SOURCE -D_REENTRANT -O2 -pipe -march=core2
> -std=gnu++98 -m64 -g -DTHIS_FILE='"jsig.c"' -c -MMD -\
> MF /home/andrew/builder/dev/hotspot/libjsig/objs/jsig.d -o
> /home/andrew/builder/dev/hotspot/libjsig/objs/jsig.o /home/andrew/p\
> rojects/openjdk/upstream/dev/hotspot/src/os/linux/vm/jsig.c >
>  >(/usr/bin/tee /home/andrew/builder/dev/hotspot/libjsig/objs/jsi\
> g.o.log) 2> >(/usr/bin/tee
> /home/andrew/builder/dev/hotspot/libjsig/objs/jsig.o.log >&2) || (
> exitcode=$? && /bin/cp /home/and\
> rew/builder/dev/hotspot/libjsig/objs/jsig.o.log
> /home/andrew/builder/dev/make-support/failure-logs/hotspot_libjsig_objs_jsig.o\
> .log && exit $exitcode ) ) && wait )
> cc1: warning: command line option '-std=gnu++98' is valid for C++/ObjC++
> but not for C
>
> It is still able to reproducible the warning for OpenJDK8 mips64el with
> GCC and LLVM toolchains.  And gcc treat it as warning,  so it is often
> be ignored,  but clang treat it as error.
> - 8<  8<  8<  8<  8<  8< ---
>
> Because CXXSTD_CXXFLAG="-std=gnu++98",  such flag might be effect
> others,  so workaround for LLVM toolchain:
> diff -r 1a87e769fb7f hotspot/make/linux/makefiles/jsig.make
> --- a/hotspot/make/linux/makefiles/jsig.makeMon Sep 03 18:02:35 2018
> +0800
> +++ b/hotspot/make/linux/makefiles/jsig.makeWed Sep 05 15:53:22 2018
> +0800
> @@ -54,7 +54,7 @@
>   $(LIBJSIG): $(JSIGSRCDIR)/jsig.c $(LIBJSIG_MAPFILE)
>   @echo Making signal interposition lib...
>   $(QUIETLY) $(CC) $(SYMFLAG) $(ARCHFLAG) $(SHARED_FLAG) $(PICFLAG) \
> - $(LFLAGS_JSIG) $(JSIG_DEBUG_CFLAGS)
> $(EXTRA_CFLAGS) -o $@ $< -ldl
> + $(LFLAGS_JSIG) $(JSIG_DEBUG_CFLAGS)
> -fintegrated-as -o $@ $< -ldl
>   ifeq ($(ENABLE_FULL_DEBUG_SYMBOLS),1)
>   $(QUIETLY) $(OBJCOPY) --only-keep-debug $@ $(LIBJSIG_DEBUGINFO)
>   $(QUIETLY) $(OBJCOPY) --add-gnu-debuglink=$(LIBJSIG_DEBUGINFO) $@
> diff -r 1a87e769fb7f hotspot/make/linux/makefiles/saproc.make
> --- a/hotspot/make/linux/makefiles/saproc.makeMon Sep 03 18:02:35
> 2018 +0800
> +++ b/hotspot/make/linux/makefiles/saproc.makeWed Sep 05 15:53:22
> 2018 +0800
> @@ -118,7 +118,7 @@
>  $(SASRCFILES) \
>  $(SA_LFLAGS) \
>  $(SA_DEBUG_CFLAGS) \
> -   $(EXTRA_CFLAGS) \
> + -fintegrated-as  \
>  -o $@ \
>  -lthread_db
>   endif
>
> Please give me some suggestion,  thanks a lot!
>
> --
> Regards,
> Leslie Zhai
>
>

Is this the OpenJDK 8 build? It's quite different to the OpenJDK 9+ build I was
referring to in my e-mail.

There are effectively three different OpenJDK build systems:

1. OpenJDK 6 & 7 - the old legacy make & Ant build throughtout
2. OpenJDK 8 - new configure & make build for most repositories, but invoking
the old legacy build for HotSpot
3. OpenJDK 9 - full  new configure & make build across the board

This leaves OpenJDK 8 as unique and quite confusing. It means when backporting
build changes from 9+, they essentially have to be duplicated to apply
to the HotSpot
part of the build as well.

I'm aware of the build warning due to -std in the backport to 8. It
was unavoidable
as the older build only uses CFLAGS, and primarily for C++ code. There is simply
no variable to add it to instead. CXXFLAGS is used there as a variable for some
preprocessor directives :

hotspot/make/linux/makefiles/rules.make:CC_COMPILE   = $(CC)
$(CXXFLAGS) $(CFLAGS)
hotspot/make/linux/makefiles/rules.make:CXX_COMPILE  = $(CXX)
$(CXXFLAGS) $(CFLAGS)

Trying to alter this at this stage in 8u's development is really too
dangerous, considering
the range of systems relying on being able to build 8u. I'm already a
little worried by the
number of build system changes coming into 8u. I would regard the
build system for
a legacy release as needing the same stability guarantees as the code itself.

Certainly removing EXTRA_CFLAGS is not appropriate as this code will
then be built
without flags passed in by configure and other additions made by
configure checks.
Being able to build without that is one thing. Being able to ensure
there are no regressions
on all builds is quite another and we've already seen an issue where
flags for GCC >= 6
were dropped because of a typo [0].

My suggestion would be that -std is filtered out locally where it is
known that only
C code is being compiled (jsig.make by the looks of it). It's not
pretty, but it's the option
which is the least risky.

Incidentally, unless things have changed, I wasn't 

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Martin Buchholz]

It would be awesome to use the sanitizers to find native code bugs in
openjdk, but it seems like a serious project.  Here at Google we are doing
our small part by improving support for clang on Linux.

On Wed, Sep 5, 2018 at 6:17 PM, Leslie Zhai  wrote:

> It might be UBSan false positive :) What about ASan?
> https://bugs.openjdk.java.net/browse/JDK-8189800
>
>
> 在 2018年09月06日 09:12, Martin Buchholz 写道:
>
>> it's difficult to use llvm tools like sanitizers on openjdk sources,
>> because of the "cheating" - relying on undefined behavior, and the JIT.
>>
>> On Wed, Sep 5, 2018 at 6:09 PM, Leslie Zhai > > wrote:
>>
>> Hi Martin,
>>
>> Thanks for your response!
>>
>> I haven't tested compiling OpenJDK 12-dev with LLVM toolchain,
>> perhaps the issue had been fixed already, because clang treat
>> invalid argument '-std=gnu++98' not allowed with 'C' as error.  It
>> is better only apply EXTRA_CFLAGS to C without EXTRA_CXXFLAGS.
>>
>> Furthermore, I just have interest, did you use clang analyzer,
>> sanitizer and libfuzzer towards hotspot and jdk native library?
>> Thanks!
>>
>>
>> 在 2018年09月06日 02:10, Martin Buchholz 写道:
>>
>> We seem to have some confusion about flags for C vs. flags for
>> C++.  Most flags for most toolchains apply to both C and C++,
>> so it's understandable that we want to unify them.  But some
>> flags, notably -std, are language-specific.  We have both
>> EXTRA_CFLAGS and EXTRA_CXXFLAGS, so we should expect
>> EXTRA_CFLAGS to only apply to C.
>>
>>
>> -- Regards,
>> Leslie Zhai
>>
>>
>>
>>
> --
> Regards,
> Leslie Zhai
>
>
>

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Leslie Zhai]

It might be UBSan false positive :) What about ASan? 
https://bugs.openjdk.java.net/browse/JDK-8189800



在 2018年09月06日 09:12, Martin Buchholz 写道:
it's difficult to use llvm tools like sanitizers on openjdk sources, 
because of the "cheating" - relying on undefined behavior, and the JIT.


On Wed, Sep 5, 2018 at 6:09 PM, Leslie Zhai > wrote:


Hi Martin,

Thanks for your response!

I haven't tested compiling OpenJDK 12-dev with LLVM toolchain,
perhaps the issue had been fixed already, because clang treat
invalid argument '-std=gnu++98' not allowed with 'C' as error.  It
is better only apply EXTRA_CFLAGS to C without EXTRA_CXXFLAGS.

Furthermore, I just have interest, did you use clang analyzer,
sanitizer and libfuzzer towards hotspot and jdk native library?
Thanks!


在 2018年09月06日 02:10, Martin Buchholz 写道:

We seem to have some confusion about flags for C vs. flags for
C++.  Most flags for most toolchains apply to both C and C++,
so it's understandable that we want to unify them.  But some
flags, notably -std, are language-specific.  We have both
EXTRA_CFLAGS and EXTRA_CXXFLAGS, so we should expect
EXTRA_CFLAGS to only apply to C.


-- 
Regards,

Leslie Zhai





--
Regards,
Leslie Zhai

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Martin Buchholz]

it's difficult to use llvm tools like sanitizers on openjdk sources,
because of the "cheating" - relying on undefined behavior, and the JIT.

On Wed, Sep 5, 2018 at 6:09 PM, Leslie Zhai  wrote:

> Hi Martin,
>
> Thanks for your response!
>
> I haven't tested compiling OpenJDK 12-dev with LLVM toolchain, perhaps the
> issue had been fixed already,  because clang treat invalid argument
> '-std=gnu++98' not allowed with 'C' as error.  It is better only apply
> EXTRA_CFLAGS to C without EXTRA_CXXFLAGS.
>
> Furthermore, I just have interest, did you use clang analyzer, sanitizer
> and libfuzzer towards hotspot and jdk native library? Thanks!
>
>
> 在 2018年09月06日 02:10, Martin Buchholz 写道:
>
>> We seem to have some confusion about flags for C vs. flags for C++.  Most
>> flags for most toolchains apply to both C and C++, so it's understandable
>> that we want to unify them.  But some flags, notably -std, are
>> language-specific.  We have both EXTRA_CFLAGS and EXTRA_CXXFLAGS, so we
>> should expect EXTRA_CFLAGS to only apply to C.
>>
>
> --
> Regards,
> Leslie Zhai
>
>
>

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Leslie Zhai]

Hi Martin,

Thanks for your response!

I haven't tested compiling OpenJDK 12-dev with LLVM toolchain, perhaps 
the issue had been fixed already,  because clang treat invalid argument 
'-std=gnu++98' not allowed with 'C' as error.  It is better only apply 
EXTRA_CFLAGS to C without EXTRA_CXXFLAGS.


Furthermore, I just have interest, did you use clang analyzer, sanitizer 
and libfuzzer towards hotspot and jdk native library? Thanks!


在 2018年09月06日 02:10, Martin Buchholz 写道:
We seem to have some confusion about flags for C vs. flags for C++.  
Most flags for most toolchains apply to both C and C++, so it's 
understandable that we want to unify them.  But some flags, notably 
-std, are language-specific.  We have both EXTRA_CFLAGS and 
EXTRA_CXXFLAGS, so we should expect EXTRA_CFLAGS to only apply to C.


--
Regards,
Leslie Zhai

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Martin Buchholz]

We seem to have some confusion about flags for C vs. flags for C++.  Most
flags for most toolchains apply to both C and C++, so it's understandable
that we want to unify them.  But some flags, notably -std, are
language-specific.  We have both EXTRA_CFLAGS and EXTRA_CXXFLAGS, so we
should expect EXTRA_CFLAGS to only apply to C.

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Leslie Zhai]

Hi Andrew,

Thanks for your response!

I just quote it from here:

http://mail.openjdk.java.net/pipermail/build-dev/2016-July/017464.html

I spotted that jsig is just a single C file and so doesn't
need the -std flag. In fact, it complains about it:

Compiling jsig.c (for libjsig.so)
( ( /usr/bin/gcc -fPIC -D_GNU_SOURCE -D_REENTRANT -O2 -pipe -march=core2 
-std=gnu++98 -m64 -g -DTHIS_FILE='"jsig.c"' -c -MMD -\
MF /home/andrew/builder/dev/hotspot/libjsig/objs/jsig.d -o 
/home/andrew/builder/dev/hotspot/libjsig/objs/jsig.o /home/andrew/p\
rojects/openjdk/upstream/dev/hotspot/src/os/linux/vm/jsig.c > 
>(/usr/bin/tee /home/andrew/builder/dev/hotspot/libjsig/objs/jsi\
g.o.log) 2> >(/usr/bin/tee 
/home/andrew/builder/dev/hotspot/libjsig/objs/jsig.o.log >&2) || ( 
exitcode=$? && /bin/cp /home/and\
rew/builder/dev/hotspot/libjsig/objs/jsig.o.log 
/home/andrew/builder/dev/make-support/failure-logs/hotspot_libjsig_objs_jsig.o\

.log && exit $exitcode ) ) && wait )
cc1: warning: command line option '-std=gnu++98' is valid for C++/ObjC++ 
but not for C


It is still able to reproducible the warning for OpenJDK8 mips64el with 
GCC and LLVM toolchains.  And gcc treat it as warning,  so it is often 
be ignored,  but clang treat it as error.

- 8<  8<  8<  8<  8<  8< ---

Because CXXSTD_CXXFLAG="-std=gnu++98",  such flag might be effect 
others,  so workaround for LLVM toolchain:

diff -r 1a87e769fb7f hotspot/make/linux/makefiles/jsig.make
--- a/hotspot/make/linux/makefiles/jsig.makeMon Sep 03 18:02:35 2018 
+0800
+++ b/hotspot/make/linux/makefiles/jsig.makeWed Sep 05 15:53:22 2018 
+0800

@@ -54,7 +54,7 @@
 $(LIBJSIG): $(JSIGSRCDIR)/jsig.c $(LIBJSIG_MAPFILE)
 @echo Making signal interposition lib...
 $(QUIETLY) $(CC) $(SYMFLAG) $(ARCHFLAG) $(SHARED_FLAG) $(PICFLAG) \
- $(LFLAGS_JSIG) $(JSIG_DEBUG_CFLAGS) 
$(EXTRA_CFLAGS) -o $@ $< -ldl
+ $(LFLAGS_JSIG) $(JSIG_DEBUG_CFLAGS) 
-fintegrated-as -o $@ $< -ldl

 ifeq ($(ENABLE_FULL_DEBUG_SYMBOLS),1)
 $(QUIETLY) $(OBJCOPY) --only-keep-debug $@ $(LIBJSIG_DEBUGINFO)
 $(QUIETLY) $(OBJCOPY) --add-gnu-debuglink=$(LIBJSIG_DEBUGINFO) $@
diff -r 1a87e769fb7f hotspot/make/linux/makefiles/saproc.make
--- a/hotspot/make/linux/makefiles/saproc.makeMon Sep 03 18:02:35 
2018 +0800
+++ b/hotspot/make/linux/makefiles/saproc.makeWed Sep 05 15:53:22 
2018 +0800

@@ -118,7 +118,7 @@
$(SASRCFILES) \
$(SA_LFLAGS) \
$(SA_DEBUG_CFLAGS) \
-   $(EXTRA_CFLAGS) \
+ -fintegrated-as  \
-o $@ \
-lthread_db
 endif

Please give me some suggestion,  thanks a lot!

--
Regards,
Leslie Zhai

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[David Holmes]

On 12/07/2016 1:40 PM, Andrew Hughes wrote:

- Original Message -

Catching up ...

On 11/07/2016 7:05 AM, Andrew Hughes wrote:



- Original Message -

On Jul 8, 2016, at 2:38 AM, Erik Joelsson 
wrote:

Hello,

This looks good except for the change in toolchain.m4, which looks like
it
might actually break cross compilation by overriding the value for
compiler version for the build compiler using the target compiler. With
this change we basically have:

if cross compilation
 TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([BUILD_], [OPENJDK_BUILD_])
else
 ...
fi
TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([], [OPENJDK_BUILD_])

The problem you are trying to solve is that in the case of not cross
compilation, the TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS macro wasn't
called with "OPENJDK_BUILD_". Kim's suggested patch was to add the call
in
the else clause.


Good catch!  I totally missed that when reviewing.



Yes, good catch! The indenting on the bug report must have confused me.

Fixed version:

http://cr.openjdk.java.net/~andrew/8156980/webrev.06/root/
http://cr.openjdk.java.net/~andrew/8156980/webrev.06/hotspot


I was glad to see you realized that parts of the hotspot build (libjsig,
libdtrace) only deal with C programs not C++ and so don't want and
should not have a C++ setting passed. But that then begs the question in
the main build in flags.m4 why we have:

  $2JVM_CFLAGS="${$2JVM_CFLAGS} ${$2CXXSTD_CXXFLAG}"

which again adds a C++ related flags to what should be the C compilation
flags ??



You'd think so, but for some reason, the HotSpot build has always called the
C++ compiler flags 'CFLAGS'. There is no JVM_CXXFLAGS.


Ah now I recall - this is the CPP -> CXX mess. CFLAGS meant 
compiler-flags; CPPFLAGS meant pre-processor-flags. People thought CPP 
meant C++ and changed to CXX.



The same behaviour was true with the old build; we had to pass these options
down to HotSpot in EXTRA_CFLAGS.

JVM_CFLAGS is the main variable used in the HotSpot build (see 
make/lib/CompileJvm.gmk).
The only C files in the HotSpot tree are for libsaproc, jsig and DTrace,
all of which are handled by separate makefiles. The only one to use JVM_CFLAGS 
is
make/lib/CompileDtracePostJvm.gmk, where it is used for a single C++ file, 
generateJvmOffsets.cpp:

generateJvmOffsets.cpp_CXXFLAGS := $(JVM_CFLAGS) -mt -xnolib 
-norunpath, \
generateJvmOffsetsMain.c_CFLAGS := -library=%none -mt -m64 -norunpath 
-z nodefs, \

Thus, it seems JVM_CFLAGS is JVM_CXXFLAGS in all but name.


Thanks for stepping through that.

David


If -std=gnu++98 does get passed to the C compiler, it produces a warning:

cc1: warning: command line option '-std=gnu++98' is valid for C++/ObjC++ but 
not for C

I don't see this at all for this patch on OpenJDK 9. It does appear in the 
backport
to 8u [0], because the aforementioned C code in the old HotSpot build does use 
the
same CFLAGS there.


Thanks,
David
-


HotSpot webrev is unchanged.

Thanks,





[0] http://mail.openjdk.java.net/pipermail/jdk8u-dev/2016-July/005690.html

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Andrew Hughes]

- Original Message -
> Catching up ...
> 
> On 11/07/2016 7:05 AM, Andrew Hughes wrote:
> >
> >
> > - Original Message -
> >>> On Jul 8, 2016, at 2:38 AM, Erik Joelsson 
> >>> wrote:
> >>>
> >>> Hello,
> >>>
> >>> This looks good except for the change in toolchain.m4, which looks like
> >>> it
> >>> might actually break cross compilation by overriding the value for
> >>> compiler version for the build compiler using the target compiler. With
> >>> this change we basically have:
> >>>
> >>> if cross compilation
> >>>  TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([BUILD_], [OPENJDK_BUILD_])
> >>> else
> >>>  ...
> >>> fi
> >>> TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([], [OPENJDK_BUILD_])
> >>>
> >>> The problem you are trying to solve is that in the case of not cross
> >>> compilation, the TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS macro wasn't
> >>> called with "OPENJDK_BUILD_". Kim's suggested patch was to add the call
> >>> in
> >>> the else clause.
> >>
> >> Good catch!  I totally missed that when reviewing.
> >>
> >>
> > Yes, good catch! The indenting on the bug report must have confused me.
> >
> > Fixed version:
> >
> > http://cr.openjdk.java.net/~andrew/8156980/webrev.06/root/
> > http://cr.openjdk.java.net/~andrew/8156980/webrev.06/hotspot
> 
> I was glad to see you realized that parts of the hotspot build (libjsig,
> libdtrace) only deal with C programs not C++ and so don't want and
> should not have a C++ setting passed. But that then begs the question in
> the main build in flags.m4 why we have:
> 
>   $2JVM_CFLAGS="${$2JVM_CFLAGS} ${$2CXXSTD_CXXFLAG}"
> 
> which again adds a C++ related flags to what should be the C compilation
> flags ??
> 

You'd think so, but for some reason, the HotSpot build has always called the
C++ compiler flags 'CFLAGS'. There is no JVM_CXXFLAGS.

The same behaviour was true with the old build; we had to pass these options
down to HotSpot in EXTRA_CFLAGS.

JVM_CFLAGS is the main variable used in the HotSpot build (see 
make/lib/CompileJvm.gmk).
The only C files in the HotSpot tree are for libsaproc, jsig and DTrace,
all of which are handled by separate makefiles. The only one to use JVM_CFLAGS 
is
make/lib/CompileDtracePostJvm.gmk, where it is used for a single C++ file, 
generateJvmOffsets.cpp:

generateJvmOffsets.cpp_CXXFLAGS := $(JVM_CFLAGS) -mt -xnolib 
-norunpath, \
generateJvmOffsetsMain.c_CFLAGS := -library=%none -mt -m64 -norunpath 
-z nodefs, \

Thus, it seems JVM_CFLAGS is JVM_CXXFLAGS in all but name.

If -std=gnu++98 does get passed to the C compiler, it produces a warning:

cc1: warning: command line option '-std=gnu++98' is valid for C++/ObjC++ but 
not for C

I don't see this at all for this patch on OpenJDK 9. It does appear in the 
backport
to 8u [0], because the aforementioned C code in the old HotSpot build does use 
the
same CFLAGS there.

> Thanks,
> David
> -
> 
> > HotSpot webrev is unchanged.
> >
> > Thanks,
> >
> 

[0] http://mail.openjdk.java.net/pipermail/jdk8u-dev/2016-July/005690.html
-- 
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

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[David Holmes]

Catching up ...

On 11/07/2016 7:05 AM, Andrew Hughes wrote:



- Original Message -

On Jul 8, 2016, at 2:38 AM, Erik Joelsson  wrote:

Hello,

This looks good except for the change in toolchain.m4, which looks like it
might actually break cross compilation by overriding the value for
compiler version for the build compiler using the target compiler. With
this change we basically have:

if cross compilation
 TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([BUILD_], [OPENJDK_BUILD_])
else
 ...
fi
TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([], [OPENJDK_BUILD_])

The problem you are trying to solve is that in the case of not cross
compilation, the TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS macro wasn't
called with "OPENJDK_BUILD_". Kim's suggested patch was to add the call in
the else clause.


Good catch!  I totally missed that when reviewing.



Yes, good catch! The indenting on the bug report must have confused me.

Fixed version:

http://cr.openjdk.java.net/~andrew/8156980/webrev.06/root/
http://cr.openjdk.java.net/~andrew/8156980/webrev.06/hotspot


I was glad to see you realized that parts of the hotspot build (libjsig, 
libdtrace) only deal with C programs not C++ and so don't want and 
should not have a C++ setting passed. But that then begs the question in 
the main build in flags.m4 why we have:


 $2JVM_CFLAGS="${$2JVM_CFLAGS} ${$2CXXSTD_CXXFLAG}"

which again adds a C++ related flags to what should be the C compilation 
flags ??


Thanks,
David
-


HotSpot webrev is unchanged.

Thanks,

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Kim Barrett]

> On Jul 10, 2016, at 5:05 PM, Andrew Hughes  wrote:
> 
> 
> 
> - Original Message -
>>> On Jul 8, 2016, at 2:38 AM, Erik Joelsson  wrote:
>>> 
>>> Hello,
>>> 
>>> This looks good except for the change in toolchain.m4, which looks like it
>>> might actually break cross compilation by overriding the value for
>>> compiler version for the build compiler using the target compiler. With
>>> this change we basically have:
>>> 
>>> if cross compilation
>>> TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([BUILD_], [OPENJDK_BUILD_])
>>> else
>>> ...
>>> fi
>>> TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([], [OPENJDK_BUILD_])
>>> 
>>> The problem you are trying to solve is that in the case of not cross
>>> compilation, the TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS macro wasn't
>>> called with "OPENJDK_BUILD_". Kim's suggested patch was to add the call in
>>> the else clause.
>> 
>> Good catch!  I totally missed that when reviewing.
>> 
>> 
> Yes, good catch! The indenting on the bug report must have confused me.
> 
> Fixed version:
> 
> http://cr.openjdk.java.net/~andrew/8156980/webrev.06/root/
> http://cr.openjdk.java.net/~andrew/8156980/webrev.06/hotspot
> 
> HotSpot webrev is unchanged.

Looks good.

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Omair Majid]

* Andrew Hughes  [2016-07-07 11:53]:
> Sadly, I celebrated too soon; it seems that change just delayed the failure
> until much later in the build.
> 
> I've found the problem though. In 
> src/share/vm/utilities/globalDefinitions.hpp,
> we have:
> 
> #ifdef max
> #undef max
> #endif
> 
> #ifdef min
> #undef min
> #endif
> 
> #define max(a,b) Do_not_use_max_use_MAX2_instead
> #define min(a,b) Do_not_use_min_use_MIN2_instead
> 
> The problem is that max and min are now longer macros in the GCC 6 C++ 
> library.
> So they can't be undefined.
> 
> The build succeeds if I disable (#if 0) the definition of max and min. So
> we need to consider either removing them or _GLIBCXX_INCLUDE_NEXT_C_HEADERS
> needs to be defined to get the old behaviour. I prefer the former.

I ran into this too and I posted a separate patch to work around this
issue that just involves changing the ordering of the includes:

http://mail.openjdk.java.net/pipermail/hotspot-dev/2016-July/023910.html

Thanks,
Omair

-- 
PGP Key: 66484681 (http://pgp.mit.edu/)
Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Andrew Hughes]

- Original Message -
> > On Jul 8, 2016, at 2:38 AM, Erik Joelsson  wrote:
> > 
> > Hello,
> > 
> > This looks good except for the change in toolchain.m4, which looks like it
> > might actually break cross compilation by overriding the value for
> > compiler version for the build compiler using the target compiler. With
> > this change we basically have:
> > 
> > if cross compilation
> >  TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([BUILD_], [OPENJDK_BUILD_])
> > else
> >  ...
> > fi
> > TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([], [OPENJDK_BUILD_])
> > 
> > The problem you are trying to solve is that in the case of not cross
> > compilation, the TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS macro wasn't
> > called with "OPENJDK_BUILD_". Kim's suggested patch was to add the call in
> > the else clause.
> 
> Good catch!  I totally missed that when reviewing.
> 
> 
Yes, good catch! The indenting on the bug report must have confused me.

Fixed version:

http://cr.openjdk.java.net/~andrew/8156980/webrev.06/root/
http://cr.openjdk.java.net/~andrew/8156980/webrev.06/hotspot

HotSpot webrev is unchanged.

Thanks,
-- 
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

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Kim Barrett]

> On Jul 8, 2016, at 2:38 AM, Erik Joelsson  wrote:
> 
> Hello,
> 
> This looks good except for the change in toolchain.m4, which looks like it 
> might actually break cross compilation by overriding the value for compiler 
> version for the build compiler using the target compiler. With this change we 
> basically have:
> 
> if cross compilation
>  TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([BUILD_], [OPENJDK_BUILD_])
> else
>  ...
> fi
> TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([], [OPENJDK_BUILD_])
> 
> The problem you are trying to solve is that in the case of not cross 
> compilation, the TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS macro wasn't 
> called with "OPENJDK_BUILD_". Kim's suggested patch was to add the call in 
> the else clause.

Good catch!  I totally missed that when reviewing.

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Erik Joelsson]

Hello,

This looks good except for the change in toolchain.m4, which looks like 
it might actually break cross compilation by overriding the value for 
compiler version for the build compiler using the target compiler. With 
this change we basically have:


if cross compilation
  TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([BUILD_], [OPENJDK_BUILD_])
else
  ...
fi
TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([], [OPENJDK_BUILD_])

The problem you are trying to solve is that in the case of not cross 
compilation, the TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS macro wasn't 
called with "OPENJDK_BUILD_". Kim's suggested patch was to add the call 
in the else clause. I would instead suggest the following:


if cross compilation
  ...
else
  ...
fi
TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS([BUILD_], [OPENJDK_BUILD_])

Just move it out of the conditional blocks and always call it the same way.

/Erik ( - on vacation and only sporadically checking email)

On 2016-07-07 22:42, Andrew Hughes wrote:


- Original Message -

On Jul 7, 2016, at 1:48 PM, Andrew Hughes  wrote:

Revised webrevs:

http://cr.openjdk.java.net/~andrew/8156980/webrev.04/root
http://cr.openjdk.java.net/~andrew/8156980/webrev.04/hotspot

These look good to me.


Minor revision:

http://cr.openjdk.java.net/~andrew/8156980/webrev.05/root
http://cr.openjdk.java.net/~andrew/8156980/webrev.05/hotspot

I spotted that jsig is just a single C file and so doesn't
need the -std flag. In fact, it complains about it:

Compiling jsig.c (for libjsig.so)
( ( /usr/bin/gcc -fPIC -D_GNU_SOURCE -D_REENTRANT -O2 -pipe -march=core2
-std=gnu++98 -m64 -g -DTHIS_FILE='"jsig.c"' -c -MMD -\
MF /home/andrew/builder/dev/hotspot/libjsig/objs/jsig.d -o
/home/andrew/builder/dev/hotspot/libjsig/objs/jsig.o /home/andrew/p\
rojects/openjdk/upstream/dev/hotspot/src/os/linux/vm/jsig.c >

(/usr/bin/tee /home/andrew/builder/dev/hotspot/libjsig/objs/jsi\

g.o.log) 2> >(/usr/bin/tee
/home/andrew/builder/dev/hotspot/libjsig/objs/jsig.o.log >&2) || (
exitcode=$? && /bin/cp /home/and\
rew/builder/dev/hotspot/libjsig/objs/jsig.o.log
/home/andrew/builder/dev/make-support/failure-logs/hotspot_libjsig_objs_jsig.o\
.log && exit $exitcode ) ) && wait )
cc1: warning: command line option '-std=gnu++98' is valid for C++/ObjC++
but not for C

So just ADLC is fixed now.

The root webrev is the same as before.

Oops, glad you caught that.

Looks even better.



The same warning was causing an error in the OpenJDK 8 backport of the original
GCC 6 patch, so I double-checked the logs for 9 and spotted this :-)

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Andrew Hughes]

- Original Message -
> > On Jul 7, 2016, at 1:48 PM, Andrew Hughes  wrote:
> >>> Revised webrevs:
> >>> 
> >>> http://cr.openjdk.java.net/~andrew/8156980/webrev.04/root
> >>> http://cr.openjdk.java.net/~andrew/8156980/webrev.04/hotspot
> >> 
> >> These look good to me.
> >> 
> > 
> > Minor revision:
> > 
> > http://cr.openjdk.java.net/~andrew/8156980/webrev.05/root
> > http://cr.openjdk.java.net/~andrew/8156980/webrev.05/hotspot
> > 
> > I spotted that jsig is just a single C file and so doesn't
> > need the -std flag. In fact, it complains about it:
> > 
> > Compiling jsig.c (for libjsig.so)
> > ( ( /usr/bin/gcc -fPIC -D_GNU_SOURCE -D_REENTRANT -O2 -pipe -march=core2
> > -std=gnu++98 -m64 -g -DTHIS_FILE='"jsig.c"' -c -MMD -\
> > MF /home/andrew/builder/dev/hotspot/libjsig/objs/jsig.d -o
> > /home/andrew/builder/dev/hotspot/libjsig/objs/jsig.o /home/andrew/p\
> > rojects/openjdk/upstream/dev/hotspot/src/os/linux/vm/jsig.c >
> > >(/usr/bin/tee /home/andrew/builder/dev/hotspot/libjsig/objs/jsi\
> > g.o.log) 2> >(/usr/bin/tee
> > /home/andrew/builder/dev/hotspot/libjsig/objs/jsig.o.log >&2) || (
> > exitcode=$? && /bin/cp /home/and\
> > rew/builder/dev/hotspot/libjsig/objs/jsig.o.log
> > /home/andrew/builder/dev/make-support/failure-logs/hotspot_libjsig_objs_jsig.o\
> > .log && exit $exitcode ) ) && wait )
> > cc1: warning: command line option '-std=gnu++98' is valid for C++/ObjC++
> > but not for C
> > 
> > So just ADLC is fixed now.
> > 
> > The root webrev is the same as before.
> 
> Oops, glad you caught that.
> 
> Looks even better.
> 
> 

The same warning was causing an error in the OpenJDK 8 backport of the original
GCC 6 patch, so I double-checked the logs for 9 and spotted this :-)
-- 
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

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Kim Barrett]

> On Jul 7, 2016, at 1:48 PM, Andrew Hughes  wrote:
>>> Revised webrevs:
>>> 
>>> http://cr.openjdk.java.net/~andrew/8156980/webrev.04/root
>>> http://cr.openjdk.java.net/~andrew/8156980/webrev.04/hotspot
>> 
>> These look good to me.
>> 
> 
> Minor revision:
> 
> http://cr.openjdk.java.net/~andrew/8156980/webrev.05/root
> http://cr.openjdk.java.net/~andrew/8156980/webrev.05/hotspot
> 
> I spotted that jsig is just a single C file and so doesn't
> need the -std flag. In fact, it complains about it:
> 
> Compiling jsig.c (for libjsig.so)
> ( ( /usr/bin/gcc -fPIC -D_GNU_SOURCE -D_REENTRANT -O2 -pipe -march=core2 
> -std=gnu++98 -m64 -g -DTHIS_FILE='"jsig.c"' -c -MMD -\
> MF /home/andrew/builder/dev/hotspot/libjsig/objs/jsig.d -o 
> /home/andrew/builder/dev/hotspot/libjsig/objs/jsig.o /home/andrew/p\
> rojects/openjdk/upstream/dev/hotspot/src/os/linux/vm/jsig.c > >(/usr/bin/tee 
> /home/andrew/builder/dev/hotspot/libjsig/objs/jsi\
> g.o.log) 2> >(/usr/bin/tee 
> /home/andrew/builder/dev/hotspot/libjsig/objs/jsig.o.log >&2) || ( 
> exitcode=$? && /bin/cp /home/and\
> rew/builder/dev/hotspot/libjsig/objs/jsig.o.log 
> /home/andrew/builder/dev/make-support/failure-logs/hotspot_libjsig_objs_jsig.o\
> .log && exit $exitcode ) ) && wait )
> cc1: warning: command line option '-std=gnu++98' is valid for C++/ObjC++ but 
> not for C
> 
> So just ADLC is fixed now.
> 
> The root webrev is the same as before.

Oops, glad you caught that.

Looks even better.

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Andrew Hughes]

snip...

> 
> > Revised webrevs:
> > 
> > http://cr.openjdk.java.net/~andrew/8156980/webrev.04/root
> > http://cr.openjdk.java.net/~andrew/8156980/webrev.04/hotspot
> 
> These look good to me.
> 

Minor revision:

http://cr.openjdk.java.net/~andrew/8156980/webrev.05/root
http://cr.openjdk.java.net/~andrew/8156980/webrev.05/hotspot

I spotted that jsig is just a single C file and so doesn't
need the -std flag. In fact, it complains about it:

Compiling jsig.c (for libjsig.so)
( ( /usr/bin/gcc -fPIC -D_GNU_SOURCE -D_REENTRANT -O2 -pipe -march=core2 
-std=gnu++98 -m64 -g -DTHIS_FILE='"jsig.c"' -c -MMD -\
MF /home/andrew/builder/dev/hotspot/libjsig/objs/jsig.d -o 
/home/andrew/builder/dev/hotspot/libjsig/objs/jsig.o /home/andrew/p\
rojects/openjdk/upstream/dev/hotspot/src/os/linux/vm/jsig.c > >(/usr/bin/tee 
/home/andrew/builder/dev/hotspot/libjsig/objs/jsi\
g.o.log) 2> >(/usr/bin/tee 
/home/andrew/builder/dev/hotspot/libjsig/objs/jsig.o.log >&2) || ( exitcode=$? 
&& /bin/cp /home/and\
rew/builder/dev/hotspot/libjsig/objs/jsig.o.log 
/home/andrew/builder/dev/make-support/failure-logs/hotspot_libjsig_objs_jsig.o\
.log && exit $exitcode ) ) && wait )
cc1: warning: command line option '-std=gnu++98' is valid for C++/ObjC++ but 
not for C

So just ADLC is fixed now.

The root webrev is the same as before.
-- 
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

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Andrew Hughes]

snip...

> > > 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.
> > 
> > What fun!
> 
> A new batch of changes just came through which seems to have fixed it.
> I'm guessing this one:
> 
> http://hg.openjdk.java.net/jdk9/dev/hotspot/rev/7862a718ec47
> 
> Either way, I'm happy I don't have to debug it :-)
> 

Sadly, I celebrated too soon; it seems that change just delayed the failure
until much later in the build.

I've found the problem though. In src/share/vm/utilities/globalDefinitions.hpp,
we have:

#ifdef max
#undef max
#endif

#ifdef min
#undef min
#endif

#define max(a,b) Do_not_use_max_use_MAX2_instead
#define min(a,b) Do_not_use_min_use_MIN2_instead

The problem is that max and min are now longer macros in the GCC 6 C++ library.
So they can't be undefined.

The build succeeds if I disable (#if 0) the definition of max and min. So
we need to consider either removing them or _GLIBCXX_INCLUDE_NEXT_C_HEADERS
needs to be defined to get the old behaviour. I prefer the former.

-- 
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

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Andrew Hughes]

snip...

> >> 
> >> 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.
> 
> Yeah, I feel the same way.  While I've dealt with cross-compilation
> issues of this sort enough to recognize the presence of problems, that
> work wasn't with this build system.
> 
> > Do you know if there's already a bug for this? If not, I'll file one.
> 
> I didn't find any relevant bugs.
> 

I've filed one: https://bugs.openjdk.java.net/browse/JDK-8160926

snip...

> 
> > Revised webrevs:
> > 
> > http://cr.openjdk.java.net/~andrew/8156980/webrev.04/root
> > http://cr.openjdk.java.net/~andrew/8156980/webrev.04/hotspot
> 
> These look good to me.
> 

Thanks.

I need someone at Oracle to push this one through, as the
configure script for the closed code needs regenerating too. Apparently,
I broke things last time :(

> > 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.
> 
> What fun!

A new batch of changes just came through which seems to have fixed it.
I'm guessing this one:

http://hg.openjdk.java.net/jdk9/dev/hotspot/rev/7862a718ec47

Either way, I'm happy I don't have to debug it :-)

> 
> > [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
> 
> 
> 

Thanks for the great feedback on this patch,
-- 
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

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Kim Barrett]

> On Jul 6, 2016, at 1:23 AM, Andrew Hughes  wrote:
> 
> - Original Message -
>>> On Jul 5, 2016, at 1:33 PM, Andrew Hughes  wrote:
>>> 
>>> - Original Message -
> On Jul 5, 2016, at 11:22 AM, Andrew Hughes  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.

Yeah, I feel the same way.  While I've dealt with cross-compilation
issues of this sort enough to recognize the presence of problems, that
work wasn't with this build system.

> Do you know if there's already a bug for this? If not, I'll file one.

I didn't find any relevant bugs.

>> 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.

I think there is interest in that direction.

>> 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 .o.cmdline files are a feature of the new build system that I
like a lot.

> 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:
> […]
> 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.

Yay!

>> 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.

Yes, that’s fine with me.

> If gcc is not being used, CXXSTD_CXXFLAG won't be set so this should be a 
> safe no-op.

Oh, right.

> Revised webrevs:
> 
> http://cr.openjdk.java.net/~andrew/8156980/webrev.04/root
> http://cr.openjdk.java.net/~andrew/8156980/webrev.04/hotspot

These look good to me.

> I'm also now seeing a problem with GCC 6 only that is unique to the latest 
> 

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Andrew Hughes]

- Original Message -
> > On Jul 5, 2016, at 1:33 PM, Andrew Hughes  wrote:
> > 
> > - Original Message -
> >>> On Jul 5, 2016, at 11:22 AM, Andrew Hughes  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
> >> 

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Kim Barrett]

> On Jul 5, 2016, at 1:33 PM, Andrew Hughes  wrote:
> 
> - Original Message -
>>> On Jul 5, 2016, at 11:22 AM, Andrew Hughes  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...

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.

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.

>> 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.

>> common/autoconf/flags.m4
>> 1458 AC_DEFUN([FLAGS_SETUP_GCC6_COMPILER_FLAGS],
>> 
>> Changing from AC_DEFUN_ONCE to AC_DEFUN is good.
>> 
>> It's needed in order to pass in the prefix.  Even ignoring that issue,
>> something needed to be done because the defun_once defined later was
>> not being expanded where used.
> 
> It was being expanded once before, which was appropriate for a block of
> code that didn't take any arguments and was only used because it made the
> source file easier to read than trying to add that logic inside the
> TOOLCHAIN_CHECK_COMPILER_VERSION invocation.

You are right about it being expanded once. I must have been
remembering one of the unsuccessful attempts I tried, where no code
was generated 

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Andrew Hughes]

- Original Message -
> > On Jul 5, 2016, at 11:22 AM, Andrew Hughes  wrote:
> > 
> > - Original Message -
> >> Hello,
> >> 
> >> In general it looks good. It's nice to see that this also fixes that
> >> warning output from configure. My only nit is the comment describing the
> >> parameters for FLAGS_SETUP_GCC6_COMPILER_FLAGS. It indicates that it
> >> takes named parameters while it actually just takes positional. Please
> >> either change it to named parameters or fix the comment.
> >> 
> > 
> > Ah, that's because it was named parameters for a while, then I changed it
> > because it was easier than trying to get ARG_PREFIX instead of $1 into
> > [$]$1CFLAGS_JDK and friends.
> > 
> > Fixed in:
> > 
> > http://cr.openjdk.java.net/~andrew/8156980/webrev.02/
> > 
> > I'll let someone on your side push it through so you can regenerate
> > your internal configure script at the same time.
> 
> I've also been looking at this area.  It looks like we've found pretty
> much the same set of problems and have similar solutions, so that's
> good.  I have a few suggestions or possible improvements.
> 
> --
> 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.

> 
> 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.

> 
> --
> 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.

> --
> common/autoconf/flags.m4
> 1458 AC_DEFUN([FLAGS_SETUP_GCC6_COMPILER_FLAGS],
> 
> Changing from AC_DEFUN_ONCE to AC_DEFUN is good.
> 
> It's needed in order to pass in the prefix.  Even ignoring that issue,
> something needed to be done because the defun_once defined later was
> not being expanded where used.

It was being expanded once before, which was appropriate for a block of
code that didn't take any arguments and was only used because it made the
source file easier to read than trying to add that logic inside the
TOOLCHAIN_CHECK_COMPILER_VERSION invocation.

Now we do need to call it for both the build and target compilers, AC_DEFUN
is more appropriate.

> 
> --
> 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 

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Kim Barrett]

> On Jul 5, 2016, at 11:22 AM, Andrew Hughes  wrote:
> 
> - Original Message -
>> Hello,
>> 
>> In general it looks good. It's nice to see that this also fixes that
>> warning output from configure. My only nit is the comment describing the
>> parameters for FLAGS_SETUP_GCC6_COMPILER_FLAGS. It indicates that it
>> takes named parameters while it actually just takes positional. Please
>> either change it to named parameters or fix the comment.
>> 
> 
> Ah, that's because it was named parameters for a while, then I changed it
> because it was easier than trying to get ARG_PREFIX instead of $1 into
> [$]$1CFLAGS_JDK and friends.
> 
> Fixed in:
> 
> http://cr.openjdk.java.net/~andrew/8156980/webrev.02/
> 
> I'll let someone on your side push it through so you can regenerate
> your internal configure script at the same time.

I've also been looking at this area.  It looks like we've found pretty
much the same set of problems and have similar solutions, so that's
good.  I have a few suggestions or possible improvements.

--
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.

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.

-- 
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.

-- 
common/autoconf/flags.m4  
1458 AC_DEFUN([FLAGS_SETUP_GCC6_COMPILER_FLAGS],

Changing from AC_DEFUN_ONCE to AC_DEFUN is good.

It's needed in order to pass in the prefix.  Even ignoring that issue,
something needed to be done because the defun_once defined later was
not being expanded where used.

--  
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.

Also, I think the variables capturing the options probably also need
to be prefixed, if they are retained.

Aside: I also think the variable name for
-fno-delete-null-pointer-checks is mildly confusing, as the absence of
"DELETE" inverts the apparent meaning.

-- 
common/autoconf/spec.gmk
[removed]
 395 NO_NULL_POINTER_CHECK_FLAG=@NO_NULL_POINTER_CHECK_CFLAG@
 396 NO_LIFETIME_DSE_CFLAG=@NO_LIFETIME_DSE_CFLAG@
 397 CXXSTD_CXXFLAG=@CXXSTD_CXXFLAG@

These removals are good. This also eliminates the "FLAG" for "CFLAG"
typo in NO_NULL_POINTER_CHECK_FLAG=.

--

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Andrew Hughes]

- Original Message -
> Hello,
> 
> In general it looks good. It's nice to see that this also fixes that
> warning output from configure. My only nit is the comment describing the
> parameters for FLAGS_SETUP_GCC6_COMPILER_FLAGS. It indicates that it
> takes named parameters while it actually just takes positional. Please
> either change it to named parameters or fix the comment.
> 

Ah, that's because it was named parameters for a while, then I changed it
because it was easier than trying to get ARG_PREFIX instead of $1 into
[$]$1CFLAGS_JDK and friends.

Fixed in:

http://cr.openjdk.java.net/~andrew/8156980/webrev.02/

I'll let someone on your side push it through so you can regenerate
your internal configure script at the same time.

> Thanks for fixing this!
> 

Thanks :)

> /Erik
> 
> On 2016-07-05 07:27, Andrew Hughes wrote:
> > Webrev: http://cr.openjdk.java.net/~andrew/8156980/webrev.01/
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8156980
> >
> > With "8151841: Build needs additional flags to compile with GCC 6",
> > we added a number of compiler flags which were needed to build
> > OpenJDK with GCC 6. Checks for these flags were added to configure,
> > and the flags passed to the JDK and HotSpot builds by CFLAGS/CXXFLAGS_JDK
> > and hotspot-spec.gmk.in respectively.
> >
> > Following "8152666: The new Hotspot Build System" and
> > "8150601: Remove the old Hotspot build system", the hotspot-spec.gmk.in
> > file was removed, so the flags were no longer passed to the HotSpot
> > build. HotSpot now uses JVM_CFLAGS via the same configure process
> > as the JDK obtains CFLAGS_JDK and CXXFLAGS_JDK[*].
> >
> > This webrev fixes the regression introduced by 8152666 by adding
> > the flags to JVM_CFLAGS. It also adapts FLAGS_SETUP_GCC6_COMPILER_FLAGS
> > to take a prefix, as other macros in flags.m4 do, and calls
> > TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS before the version check.
> > The lack of the latter lead to configure executing:
> >
> > if test $OPENJDK_BUILD_COMPARABLE_ACTUAL_VERSION -ge
> > $COMPARABLE_REFERENCE_VERSION
> >
> > with OPENJDK_BUILD_COMPARABLE_ACTUAL_VERSION ever being set, causing
> > test to print an error.
> >
> > With this fix, the flags again make it into the build.
> >
> > [*] It should really be CXXFLAGS_JVM as they are flags for the C++
> > compiler.
> 
> 

-- 
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

Re: [RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Erik Joelsson]

Hello,

In general it looks good. It's nice to see that this also fixes that 
warning output from configure. My only nit is the comment describing the 
parameters for FLAGS_SETUP_GCC6_COMPILER_FLAGS. It indicates that it 
takes named parameters while it actually just takes positional. Please 
either change it to named parameters or fix the comment.


Thanks for fixing this!

/Erik

On 2016-07-05 07:27, Andrew Hughes wrote:

Webrev: http://cr.openjdk.java.net/~andrew/8156980/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8156980

With "8151841: Build needs additional flags to compile with GCC 6",
we added a number of compiler flags which were needed to build
OpenJDK with GCC 6. Checks for these flags were added to configure,
and the flags passed to the JDK and HotSpot builds by CFLAGS/CXXFLAGS_JDK
and hotspot-spec.gmk.in respectively.

Following "8152666: The new Hotspot Build System" and
"8150601: Remove the old Hotspot build system", the hotspot-spec.gmk.in
file was removed, so the flags were no longer passed to the HotSpot
build. HotSpot now uses JVM_CFLAGS via the same configure process
as the JDK obtains CFLAGS_JDK and CXXFLAGS_JDK[*].

This webrev fixes the regression introduced by 8152666 by adding
the flags to JVM_CFLAGS. It also adapts FLAGS_SETUP_GCC6_COMPILER_FLAGS
to take a prefix, as other macros in flags.m4 do, and calls
TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS before the version check.
The lack of the latter lead to configure executing:

if test $OPENJDK_BUILD_COMPARABLE_ACTUAL_VERSION -ge 
$COMPARABLE_REFERENCE_VERSION

with OPENJDK_BUILD_COMPARABLE_ACTUAL_VERSION ever being set, causing
test to print an error.

With this fix, the flags again make it into the build.

[*] It should really be CXXFLAGS_JVM as they are flags for the C++ compiler.

[RFR] JDK-8156980: Hotspot build doesn't have -std=gnu++98 gcc option

[Andrew Hughes]

Webrev: http://cr.openjdk.java.net/~andrew/8156980/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8156980

With "8151841: Build needs additional flags to compile with GCC 6",
we added a number of compiler flags which were needed to build
OpenJDK with GCC 6. Checks for these flags were added to configure,
and the flags passed to the JDK and HotSpot builds by CFLAGS/CXXFLAGS_JDK
and hotspot-spec.gmk.in respectively.

Following "8152666: The new Hotspot Build System" and
"8150601: Remove the old Hotspot build system", the hotspot-spec.gmk.in
file was removed, so the flags were no longer passed to the HotSpot
build. HotSpot now uses JVM_CFLAGS via the same configure process
as the JDK obtains CFLAGS_JDK and CXXFLAGS_JDK[*].

This webrev fixes the regression introduced by 8152666 by adding
the flags to JVM_CFLAGS. It also adapts FLAGS_SETUP_GCC6_COMPILER_FLAGS
to take a prefix, as other macros in flags.m4 do, and calls
TOOLCHAIN_PREPARE_FOR_VERSION_COMPARISONS before the version check.
The lack of the latter lead to configure executing:

if test $OPENJDK_BUILD_COMPARABLE_ACTUAL_VERSION -ge 
$COMPARABLE_REFERENCE_VERSION

with OPENJDK_BUILD_COMPARABLE_ACTUAL_VERSION ever being set, causing
test to print an error.

With this fix, the flags again make it into the build.

[*] It should really be CXXFLAGS_JVM as they are flags for the C++ compiler.
-- 
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