Integrated: 8248584: Enable CHECK_UNHANDLED_OOPS for Windows fastdebug builds

[Harold Seigel]

On Tue, 19 Oct 2021 19:21:24 GMT, Harold Seigel  wrote:

> Please review this small change to enable CHECK_UNHANDLED_OOPs for Windows 
> fastdebug builds.  The change was tested by running Mach5 tiers 1-6 on 
> Windows-x64-debug.
> 
> Thanks, Harold

This pull request has now been integrated.

Changeset: 1271fbf3
Author:Harold Seigel 
URL:   
https://git.openjdk.java.net/jdk/commit/1271fbf3d45ee654faf6e3003c7fb2e5c4a0
Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod

8248584: Enable CHECK_UNHANDLED_OOPS for Windows fastdebug builds

Reviewed-by: dholmes, erikj

-

PR: https://git.openjdk.java.net/jdk/pull/6023

Re: RFR: 8248584: Enable CHECK_UNHANDLED_OOPS for Windows fastdebug builds

[Harold Seigel]

On Tue, 19 Oct 2021 19:21:24 GMT, Harold Seigel  wrote:

> Please review this small change to enable CHECK_UNHANDLED_OOPs for Windows 
> fastdebug builds.  The change was tested by running Mach5 tiers 1-6 on 
> Windows-x64-debug.
> 
> Thanks, Harold

Thanks David and Erik!

-

PR: https://git.openjdk.java.net/jdk/pull/6023

Re: RFR: 8248584: Enable CHECK_UNHANDLED_OOPS for Windows fastdebug builds

[Erik Joelsson]

On Tue, 19 Oct 2021 19:21:24 GMT, Harold Seigel  wrote:

> Please review this small change to enable CHECK_UNHANDLED_OOPs for Windows 
> fastdebug builds.  The change was tested by running Mach5 tiers 1-6 on 
> Windows-x64-debug.
> 
> Thanks, Harold

Looks good to me.

-

Marked as reviewed by erikj (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6023

Re: RFR: 8248584: Enable CHECK_UNHANDLED_OOPS for Windows fastdebug builds

[David Holmes]

On Tue, 19 Oct 2021 19:21:24 GMT, Harold Seigel  wrote:

> Please review this small change to enable CHECK_UNHANDLED_OOPs for Windows 
> fastdebug builds.  The change was tested by running Mach5 tiers 1-6 on 
> Windows-x64-debug.
> 
> Thanks, Harold

Seems fine to me.

Thanks,
David

-

Marked as reviewed by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6023

RFR: 8248584: Enable CHECK_UNHANDLED_OOPS for Windows fastdebug builds

[Harold Seigel]

Please review this small change to enable CHECK_UNHANDLED_OOPs for Windows 
fastdebug builds.  The change was tested by running Mach5 tiers 1-6 on 
Windows-x64-debug.

Thanks, Harold

-

Commit messages:
 - 8248584: Enable CHECK_UNHANDLED_OOPS for Windows fastdebug builds

Changes: https://git.openjdk.java.net/jdk/pull/6023/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6023=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8248584
  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6023.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6023/head:pull/6023

PR: https://git.openjdk.java.net/jdk/pull/6023

RE: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds - was : RE: gcc FORTIFY_SOURCE application security flags

[Baesken, Matthias]

Thanks !

May I have a second review ?

Btw I added a comment  to 

https://bugs.openjdk.java.net/browse/JDK-8130017

describing a bit  the "current situation"   (minimum  gcc 4.8  +   
"-U_FORTIFY_SOURCE" for lower level OPT-flags   (< -O1) )  .

Best regards, Matthias


> 
> Looks good, thanks!
> 
> /Erik
> 
> On 2019-05-14 03:16, Baesken, Matthias wrote:
> > Hi Erik, here is the updated webrev :
> >
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8130017.2/
> >
> >
> > Best regards, Matthias
> >
> >
> >> -Original Message-
> >> From: Erik Joelsson 
> >> Sent: Freitag, 10. Mai 2019 16:29
> >> To: Baesken, Matthias ; David Holmes
> >> ; 'build-dev@openjdk.java.net'  >> d...@openjdk.java.net>
> >> Subject: Re: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds -
> >> was : RE: gcc FORTIFY_SOURCE application security flags
> >>
> >> Hello Matthias,
> >>
> >> I think just -U_FORTIFY_SOURCE should be enough to unset it, no need to
> >> also set it to 0. Also, I think it would be good to use an extra set of
> >> variables to avoid repeating the flag, like this:
> >>
> >> ENABLE_FORTIFY_CFLAGS="-D_FORTIFY_SOURCE=2"
> >> DISABLE_FORTIFY_CFLAGS="-U_FORTIFY_SOURCE"
> >> C_O_FLAG_HIGHEST_JVM="${C_O_FLAG_HIGHEST_JVM}
> >> ${ENABLE_FORTIFY_CFLAGS}"
> >> ...
> >>

Re: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds - was : RE: gcc FORTIFY_SOURCE application security flags

[Erik Joelsson]

Looks good, thanks!

/Erik

On 2019-05-14 03:16, Baesken, Matthias wrote:

Hi Erik, here is the updated webrev :

http://cr.openjdk.java.net/~mbaesken/webrevs/8130017.2/


Best regards, Matthias



-Original Message-
From: Erik Joelsson 
Sent: Freitag, 10. Mai 2019 16:29
To: Baesken, Matthias ; David Holmes
; 'build-dev@openjdk.java.net' 
Subject: Re: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds -
was : RE: gcc FORTIFY_SOURCE application security flags

Hello Matthias,

I think just -U_FORTIFY_SOURCE should be enough to unset it, no need to
also set it to 0. Also, I think it would be good to use an extra set of
variables to avoid repeating the flag, like this:

ENABLE_FORTIFY_CFLAGS="-D_FORTIFY_SOURCE=2"
DISABLE_FORTIFY_CFLAGS="-U_FORTIFY_SOURCE"
C_O_FLAG_HIGHEST_JVM="${C_O_FLAG_HIGHEST_JVM}
${ENABLE_FORTIFY_CFLAGS}"
...

/Erik

On 2019-05-09 22:46, Baesken, Matthias wrote:

Hello, here  is  the  new webrev  with the

"-U_FORTIFY_SOURCE  -D_FORTIFY_SOURCE=0"

Set for the lower level optimization flags :

http://cr.openjdk.java.net/~mbaesken/webrevs/8130017.1/


I would suggest to leave the pre-gcc4.8 cleanup to a separate change.

Best regards, Matthias




Configure will protest if GCC version is less than 4.8 (see toolchain.m4
*_MINIMUM_VERSION variables).

That said, as long as we conditionally set the FDLIBM_CFLAGS like this,
I would say we need to continue honoring the result of that check. You
could also remove the check altogether since it seems to no longer be
needed.

/Erik

On 2019-05-09 07:14, Baesken, Matthias wrote:

Hello,
I tried  setting

"-U_FORTIFY_SOURCE  -D_FORTIFY_SOURCE=0"

And this seems indeed to work , no warning any more .

Let's hope gcc does not change  the command line parsing .

Btw.  is there a gcc version  that   a) still compiles jdk/jdkand  b)   
would

show the issue  ?

(with our internally used gcc's we are always > 4.6   in jdk/jdk )

Best regards, Matthias



-Original Message-
From: Erik Joelsson 
Sent: Donnerstag, 9. Mai 2019 15:18
To: Baesken, Matthias ; David Holmes
; 'build-dev@openjdk.java.net' 
Subject: Re: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug

builds -

was : RE: gcc FORTIFY_SOURCE application security flags

Hello,

I just tried this and you are correct. However, it does seem to work if
you instead use -U_FORTIFY_SOURCE.

/Erik

On 2019-05-09 05:36, Baesken, Matthias wrote:

Hi Erik, while  setting -O  and -O  (with x != y )   in one gcc/g++

command line call  works ,

  setting  together  -D_FORTIFY_SOURCE=2  and   -

D_FORTIFY_SOURCE=0

in one command line call  generates a warning , so I think we cannot do

that .

Best regards, Matthias

RE: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds - was : RE: gcc FORTIFY_SOURCE application security flags

[Baesken, Matthias]

Hi Erik, here is the updated webrev :

http://cr.openjdk.java.net/~mbaesken/webrevs/8130017.2/


Best regards, Matthias


> -Original Message-
> From: Erik Joelsson 
> Sent: Freitag, 10. Mai 2019 16:29
> To: Baesken, Matthias ; David Holmes
> ; 'build-dev@openjdk.java.net'  d...@openjdk.java.net>
> Subject: Re: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds -
> was : RE: gcc FORTIFY_SOURCE application security flags
> 
> Hello Matthias,
> 
> I think just -U_FORTIFY_SOURCE should be enough to unset it, no need to
> also set it to 0. Also, I think it would be good to use an extra set of
> variables to avoid repeating the flag, like this:
> 
> ENABLE_FORTIFY_CFLAGS="-D_FORTIFY_SOURCE=2"
> DISABLE_FORTIFY_CFLAGS="-U_FORTIFY_SOURCE"
> C_O_FLAG_HIGHEST_JVM="${C_O_FLAG_HIGHEST_JVM}
> ${ENABLE_FORTIFY_CFLAGS}"
> ...
> 
> /Erik
> 
> On 2019-05-09 22:46, Baesken, Matthias wrote:
> > Hello, here  is  the  new webrev  with the
> >
> > "-U_FORTIFY_SOURCE  -D_FORTIFY_SOURCE=0"
> >
> > Set for the lower level optimization flags :
> >
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8130017.1/
> >
> >
> > I would suggest to leave the pre-gcc4.8 cleanup to a separate change.
> >
> > Best regards, Matthias
> >
> >
> >
> >> Configure will protest if GCC version is less than 4.8 (see toolchain.m4
> >> *_MINIMUM_VERSION variables).
> >>
> >> That said, as long as we conditionally set the FDLIBM_CFLAGS like this,
> >> I would say we need to continue honoring the result of that check. You
> >> could also remove the check altogether since it seems to no longer be
> >> needed.
> >>
> >> /Erik
> >>
> >> On 2019-05-09 07:14, Baesken, Matthias wrote:
> >>> Hello,
> >>> I tried  setting
> >>>
> >>> "-U_FORTIFY_SOURCE  -D_FORTIFY_SOURCE=0"
> >>>
> >>> And this seems indeed to work , no warning any more .
> >>>
> >>> Let's hope gcc does not change  the command line parsing .
> >>>
> >>> Btw.  is there a gcc version  that   a) still compiles jdk/jdkand  b) 
> >>>   would
> >> show the issue  ?
> >>> (with our internally used gcc's we are always > 4.6   in jdk/jdk )
> >>>
> >>> Best regards, Matthias
> >>>
> >>>
> >>>> -Original Message-
> >>>> From: Erik Joelsson 
> >>>> Sent: Donnerstag, 9. Mai 2019 15:18
> >>>> To: Baesken, Matthias ; David Holmes
> >>>> ; 'build-dev@openjdk.java.net'  >>>> d...@openjdk.java.net>
> >>>> Subject: Re: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug
> builds -
> >>>> was : RE: gcc FORTIFY_SOURCE application security flags
> >>>>
> >>>> Hello,
> >>>>
> >>>> I just tried this and you are correct. However, it does seem to work if
> >>>> you instead use -U_FORTIFY_SOURCE.
> >>>>
> >>>> /Erik
> >>>>
> >>>> On 2019-05-09 05:36, Baesken, Matthias wrote:
> >>>>> Hi Erik, while  setting -O  and -O  (with x != y )   in one 
> >>>>> gcc/g++
> >>>> command line call  works ,
> >>>>>  setting  together  -D_FORTIFY_SOURCE=2  and   -
> >> D_FORTIFY_SOURCE=0
> >>>> in one command line call  generates a warning , so I think we cannot do
> >> that .
> >>>>> Best regards, Matthias
> >>>>>

Re: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds - was : RE: gcc FORTIFY_SOURCE application security flags

[Erik Joelsson]

Hello Matthias,

I think just -U_FORTIFY_SOURCE should be enough to unset it, no need to 
also set it to 0. Also, I think it would be good to use an extra set of 
variables to avoid repeating the flag, like this:


ENABLE_FORTIFY_CFLAGS="-D_FORTIFY_SOURCE=2"
DISABLE_FORTIFY_CFLAGS="-U_FORTIFY_SOURCE"
C_O_FLAG_HIGHEST_JVM="${C_O_FLAG_HIGHEST_JVM} ${ENABLE_FORTIFY_CFLAGS}"
...

/Erik

On 2019-05-09 22:46, Baesken, Matthias wrote:

Hello, here  is  the  new webrev  with the

"-U_FORTIFY_SOURCE  -D_FORTIFY_SOURCE=0"

Set for the lower level optimization flags :

http://cr.openjdk.java.net/~mbaesken/webrevs/8130017.1/


I would suggest to leave the pre-gcc4.8 cleanup to a separate change.

Best regards, Matthias




Configure will protest if GCC version is less than 4.8 (see toolchain.m4
*_MINIMUM_VERSION variables).

That said, as long as we conditionally set the FDLIBM_CFLAGS like this,
I would say we need to continue honoring the result of that check. You
could also remove the check altogether since it seems to no longer be
needed.

/Erik

On 2019-05-09 07:14, Baesken, Matthias wrote:

Hello,
I tried  setting

"-U_FORTIFY_SOURCE  -D_FORTIFY_SOURCE=0"

And this seems indeed to work , no warning any more .

Let's hope gcc does not change  the command line parsing .

Btw.  is there a gcc version  that   a) still compiles jdk/jdkand  b)   
would

show the issue  ?

(with our internally used gcc's we are always > 4.6   in jdk/jdk )

Best regards, Matthias



-Original Message-
From: Erik Joelsson 
Sent: Donnerstag, 9. Mai 2019 15:18
To: Baesken, Matthias ; David Holmes
; 'build-dev@openjdk.java.net' 
Subject: Re: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds -
was : RE: gcc FORTIFY_SOURCE application security flags

Hello,

I just tried this and you are correct. However, it does seem to work if
you instead use -U_FORTIFY_SOURCE.

/Erik

On 2019-05-09 05:36, Baesken, Matthias wrote:

Hi Erik, while  setting -O  and -O  (with x != y )   in one gcc/g++

command line call  works ,

 setting  together  -D_FORTIFY_SOURCE=2  and   -

D_FORTIFY_SOURCE=0

in one command line call  generates a warning , so I think we cannot do

that .

Best regards, Matthias

Re: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds - was : RE: gcc FORTIFY_SOURCE application security flags

[Erik Joelsson]

Configure will protest if GCC version is less than 4.8 (see toolchain.m4 
*_MINIMUM_VERSION variables).


That said, as long as we conditionally set the FDLIBM_CFLAGS like this, 
I would say we need to continue honoring the result of that check. You 
could also remove the check altogether since it seems to no longer be 
needed.


/Erik

On 2019-05-09 07:14, Baesken, Matthias wrote:

Hello,
I tried  setting

"-U_FORTIFY_SOURCE  -D_FORTIFY_SOURCE=0"

And this seems indeed to work , no warning any more .

Let's hope gcc does not change  the command line parsing .

Btw.  is there a gcc version  that   a) still compiles jdk/jdkand  b)   
would show the issue  ?

(with our internally used gcc's we are always > 4.6   in jdk/jdk )

Best regards, Matthias



-Original Message-
From: Erik Joelsson 
Sent: Donnerstag, 9. Mai 2019 15:18
To: Baesken, Matthias ; David Holmes
; 'build-dev@openjdk.java.net' 
Subject: Re: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds -
was : RE: gcc FORTIFY_SOURCE application security flags

Hello,

I just tried this and you are correct. However, it does seem to work if
you instead use -U_FORTIFY_SOURCE.

/Erik

On 2019-05-09 05:36, Baesken, Matthias wrote:

Hi Erik, while  setting -O  and -O  (with x != y )   in one gcc/g++

command line call  works ,

setting  together  -D_FORTIFY_SOURCE=2  and   -D_FORTIFY_SOURCE=0

in one command line call  generates a warning , so I think we cannot do that .


Best regards, Matthias

RE: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds - was : RE: gcc FORTIFY_SOURCE application security flags

[Baesken, Matthias]

Hello,
I tried  setting

"-U_FORTIFY_SOURCE  -D_FORTIFY_SOURCE=0"

And this seems indeed to work , no warning any more .

Let's hope gcc does not change  the command line parsing .

Btw.  is there a gcc version  that   a) still compiles jdk/jdkand  b)   
would show the issue  ?

(with our internally used gcc's we are always > 4.6   in jdk/jdk )

Best regards, Matthias


> -Original Message-
> From: Erik Joelsson 
> Sent: Donnerstag, 9. Mai 2019 15:18
> To: Baesken, Matthias ; David Holmes
> ; 'build-dev@openjdk.java.net'  d...@openjdk.java.net>
> Subject: Re: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds -
> was : RE: gcc FORTIFY_SOURCE application security flags
> 
> Hello,
> 
> I just tried this and you are correct. However, it does seem to work if
> you instead use -U_FORTIFY_SOURCE.
> 
> /Erik
> 
> On 2019-05-09 05:36, Baesken, Matthias wrote:
> > Hi Erik, while  setting -O  and -O  (with x != y )   in one gcc/g++
> command line call  works ,
> >setting  together  -D_FORTIFY_SOURCE=2  and   -D_FORTIFY_SOURCE=0
> in one command line call  generates a warning , so I think we cannot do that .
> >
> >
> > Best regards, Matthias
> >

Re: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds - was : RE: gcc FORTIFY_SOURCE application security flags

[Erik Joelsson]

Hello,

I just tried this and you are correct. However, it does seem to work if 
you instead use -U_FORTIFY_SOURCE.


/Erik

On 2019-05-09 05:36, Baesken, Matthias wrote:

Hi Erik, while  setting -O  and -O  (with x != y )   in one gcc/g++  
command line call  works ,
   setting  together  -D_FORTIFY_SOURCE=2  and   -D_FORTIFY_SOURCE=0   in one 
command line call  generates a warning , so I think we cannot do that .


Best regards, Matthias
  


Hello Matthias,

On 2019-05-08 06:27, Baesken, Matthias wrote:

Hello,I looked a bit  more  into it .
It seems to me ,  that   when  -ffp-contract=off   is available  which is the

case with  current gcc versions ,  we want to optimize the 2 special files (
sharedRuntimeTrig.cpp / sharedRuntimeTrans.cpp ).

   see the following comments :

jdk/make/hotspot/lib/JvmOverrideFiles.gmk

47# If the FDLIBM_CFLAGS variable is non-empty we know
48# that the fdlibm-fork in hotspot can get optimized
49# by using -ffp-contract=off on GCC/Clang platforms.
..
58  BUILD_LIBJVM_sharedRuntimeTrig.cpp_CXXFLAGS := -DNO_PCH

$(FDLIBM_CFLAGS) $(LIBJVM_FDLIBM_COPY_OPT_FLAG)

59  BUILD_LIBJVM_sharedRuntimeTrans.cpp_CXXFLAGS := -DNO_PCH

$(FDLIBM_CFLAGS) $(LIBJVM_FDLIBM_COPY_OPT_FLAG)

60

Will this not just resolve itself if you also add -D_FORTIFY_SOURCE=0 to
C_O_FLAG_NONE (and all the other optimization flag variables that have
the value "-O0")?

But still, setting both -O3 and -O2  in one compile call looks not nice to me .

This may not look nice, but is how we have to do things. The last flag
on a compiler command line takes precedence. We rely on this to override
general flags with more specific ones (typically a general flag for the
whole library with specific ones for certain compilation units). This
technique is quite common.

In case of  ancient gcc  ***without***  -ffp-contract=off   ,  we might still

run into issues for these  2 special files  when _FORTIFY_SOURCE  is set .

Don't know if this is  still relevant .
In case  we want to be on the very safe side , we might  need  to  filter out

-D_FORTIFY_SOURCE=2for these 2 compilation units .

We don't want to filter out flags. It creates very brittle code that is
likely to break in the future.

For the patch, I think it would make sense to introduce a variable for
the value -D_FORTIFY_SOURCE=2 (and
-D_FORTIFY_SOURCE=0/-U_FORTIFY_SOURCE) to avoid repeating it.

/Erik

RE: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds - was : RE: gcc FORTIFY_SOURCE application security flags

[Baesken, Matthias]

Hi Erik, while  setting -O  and -O  (with x != y )   in one gcc/g++  
command line call  works ,
  setting  together  -D_FORTIFY_SOURCE=2  and   -D_FORTIFY_SOURCE=0   in one 
command line call  generates a warning , so I think we cannot do that .


Best regards, Matthias
 

> Hello Matthias,
> 
> On 2019-05-08 06:27, Baesken, Matthias wrote:
> > Hello,I looked a bit  more  into it .
> > It seems to me ,  that   when  -ffp-contract=off   is available  which is 
> > the
> case with  current gcc versions ,  we want to optimize the 2 special files (
> sharedRuntimeTrig.cpp / sharedRuntimeTrans.cpp ).
> >   see the following comments :
> >
> > jdk/make/hotspot/lib/JvmOverrideFiles.gmk
> >
> > 47# If the FDLIBM_CFLAGS variable is non-empty we know
> > 48# that the fdlibm-fork in hotspot can get optimized
> > 49# by using -ffp-contract=off on GCC/Clang platforms.
> >..
> > 58  BUILD_LIBJVM_sharedRuntimeTrig.cpp_CXXFLAGS := -DNO_PCH
> $(FDLIBM_CFLAGS) $(LIBJVM_FDLIBM_COPY_OPT_FLAG)
> > 59  BUILD_LIBJVM_sharedRuntimeTrans.cpp_CXXFLAGS := -DNO_PCH
> $(FDLIBM_CFLAGS) $(LIBJVM_FDLIBM_COPY_OPT_FLAG)
> > 60
> Will this not just resolve itself if you also add -D_FORTIFY_SOURCE=0 to
> C_O_FLAG_NONE (and all the other optimization flag variables that have
> the value "-O0")?
> > But still, setting both -O3 and -O2  in one compile call looks not nice to 
> > me .
> This may not look nice, but is how we have to do things. The last flag
> on a compiler command line takes precedence. We rely on this to override
> general flags with more specific ones (typically a general flag for the
> whole library with specific ones for certain compilation units). This
> technique is quite common.
> >
> > In case of  ancient gcc  ***without***  -ffp-contract=off   ,  we might 
> > still
> run into issues for these  2 special files  when _FORTIFY_SOURCE  is set .
> > Don't know if this is  still relevant .
> > In case  we want to be on the very safe side , we might  need  to  filter 
> > out
> -D_FORTIFY_SOURCE=2for these 2 compilation units .
> 
> We don't want to filter out flags. It creates very brittle code that is
> likely to break in the future.
> 
> For the patch, I think it would make sense to introduce a variable for
> the value -D_FORTIFY_SOURCE=2 (and
> -D_FORTIFY_SOURCE=0/-U_FORTIFY_SOURCE) to avoid repeating it.
> 
> /Erik

Re: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds - was : RE: gcc FORTIFY_SOURCE application security flags

[sgehwolf]

Hi Matthias,

For some background about the -O2 -ffp-contract=off flags for
sharedRuntimeTrig.cpp and sharedRuntimeTrans.cpp see:
https://bugs.openjdk.java.net/browse/JDK-8210425

My understanding is that if there is a per-file override of the
optimization level, the compilation command will end up containing both
-O3 and a later, possible override, via the per-file settings. The -O3
flag originates from it being a release/fastdebug build.

Thanks,
Severin

On Wed, 2019-05-08 at 13:27 +, Baesken, Matthias wrote:
> Hello,I looked a bit  more  into it .
> It seems to me ,  that   when  -ffp-contract=off   is
> available  which is the case with  current gcc versions ,  we want to
> optimize the 2 special files ( sharedRuntimeTrig.cpp /
> sharedRuntimeTrans.cpp ).
>  see the following comments :
> 
> jdk/make/hotspot/lib/JvmOverrideFiles.gmk
> 
> 47# If the FDLIBM_CFLAGS variable is non-empty we know
> 48# that the fdlibm-fork in hotspot can get optimized
> 49# by using -ffp-contract=off on GCC/Clang platforms.
>   ..
> 58  BUILD_LIBJVM_sharedRuntimeTrig.cpp_CXXFLAGS := -DNO_PCH
> $(FDLIBM_CFLAGS) $(LIBJVM_FDLIBM_COPY_OPT_FLAG)
> 59  BUILD_LIBJVM_sharedRuntimeTrans.cpp_CXXFLAGS := -DNO_PCH
> $(FDLIBM_CFLAGS) $(LIBJVM_FDLIBM_COPY_OPT_FLAG)
> 60
> 
> But still, setting both -O3 and -O2  in one compile call looks not
> nice to me .
> 
> In case of  ancient gcc  ***without***  -ffp-contract=off   ,  we
> might still run into issues for these  2 special files  when
> _FORTIFY_SOURCE  is set .
> Don't know if this is  still relevant .
> In case  we want to be on the very safe side , we
> might  need  to  filter out   -D_FORTIFY_SOURCE=2for these 2
> compilation units .
> 
> 
> Best regards, Matthias
> 
> 
> 
> > Hi David,   thanks for the comment .
> > 
> > Currently  I do not see the issue in our fastdebug  builds .
> > So I think the  opt-flag filtering got changed/removed in the
> > years  after the
> > issues were reported .
> > 
> > 
> > https://bugs.openjdk.java.net/browse/JDK-8047952
> > 
> > mentions special O-level settings  for  sharedRuntimeTrig.cpp and
> > sharedRuntimeTrans.cpp .
> > 
> > But the files  have optimization set in both  fastdebug
> > and  opt  builds  :
> > 
> >  Linux  x86_64  gcc-7 based builds :
> > 
> >fastdebug build  (with the added  -D_FORTIFY_SOURCE=2  flag) :
> > 
> > -Werror -O3 -D_FORTIFY_SOURCE=2 -DNO_PCH -ffp-contract=off -O2 -
> > D_FORTIFY_SOURCE=2
> > 
> > 
> > Opt build  (without  -D_FORTIFY_SOURCE=... ) :
> > 
> > -O3 -DNO_PCH -ffp-contract=off -O2  
> > 
> > 
> > (btw.  the setting  of  both-O3 AND -O2   looks strange to me ,
> > but that’s
> > unrelated to my change ;  I  noticed that already in OpenJDK 11  ).
> > 
> > 
> > Best regards, Matthias
> > 
> > 
> > 
> > 
> > > Hi Matthias,
> > > 
> > > On 8/05/2019 6:05 pm, Baesken, Matthias wrote:
> > > > Hello, here is  a   webrev,  I used the existing bug
> > > > "JDK-8130017 : use _FORTIFY_SOURCE in gcc fastdebug builds"
> > > > 
> > > > Hope that’s fine .
> > > 
> > > That is fine, but please add a comment to the bug explaining
> > > exactly how
> > > you fixed the issue and how the issues raised in the bug
> > > description
> > > regarding optimisation levels have been addressed.
> > > 
> > > Not a review - I'll leave that to build team. The proof of this
> > > will be
> > > in the building and testing.
> > > 
> > > Thanks,
> > > David
> > > 
> > > > https://bugs.openjdk.java.net/browse/JDK-8130017
> > > > 
> > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8130017.0/
> > > > 
> > > > 
> > > > Our internal OpenJDK Linux   (x86_64, ppc64, ppc64le ,
> > > > s390x)   fastdebug
> > > builds  are fine with the added flag .
> > > > 

RE: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds - was : RE: gcc FORTIFY_SOURCE application security flags

[Baesken, Matthias]

Hello,I looked a bit  more  into it .
It seems to me ,  that   when  -ffp-contract=off   is available  which is the 
case with  current gcc versions ,  we want to optimize the 2 special files ( 
sharedRuntimeTrig.cpp / sharedRuntimeTrans.cpp ).
 see the following comments :

jdk/make/hotspot/lib/JvmOverrideFiles.gmk

47# If the FDLIBM_CFLAGS variable is non-empty we know
48# that the fdlibm-fork in hotspot can get optimized
49# by using -ffp-contract=off on GCC/Clang platforms.
  ..
58  BUILD_LIBJVM_sharedRuntimeTrig.cpp_CXXFLAGS := -DNO_PCH $(FDLIBM_CFLAGS) 
$(LIBJVM_FDLIBM_COPY_OPT_FLAG)
59  BUILD_LIBJVM_sharedRuntimeTrans.cpp_CXXFLAGS := -DNO_PCH $(FDLIBM_CFLAGS) 
$(LIBJVM_FDLIBM_COPY_OPT_FLAG)
60

But still, setting both -O3 and -O2  in one compile call looks not nice to me .

In case of  ancient gcc  ***without***  -ffp-contract=off   ,  we might still 
run into issues for these  2 special files  when _FORTIFY_SOURCE  is set .
Don't know if this is  still relevant .
In case  we want to be on the very safe side , we might  need  to  filter out   
-D_FORTIFY_SOURCE=2for these 2 compilation units .


Best regards, Matthias



> 
> Hi David,   thanks for the comment .
> 
> Currently  I do not see the issue in our fastdebug  builds .
> So I think the  opt-flag filtering got changed/removed in the years  after the
> issues were reported .
> 
> 
> https://bugs.openjdk.java.net/browse/JDK-8047952
> 
> mentions special O-level settings  for  sharedRuntimeTrig.cpp and
> sharedRuntimeTrans.cpp .
> 
> But the files  have optimization set in both  fastdebug and  opt  builds  :
> 
>  Linux  x86_64  gcc-7 based builds :
> 
>fastdebug build  (with the added  -D_FORTIFY_SOURCE=2  flag) :
> 
> -Werror -O3 -D_FORTIFY_SOURCE=2 -DNO_PCH -ffp-contract=off -O2 -
> D_FORTIFY_SOURCE=2
> 
> 
> Opt build  (without  -D_FORTIFY_SOURCE=... ) :
> 
> -O3 -DNO_PCH -ffp-contract=off -O2  
> 
> 
> (btw.  the setting  of  both-O3 AND -O2   looks strange to me , but that’s
> unrelated to my change ;  I  noticed that already in OpenJDK 11  ).
> 
> 
> Best regards, Matthias
> 
> 
> 
> 
> >
> > Hi Matthias,
> >
> > On 8/05/2019 6:05 pm, Baesken, Matthias wrote:
> > > Hello, here is  a   webrev,  I used the existing bug
> > > "JDK-8130017 : use _FORTIFY_SOURCE in gcc fastdebug builds"
> > >
> > > Hope that’s fine .
> >
> > That is fine, but please add a comment to the bug explaining exactly how
> > you fixed the issue and how the issues raised in the bug description
> > regarding optimisation levels have been addressed.
> >
> > Not a review - I'll leave that to build team. The proof of this will be
> > in the building and testing.
> >
> > Thanks,
> > David
> >
> > > https://bugs.openjdk.java.net/browse/JDK-8130017
> > >
> > > http://cr.openjdk.java.net/~mbaesken/webrevs/8130017.0/
> > >
> > >
> > > Our internal OpenJDK Linux   (x86_64, ppc64, ppc64le , s390x)   fastdebug
> > builds  are fine with the added flag .
> > >
> > >

RE: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds - was : RE: gcc FORTIFY_SOURCE application security flags

[Baesken, Matthias]

Hi David,   thanks for the comment .

Currently  I do not see the issue in our fastdebug  builds .
So I think the  opt-flag filtering got changed/removed in the years  after the 
issues were reported .


https://bugs.openjdk.java.net/browse/JDK-8047952

mentions special O-level settings  for  sharedRuntimeTrig.cpp and 
sharedRuntimeTrans.cpp .

But the files  have optimization set in both  fastdebug and  opt  builds  :

 Linux  x86_64  gcc-7 based builds :

   fastdebug build  (with the added  -D_FORTIFY_SOURCE=2  flag) :

-Werror -O3 -D_FORTIFY_SOURCE=2 -DNO_PCH -ffp-contract=off -O2 
-D_FORTIFY_SOURCE=2


Opt build  (without  -D_FORTIFY_SOURCE=... ) :

-O3 -DNO_PCH -ffp-contract=off -O2  


(btw.  the setting  of  both-O3 AND -O2   looks strange to me , but that’s 
unrelated to my change ;  I  noticed that already in OpenJDK 11  ).


Best regards, Matthias




> 
> Hi Matthias,
> 
> On 8/05/2019 6:05 pm, Baesken, Matthias wrote:
> > Hello, here is  a   webrev,  I used the existing bug
> > "JDK-8130017 : use _FORTIFY_SOURCE in gcc fastdebug builds"
> >
> > Hope that’s fine .
> 
> That is fine, but please add a comment to the bug explaining exactly how
> you fixed the issue and how the issues raised in the bug description
> regarding optimisation levels have been addressed.
> 
> Not a review - I'll leave that to build team. The proof of this will be
> in the building and testing.
> 
> Thanks,
> David
> 
> > https://bugs.openjdk.java.net/browse/JDK-8130017
> >
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8130017.0/
> >
> >
> > Our internal OpenJDK Linux   (x86_64, ppc64, ppc64le , s390x)   fastdebug
> builds  are fine with the added flag .
> >
> >
> > Best Regards, Matthias
> >
> >
> >
> >> -Original Message-
> >> From: Baesken, Matthias
> >> Sent: Dienstag, 7. Mai 2019 16:55
> >> To: 'Erik Joelsson' ; 'build-
> >> d...@openjdk.java.net' 
> >> Cc: 'Kim Barrett' ; Zeller, Arno
> >> 
> >> Subject: RE: gcc FORTIFY_SOURCE application security flags
> >>
> >> Hello, I looked  at  JDK-8050803 .
> >> There are build issues reported  when using the  _FORTIFY_SOURCE  flag .
> >> However I only noticed one build issue,  this  is related to an additional
> >> warning  ("no result checking of fwrite call") ,  most likely  generated by
> the
> >> added compile time checks of   -D_FORTIFY_SOURCE=2 .
> >> Obviously ,  the  _FORTIFY_SOURCE  flagmust be used together   with
> >> optimization flags , otherwise  the feature does not work .
> >> So I propose to add it  to the optimization flags, but only in case  we 
> >> have a
> >> fastdebug build. See the patch below .
> >>
> >> Best regards, Matthias
> >>
> >>
> >> Patch :
> >> ---
> >>
> >> diff -r 26748009f2e5 make/autoconf/flags-cflags.m4
> >> --- a/make/autoconf/flags-cflags.m4 Thu May 02 20:47:23 2019 +0200
> >> +++ b/make/autoconf/flags-cflags.m4 Tue May 07 16:07:32 2019 +0200
> >> @@ -300,6 +300,13 @@
> >>   C_O_FLAG_DEBUG="-O0"
> >>   C_O_FLAG_DEBUG_JVM="-O0"
> >>   C_O_FLAG_NONE="-O0"
> >> +# -D_FORTIFY_SOURCE=2 hardening option needs optimization
> enabled
> >> +if test "x$OPENJDK_TARGET_OS" = xlinux -a "x$DEBUG_LEVEL" =
> >> "xfastdebug"; then
> >> +  C_O_FLAG_HIGHEST_JVM="${C_O_FLAG_HIGHEST_JVM} -
> >> D_FORTIFY_SOURCE=2"
> >> +  C_O_FLAG_HIGHEST="${C_O_FLAG_HIGHEST} -
> D_FORTIFY_SOURCE=2"
> >> +  C_O_FLAG_HI="${C_O_FLAG_HI} -D_FORTIFY_SOURCE=2"
> >> +  C_O_FLAG_NORM="${C_O_FLAG_NORM} -D_FORTIFY_SOURCE=2"
> >> +fi
> >> elif test "x$TOOLCHAIN_TYPE" = xclang; then
> >>   if test "x$OPENJDK_TARGET_OS" = xmacosx; then
> >> # On MacOSX we optimize for size, something
> >> diff -r 26748009f2e5 test/fmw/gtest/src/gtest.cc
> >> --- a/test/fmw/gtest/src/gtest.cc   Thu May 02 20:47:23 2019 +0200
> >> +++ b/test/fmw/gtest/src/gtest.cc   Tue May 07 16:07:32 2019 +0200
> >> @@ -34,6 +34,7 @@
> >>   #include "gtest/gtest.h"
> >>   #include "gtest/gtest-spi.h"
> >>
> >> +#include 
> >>   #include 
> >>   #include 
> >>   #include 
> >> @@ -3538,7 +3539,8 @@
> >> // errors are ignored as there's nothing better we can do and we
> >>

Re: RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds - was : RE: gcc FORTIFY_SOURCE application security flags

[David Holmes]

Hi Matthias,

On 8/05/2019 6:05 pm, Baesken, Matthias wrote:

Hello, here is  a   webrev,  I used the existing bug
"JDK-8130017 : use _FORTIFY_SOURCE in gcc fastdebug builds"

Hope that’s fine .


That is fine, but please add a comment to the bug explaining exactly how 
you fixed the issue and how the issues raised in the bug description 
regarding optimisation levels have been addressed.


Not a review - I'll leave that to build team. The proof of this will be 
in the building and testing.


Thanks,
David


https://bugs.openjdk.java.net/browse/JDK-8130017

http://cr.openjdk.java.net/~mbaesken/webrevs/8130017.0/


Our internal OpenJDK Linux   (x86_64, ppc64, ppc64le , s390x)   fastdebug 
builds  are fine with the added flag .


Best Regards, Matthias




-Original Message-
From: Baesken, Matthias
Sent: Dienstag, 7. Mai 2019 16:55
To: 'Erik Joelsson' ; 'build-
d...@openjdk.java.net' 
Cc: 'Kim Barrett' ; Zeller, Arno

Subject: RE: gcc FORTIFY_SOURCE application security flags

Hello, I looked  at  JDK-8050803 .
There are build issues reported  when using the  _FORTIFY_SOURCE  flag .
However I only noticed one build issue,  this  is related to an additional
warning  ("no result checking of fwrite call") ,  most likely  generated by the
added compile time checks of   -D_FORTIFY_SOURCE=2 .
Obviously ,  the  _FORTIFY_SOURCE  flagmust be used together   with
optimization flags , otherwise  the feature does not work .
So I propose to add it  to the optimization flags, but only in case  we have a
fastdebug build. See the patch below .

Best regards, Matthias


Patch :
---

diff -r 26748009f2e5 make/autoconf/flags-cflags.m4
--- a/make/autoconf/flags-cflags.m4 Thu May 02 20:47:23 2019 +0200
+++ b/make/autoconf/flags-cflags.m4 Tue May 07 16:07:32 2019 +0200
@@ -300,6 +300,13 @@
  C_O_FLAG_DEBUG="-O0"
  C_O_FLAG_DEBUG_JVM="-O0"
  C_O_FLAG_NONE="-O0"
+# -D_FORTIFY_SOURCE=2 hardening option needs optimization enabled
+if test "x$OPENJDK_TARGET_OS" = xlinux -a "x$DEBUG_LEVEL" =
"xfastdebug"; then
+  C_O_FLAG_HIGHEST_JVM="${C_O_FLAG_HIGHEST_JVM} -
D_FORTIFY_SOURCE=2"
+  C_O_FLAG_HIGHEST="${C_O_FLAG_HIGHEST} -D_FORTIFY_SOURCE=2"
+  C_O_FLAG_HI="${C_O_FLAG_HI} -D_FORTIFY_SOURCE=2"
+  C_O_FLAG_NORM="${C_O_FLAG_NORM} -D_FORTIFY_SOURCE=2"
+fi
elif test "x$TOOLCHAIN_TYPE" = xclang; then
  if test "x$OPENJDK_TARGET_OS" = xmacosx; then
# On MacOSX we optimize for size, something
diff -r 26748009f2e5 test/fmw/gtest/src/gtest.cc
--- a/test/fmw/gtest/src/gtest.cc   Thu May 02 20:47:23 2019 +0200
+++ b/test/fmw/gtest/src/gtest.cc   Tue May 07 16:07:32 2019 +0200
@@ -34,6 +34,7 @@
  #include "gtest/gtest.h"
  #include "gtest/gtest-spi.h"

+#include 
  #include 
  #include 
  #include 
@@ -3538,7 +3539,8 @@
// errors are ignored as there's nothing better we can do and we
// don't want to fail the test because of this.
FILE* pfile = posix::FOpen(premature_exit_filepath, "w");
-  fwrite("0", 1, 1, pfile);
+  size_t cnt= fwrite("0", 1, 1, pfile);
+  assert(cnt == (size_t)1);
fclose(pfile);
  }
}



-Original Message-
From: Erik Joelsson 
Sent: Freitag, 3. Mai 2019 17:40
To: Baesken, Matthias ; 'build-
d...@openjdk.java.net' 
Subject: Re: gcc FORTIFY_SOURCE application security flags

Hello Matthias,

We have tried to use it before but later removed it. See
https://bugs.openjdk.java.net/browse/JDK-8050803

/Erik

On 2019-05-03 08:12, Baesken, Matthias wrote:



Hello.
  maybe some of you are aware of the gcc  FORTIFY_SOURCE application

security flags.

Developers can enable compile and also runtime checks for some string /

memory related operations with the flag.


See details :
https://access.redhat.com/blogs/766093/posts/1976213

Have you tried already those flags in the OpenJDK ?

One issue I experienced when using the flag  (-D_FORTIFY_SOURCE=2) is

that in case that a runtime issue is detected,

no hs_err file is written , only a "*** buffer overflow detected ***"  +

backtrace + Memory map  output is written, looks like this :



*** buffer overflow detected ***: /bin/java terminated
=== Backtrace: =
/lib64/libc.so.6(__fortify_fail+0x37)[0x7f5b500b7177]
/lib64/libc.so.6(+0xe8e10)[0x7f5b500b4e10]
/lib64/libc.so.6(+0xe8109)[0x7f5b500b4109]
/lib64/libc.so.6(_IO_default_xsputn+0x85)[0x7f5b5003f705]
/lib64/libc.so.6(_IO_vfprintf+0x18e)[0x7f5b5000f0ce]
/lib64/libc.so.6(__vsprintf_chk+0x9d)[0x7f5b500b41ad]
/lib64/libc.so.6(__sprintf_chk+0x80)[0x7f5b500b40f0]


=== Memory map: 
c000-c070 rw-p  00:00 0
.



I would prefer to get a hs_err file, do you know a way to get this in

context

of the gcc flag _FORTIFY_SOURCE ?


In case this is

RFR: 8130017: use _FORTIFY_SOURCE in gcc fastdebug builds - was : RE: gcc FORTIFY_SOURCE application security flags

[Baesken, Matthias]

Hello, here is  a   webrev,  I used the existing bug  
"JDK-8130017 : use _FORTIFY_SOURCE in gcc fastdebug builds"

Hope that’s fine .

https://bugs.openjdk.java.net/browse/JDK-8130017

http://cr.openjdk.java.net/~mbaesken/webrevs/8130017.0/


Our internal OpenJDK Linux   (x86_64, ppc64, ppc64le , s390x)   fastdebug 
builds  are fine with the added flag .


Best Regards, Matthias



> -Original Message-
> From: Baesken, Matthias
> Sent: Dienstag, 7. Mai 2019 16:55
> To: 'Erik Joelsson' ; 'build-
> d...@openjdk.java.net' 
> Cc: 'Kim Barrett' ; Zeller, Arno
> 
> Subject: RE: gcc FORTIFY_SOURCE application security flags
> 
> Hello, I looked  at  JDK-8050803 .
> There are build issues reported  when using the  _FORTIFY_SOURCE  flag .
> However I only noticed one build issue,  this  is related to an additional
> warning  ("no result checking of fwrite call") ,  most likely  generated by 
> the
> added compile time checks of   -D_FORTIFY_SOURCE=2 .
> Obviously ,  the  _FORTIFY_SOURCE  flagmust be used together   with
> optimization flags , otherwise  the feature does not work .
> So I propose to add it  to the optimization flags, but only in case  we have a
> fastdebug build. See the patch below .
> 
> Best regards, Matthias
> 
> 
> Patch :
> ---
> 
> diff -r 26748009f2e5 make/autoconf/flags-cflags.m4
> --- a/make/autoconf/flags-cflags.m4 Thu May 02 20:47:23 2019 +0200
> +++ b/make/autoconf/flags-cflags.m4 Tue May 07 16:07:32 2019 +0200
> @@ -300,6 +300,13 @@
>  C_O_FLAG_DEBUG="-O0"
>  C_O_FLAG_DEBUG_JVM="-O0"
>  C_O_FLAG_NONE="-O0"
> +# -D_FORTIFY_SOURCE=2 hardening option needs optimization enabled
> +if test "x$OPENJDK_TARGET_OS" = xlinux -a "x$DEBUG_LEVEL" =
> "xfastdebug"; then
> +  C_O_FLAG_HIGHEST_JVM="${C_O_FLAG_HIGHEST_JVM} -
> D_FORTIFY_SOURCE=2"
> +  C_O_FLAG_HIGHEST="${C_O_FLAG_HIGHEST} -D_FORTIFY_SOURCE=2"
> +  C_O_FLAG_HI="${C_O_FLAG_HI} -D_FORTIFY_SOURCE=2"
> +  C_O_FLAG_NORM="${C_O_FLAG_NORM} -D_FORTIFY_SOURCE=2"
> +fi
>elif test "x$TOOLCHAIN_TYPE" = xclang; then
>  if test "x$OPENJDK_TARGET_OS" = xmacosx; then
># On MacOSX we optimize for size, something
> diff -r 26748009f2e5 test/fmw/gtest/src/gtest.cc
> --- a/test/fmw/gtest/src/gtest.cc   Thu May 02 20:47:23 2019 +0200
> +++ b/test/fmw/gtest/src/gtest.cc   Tue May 07 16:07:32 2019 +0200
> @@ -34,6 +34,7 @@
>  #include "gtest/gtest.h"
>  #include "gtest/gtest-spi.h"
> 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -3538,7 +3539,8 @@
>// errors are ignored as there's nothing better we can do and we
>// don't want to fail the test because of this.
>FILE* pfile = posix::FOpen(premature_exit_filepath, "w");
> -  fwrite("0", 1, 1, pfile);
> +  size_t cnt= fwrite("0", 1, 1, pfile);
> +  assert(cnt == (size_t)1);
>fclose(pfile);
>  }
>}
> 
> 
> > -Original Message-
> > From: Erik Joelsson 
> > Sent: Freitag, 3. Mai 2019 17:40
> > To: Baesken, Matthias ; 'build-
> > d...@openjdk.java.net' 
> > Subject: Re: gcc FORTIFY_SOURCE application security flags
> >
> > Hello Matthias,
> >
> > We have tried to use it before but later removed it. See
> > https://bugs.openjdk.java.net/browse/JDK-8050803
> >
> > /Erik
> >
> > On 2019-05-03 08:12, Baesken, Matthias wrote:
> > >
> > >
> > > Hello.
> > >  maybe some of you are aware of the gcc  FORTIFY_SOURCE application
> > security flags.
> > > Developers can enable compile and also runtime checks for some string /
> > memory related operations with the flag.
> > >
> > > See details :
> > > https://access.redhat.com/blogs/766093/posts/1976213
> > >
> > > Have you tried already those flags in the OpenJDK ?
> > >
> > > One issue I experienced when using the flag  (-D_FORTIFY_SOURCE=2) is
> > that in case that a runtime issue is detected,
> > > no hs_err file is written , only a "*** buffer overflow detected ***"  +
> > backtrace + Memory map  output is written, looks like this :
> > >
> > >
> > > *** buffer overflow detected ***: /bin/java terminated
> > > === Backtrace: =
> > > /lib64/libc.so.6(__fortify_fail+0x37)[0x7f5b500b7177]
> > > /lib64/libc.so.6(+0xe8e10)[0x7f5b500b4e10]
> > > /lib64/libc.so.6(+0xe8109)[0x7f5b500b4109]
> > > /lib64/li

Re: RFR: 8186780: clang-4.0 fastdebug assertion failure in os_linux_x86:os::verify_stack_alignment()

[Martin Buchholz]

Since the question of how to get the stack pointer in hotspot is
unexpectedly difficult, I propose we split into two separate changes and we
can submit the make/ changes that Erik is happy with.

Re: RFR: 8186780: clang-4.0 fastdebug assertion failure in os_linux_x86:os::verify_stack_alignment()

[Martin Buchholz]

I see stack pointers (and frame pointers) declared as intptr_t* which makes
no sense to me.  These are all "just" pointers, so declare them as either
void* or intptr_t or address

In the construct below, we could elide the cast if we had declared esp
right in the first place?!

  intptr_t* esp;
  __asm__ __volatile__ ("mov %%"SPELL_REG_SP", %0":"=r"(esp):);
  return (address) esp;

Re: RFR: 8186780: clang-4.0 fastdebug assertion failure in os_linux_x86:os::verify_stack_alignment()

[Martin Buchholz]

There sure seems to be a lot of confusion about how many stack frames we're
getting at the hot end of the stack, e.g.

  register intptr_t **fp __asm__ (SPELL_REG_FP);

  // fp is for this frame (_get_previous_fp). We want the fp for the
  // caller of os::current_frame*(), so go up two frames. However, for
  // optimized builds, _get_previous_fp() will be inlined, so only go
  // up 1 frame in that case.
  #ifdef _NMT_NOINLINE_
return **(intptr_t***)fp;
  #else
return *fp;
  #endif

Re: RFR: 8186780: clang-4.0 fastdebug assertion failure in os_linux_x86:os::verify_stack_alignment()

[Martin Buchholz]

(I keep trying not to become a hotspot engineer...)

I would define current_stack_pointer as a macro using expression
statements: prototype is:

#if defined(__clang__)
#define sp_3() ({ intptr_t rsp; __asm__ __volatile__ ("mov %% rsp,
%0":"=r"(rsp):); rsp; })
#else
#define sp_3() ({ register intptr_t rsp asm ("rsp"); rsp; })
#endif

Then we can call that everywhere and actually get the right answer.
(Other compilers and cpu architectures left as an exercise for the reader)
(Probably we won't be able to do this for every compiler we'd like to use)

Then we can make all the
os::verify_stack_alignment functions in non-product mode NOINLINE so that
they have a real stack frame with a real stack pointer.

But that's starting to be a big change and a hotspot culture change, so can
hotspot engineers please take over ?!

On Fri, Jun 22, 2018 at 8:27 AM, Thomas Stüfe 
wrote:

> On Fri, Jun 22, 2018 at 1:57 PM, David Holmes 
> wrote:
> > On 21/06/2018 10:37 PM, Thomas Stüfe wrote:
> >>
> >> On Thu, Jun 21, 2018 at 1:27 PM, David Holmes 
> >> wrote:
> >>>
> >>> On 21/06/2018 10:05 AM, Martin Buchholz wrote:
> 
> 
>  On Wed, Jun 20, 2018 at 4:03 PM, Martin Buchholz   > wrote:
> 
>   Hi David and build-dev folk,
> 
>   After way too much build/hotspot hacking, I have a better fix:
> 
>   clang inlined os::current_stack_pointer into its caller __in the
>   same translation unit___ (that could be fixed in a separate
> change)
>   so of course in this case it didn't have to follow the ABI.  Fix
> is
>   obvious in hindsight:
> 
>   -address os::current_stack_pointer() {
>   +NOINLINE address os::current_stack_pointer() {
> 
> 
>  If y'all like the addition of NOINLINE, it should probably be added to
>  all
>  of the 14 variants of os::current_stack_pointer.
>  Gives me a chance to try out the submit repo.
> >>>
> >>>
> >>>
> >>> I can't help but think other platforms actually rely on it being
> inlined
> >>> so
> >>> that it really does return the stack pointer of the method calling
> >>> os::current_stack_pointer!
> >>>
> >>
> >> But we only inline today if caller is in the same translation unit and
> >> builds with optimization, no?
> >
> >
> > Don't know, but Martin's encountering a case where it is being inlined -
> is
> > that likely to be unique for some reason?
> >
>
> Okay I may have confused myself.
>
> My original reasoning was: A lot of calls to
> os::current_stack_pointer() today happen not-inlined. That includes
> calls from outside os__.cpp, and calls from inside
> os__.cpp if slowdebug. Hence, since the VM - in particular
> the slowdebug one - seems to work fine, it should be okay to mark
> os::current_stack_pointer() unconditionally as NOINLINE.
>
> However, then I saw that the only "real" function (real meaning not
> just asserting something) using os::current_stack_pointer() and living
> in the same translation unit is os::current_frame(), e.g. on linux:
>
> frame os::current_frame() {
>   intptr_t* fp = _get_previous_fp();
>   frame myframe((intptr_t*)os::current_stack_pointer(),
> (intptr_t*)fp,
> CAST_FROM_FN_PTR(address, os::current_frame));
>   if (os::is_first_C_frame()) {
> // stack is not walkable
> return frame();
>   } else {
> return os::get_sender_for_C_frame();
>   }
> }
>
> how does this even work if os::current_stack_pointer() is not inlined?
> In slowdebug? Would the frame object in this case not contain the SP
> from the frame built up for os::current_stack_pointer()?
>
> So, now I wonder if making os::current_stack_pointer() NOINLINE would
> break os::current_frame() - make if produce frames with a slightly-off
> SP. os::current_frame() is used e.g. in NMT. So, I wonder if NMT still
> works if os::current_stack_pointer is made NOINLINE, or in slowdebug.
>
> > I never assume the compiler does the obvious these days :) or that there
> > can't be clever link-time tricks as well.
>
> Oh. I did not think of that at all, you are right.
>
> >
> > Maybe the safest thing to do is to only make a change for the clang case
> and
> > leave everything else alone.
> >
>
> It would be better to know for sure, though.
>
> ..Thomas
>
> > David
> > -
> >
> >>
> >> ..Thomas
> >>
> >>> David
>

Re: RFR: 8186780: clang-4.0 fastdebug assertion failure in os_linux_x86:os::verify_stack_alignment()

[Thomas Stüfe]

On Fri, Jun 22, 2018 at 1:57 PM, David Holmes  wrote:
> On 21/06/2018 10:37 PM, Thomas Stüfe wrote:
>>
>> On Thu, Jun 21, 2018 at 1:27 PM, David Holmes 
>> wrote:
>>>
>>> On 21/06/2018 10:05 AM, Martin Buchholz wrote:


 On Wed, Jun 20, 2018 at 4:03 PM, Martin Buchholz >>> > wrote:

  Hi David and build-dev folk,

  After way too much build/hotspot hacking, I have a better fix:

  clang inlined os::current_stack_pointer into its caller __in the
  same translation unit___ (that could be fixed in a separate change)
  so of course in this case it didn't have to follow the ABI.  Fix is
  obvious in hindsight:

  -address os::current_stack_pointer() {
  +NOINLINE address os::current_stack_pointer() {


 If y'all like the addition of NOINLINE, it should probably be added to
 all
 of the 14 variants of os::current_stack_pointer.
 Gives me a chance to try out the submit repo.
>>>
>>>
>>>
>>> I can't help but think other platforms actually rely on it being inlined
>>> so
>>> that it really does return the stack pointer of the method calling
>>> os::current_stack_pointer!
>>>
>>
>> But we only inline today if caller is in the same translation unit and
>> builds with optimization, no?
>
>
> Don't know, but Martin's encountering a case where it is being inlined - is
> that likely to be unique for some reason?
>

Okay I may have confused myself.

My original reasoning was: A lot of calls to
os::current_stack_pointer() today happen not-inlined. That includes
calls from outside os__.cpp, and calls from inside
os__.cpp if slowdebug. Hence, since the VM - in particular
the slowdebug one - seems to work fine, it should be okay to mark
os::current_stack_pointer() unconditionally as NOINLINE.

However, then I saw that the only "real" function (real meaning not
just asserting something) using os::current_stack_pointer() and living
in the same translation unit is os::current_frame(), e.g. on linux:

frame os::current_frame() {
  intptr_t* fp = _get_previous_fp();
  frame myframe((intptr_t*)os::current_stack_pointer(),
(intptr_t*)fp,
CAST_FROM_FN_PTR(address, os::current_frame));
  if (os::is_first_C_frame()) {
// stack is not walkable
return frame();
  } else {
return os::get_sender_for_C_frame();
  }
}

how does this even work if os::current_stack_pointer() is not inlined?
In slowdebug? Would the frame object in this case not contain the SP
from the frame built up for os::current_stack_pointer()?

So, now I wonder if making os::current_stack_pointer() NOINLINE would
break os::current_frame() - make if produce frames with a slightly-off
SP. os::current_frame() is used e.g. in NMT. So, I wonder if NMT still
works if os::current_stack_pointer is made NOINLINE, or in slowdebug.

> I never assume the compiler does the obvious these days :) or that there
> can't be clever link-time tricks as well.

Oh. I did not think of that at all, you are right.

>
> Maybe the safest thing to do is to only make a change for the clang case and
> leave everything else alone.
>

It would be better to know for sure, though.

..Thomas

> David
> -
>
>>
>> ..Thomas
>>
>>> David

Re: RFR: 8186780: clang-4.0 fastdebug assertion failure in os_linux_x86:os::verify_stack_alignment()

[David Holmes]

On 21/06/2018 10:37 PM, Thomas Stüfe wrote:

On Thu, Jun 21, 2018 at 1:27 PM, David Holmes  wrote:

On 21/06/2018 10:05 AM, Martin Buchholz wrote:


On Wed, Jun 20, 2018 at 4:03 PM, Martin Buchholz mailto:marti...@google.com>> wrote:

 Hi David and build-dev folk,

 After way too much build/hotspot hacking, I have a better fix:

 clang inlined os::current_stack_pointer into its caller __in the
 same translation unit___ (that could be fixed in a separate change)
 so of course in this case it didn't have to follow the ABI.  Fix is
 obvious in hindsight:

 -address os::current_stack_pointer() {
 +NOINLINE address os::current_stack_pointer() {


If y'all like the addition of NOINLINE, it should probably be added to all
of the 14 variants of os::current_stack_pointer.
Gives me a chance to try out the submit repo.



I can't help but think other platforms actually rely on it being inlined so
that it really does return the stack pointer of the method calling
os::current_stack_pointer!



But we only inline today if caller is in the same translation unit and
builds with optimization, no?


Don't know, but Martin's encountering a case where it is being inlined - 
is that likely to be unique for some reason?


I never assume the compiler does the obvious these days :) or that there 
can't be clever link-time tricks as well.


Maybe the safest thing to do is to only make a change for the clang case 
and leave everything else alone.


David
-



..Thomas


David

Re: RFR: 8186780: clang-4.0 fastdebug assertion failure in os_linux_x86:os::verify_stack_alignment()

[Martin Buchholz]

I see:

Don't use
  // os::current_stack_pointer(), as its result can be slightly below
current
  // stack pointer, causing us to not alloca enough to reach "bottom".

If you really really want to get the stack pointer of the current frame,
you can't put it in a function!  Use magic compiler extensions via a macro.

gcc and clang both have __builtin_frame_address(0).

gcc BUT not clang has
register uint64_t rsp asm ("rsp");
BUT that gives a slightly different value from __builtin_frame_address(0)
(different register? don't know much about x86 assembly)


On Thu, Jun 21, 2018 at 5:37 AM, Thomas Stüfe 
wrote:

> On Thu, Jun 21, 2018 at 1:27 PM, David Holmes 
> wrote:
> > On 21/06/2018 10:05 AM, Martin Buchholz wrote:
> >>
> >> On Wed, Jun 20, 2018 at 4:03 PM, Martin Buchholz  >> > wrote:
> >>
> >> Hi David and build-dev folk,
> >>
> >> After way too much build/hotspot hacking, I have a better fix:
> >>
> >> clang inlined os::current_stack_pointer into its caller __in the
> >> same translation unit___ (that could be fixed in a separate change)
> >> so of course in this case it didn't have to follow the ABI.  Fix is
> >> obvious in hindsight:
> >>
> >> -address os::current_stack_pointer() {
> >> +NOINLINE address os::current_stack_pointer() {
> >>
> >>
> >> If y'all like the addition of NOINLINE, it should probably be added to
> all
> >> of the 14 variants of os::current_stack_pointer.
> >> Gives me a chance to try out the submit repo.
> >
> >
> > I can't help but think other platforms actually rely on it being inlined
> so
> > that it really does return the stack pointer of the method calling
> > os::current_stack_pointer!
> >
>
> But we only inline today if caller is in the same translation unit and
> builds with optimization, no?
>
> ..Thomas
>
> > David
>

Re: RFR: 8186780: clang-4.0 fastdebug assertion failure in os_linux_x86:os::verify_stack_alignment()

[Thomas Stüfe]

On Thu, Jun 21, 2018 at 1:27 PM, David Holmes  wrote:
> On 21/06/2018 10:05 AM, Martin Buchholz wrote:
>>
>> On Wed, Jun 20, 2018 at 4:03 PM, Martin Buchholz > > wrote:
>>
>> Hi David and build-dev folk,
>>
>> After way too much build/hotspot hacking, I have a better fix:
>>
>> clang inlined os::current_stack_pointer into its caller __in the
>> same translation unit___ (that could be fixed in a separate change)
>> so of course in this case it didn't have to follow the ABI.  Fix is
>> obvious in hindsight:
>>
>> -address os::current_stack_pointer() {
>> +NOINLINE address os::current_stack_pointer() {
>>
>>
>> If y'all like the addition of NOINLINE, it should probably be added to all
>> of the 14 variants of os::current_stack_pointer.
>> Gives me a chance to try out the submit repo.
>
>
> I can't help but think other platforms actually rely on it being inlined so
> that it really does return the stack pointer of the method calling
> os::current_stack_pointer!
>

But we only inline today if caller is in the same translation unit and
builds with optimization, no?

..Thomas

> David

Re: RFR: 8186780: clang-4.0 fastdebug assertion failure in os_linux_x86:os::verify_stack_alignment()

[David Holmes]

On 21/06/2018 10:05 AM, Martin Buchholz wrote:
On Wed, Jun 20, 2018 at 4:03 PM, Martin Buchholz > wrote:


Hi David and build-dev folk,

After way too much build/hotspot hacking, I have a better fix:

clang inlined os::current_stack_pointer into its caller __in the
same translation unit___ (that could be fixed in a separate change)
so of course in this case it didn't have to follow the ABI.  Fix is
obvious in hindsight:

-address os::current_stack_pointer() {
+NOINLINE address os::current_stack_pointer() {


If y'all like the addition of NOINLINE, it should probably be added to 
all of the 14 variants of os::current_stack_pointer.

Gives me a chance to try out the submit repo.


I can't help but think other platforms actually rely on it being inlined 
so that it really does return the stack pointer of the method calling 
os::current_stack_pointer!


David

Re: RFR: 8186780: clang-4.0 fastdebug assertion failure in os_linux_x86:os::verify_stack_alignment()

[Martin Buchholz]

On Wed, Jun 20, 2018 at 4:03 PM, Martin Buchholz 
wrote:

> Hi David and build-dev folk,
>
> After way too much build/hotspot hacking, I have a better fix:
>
> clang inlined os::current_stack_pointer into its caller __in the same
> translation unit___ (that could be fixed in a separate change) so of course
> in this case it didn't have to follow the ABI.  Fix is obvious in hindsight:
>
> -address os::current_stack_pointer() {
> +NOINLINE address os::current_stack_pointer() {
>

If y'all like the addition of NOINLINE, it should probably be added to all
of the 14 variants of  os::current_stack_pointer.
Gives me a chance to try out the submit repo.

Re: RFR: 8186780: clang-4.0 fastdebug assertion failure in os_linux_x86:os::verify_stack_alignment()

[Erik Joelsson]

Looks good to me.

/Erik


On 2018-06-20 16:49, Martin Buchholz wrote:

Thanks Erik.

On Wed, Jun 20, 2018 at 4:19 PM, Erik Joelsson 
mailto:erik.joels...@oracle.com>> wrote:


Hello,

It's very probable that we have made several such mistakes with
our flags, since for the most part, we have a 1-1 mapping of
compiler and OS. We certainly appreciate correcting this whenever
possible. I don't have the expertise to verify your claim here,
but I will take your word for it.

The change looks ok, but there is already a big block of clang
specific stuff close by. Perhaps move this assignment there?


Yes, that does look like a better place:

 --- a/make/autoconf/flags-cflags.m4
+++ b/make/autoconf/flags-cflags.m4
@@ -470,14 +470,6 @@ AC_DEFUN([FLAGS_SETUP_CFLAGS_HELPER],
     # COMMON to gcc and clang
     TOOLCHAIN_CFLAGS_JVM="-pipe -fno-rtti -fno-exceptions \
         -fvisibility=hidden -fno-strict-aliasing -fno-omit-frame-pointer"
-
-    if test "x$TOOLCHAIN_TYPE" = xclang; then
-      # In principle the stack alignment below is cpu- and 
ABI-dependent and

-      # should agree with values of StackAlignmentInBytes in various
-      # src/hotspot/cpu/*/globalDefinitions_*.hpp files, but this value
-      # currently works for all platforms.
-      TOOLCHAIN_CFLAGS_JVM="$TOOLCHAIN_CFLAGS_JVM 
-mno-omit-leaf-frame-pointer -mstack-alignment=16"

-    fi
   fi
   if test "x$TOOLCHAIN_TYPE" = xgcc; then
@@ -499,6 +491,12 @@ AC_DEFUN([FLAGS_SETUP_CFLAGS_HELPER],
     # (see http://llvm.org/bugs/show_bug.cgi?id=7554)
     TOOLCHAIN_CFLAGS_JVM="$TOOLCHAIN_CFLAGS_JVM -flimit-debug-info"
+    # In principle the stack alignment below is cpu- and 
ABI-dependent and

+    # should agree with values of StackAlignmentInBytes in various
+    # src/hotspot/cpu/*/globalDefinitions_*.hpp files, but this value
+    # currently works for all platforms.
+    TOOLCHAIN_CFLAGS_JVM="$TOOLCHAIN_CFLAGS_JVM 
-mno-omit-leaf-frame-pointer -mstack-alignment=16"

+
     if test "x$OPENJDK_TARGET_OS" = xlinux; then
       TOOLCHAIN_CFLAGS_JDK="-pipe"
 TOOLCHAIN_CFLAGS_JDK_CONLY="-fno-strict-aliasing" # technically NOT 
for CXX

Re: RFR: 8186780: clang-4.0 fastdebug assertion failure in os_linux_x86:os::verify_stack_alignment()

[Martin Buchholz]

Thanks Erik.

On Wed, Jun 20, 2018 at 4:19 PM, Erik Joelsson 
wrote:

> Hello,
>
> It's very probable that we have made several such mistakes with our flags,
> since for the most part, we have a 1-1 mapping of compiler and OS. We
> certainly appreciate correcting this whenever possible. I don't have the
> expertise to verify your claim here, but I will take your word for it.
>
> The change looks ok, but there is already a big block of clang specific
> stuff close by. Perhaps move this assignment there?


Yes, that does look like a better place:

 --- a/make/autoconf/flags-cflags.m4
+++ b/make/autoconf/flags-cflags.m4
@@ -470,14 +470,6 @@ AC_DEFUN([FLAGS_SETUP_CFLAGS_HELPER],
 # COMMON to gcc and clang
 TOOLCHAIN_CFLAGS_JVM="-pipe -fno-rtti -fno-exceptions \
 -fvisibility=hidden -fno-strict-aliasing -fno-omit-frame-pointer"
-
-if test "x$TOOLCHAIN_TYPE" = xclang; then
-  # In principle the stack alignment below is cpu- and ABI-dependent
and
-  # should agree with values of StackAlignmentInBytes in various
-  # src/hotspot/cpu/*/globalDefinitions_*.hpp files, but this value
-  # currently works for all platforms.
-  TOOLCHAIN_CFLAGS_JVM="$TOOLCHAIN_CFLAGS_JVM
-mno-omit-leaf-frame-pointer -mstack-alignment=16"
-fi
   fi

   if test "x$TOOLCHAIN_TYPE" = xgcc; then
@@ -499,6 +491,12 @@ AC_DEFUN([FLAGS_SETUP_CFLAGS_HELPER],
 # (see http://llvm.org/bugs/show_bug.cgi?id=7554)
 TOOLCHAIN_CFLAGS_JVM="$TOOLCHAIN_CFLAGS_JVM -flimit-debug-info"

+# In principle the stack alignment below is cpu- and ABI-dependent and
+# should agree with values of StackAlignmentInBytes in various
+# src/hotspot/cpu/*/globalDefinitions_*.hpp files, but this value
+# currently works for all platforms.
+TOOLCHAIN_CFLAGS_JVM="$TOOLCHAIN_CFLAGS_JVM
-mno-omit-leaf-frame-pointer -mstack-alignment=16"
+
 if test "x$OPENJDK_TARGET_OS" = xlinux; then
   TOOLCHAIN_CFLAGS_JDK="-pipe"
   TOOLCHAIN_CFLAGS_JDK_CONLY="-fno-strict-aliasing" # technically NOT
for CXX

Re: RFR: 8186780: clang-4.0 fastdebug assertion failure in os_linux_x86:os::verify_stack_alignment()

[Erik Joelsson]

Hello,

It's very probable that we have made several such mistakes with our 
flags, since for the most part, we have a 1-1 mapping of compiler and 
OS. We certainly appreciate correcting this whenever possible. I don't 
have the expertise to verify your claim here, but I will take your word 
for it.


The change looks ok, but there is already a big block of clang specific 
stuff close by. Perhaps move this assignment there?


/Erik


On 2018-06-20 16:03, Martin Buchholz wrote:

Hi David and build-dev folk,

After way too much build/hotspot hacking, I have a better fix:

clang inlined os::current_stack_pointer into its caller __in the same
translation unit___ (that could be fixed in a separate change) so of course
in this case it didn't have to follow the ABI.  Fix is obvious in hindsight:

-address os::current_stack_pointer() {
+NOINLINE address os::current_stack_pointer() {

While working on this I noticed that the clang stack alignment flags on
Linux are missing.  They should be moved from a macosx-specific check to a
clang-specific check.  macosx is not synonymous with clang!

--- a/make/autoconf/flags-cflags.m4
+++ b/make/autoconf/flags-cflags.m4
@@ -470,6 +470,14 @@ AC_DEFUN([FLAGS_SETUP_CFLAGS_HELPER],
  # COMMON to gcc and clang
  TOOLCHAIN_CFLAGS_JVM="-pipe -fno-rtti -fno-exceptions \
  -fvisibility=hidden -fno-strict-aliasing -fno-omit-frame-pointer"
+
+if test "x$TOOLCHAIN_TYPE" = xclang; then
+  # In principle the stack alignment below is cpu- and ABI-dependent
and
+  # should agree with values of StackAlignmentInBytes in various
+  # src/hotspot/cpu/*/globalDefinitions_*.hpp files, but this value
+  # currently works for all platforms.
+  TOOLCHAIN_CFLAGS_JVM="$TOOLCHAIN_CFLAGS_JVM
-mno-omit-leaf-frame-pointer -mstack-alignment=16"
+fi
fi

if test "x$TOOLCHAIN_TYPE" = xgcc; then
@@ -601,10 +609,6 @@ AC_DEFUN([FLAGS_SETUP_CFLAGS_HELPER],
  fi
fi

-  if test "x$OPENJDK_TARGET_OS" = xmacosx; then
-OS_CFLAGS_JVM="$OS_CFLAGS_JVM -mno-omit-leaf-frame-pointer
-mstack-alignment=16"
-  fi
-
# Optional POSIX functionality needed by the JVM
#
# Check if clock_gettime is available and in which library. This
indicates


8186780: clang-4.0 fastdebug assertion failure in
os_linux_x86:os::verify_stack_alignment()
http://cr.openjdk.java.net/~martin/webrevs/jdk/clang-stack-alignment/
https://bugs.openjdk.java.net/browse/JDK-8186780

On Wed, Jun 20, 2018 at 12:30 AM, David Holmes 
wrote:


Hi Martin,


On 20/06/2018 3:03 AM, Martin Buchholz wrote:


(There's surely a better fix that involves refactoring os/cpu/compiler
support)

8186780: clang-4.0 fastdebug assertion failure in
os_linux_x86:os::verify_stack_alignment()
http://cr.openjdk.java.net/~martin/webrevs/jdk/clang-verify_
stack_alignment/
https://bugs.openjdk.java.net/browse/JDK-8186780


I remain concerned about what it may mean for the stack pointer to not be
aligned. I would have thought stack pointer alignment was part of the ABI
for a CPU architecture, not something the compiler could choose at will?
What about all the other code that uses StackAlignmentInBytes ??

That aside your fix excludes the assert when building with clang for linux
x86 as intended. And I see that for BSD x86 (where we also use clang) that
verify_stack_alignment is empty.

Thanks,
David

Re: RFR: 8186780: clang-4.0 fastdebug assertion failure in os_linux_x86:os::verify_stack_alignment()

[Martin Buchholz]

Hi David and build-dev folk,

After way too much build/hotspot hacking, I have a better fix:

clang inlined os::current_stack_pointer into its caller __in the same
translation unit___ (that could be fixed in a separate change) so of course
in this case it didn't have to follow the ABI.  Fix is obvious in hindsight:

-address os::current_stack_pointer() {
+NOINLINE address os::current_stack_pointer() {

While working on this I noticed that the clang stack alignment flags on
Linux are missing.  They should be moved from a macosx-specific check to a
clang-specific check.  macosx is not synonymous with clang!

--- a/make/autoconf/flags-cflags.m4
+++ b/make/autoconf/flags-cflags.m4
@@ -470,6 +470,14 @@ AC_DEFUN([FLAGS_SETUP_CFLAGS_HELPER],
 # COMMON to gcc and clang
 TOOLCHAIN_CFLAGS_JVM="-pipe -fno-rtti -fno-exceptions \
 -fvisibility=hidden -fno-strict-aliasing -fno-omit-frame-pointer"
+
+if test "x$TOOLCHAIN_TYPE" = xclang; then
+  # In principle the stack alignment below is cpu- and ABI-dependent
and
+  # should agree with values of StackAlignmentInBytes in various
+  # src/hotspot/cpu/*/globalDefinitions_*.hpp files, but this value
+  # currently works for all platforms.
+  TOOLCHAIN_CFLAGS_JVM="$TOOLCHAIN_CFLAGS_JVM
-mno-omit-leaf-frame-pointer -mstack-alignment=16"
+fi
   fi

   if test "x$TOOLCHAIN_TYPE" = xgcc; then
@@ -601,10 +609,6 @@ AC_DEFUN([FLAGS_SETUP_CFLAGS_HELPER],
 fi
   fi

-  if test "x$OPENJDK_TARGET_OS" = xmacosx; then
-OS_CFLAGS_JVM="$OS_CFLAGS_JVM -mno-omit-leaf-frame-pointer
-mstack-alignment=16"
-  fi
-
   # Optional POSIX functionality needed by the JVM
   #
   # Check if clock_gettime is available and in which library. This
indicates


8186780: clang-4.0 fastdebug assertion failure in
os_linux_x86:os::verify_stack_alignment()
http://cr.openjdk.java.net/~martin/webrevs/jdk/clang-stack-alignment/
https://bugs.openjdk.java.net/browse/JDK-8186780

On Wed, Jun 20, 2018 at 12:30 AM, David Holmes 
wrote:

> Hi Martin,
>
>
> On 20/06/2018 3:03 AM, Martin Buchholz wrote:
>
>> (There's surely a better fix that involves refactoring os/cpu/compiler
>> support)
>>
>> 8186780: clang-4.0 fastdebug assertion failure in
>> os_linux_x86:os::verify_stack_alignment()
>> http://cr.openjdk.java.net/~martin/webrevs/jdk/clang-verify_
>> stack_alignment/
>> https://bugs.openjdk.java.net/browse/JDK-8186780
>>
>
> I remain concerned about what it may mean for the stack pointer to not be
> aligned. I would have thought stack pointer alignment was part of the ABI
> for a CPU architecture, not something the compiler could choose at will?
> What about all the other code that uses StackAlignmentInBytes ??
>
> That aside your fix excludes the assert when building with clang for linux
> x86 as intended. And I see that for BSD x86 (where we also use clang) that
> verify_stack_alignment is empty.
>
> Thanks,
> David
>

Re: RFR: JDK-8199331 Don't limit debug information for fastdebug JDK native libraries

[Erik Joelsson]

Looks good!

/Erik


On 2018-03-08 06:19, Magnus Ihse Bursie wrote:

Since long time ago, native libraries in JDK has been compiled with -g1 instead 
of -g, when doing a fastdebug build with gcc. (This does not apply to hotspot 
which is always compiled with -g.)

This means that the debug information generated is limited. The gcc manual says 
this about level 1:

"Level 1 produces minimal information, enough for making backtraces in parts of the 
program that you don't plan to debug. This includes descriptions of functions and 
external variables, and line number tables, but no information about local 
variables."

The reason for doing this is claimed to be concerns over size. However, debug 
symbols images only increased from 129 MB to 135 MB with -g instead of -g1. 
This does not seem like a valid concern.

As part of a general effort of simplifying and unifying the various options, 
I'd like to turn this to -g, so we have consistent debug capabilities 
regardless of build type.

Bug: https://bugs.openjdk.java.net/browse/JDK-8199331
Patch inline:
diff --git a/make/autoconf/flags-cflags.m4 b/make/autoconf/flags-cflags.m4
--- a/make/autoconf/flags-cflags.m4
+++ b/make/autoconf/flags-cflags.m4
@@ -108,11 +108,7 @@
  [
# Debug symbols
if test "x$TOOLCHAIN_TYPE" = xgcc; then
-if test "x$OPENJDK_TARGET_CPU_BITS" = "x64" && test "x$DEBUG_LEVEL" = 
"xfastdebug"; then
-  CFLAGS_DEBUG_SYMBOLS="-g1"
-else
-  CFLAGS_DEBUG_SYMBOLS="-g"
-fi
+CFLAGS_DEBUG_SYMBOLS="-g"
elif test "x$TOOLCHAIN_TYPE" = xclang; then
  CFLAGS_DEBUG_SYMBOLS="-g"
elif test "x$TOOLCHAIN_TYPE" = xsolstudio; then

/Magnus

RFR: JDK-8199331 Don't limit debug information for fastdebug JDK native libraries

[Magnus Ihse Bursie]

Since long time ago, native libraries in JDK has been compiled with -g1 instead 
of -g, when doing a fastdebug build with gcc. (This does not apply to hotspot 
which is always compiled with -g.)

This means that the debug information generated is limited. The gcc manual says 
this about level 1:

"Level 1 produces minimal information, enough for making backtraces in parts of 
the program that you don't plan to debug. This includes descriptions of 
functions and external variables, and line number tables, but no information 
about local variables."

The reason for doing this is claimed to be concerns over size. However, debug 
symbols images only increased from 129 MB to 135 MB with -g instead of -g1. 
This does not seem like a valid concern.

As part of a general effort of simplifying and unifying the various options, 
I'd like to turn this to -g, so we have consistent debug capabilities 
regardless of build type.

Bug: https://bugs.openjdk.java.net/browse/JDK-8199331
Patch inline:
diff --git a/make/autoconf/flags-cflags.m4 b/make/autoconf/flags-cflags.m4
--- a/make/autoconf/flags-cflags.m4
+++ b/make/autoconf/flags-cflags.m4
@@ -108,11 +108,7 @@
 [
   # Debug symbols
   if test "x$TOOLCHAIN_TYPE" = xgcc; then
-if test "x$OPENJDK_TARGET_CPU_BITS" = "x64" && test "x$DEBUG_LEVEL" = 
"xfastdebug"; then
-  CFLAGS_DEBUG_SYMBOLS="-g1"
-else
-  CFLAGS_DEBUG_SYMBOLS="-g"
-fi
+CFLAGS_DEBUG_SYMBOLS="-g"
   elif test "x$TOOLCHAIN_TYPE" = xclang; then
 CFLAGS_DEBUG_SYMBOLS="-g"
   elif test "x$TOOLCHAIN_TYPE" = xsolstudio; then

/Magnus

Re: RFR [xxs, aix]: JDK-8196488: [aix] TOC overflow for libjvm.so in fastdebug build

[Thomas Stüfe]

On Mon, Feb 12, 2018 at 4:41 PM, Magnus Ihse Bursie <
magnus.ihse.bur...@oracle.com> wrote:

> On 2018-02-12 15:32, Thomas Stüfe wrote:
>
>> Hi all,
>>
>> may I have a sponsor please? I am not sure whether I am allowed to push
>> myself, and if yes, to which repository.
>>
>
> Since the removal of generated-configure.sh you don't need a sponsor just
> because you touch autoconf files. So for jdk/jdk, you're free to just push
> it. As far as I understand, you're also free to just push it to jdk/hs
> nowadays, if that were you want it to go, but someone from hotspot should
> probably confirm this.
>
>
Ok. Nice to know. Thanks.

..Thomas


> /Magnus
>
>
>
>> I have three reviewers already.
>>
>> Thank you!
>>
>> Thomas
>>
>>
>> On Thu, Feb 8, 2018 at 3:42 PM, Thomas Stüfe <thomas.stu...@gmail.com>
>> wrote:
>>
>> Hi,
>>>
>>> may I please have reviews for this tiny fix:
>>>
>>> Bug:  https://bugs.openjdk.java.net/browse/JDK-8196488
>>> webrev: http://cr.openjdk.java.net/~stuefe/webrevs/
>>> 8196488-aix-toc-overflow-in-fastdebug/webrev.00/webrev/
>>>
>>> Thanks and Kind Regards, Thomas
>>>
>>>
>

Re: RFR [xxs, aix]: JDK-8196488: [aix] TOC overflow for libjvm.so in fastdebug build

[Magnus Ihse Bursie]

On 2018-02-12 15:32, Thomas Stüfe wrote:

Hi all,

may I have a sponsor please? I am not sure whether I am allowed to push
myself, and if yes, to which repository.


Since the removal of generated-configure.sh you don't need a sponsor 
just because you touch autoconf files. So for jdk/jdk, you're free to 
just push it. As far as I understand, you're also free to just push it 
to jdk/hs nowadays, if that were you want it to go, but someone from 
hotspot should probably confirm this.


/Magnus



I have three reviewers already.

Thank you!

Thomas


On Thu, Feb 8, 2018 at 3:42 PM, Thomas Stüfe <thomas.stu...@gmail.com>
wrote:


Hi,

may I please have reviews for this tiny fix:

Bug:  https://bugs.openjdk.java.net/browse/JDK-8196488
webrev: http://cr.openjdk.java.net/~stuefe/webrevs/
8196488-aix-toc-overflow-in-fastdebug/webrev.00/webrev/

Thanks and Kind Regards, Thomas

Re: RFR [xxs, aix]: JDK-8196488: [aix] TOC overflow for libjvm.so in fastdebug build

[Thomas Stüfe]

Please disregard my last mail, the change has just been pushed.

Thanks, Thomas

On Mon, Feb 12, 2018 at 3:32 PM, Thomas Stüfe <thomas.stu...@gmail.com>
wrote:

> Hi all,
>
> may I have a sponsor please? I am not sure whether I am allowed to push
> myself, and if yes, to which repository.
>
> I have three reviewers already.
>
> Thank you!
>
> Thomas
>
>
> On Thu, Feb 8, 2018 at 3:42 PM, Thomas Stüfe <thomas.stu...@gmail.com>
> wrote:
>
>> Hi,
>>
>> may I please have reviews for this tiny fix:
>>
>> Bug:  https://bugs.openjdk.java.net/browse/JDK-8196488
>> webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8196488-
>> aix-toc-overflow-in-fastdebug/webrev.00/webrev/
>>
>> Thanks and Kind Regards, Thomas
>>
>
>

Re: RFR [xxs, aix]: JDK-8196488: [aix] TOC overflow for libjvm.so in fastdebug build

[Thomas Stüfe]

Hi all,

may I have a sponsor please? I am not sure whether I am allowed to push
myself, and if yes, to which repository.

I have three reviewers already.

Thank you!

Thomas


On Thu, Feb 8, 2018 at 3:42 PM, Thomas Stüfe <thomas.stu...@gmail.com>
wrote:

> Hi,
>
> may I please have reviews for this tiny fix:
>
> Bug:  https://bugs.openjdk.java.net/browse/JDK-8196488
> webrev: http://cr.openjdk.java.net/~stuefe/webrevs/
> 8196488-aix-toc-overflow-in-fastdebug/webrev.00/webrev/
>
> Thanks and Kind Regards, Thomas
>

Re: RFR [xxs, aix]: JDK-8196488: [aix] TOC overflow for libjvm.so in fastdebug build

[Thomas Stüfe]

Thanks!

On Thu, Feb 8, 2018 at 3:49 PM, Lindenmaier, Goetz <
goetz.lindenma...@sap.com> wrote:

> HI Thomas,
>
> looks good, thanks!
>
> Best regards,
>   Goetz.
>
> > -Original Message-
> > From: ppc-aix-port-dev [mailto:ppc-aix-port-dev-
> > boun...@openjdk.java.net] On Behalf Of Thomas Stüfe
> > Sent: Donnerstag, 8. Februar 2018 15:42
> > To: ppc-aix-port-...@openjdk.java.net; build-dev  > d...@openjdk.java.net>
> > Subject: RFR [xxs, aix]: JDK-8196488: [aix] TOC overflow for libjvm.so in
> > fastdebug build
> >
> > Hi,
> >
> > may I please have reviews for this tiny fix:
> >
> > Bug:  https://bugs.openjdk.java.net/browse/JDK-8196488
> > webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8196488-aix-toc-
> > overflow-in-fastdebug/webrev.00/webrev/
> >
> > Thanks and Kind Regards, Thomas
>

Re: RFR [xxs, aix]: JDK-8196488: [aix] TOC overflow for libjvm.so in fastdebug build

[Thomas Stüfe]

Thanks Matthias!

On Thu, Feb 8, 2018 at 3:58 PM, Baesken, Matthias <matthias.baes...@sap.com>
wrote:

> Hi Thomas , looks good to me( not a Reviever however).
>
>
>
> Best Regards, Matthias
>
>
>
> *From:* ppc-aix-port-dev [mailto:ppc-aix-port-dev-boun...@openjdk.java.net]
> *On Behalf Of *Thomas Stüfe
> *Sent:* Donnerstag, 8. Februar 2018 15:42
> *To:* ppc-aix-port-...@openjdk.java.net; build-dev <
> build-dev@openjdk.java.net>
> *Subject:* RFR [xxs, aix]: JDK-8196488: [aix] TOC overflow for libjvm.so
> in fastdebug build
>
>
>
> Hi,
>
>
>
> may I please have reviews for this tiny fix:
>
>
>
> Bug:  https://bugs.openjdk.java.net/browse/JDK-8196488
>
> webrev: http://cr.openjdk.java.net/~stuefe/webrevs/
> 8196488-aix-toc-overflow-in-fastdebug/webrev.00/webrev/
>
>
>
> Thanks and Kind Regards, Thomas
>

Re: RFR [xxs, aix]: JDK-8196488: [aix] TOC overflow for libjvm.so in fastdebug build

[Thomas Stüfe]

Hi Eric,

On Thu, Feb 8, 2018 at 6:09 PM, Erik Joelsson <erik.joels...@oracle.com>
wrote:

> Looks good, happy pushing without a sponsor!
>
> /Erik
>
>
Yes please, that would be nice!

..Thomas


>
>
> On 2018-02-08 06:42, Thomas Stüfe wrote:
>
>> Hi,
>>
>> may I please have reviews for this tiny fix:
>>
>> Bug:  https://bugs.openjdk.java.net/browse/JDK-8196488
>> webrev:
>> http://cr.openjdk.java.net/~stuefe/webrevs/8196488-aix-toc-
>> overflow-in-fastdebug/webrev.00/webrev/
>>
>> Thanks and Kind Regards, Thomas
>>
>
>

Re: RFR [xxs, aix]: JDK-8196488: [aix] TOC overflow for libjvm.so in fastdebug build

[Erik Joelsson]

Looks good, happy pushing without a sponsor!

/Erik


On 2018-02-08 06:42, Thomas Stüfe wrote:

Hi,

may I please have reviews for this tiny fix:

Bug:  https://bugs.openjdk.java.net/browse/JDK-8196488
webrev:
http://cr.openjdk.java.net/~stuefe/webrevs/8196488-aix-toc-overflow-in-fastdebug/webrev.00/webrev/

Thanks and Kind Regards, Thomas

RE: RFR [xxs, aix]: JDK-8196488: [aix] TOC overflow for libjvm.so in fastdebug build

[Baesken, Matthias]

Hi Thomas , looks good to me( not a Reviever however).

Best Regards, Matthias

From: ppc-aix-port-dev [mailto:ppc-aix-port-dev-boun...@openjdk.java.net] On 
Behalf Of Thomas Stüfe
Sent: Donnerstag, 8. Februar 2018 15:42
To: ppc-aix-port-...@openjdk.java.net; build-dev <build-dev@openjdk.java.net>
Subject: RFR [xxs, aix]: JDK-8196488: [aix] TOC overflow for libjvm.so in 
fastdebug build

Hi,

may I please have reviews for this tiny fix:

Bug:  https://bugs.openjdk.java.net/browse/JDK-8196488
webrev: 
http://cr.openjdk.java.net/~stuefe/webrevs/8196488-aix-toc-overflow-in-fastdebug/webrev.00/webrev/

Thanks and Kind Regards, Thomas

RE: RFR [xxs, aix]: JDK-8196488: [aix] TOC overflow for libjvm.so in fastdebug build

[Lindenmaier, Goetz]

HI Thomas, 

looks good, thanks!

Best regards,
  Goetz.

> -Original Message-
> From: ppc-aix-port-dev [mailto:ppc-aix-port-dev-
> boun...@openjdk.java.net] On Behalf Of Thomas Stüfe
> Sent: Donnerstag, 8. Februar 2018 15:42
> To: ppc-aix-port-...@openjdk.java.net; build-dev  d...@openjdk.java.net>
> Subject: RFR [xxs, aix]: JDK-8196488: [aix] TOC overflow for libjvm.so in
> fastdebug build
> 
> Hi,
> 
> may I please have reviews for this tiny fix:
> 
> Bug:  https://bugs.openjdk.java.net/browse/JDK-8196488
> webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8196488-aix-toc-
> overflow-in-fastdebug/webrev.00/webrev/
> 
> Thanks and Kind Regards, Thomas

RFR [xxs, aix]: JDK-8196488: [aix] TOC overflow for libjvm.so in fastdebug build

[Thomas Stüfe]

Hi,

may I please have reviews for this tiny fix:

Bug:  https://bugs.openjdk.java.net/browse/JDK-8196488
webrev:
http://cr.openjdk.java.net/~stuefe/webrevs/8196488-aix-toc-overflow-in-fastdebug/webrev.00/webrev/

Thanks and Kind Regards, Thomas

Re: RFR: JDK-8157574: Mac fastdebug bundles have wrong directory layout

[Mandy Chung]

Looks okay to me.

Mandy

> On May 23, 2016, at 7:00 AM, Erik Joelsson  wrote:
> 
> Hello,
> 
> In JDK-8136777 I introduced the new bundle targets which creates .tar.gz 
> bundles of all the images. The naming and layout of these bundles were based 
> on how Oracle has created such bundles internally before. Except in one case, 
> where I missed: Macosx debug.
> 
> As I see it, there is no real right or wrong here, just opinion and history. 
> On Macosx, for release builds, the image layout is a bit different compared 
> to other platforms:
> 
> jdk-9.jdk/Contents/Home/
> 
> compared to:
> 
> jdk-9/
> 
> On all other platforms, the debug bundles look like this:
> 
> jdk-9/$(DEBUG_LEVEL)/
> 
> Which makes it possible to overlay both release and debug bundles in the same 
> directory.
> 
> Now the question is what the layout in a debug bundle for macosx should look 
> like. JDK-8136777 resulted in the same as for macosx release:
> 
> jdk-9.jdk/Contents/Home/
> 
> While Oracle historically used the same as other platforms:
> 
> jdk-9/$(DEBUG_LEVEL)/
> 
> This change aligns with Oracle's old way so as to reduce friction and 
> adoption internally.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8157574
> Webrev: http://cr.openjdk.java.net/~erikj/8157574/webrev.top.01/
> 
> /Erik

RFR: JDK-8157574: Mac fastdebug bundles have wrong directory layout

[Erik Joelsson]

Hello,

In JDK-8136777 I introduced the new bundle targets which creates .tar.gz 
bundles of all the images. The naming and layout of these bundles were 
based on how Oracle has created such bundles internally before. Except 
in one case, where I missed: Macosx debug.


As I see it, there is no real right or wrong here, just opinion and 
history. On Macosx, for release builds, the image layout is a bit 
different compared to other platforms:


jdk-9.jdk/Contents/Home/

compared to:

jdk-9/

On all other platforms, the debug bundles look like this:

jdk-9/$(DEBUG_LEVEL)/

Which makes it possible to overlay both release and debug bundles in the 
same directory.


Now the question is what the layout in a debug bundle for macosx should 
look like. JDK-8136777 resulted in the same as for macosx release:


jdk-9.jdk/Contents/Home/

While Oracle historically used the same as other platforms:

jdk-9/$(DEBUG_LEVEL)/

This change aligns with Oracle's old way so as to reduce friction and 
adoption internally.


Bug: https://bugs.openjdk.java.net/browse/JDK-8157574
Webrev: http://cr.openjdk.java.net/~erikj/8157574/webrev.top.01/

/Erik

Re: Is there any valid reason that a debug or fastdebug build should define PRODUCT and not ASSERT during compiles?

[Derek White]

Hi Magnus,

On 1/14/16 6:06 AM, Magnus Ihse Bursie wrote:

On 2016-01-08 23:07, Derek White wrote:

[This is likely a bug, but thought I'd ask it as a question first].

I'm new to jdk builds on windows, and have spent way more time than 
I'd like to admit on figuring out why my fastdebug builds did not 
have asserts turned on.


The TL;DR; answer is that anyone can build this way on Windows by 
simply executing this in cygwin before doing a make:

> export release="Derek is having a fun day"
> bash common/bin/jib.sh make -p windows-x86-debug -- images

(This bug has nothing to do with jib, it's just the actual command 
line I used.)


This results in a build with both /D "DEBUG_LEVEL=\"fastdebug\"" and 
/D "PRODUCT" set, because nmake.exe uppercases all environment 
variables as it imports them (as a convenience to the user).


I'd think that there is no useful purpose to supporting such a 
configuration, and it's a bug to attempt it. I suggest doing an 
"!ifdef RELEASE" check in windows/makefiles/debug.make and 
windows/makefiles/fastdebug.make that either errors or unsets RELEASE 
before including vm.make.


Any thoughts on this? Thanks!


In general, environment variables can have unexpected effects on the 
build, if they match a name used in the make files. There is no 
general solution to this problem.


As for this specific problem, as Erik said in a reply to your other 
mail, we are currently working on replacing the use of nmake, so we 
are not keen on spending time fixing issues in the current nmake scripts.


Since this problem has existed since the dawn of time (or so...), and 
it has not been reported until now, and given that there is a good 
workaround, I don't consider it worth fixing.


I appreciate that you spent time to figure out the issue and that you 
reported it to us in such a polite manner! :-) I hope I don't come 
across as rude by dismissing your suggested fix, it's just that I want 
to spend all possible time on getting rid of nmake instead of fixing 
issues in it.


/Magnus
Unfortunately I ended up modifying a unix .bashrc file provided by Sun 
IT back in the day to use in my cygwin env. For whatever reason Sun IT 
defined the variable "release" :-(.


I am happy to see the nmake dependency go away. I'd like to make sure 
that the new windows make files don't depend unnecessarily on a RELEASE 
variable, but instead follow the linux/mac/solaris convention of having 
the top-level make files (product/fastdebug/debug) add in the correct 
options (ASSERT/PRODUCT, etc) as needed.


 - Derek

Re: Is there any valid reason that a debug or fastdebug build should define PRODUCT and not ASSERT during compiles?

[Magnus Ihse Bursie]

On 2016-01-08 23:07, Derek White wrote:

[This is likely a bug, but thought I'd ask it as a question first].

I'm new to jdk builds on windows, and have spent way more time than 
I'd like to admit on figuring out why my fastdebug builds did not have 
asserts turned on.


The TL;DR; answer is that anyone can build this way on Windows by 
simply executing this in cygwin before doing a make:

> export release="Derek is having a fun day"
> bash common/bin/jib.sh make -p windows-x86-debug -- images

(This bug has nothing to do with jib, it's just the actual command 
line I used.)


This results in a build with both /D "DEBUG_LEVEL=\"fastdebug\"" and 
/D "PRODUCT" set, because nmake.exe uppercases all environment 
variables as it imports them (as a convenience to the user).


I'd think that there is no useful purpose to supporting such a 
configuration, and it's a bug to attempt it. I suggest doing an 
"!ifdef RELEASE" check in windows/makefiles/debug.make and 
windows/makefiles/fastdebug.make that either errors or unsets RELEASE 
before including vm.make.


Any thoughts on this? Thanks!


In general, environment variables can have unexpected effects on the 
build, if they match a name used in the make files. There is no general 
solution to this problem.


As for this specific problem, as Erik said in a reply to your other 
mail, we are currently working on replacing the use of nmake, so we are 
not keen on spending time fixing issues in the current nmake scripts.


Since this problem has existed since the dawn of time (or so...), and it 
has not been reported until now, and given that there is a good 
workaround, I don't consider it worth fixing.


I appreciate that you spent time to figure out the issue and that you 
reported it to us in such a polite manner! :-) I hope I don't come 
across as rude by dismissing your suggested fix, it's just that I want 
to spend all possible time on getting rid of nmake instead of fixing 
issues in it.


/Magnus

Is there any valid reason that a debug or fastdebug build should define PRODUCT and not ASSERT during compiles?

[Derek White]

[This is likely a bug, but thought I'd ask it as a question first].

I'm new to jdk builds on windows, and have spent way more time than I'd 
like to admit on figuring out why my fastdebug builds did not have 
asserts turned on.


The TL;DR; answer is that anyone can build this way on Windows by simply 
executing this in cygwin before doing a make:

> export release="Derek is having a fun day"
> bash common/bin/jib.sh make -p windows-x86-debug -- images

(This bug has nothing to do with jib, it's just the actual command line 
I used.)


This results in a build with both /D "DEBUG_LEVEL=\"fastdebug\"" and /D 
"PRODUCT" set, because nmake.exe uppercases all environment variables as 
it imports them (as a convenience to the user).


I'd think that there is no useful purpose to supporting such a 
configuration, and it's a bug to attempt it. I suggest doing an "!ifdef 
RELEASE" check in windows/makefiles/debug.make and 
windows/makefiles/fastdebug.make that either errors or unsets RELEASE 
before including vm.make.


Any thoughts on this? Thanks!

 - Derek

Re: RFR: JDK-8145564: 8036003: startup regression on linux fastdebug builds

[Magnus Ihse Bursie]

On 2015-12-17 22:24, David Holmes wrote:

On 17/12/2015 6:18 PM, Magnus Ihse Bursie wrote:

On 2015-12-17 09:08, David Holmes wrote:

On 17/12/2015 5:54 PM, Erik Joelsson wrote:



On 2015-12-17 01:40, David Holmes wrote:

On 17/12/2015 7:35 AM, Erik Joelsson wrote:

One more thing, where should this fix be pushed? Do you need it
urgently
in hs-rt?


It is urgently needed in both the hs-rt and hs-comp repos as it
affects nightly testing and JPRT. If Alejandro agrees I suggest
pushing to jdk9/hs and it can then be pulled down to the team repos.


Will do.

With regard to the fix ... previously DEBUG_BINARIES was never set in
spec.gmk and so was never externally introduced into the hotspot 
build
this way. So why not simply change the name of the variable so 
that it

has no affect on the hotspot part of the build?


Well, we don't want it affecting the jdk part of the build either at
this point. This patch aims to restore the "external" and "zipped"
settings to exactly what they were before the patch, as was promised.


I had not inferred from any of this that what was being done via
NativeCompilation.gmk was in any way a problem. I would have expected
any problems there to be readily seen before this was reviewed and
pushed. So I'm a bit confused about this.


The changes in NativeCompilation.gmk looked perfectly fine. They turned
out to trigger a bug (or, at the very least, very unsuspected behavior)
which was deeply hidden in the hotspot linux makefiles, where setting
DEBUG_BINARIES did indeed enable the -g flag, but also, as a side
effect, turned fastdebug builds into slowdebug builds. This was not
something that we could possibly forsee.


That was my understanding too - that this was simply a side-effect on 
hotspot, hence my suggestion to rename the variable. But Erik then 
said this was also not working correctly for the JDK side - which 
means the problem is far worse.


JDK-8036003 introduced the the use of DEBUG_BINARIES in jdk code as 
well. So it "affects" jdk libraries as well, but not with the kind of 
performance regression. However, it did enable debug information were we 
didn't have it before in the Oracle builds. While I believe this is a 
good thing, it should be added under more controlled forms. Therefore we 
want to revert the effect of DEBUG_BINARIES for the kinds of build we do 
at Oracle (external/zipped), keep the effect added in JDK-8036003 for 
the new kind of builds the community requested (internal), and finally 
we want to enable debug symbols for all our jdk libraries as a separate 
change, JDK-8145596.


The only thing left to do is for the community to provide a fix to the 
problem that the newly added functionality of debug-symbols=internal 
means that hotspot in effect turns into a slowdebug build. Since the 
community regularly builds by setting DEBUG_BINARIES=true on the command 
line, it's not really a regression for them, but it's still a bug since 
they get unnecessarily slow builds. Either they need to provide a patch, 
or wait for the new hotspot build where this won't be a problem.


/Magnus


It was not "far worse", but



David
-




I'm tempted to say rollback the whole thing rather than hack at it.


I don't think there is any compelling reason to rollback the whole
change. The basic idea is sound. However, we do need to work on how to
handle the debug symbols "under the hood". This deficit has been known
for a long time to me and Erik but we have not spent any time on it.
This change brought the problems up to daylight, and I think that's a
good thing.

I've started working on JDK-8145596, which will provide a proper
solution to how we handle debug symbols. It is based on JDK-8036003.

/Magnus




And apologies as I'm just about to go offline for a few hours at least.

David
-




We will followup with a more complete fix.

/Erik

Thanks,
David


/Erik

On 2015-12-16 22:34, Erik Joelsson wrote:

Hello,

Please review this quick fix for the build issue introduced in
Hotspot
by JDK-8036003. The short story is that if you set
DEBUG_BINARIES=true
when building Hotspot fastdebug, you essentially get a slowdebug
build. For an explanation of why, see comment in bug. This
behavior is
of course also a bug, but not something I will address in this 
quick

fix.

What happened in JDK-8036003 was that a new configure API for
controlling debug symbols was introduced. The two main settings of
this new parameter, --with-native-debug-symbols, that we use
internally at Oracle are "external" and "zipped". It was 
important to

us that the behavior of these did not change with JDK-8036003, but
exactly that did happen, because both of these settings now cause
DEBUG_BINARIES=true to be set. This variable has never been set by
configure before and because of the above weird behavior in the
Hotspot makefiles, we are having problems.

My proposed quick fix is to not set DEBUG_BINARIES=true for
"

Re: RFR: JDK-8145564: 8036003: startup regression on linux fastdebug builds

[Magnus Ihse Bursie]

On 2015-12-17 22:32, David Holmes wrote:

On 17/12/2015 7:24 PM, Erik Joelsson wrote:

On 2015-12-17 09:08, David Holmes wrote:

On 17/12/2015 5:54 PM, Erik Joelsson wrote:



On 2015-12-17 01:40, David Holmes wrote:

On 17/12/2015 7:35 AM, Erik Joelsson wrote:

One more thing, where should this fix be pushed? Do you need it
urgently
in hs-rt?


It is urgently needed in both the hs-rt and hs-comp repos as it
affects nightly testing and JPRT. If Alejandro agrees I suggest
pushing to jdk9/hs and it can then be pulled down to the team repos.


Will do.

With regard to the fix ... previously DEBUG_BINARIES was never set in
spec.gmk and so was never externally introduced into the hotspot 
build
this way. So why not simply change the name of the variable so 
that it

has no affect on the hotspot part of the build?


Well, we don't want it affecting the jdk part of the build either at
this point. This patch aims to restore the "external" and "zipped"
settings to exactly what they were before the patch, as was promised.


I had not inferred from any of this that what was being done via
NativeCompilation.gmk was in any way a problem. I would have expected
any problems there to be readily seen before this was reviewed and
pushed. So I'm a bit confused about this.


I didn't follow this particular review closely as Magnus was on it and
so I had missed the NativeCompilation part. It's true that it is very
unlikely to be part of the problem described in this bug, but I still
feel that the safest action at this point is to restore the behavior of
"external" and "zipped" to what they used to be.


My concern is recognizing that these adjustments do in fact restore 
the old behaviour. From the hotspot side it seemed cleaner to avoid 
the breakage by using a different variable name.


I'm not sure about this talk about a different variable name? The 
*point* of the patch was *exactly* to set DEBUG_BINARIES=true for 
internal builds. The problem was that it also set it for 
external/zipped, which was incorrect. But neither the writer of the 
patch nor me (or any other reviewer) realized that this would have this 
"hammer" effect.






Magnus is working on a
complete fix where debug symbols will be enabled for everything in
NativeCompilation.

I'm tempted to say rollback the whole thing rather than hack at it.


Rolling back will be much more work for me than submitting this patch.
There are two changes already that depend on this change. I also don't
dislike the idea of the new configure parameter.


Rolling back the new configure parameters and then reinstating them 
again would also not win us any friends as it would be very 
disruptive. As others have noted the way we introduce and remove 
configure parameters needs to be looked at.
This makes it sound like we're sloppy with configure arguments when we 
actually try hard not to be.


Normally, we don't have any problems with introducing new configure 
arguments. Most often, when we introduce a new argument, the behaviour 
falls back to the old behaviour as default if the argument is not 
specified. From time to time, we actually need to change the behaviour 
of configure, and this does not really apply. But then again, all 
modification somehow change behaviour regardless of if any options are 
changed, an all changes in behaviour breaks someones workflow 
(compulsory xkcd reference: https://xkcd.com/1172/).


In this case, not setting the new configure option resulted in the old 
default behaviour.


As for removing options, we are more conservative. We never remove 
options, we just deprecate them. (With a few exceptions, e.g. sometimes 
we have introduced temporary options which are clearly announced as 
such.) Our plan is to remove deprecated options once some time has 
passed (e.g. next major release) but, like the Java language itself, so 
far we have not actually removed any deprecated options. :-)


When we deprecate an option, we ignore the value it sets, but prints a 
warning stating that the option is deprecated. The configure call will 
not fail, though. The warning is repeated as the very last thing so it 
should be easy for the user to spot, like this:


The following warnings were produced. Repeated here for convenience:
WARNING: Option --with-override-jaxp is deprecated and will be ignored.

In some cases we've tried to write some "glue" to interpret old and 
deprecated options in terms of new. (I only think we've done this in the 
closed source). I don't really like it. It means a lot of tests, 
handling situations like what if both old and new are set, and they 
conflict? Or if both old and new are set and they do *not* conflict? 
What if the behaviour has changed slightly, so it does not really match? 
When should we stop helping the user to convert from the old to the new?


I was mainly concerned that the out-of-the-box default behaviour was 
unchanged.


The stated goal of the patch was that out of the box behaviour should 
have been unchanged. 

Re: RFR: JDK-8145564: 8036003: startup regression on linux fastdebug builds

[Magnus Ihse Bursie]

On 2015-12-17 09:08, David Holmes wrote:

On 17/12/2015 5:54 PM, Erik Joelsson wrote:



On 2015-12-17 01:40, David Holmes wrote:

On 17/12/2015 7:35 AM, Erik Joelsson wrote:
One more thing, where should this fix be pushed? Do you need it 
urgently

in hs-rt?


It is urgently needed in both the hs-rt and hs-comp repos as it
affects nightly testing and JPRT. If Alejandro agrees I suggest
pushing to jdk9/hs and it can then be pulled down to the team repos.


Will do.

With regard to the fix ... previously DEBUG_BINARIES was never set in
spec.gmk and so was never externally introduced into the hotspot build
this way. So why not simply change the name of the variable so that it
has no affect on the hotspot part of the build?


Well, we don't want it affecting the jdk part of the build either at
this point. This patch aims to restore the "external" and "zipped"
settings to exactly what they were before the patch, as was promised.


I had not inferred from any of this that what was being done via 
NativeCompilation.gmk was in any way a problem. I would have expected 
any problems there to be readily seen before this was reviewed and 
pushed. So I'm a bit confused about this.


The changes in NativeCompilation.gmk looked perfectly fine. They turned 
out to trigger a bug (or, at the very least, very unsuspected behavior) 
which was deeply hidden in the hotspot linux makefiles, where setting 
DEBUG_BINARIES did indeed enable the -g flag, but also, as a side 
effect, turned fastdebug builds into slowdebug builds. This was not 
something that we could possibly forsee.




I'm tempted to say rollback the whole thing rather than hack at it.


I don't think there is any compelling reason to rollback the whole 
change. The basic idea is sound. However, we do need to work on how to 
handle the debug symbols "under the hood". This deficit has been known 
for a long time to me and Erik but we have not spent any time on it. 
This change brought the problems up to daylight, and I think that's a 
good thing.


I've started working on JDK-8145596, which will provide a proper 
solution to how we handle debug symbols. It is based on JDK-8036003.


/Magnus




And apologies as I'm just about to go offline for a few hours at least.

David
-




We will followup with a more complete fix.

/Erik

Thanks,
David


/Erik

On 2015-12-16 22:34, Erik Joelsson wrote:

Hello,

Please review this quick fix for the build issue introduced in 
Hotspot
by JDK-8036003. The short story is that if you set 
DEBUG_BINARIES=true

when building Hotspot fastdebug, you essentially get a slowdebug
build. For an explanation of why, see comment in bug. This 
behavior is

of course also a bug, but not something I will address in this quick
fix.

What happened in JDK-8036003 was that a new configure API for
controlling debug symbols was introduced. The two main settings of
this new parameter, --with-native-debug-symbols, that we use
internally at Oracle are "external" and "zipped". It was important to
us that the behavior of these did not change with JDK-8036003, but
exactly that did happen, because both of these settings now cause
DEBUG_BINARIES=true to be set. This variable has never been set by
configure before and because of the above weird behavior in the
Hotspot makefiles, we are having problems.

My proposed quick fix is to not set DEBUG_BINARIES=true for 
"external"

or "zipped". It can remain true for "internal" since Oracle never
builds it that way, and I understand those that requested this new
configure parameter were setting DEBUG_BINARIES=true as a workaround
before this anyway, so they should be fine with the broken fastdebug
behavior for a while more. I will file a follow up bug to properly
clean up this mess, but it will take some more time.

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

/Erik

Re: RFR: JDK-8145564: 8036003: startup regression on linux fastdebug builds

[David Holmes]

On 17/12/2015 5:54 PM, Erik Joelsson wrote:



On 2015-12-17 01:40, David Holmes wrote:

On 17/12/2015 7:35 AM, Erik Joelsson wrote:

One more thing, where should this fix be pushed? Do you need it urgently
in hs-rt?


It is urgently needed in both the hs-rt and hs-comp repos as it
affects nightly testing and JPRT. If Alejandro agrees I suggest
pushing to jdk9/hs and it can then be pulled down to the team repos.


Will do.

With regard to the fix ... previously DEBUG_BINARIES was never set in
spec.gmk and so was never externally introduced into the hotspot build
this way. So why not simply change the name of the variable so that it
has no affect on the hotspot part of the build?


Well, we don't want it affecting the jdk part of the build either at
this point. This patch aims to restore the "external" and "zipped"
settings to exactly what they were before the patch, as was promised.


I had not inferred from any of this that what was being done via 
NativeCompilation.gmk was in any way a problem. I would have expected 
any problems there to be readily seen before this was reviewed and 
pushed. So I'm a bit confused about this.


I'm tempted to say rollback the whole thing rather than hack at it.

And apologies as I'm just about to go offline for a few hours at least.

David
-




We will followup with a more complete fix.

/Erik

Thanks,
David


/Erik

On 2015-12-16 22:34, Erik Joelsson wrote:

Hello,

Please review this quick fix for the build issue introduced in Hotspot
by JDK-8036003. The short story is that if you set DEBUG_BINARIES=true
when building Hotspot fastdebug, you essentially get a slowdebug
build. For an explanation of why, see comment in bug. This behavior is
of course also a bug, but not something I will address in this quick
fix.

What happened in JDK-8036003 was that a new configure API for
controlling debug symbols was introduced. The two main settings of
this new parameter, --with-native-debug-symbols, that we use
internally at Oracle are "external" and "zipped". It was important to
us that the behavior of these did not change with JDK-8036003, but
exactly that did happen, because both of these settings now cause
DEBUG_BINARIES=true to be set. This variable has never been set by
configure before and because of the above weird behavior in the
Hotspot makefiles, we are having problems.

My proposed quick fix is to not set DEBUG_BINARIES=true for "external"
or "zipped". It can remain true for "internal" since Oracle never
builds it that way, and I understand those that requested this new
configure parameter were setting DEBUG_BINARIES=true as a workaround
before this anyway, so they should be fine with the broken fastdebug
behavior for a while more. I will file a follow up bug to properly
clean up this mess, but it will take some more time.

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

/Erik

Re: RFR: JDK-8145564: 8036003: startup regression on linux fastdebug builds

[Daniel D. Daugherty]

DEBUG_BINARIES is one of those "hidden" HotSpot big hammers that only
affects Linux (IIRC). Basically, many years ago someone got tired of
trying to figure out how to get completely debuggable HotSpot build
on Linux and this big hammer was dropped in. I could chase down who
and when, but I don't think that really matters...

When I did FDS a few years back, I was asked to not break the semantics
of DEBUG_BINARIES and so the big hammer was left in. Solaris is my
primary dev platform and DEBUG_BINARIES doesn't work there so I didn't
mind leaving in DEBUG_BINARIES for the Linux folks...

Fast forward to today... I don't think the entire patch needs to be
backed out. Not touching DEBUG_BINARIES via configure is a "good idea"
(TM) because it is such a big hammer. I do recommend trying to deprecate
the DEBUG_BINARIES setting in the big HotSpot Makefile rewrite...

I'm very much looking forward to a cleaner HotSpot build...

Dan


On 12/17/15 2:24 AM, Erik Joelsson wrote:



On 2015-12-17 09:08, David Holmes wrote:

On 17/12/2015 5:54 PM, Erik Joelsson wrote:



On 2015-12-17 01:40, David Holmes wrote:

On 17/12/2015 7:35 AM, Erik Joelsson wrote:
One more thing, where should this fix be pushed? Do you need it 
urgently

in hs-rt?


It is urgently needed in both the hs-rt and hs-comp repos as it
affects nightly testing and JPRT. If Alejandro agrees I suggest
pushing to jdk9/hs and it can then be pulled down to the team repos.


Will do.

With regard to the fix ... previously DEBUG_BINARIES was never set in
spec.gmk and so was never externally introduced into the hotspot build
this way. So why not simply change the name of the variable so that it
has no affect on the hotspot part of the build?


Well, we don't want it affecting the jdk part of the build either at
this point. This patch aims to restore the "external" and "zipped"
settings to exactly what they were before the patch, as was promised.


I had not inferred from any of this that what was being done via 
NativeCompilation.gmk was in any way a problem. I would have expected 
any problems there to be readily seen before this was reviewed and 
pushed. So I'm a bit confused about this.


I didn't follow this particular review closely as Magnus was on it and 
so I had missed the NativeCompilation part. It's true that it is very 
unlikely to be part of the problem described in this bug, but I still 
feel that the safest action at this point is to restore the behavior 
of "external" and "zipped" to what they used to be. Magnus is working 
on a complete fix where debug symbols will be enabled for everything 
in NativeCompilation.

I'm tempted to say rollback the whole thing rather than hack at it.

Rolling back will be much more work for me than submitting this patch. 
There are two changes already that depend on this change. I also don't 
dislike the idea of the new configure parameter.

And apologies as I'm just about to go offline for a few hours at least.

I hope you won't object to me pushing this with just ihse as reviewer. 
I feel this is rather a priority to get fixed.


/Erik

Re: RFR: JDK-8145564: 8036003: startup regression on linux fastdebug builds

[David Holmes]

On 18/12/2015 1:58 AM, Daniel D. Daugherty wrote:

DEBUG_BINARIES is one of those "hidden" HotSpot big hammers that only
affects Linux (IIRC). Basically, many years ago someone got tired of
trying to figure out how to get completely debuggable HotSpot build
on Linux and this big hammer was dropped in. I could chase down who
and when, but I don't think that really matters...


Unfortunately it also got "cloned" into BSD and AIX ports.

Thanks for the additional info.

David
-


When I did FDS a few years back, I was asked to not break the semantics
of DEBUG_BINARIES and so the big hammer was left in. Solaris is my
primary dev platform and DEBUG_BINARIES doesn't work there so I didn't
mind leaving in DEBUG_BINARIES for the Linux folks...

Fast forward to today... I don't think the entire patch needs to be
backed out. Not touching DEBUG_BINARIES via configure is a "good idea"
(TM) because it is such a big hammer. I do recommend trying to deprecate
the DEBUG_BINARIES setting in the big HotSpot Makefile rewrite...

I'm very much looking forward to a cleaner HotSpot build...

Dan


On 12/17/15 2:24 AM, Erik Joelsson wrote:



On 2015-12-17 09:08, David Holmes wrote:

On 17/12/2015 5:54 PM, Erik Joelsson wrote:



On 2015-12-17 01:40, David Holmes wrote:

On 17/12/2015 7:35 AM, Erik Joelsson wrote:

One more thing, where should this fix be pushed? Do you need it
urgently
in hs-rt?


It is urgently needed in both the hs-rt and hs-comp repos as it
affects nightly testing and JPRT. If Alejandro agrees I suggest
pushing to jdk9/hs and it can then be pulled down to the team repos.


Will do.

With regard to the fix ... previously DEBUG_BINARIES was never set in
spec.gmk and so was never externally introduced into the hotspot build
this way. So why not simply change the name of the variable so that it
has no affect on the hotspot part of the build?


Well, we don't want it affecting the jdk part of the build either at
this point. This patch aims to restore the "external" and "zipped"
settings to exactly what they were before the patch, as was promised.


I had not inferred from any of this that what was being done via
NativeCompilation.gmk was in any way a problem. I would have expected
any problems there to be readily seen before this was reviewed and
pushed. So I'm a bit confused about this.


I didn't follow this particular review closely as Magnus was on it and
so I had missed the NativeCompilation part. It's true that it is very
unlikely to be part of the problem described in this bug, but I still
feel that the safest action at this point is to restore the behavior
of "external" and "zipped" to what they used to be. Magnus is working
on a complete fix where debug symbols will be enabled for everything
in NativeCompilation.

I'm tempted to say rollback the whole thing rather than hack at it.


Rolling back will be much more work for me than submitting this patch.
There are two changes already that depend on this change. I also don't
dislike the idea of the new configure parameter.

And apologies as I'm just about to go offline for a few hours at least.


I hope you won't object to me pushing this with just ihse as reviewer.
I feel this is rather a priority to get fixed.

/Erik

Re: RFR: JDK-8145564: 8036003: startup regression on linux fastdebug builds

[David Holmes]

On 17/12/2015 6:18 PM, Magnus Ihse Bursie wrote:

On 2015-12-17 09:08, David Holmes wrote:

On 17/12/2015 5:54 PM, Erik Joelsson wrote:



On 2015-12-17 01:40, David Holmes wrote:

On 17/12/2015 7:35 AM, Erik Joelsson wrote:

One more thing, where should this fix be pushed? Do you need it
urgently
in hs-rt?


It is urgently needed in both the hs-rt and hs-comp repos as it
affects nightly testing and JPRT. If Alejandro agrees I suggest
pushing to jdk9/hs and it can then be pulled down to the team repos.


Will do.

With regard to the fix ... previously DEBUG_BINARIES was never set in
spec.gmk and so was never externally introduced into the hotspot build
this way. So why not simply change the name of the variable so that it
has no affect on the hotspot part of the build?


Well, we don't want it affecting the jdk part of the build either at
this point. This patch aims to restore the "external" and "zipped"
settings to exactly what they were before the patch, as was promised.


I had not inferred from any of this that what was being done via
NativeCompilation.gmk was in any way a problem. I would have expected
any problems there to be readily seen before this was reviewed and
pushed. So I'm a bit confused about this.


The changes in NativeCompilation.gmk looked perfectly fine. They turned
out to trigger a bug (or, at the very least, very unsuspected behavior)
which was deeply hidden in the hotspot linux makefiles, where setting
DEBUG_BINARIES did indeed enable the -g flag, but also, as a side
effect, turned fastdebug builds into slowdebug builds. This was not
something that we could possibly forsee.


That was my understanding too - that this was simply a side-effect on 
hotspot, hence my suggestion to rename the variable. But Erik then said 
this was also not working correctly for the JDK side - which means the 
problem is far worse.


David
-




I'm tempted to say rollback the whole thing rather than hack at it.


I don't think there is any compelling reason to rollback the whole
change. The basic idea is sound. However, we do need to work on how to
handle the debug symbols "under the hood". This deficit has been known
for a long time to me and Erik but we have not spent any time on it.
This change brought the problems up to daylight, and I think that's a
good thing.

I've started working on JDK-8145596, which will provide a proper
solution to how we handle debug symbols. It is based on JDK-8036003.

/Magnus




And apologies as I'm just about to go offline for a few hours at least.

David
-




We will followup with a more complete fix.

/Erik

Thanks,
David


/Erik

On 2015-12-16 22:34, Erik Joelsson wrote:

Hello,

Please review this quick fix for the build issue introduced in
Hotspot
by JDK-8036003. The short story is that if you set
DEBUG_BINARIES=true
when building Hotspot fastdebug, you essentially get a slowdebug
build. For an explanation of why, see comment in bug. This
behavior is
of course also a bug, but not something I will address in this quick
fix.

What happened in JDK-8036003 was that a new configure API for
controlling debug symbols was introduced. The two main settings of
this new parameter, --with-native-debug-symbols, that we use
internally at Oracle are "external" and "zipped". It was important to
us that the behavior of these did not change with JDK-8036003, but
exactly that did happen, because both of these settings now cause
DEBUG_BINARIES=true to be set. This variable has never been set by
configure before and because of the above weird behavior in the
Hotspot makefiles, we are having problems.

My proposed quick fix is to not set DEBUG_BINARIES=true for
"external"
or "zipped". It can remain true for "internal" since Oracle never
builds it that way, and I understand those that requested this new
configure parameter were setting DEBUG_BINARIES=true as a workaround
before this anyway, so they should be fine with the broken fastdebug
behavior for a while more. I will file a follow up bug to properly
clean up this mess, but it will take some more time.

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

/Erik

Re: RFR: JDK-8145564: 8036003: startup regression on linux fastdebug builds

[David Holmes]

On 17/12/2015 7:24 PM, Erik Joelsson wrote:

On 2015-12-17 09:08, David Holmes wrote:

On 17/12/2015 5:54 PM, Erik Joelsson wrote:



On 2015-12-17 01:40, David Holmes wrote:

On 17/12/2015 7:35 AM, Erik Joelsson wrote:

One more thing, where should this fix be pushed? Do you need it
urgently
in hs-rt?


It is urgently needed in both the hs-rt and hs-comp repos as it
affects nightly testing and JPRT. If Alejandro agrees I suggest
pushing to jdk9/hs and it can then be pulled down to the team repos.


Will do.

With regard to the fix ... previously DEBUG_BINARIES was never set in
spec.gmk and so was never externally introduced into the hotspot build
this way. So why not simply change the name of the variable so that it
has no affect on the hotspot part of the build?


Well, we don't want it affecting the jdk part of the build either at
this point. This patch aims to restore the "external" and "zipped"
settings to exactly what they were before the patch, as was promised.


I had not inferred from any of this that what was being done via
NativeCompilation.gmk was in any way a problem. I would have expected
any problems there to be readily seen before this was reviewed and
pushed. So I'm a bit confused about this.


I didn't follow this particular review closely as Magnus was on it and
so I had missed the NativeCompilation part. It's true that it is very
unlikely to be part of the problem described in this bug, but I still
feel that the safest action at this point is to restore the behavior of
"external" and "zipped" to what they used to be.


My concern is recognizing that these adjustments do in fact restore the 
old behaviour. From the hotspot side it seemed cleaner to avoid the 
breakage by using a different variable name.



Magnus is working on a
complete fix where debug symbols will be enabled for everything in
NativeCompilation.

I'm tempted to say rollback the whole thing rather than hack at it.


Rolling back will be much more work for me than submitting this patch.
There are two changes already that depend on this change. I also don't
dislike the idea of the new configure parameter.


Rolling back the new configure parameters and then reinstating them 
again would also not win us any friends as it would be very disruptive. 
As others have noted the way we introduce and remove configure 
parameters needs to be looked at. I was mainly concerned that the 
out-of-the-box default behaviour was unchanged.



And apologies as I'm just about to go offline for a few hours at least.


I hope you won't object to me pushing this with just ihse as reviewer. I
feel this is rather a priority to get fixed.


Sure. I had verified that DEBUG_BINARIES is only ever tested against 
"true" so setting it to false should be as effective as not setting it 
at all.


I'll follow up separately to see where this was pushed and whether we 
need to pull it into anywhere else urgently.


Thanks,
David


/Erik

Re: RFR: JDK-8145564: 8036003: startup regression on linux fastdebug builds

[Magnus Ihse Bursie]

On 2015-12-16 22:34, Erik Joelsson wrote:

Hello,

Please review this quick fix for the build issue introduced in Hotspot 
by JDK-8036003. The short story is that if you set DEBUG_BINARIES=true 
when building Hotspot fastdebug, you essentially get a slowdebug 
build. For an explanation of why, see comment in bug. This behavior is 
of course also a bug, but not something I will address in this quick fix.


What happened in JDK-8036003 was that a new configure API for 
controlling debug symbols was introduced. The two main settings of 
this new parameter, --with-native-debug-symbols, that we use 
internally at Oracle are "external" and "zipped". It was important to 
us that the behavior of these did not change with JDK-8036003, but 
exactly that did happen, because both of these settings now cause 
DEBUG_BINARIES=true to be set. This variable has never been set by 
configure before and because of the above weird behavior in the 
Hotspot makefiles, we are having problems.


My proposed quick fix is to not set DEBUG_BINARIES=true for "external" 
or "zipped". It can remain true for "internal" since Oracle never 
builds it that way, and I understand those that requested this new 
configure parameter were setting DEBUG_BINARIES=true as a workaround 
before this anyway, so they should be fine with the broken fastdebug 
behavior for a while more. I will file a follow up bug to properly 
clean up this mess, but it will take some more time.


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


Looks good to me.

/Magnus

Re: RFR: JDK-8145564: 8036003: startup regression on linux fastdebug builds

[Erik Joelsson]

One more thing, where should this fix be pushed? Do you need it urgently 
in hs-rt?


/Erik

On 2015-12-16 22:34, Erik Joelsson wrote:

Hello,

Please review this quick fix for the build issue introduced in Hotspot 
by JDK-8036003. The short story is that if you set DEBUG_BINARIES=true 
when building Hotspot fastdebug, you essentially get a slowdebug 
build. For an explanation of why, see comment in bug. This behavior is 
of course also a bug, but not something I will address in this quick fix.


What happened in JDK-8036003 was that a new configure API for 
controlling debug symbols was introduced. The two main settings of 
this new parameter, --with-native-debug-symbols, that we use 
internally at Oracle are "external" and "zipped". It was important to 
us that the behavior of these did not change with JDK-8036003, but 
exactly that did happen, because both of these settings now cause 
DEBUG_BINARIES=true to be set. This variable has never been set by 
configure before and because of the above weird behavior in the 
Hotspot makefiles, we are having problems.


My proposed quick fix is to not set DEBUG_BINARIES=true for "external" 
or "zipped". It can remain true for "internal" since Oracle never 
builds it that way, and I understand those that requested this new 
configure parameter were setting DEBUG_BINARIES=true as a workaround 
before this anyway, so they should be fine with the broken fastdebug 
behavior for a while more. I will file a follow up bug to properly 
clean up this mess, but it will take some more time.


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

/Erik

Re: RFR: JDK-8145564: 8036003: startup regression on linux fastdebug builds

[Thomas Stüfe]

Hi Eric,

short question, are other platforms beside Linux affected or is this
Linux-specific (I saw you said Windows x64 showed no regression)?

Reason I ask is, we just changed the default for AIX to "internal" because
this is the only configuration we support and the build was broken
after  JDK-8036003
(see  https://bugs.openjdk.java.net/browse/JDK-8145560)

Kind Regards, Thomas


On Wed, Dec 16, 2015 at 10:34 PM, Erik Joelsson <erik.joels...@oracle.com>
wrote:

> Hello,
>
> Please review this quick fix for the build issue introduced in Hotspot by
> JDK-8036003. The short story is that if you set DEBUG_BINARIES=true when
> building Hotspot fastdebug, you essentially get a slowdebug build. For an
> explanation of why, see comment in bug. This behavior is of course also a
> bug, but not something I will address in this quick fix.
>
> What happened in JDK-8036003 was that a new configure API for controlling
> debug symbols was introduced. The two main settings of this new parameter,
> --with-native-debug-symbols, that we use internally at Oracle are
> "external" and "zipped". It was important to us that the behavior of these
> did not change with JDK-8036003, but exactly that did happen, because both
> of these settings now cause DEBUG_BINARIES=true to be set. This variable
> has never been set by configure before and because of the above weird
> behavior in the Hotspot makefiles, we are having problems.
>
> My proposed quick fix is to not set DEBUG_BINARIES=true for "external" or
> "zipped". It can remain true for "internal" since Oracle never builds it
> that way, and I understand those that requested this new configure
> parameter were setting DEBUG_BINARIES=true as a workaround before this
> anyway, so they should be fine with the broken fastdebug behavior for a
> while more. I will file a follow up bug to properly clean up this mess, but
> it will take some more time.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8145564
> Webrev: http://cr.openjdk.java.net/~erikj/8145564/webrev.01/
>
> /Erik
>

Re: RFR: JDK-8145564: 8036003: startup regression on linux fastdebug builds

[Alejandro E Murillo]

On 12/16/2015 5:40 PM, David Holmes wrote:

On 17/12/2015 7:35 AM, Erik Joelsson wrote:

One more thing, where should this fix be pushed? Do you need it urgently
in hs-rt?


It is urgently needed in both the hs-rt and hs-comp repos as it 
affects nightly testing and JPRT. If Alejandro agrees I suggest 
pushing to jdk9/hs and it can then be pulled down to the team repos.

Fine with me.
It will go through PIT this weekend
Alejandro



With regard to the fix ... previously DEBUG_BINARIES was never set in 
spec.gmk and so was never externally introduced into the hotspot build 
this way. So why not simply change the name of the variable so that it 
has no affect on the hotspot part of the build?


Thanks,
David


/Erik

On 2015-12-16 22:34, Erik Joelsson wrote:

Hello,

Please review this quick fix for the build issue introduced in Hotspot
by JDK-8036003. The short story is that if you set DEBUG_BINARIES=true
when building Hotspot fastdebug, you essentially get a slowdebug
build. For an explanation of why, see comment in bug. This behavior is
of course also a bug, but not something I will address in this quick 
fix.


What happened in JDK-8036003 was that a new configure API for
controlling debug symbols was introduced. The two main settings of
this new parameter, --with-native-debug-symbols, that we use
internally at Oracle are "external" and "zipped". It was important to
us that the behavior of these did not change with JDK-8036003, but
exactly that did happen, because both of these settings now cause
DEBUG_BINARIES=true to be set. This variable has never been set by
configure before and because of the above weird behavior in the
Hotspot makefiles, we are having problems.

My proposed quick fix is to not set DEBUG_BINARIES=true for "external"
or "zipped". It can remain true for "internal" since Oracle never
builds it that way, and I understand those that requested this new
configure parameter were setting DEBUG_BINARIES=true as a workaround
before this anyway, so they should be fine with the broken fastdebug
behavior for a while more. I will file a follow up bug to properly
clean up this mess, but it will take some more time.

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

/Erik




--
Alejandro

Re: RFR: JDK-8145564: 8036003: startup regression on linux fastdebug builds

[David Holmes]

On 17/12/2015 7:35 AM, Erik Joelsson wrote:

One more thing, where should this fix be pushed? Do you need it urgently
in hs-rt?


It is urgently needed in both the hs-rt and hs-comp repos as it affects 
nightly testing and JPRT. If Alejandro agrees I suggest pushing to 
jdk9/hs and it can then be pulled down to the team repos.


With regard to the fix ... previously DEBUG_BINARIES was never set in 
spec.gmk and so was never externally introduced into the hotspot build 
this way. So why not simply change the name of the variable so that it 
has no affect on the hotspot part of the build?


Thanks,
David


/Erik

On 2015-12-16 22:34, Erik Joelsson wrote:

Hello,

Please review this quick fix for the build issue introduced in Hotspot
by JDK-8036003. The short story is that if you set DEBUG_BINARIES=true
when building Hotspot fastdebug, you essentially get a slowdebug
build. For an explanation of why, see comment in bug. This behavior is
of course also a bug, but not something I will address in this quick fix.

What happened in JDK-8036003 was that a new configure API for
controlling debug symbols was introduced. The two main settings of
this new parameter, --with-native-debug-symbols, that we use
internally at Oracle are "external" and "zipped". It was important to
us that the behavior of these did not change with JDK-8036003, but
exactly that did happen, because both of these settings now cause
DEBUG_BINARIES=true to be set. This variable has never been set by
configure before and because of the above weird behavior in the
Hotspot makefiles, we are having problems.

My proposed quick fix is to not set DEBUG_BINARIES=true for "external"
or "zipped". It can remain true for "internal" since Oracle never
builds it that way, and I understand those that requested this new
configure parameter were setting DEBUG_BINARIES=true as a workaround
before this anyway, so they should be fine with the broken fastdebug
behavior for a while more. I will file a follow up bug to properly
clean up this mess, but it will take some more time.

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

/Erik

Re: RFR: JDK-8145564: 8036003: startup regression on linux fastdebug builds

[Erik Joelsson]

On 2015-12-17 01:40, David Holmes wrote:

On 17/12/2015 7:35 AM, Erik Joelsson wrote:

One more thing, where should this fix be pushed? Do you need it urgently
in hs-rt?


It is urgently needed in both the hs-rt and hs-comp repos as it 
affects nightly testing and JPRT. If Alejandro agrees I suggest 
pushing to jdk9/hs and it can then be pulled down to the team repos.



Will do.
With regard to the fix ... previously DEBUG_BINARIES was never set in 
spec.gmk and so was never externally introduced into the hotspot build 
this way. So why not simply change the name of the variable so that it 
has no affect on the hotspot part of the build?


Well, we don't want it affecting the jdk part of the build either at 
this point. This patch aims to restore the "external" and "zipped" 
settings to exactly what they were before the patch, as was promised.


We will followup with a more complete fix.

/Erik

Thanks,
David


/Erik

On 2015-12-16 22:34, Erik Joelsson wrote:

Hello,

Please review this quick fix for the build issue introduced in Hotspot
by JDK-8036003. The short story is that if you set DEBUG_BINARIES=true
when building Hotspot fastdebug, you essentially get a slowdebug
build. For an explanation of why, see comment in bug. This behavior is
of course also a bug, but not something I will address in this quick 
fix.


What happened in JDK-8036003 was that a new configure API for
controlling debug symbols was introduced. The two main settings of
this new parameter, --with-native-debug-symbols, that we use
internally at Oracle are "external" and "zipped". It was important to
us that the behavior of these did not change with JDK-8036003, but
exactly that did happen, because both of these settings now cause
DEBUG_BINARIES=true to be set. This variable has never been set by
configure before and because of the above weird behavior in the
Hotspot makefiles, we are having problems.

My proposed quick fix is to not set DEBUG_BINARIES=true for "external"
or "zipped". It can remain true for "internal" since Oracle never
builds it that way, and I understand those that requested this new
configure parameter were setting DEBUG_BINARIES=true as a workaround
before this anyway, so they should be fine with the broken fastdebug
behavior for a while more. I will file a follow up bug to properly
clean up this mess, but it will take some more time.

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

/Erik

Re: Why does a fastdebug build compile with -O3 *and* -O0?

[Severin Gehwolf]

Hi Mikael,

Adding in build-dev for input.

On Tue, 2015-04-28 at 10:55 +0200, Mikael Gerdin wrote:
 Severin,
 
 On 2015-04-27 16:33, Severin Gehwolf wrote:
  Hi,
 
  I've noticed that a fastdebug build of recent hs-rt compiles objects
  with -O3 *and* -O0. Is this a bug or a feature? If it's a feature,
  what's the rationale?
 
 This does not appear to be the case when I build on my machine.
 I also don't get -fstack-protector-all --param ssp-buffer-size=1
 Those appear to be set in DEBUG_CFLAGS in gcc.make.
 The first line of fastdebug.make uses an interesting hack to set 
 FASTDEBUG_CFLAGS to DEBUG_CFLAGS unless FASTDEBUG_CFLAGS is already set.
 FASTDEBUG_CFLAGS is only set in gcc.make if ENABLE_FULL_DEBUG_SYMBOLS is 0.
 So I tried with:
 hs-rt-open/hotspot$ make -C make/ 
 ALT_BOOTDIR=/usr/lib/jvm/jdk-8-oracle-x64 HOTSPOT_BUILD_JOBS=24 
 ARCH_DATA_MODEL=64 ENABLE_FULL_DEBUG_SYMBOLS=0 fastdebug
 
 And that reproduces your problem. This is most certainly a bug in the 
 build system.

OK, thanks. Any suggestions as to how to fix this? FWIW, I don't see
this problem for JDK 8 fastdebug builds. Example:
/usr/bin/g++ -DLINUX -D_GNU_SOURCE -DAMD64 -DASSERT -DCHECK_UNHANDLED_OOPS -I. 
-I/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk8u-dev/hotspot/src/share/vm/prims
 
-I/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk8u-dev/hotspot/src/share/vm
 
-I/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk8u-dev/hotspot/src/share/vm/precompiled
 
-I/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk8u-dev/hotspot/src/cpu/x86/vm
 
-I/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk8u-dev/hotspot/src/os_cpu/linux_x86/vm
 
-I/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk8u-dev/hotspot/src/os/linux/vm
 
-I/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk8u-dev/hotspot/src/os/posix/vm
 -I../generated -DHOTSPOT_RELEASE_VERSION=\25.60-b12\ 
-DHOTSPOT_BUILD_TARGET=\fastdebug\ -DHOTSPOT_BUILD_USER=\sgehwolf\ 
-DHOTSPOT_LIB_ARCH=\amd64\ -DHOTSPOT_VM_DISTRO=\OpenJDK\  
-DTARGET_OS_FAMILY_linux -DTARGET_ARCH_x86 -DTARGET_ARCH_MODEL_x86_64 
-DTARGET_OS_ARCH_linux_x86 -DTARGET_OS_ARCH_MODEL_linux_x86_64 
-DTARGET_COMPILER_gcc -DCOMPILER2 -DCOMPILER1 -fPIC -fno-rtti -fno-exceptions 
-D_REENTRANT -fcheck-new -fvisibility=hidden -m64  -pipe -fno-strict-aliasing  
-g -fno-omit-frame-pointer -O3  -DVM_LITTLE_ENDIAN -D_LP64=1 -Werror 
-Wpointer-arith -Wsign-compare -Wundef -Wunused-function -Wunused-value
-DDTRACE_ENABLED -c -MMD -MP -MF 
../generated/dependencies/asParNewGeneration.o.d -fpch-deps -o 
asParNewGeneration.o 
/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk8u-dev/hotspot/src/share/vm/gc_implementation/parNew/asParNewGeneration.cpp

My make invocation does indeed have ENABLE_FULL_DEBUG_SYMBOLS=0. The
full make command line is (the comment says it all, but may be outdated
since it's a recurring issue for us distro people):

# In order to get correct debug symbols in libjvm.so we need
# DEBUG_BINARIES=true ENABLE_FULL_DEBUG_SYMBOLS=0 POST_STRIP_CMD=
make \
 SCTP_WERROR= \
 DEBUG_BINARIES=true \
 ENABLE_FULL_DEBUG_SYMBOLS=0 \
 POST_STRIP_CMD= \
 DISABLE_INTREE_EC=true \
 LOG=debug
 images

The rationale for DEBUG_BINARIES=true ENABLE_FULL_DEBUG_SYMBOLS=0
POST_STRIP_CMD= is here (short version: we want full debug symbols in
object files and leave the stripping to rpm):
http://mail.openjdk.java.net/pipermail/build-dev/2014-March/012039.html

Thread started here:
http://mail.openjdk.java.net/pipermail/build-dev/2014-February/012019.html

A somewhat related bug I've filed about this a while ago:
https://bugs.openjdk.java.net/browse/JDK-8073461

If we could fix this proper, then we wouldn't have to use those
non-standard make variables...

Cheers,
Severin

 The hotspot makefiles are horrible.
 
 /Mikael
 
 
 
  I'm not sure what GCC does with that, but it appears it just uses -O0
  (last seen option?).
 
  Example:
  /usr/bin/g++ -DLINUX -D_GNU_SOURCE -DAMD64 -DASSERT -DCHECK_UNHANDLED_OOPS 
  -I. 
  -I/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk9-hs-rt/hotspot/src/share/vm/prims
   
  -I/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk9-hs-rt/hotspot/src/share/vm
   
  -I/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk9-hs-rt/hotspot/src/share/vm/precompiled
   
  -I/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk9-hs-rt/hotspot/src/cpu/x86/vm
   
  -I/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk9-hs-rt/hotspot/src/os_cpu/linux_x86/vm
   
  -I/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk9-hs-rt/hotspot/src/os/linux/vm
   
  -I/home/sgehwolf/Documents/openjdk/upstream-sources/openjdk9-hs-rt/hotspot/src/os/posix/vm
   -I../generated -DHOTSPOT_BUILD_USER=\sgehwolf\ 
  -DHOTSPOT_LIB_ARCH=\amd64\ -DHOTSPOT_VM_DISTRO=\OpenJDK\  
  -DTARGET_OS_FAMILY_linux -DTARGET_ARCH_x86 -DTARGET_ARCH_MODEL_x86_64 
  -DTARGET_OS_ARCH_linux_x86 -DTARGET_OS_ARCH_MODEL_linux_x86_64 -DTARGE!
  T_COMPILER
 _gcc -DCOMPILER2

RE: RFR(S): 8027480 - Build Windows x64 fastdebug builds using /homeparams

[Christian Tornqvist]

Adding build-dev as well

-Original Message-
From: hotspot-dev [mailto:hotspot-dev-boun...@openjdk.java.net] On Behalf Of
Christian Tornqvist
Sent: Tuesday, August 19, 2014 7:39 PM
To: hotspot-...@openjdk.java.net
Subject: RFR(S): 8027480 - Build Windows x64 fastdebug builds using
/homeparams

Hi everyone,

 

This change adds /homeparams
(http://msdn.microsoft.com/en-us/library/6exwh0y6.aspx) to compiler flags
when building fastdebug on Windows x64. This causes the compiler to generate
code to spill the first 4 arguments to the stack (they're normally only
passed in registers), which should make it easier to debug. 

 

I also changed the ProjectCreator to enable building with this using Visual
Studio. The size of jvm.dll increases with about 3% (about 504k increase).

 

Verified that it builds correctly using Visual Studio and JPRT, the
generation of the spill code has been verified by comparing prologue code
for several functions in the JVM with/without using /homeparams. 

 

Webrev:

http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.00/

 

Bug:

https://bugs.openjdk.java.net/browse/JDK-8027480

 

Thanks,

Christian

 

RE: RFR(S): 8027480 - Build Windows x64 fastdebug builds using /homeparams

[Christian Tornqvist]

 Do we have any idea how much of a change? Do we care? I'm presuming the
increased debuggability is worth it.

It adds 0-4 additional stack movs in function prologue code which should
have a very low impact, it's only on debug/fastdebug builds.

 The above block will also apply to an ia64 build. We don't support that
anymore, but I don't know if any licensees support it.

I've changed the checks in vm.make and WinGammaPlatformVC10.java

 Do we need to do anything about the new, but unused  platform to make
lint-like tools not squawk?

I'll open a new bug to clean up the VC7/8/9 files in ProjectCreator.

New webrev:
http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.01/

Thanks,
Christian

-Original Message-
From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com] 
Sent: Wednesday, August 20, 2014 9:48 AM
To: Christian Tornqvist
Cc: hotspot-...@openjdk.java.net
Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds using
/homeparams

  http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.00/

General Comment

The MSDN note says:

  /homeparams does imply a performance disadvantage, because it   does
require a cycle to load the register parameters on to the stack.

Do we have any idea how much of a change? Do we care? I'm presuming the
increased debuggability is worth it.

make/windows/makefiles/vm.make
 line 38: !else
 line 39: CXX_FLAGS=$(CXX_FLAGS) /D ASSERT /homeparams
 line 40: !endif
 The above block will also apply to an ia64 build.
 We don't support that anymore, but I don't know if
 any licensees support it.

src/share/tools/ProjectCreator/BuildConfig.java
 No comments.

src/share/tools/ProjectCreator/WinGammaPlatformVC10.java
 line 373: if(!platformName.equals(Win32)) {
 line 374: addAttr(rv, AdditionalOptions, /homeparams);
 The above block will also apply to an ia64 build.

src/share/tools/ProjectCreator/WinGammaPlatformVC7.java
src/share/tools/ProjectCreator/WinGammaPlatformVC8.java
 Do we need to do anything about the new, but unused
 platform to make lint-like tools not squawk?

Thumbs up.

Dan


On 8/19/14 5:39 PM, Christian Tornqvist wrote:
 Hi everyone,

   

 This change adds /homeparams
 (http://msdn.microsoft.com/en-us/library/6exwh0y6.aspx) to compiler 
 flags when building fastdebug on Windows x64. This causes the compiler 
 to generate code to spill the first 4 arguments to the stack (they're 
 normally only passed in registers), which should make it easier to debug.

   

 I also changed the ProjectCreator to enable building with this using 
 Visual Studio. The size of jvm.dll increases with about 3% (about 504k
increase).

   

 Verified that it builds correctly using Visual Studio and JPRT, the 
 generation of the spill code has been verified by comparing prologue 
 code for several functions in the JVM with/without using /homeparams.

   

 Webrev:

 http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.00/

   

 Bug:

 https://bugs.openjdk.java.net/browse/JDK-8027480

   

 Thanks,

 Christian

   

Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds using /homeparams

[Daniel D. Daugherty]

On 8/20/14 8:49 AM, Christian Tornqvist wrote:

Do we have any idea how much of a change? Do we care? I'm presuming the

increased debuggability is worth it.

It adds 0-4 additional stack movs in function prologue code which should
have a very low impact, it's only on debug/fastdebug builds.


I missed that this was a non-RELEASE build change in my
original review.

These two blurbs from the MSDN note:

 However, by default in a release build, the register parameters
 will not be written to the stack, into the space that is already
 provided for the parameters. This makes it difficult to debug an
 optimized (release) build of your program.

and

 In a debug build, the stack is always populated with parameters
 passed in registers.

indicate that we shouldn't need this change in our debug/fastdebug
builds. However, you looked at the generated prologue code before
and after turning on this option right?

Does this mean that the MSDN note is not correct for the way that
we do debug and fastdebug builds? We might have some option enabled
that prevents the default /homeparams behavior from working...

Dan





The above block will also apply to an ia64 build. We don't support that

anymore, but I don't know if any licensees support it.

I've changed the checks in vm.make and WinGammaPlatformVC10.java


Do we need to do anything about the new, but unused  platform to make

lint-like tools not squawk?

I'll open a new bug to clean up the VC7/8/9 files in ProjectCreator.

New webrev:
http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.01/

Thanks,
Christian

-Original Message-
From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com]
Sent: Wednesday, August 20, 2014 9:48 AM
To: Christian Tornqvist
Cc: hotspot-...@openjdk.java.net
Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds using
/homeparams

   http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.00/

General Comment

The MSDN note says:

   /homeparams does imply a performance disadvantage, because it   does
require a cycle to load the register parameters on to the stack.

Do we have any idea how much of a change? Do we care? I'm presuming the
increased debuggability is worth it.

make/windows/makefiles/vm.make
  line 38: !else
  line 39: CXX_FLAGS=$(CXX_FLAGS) /D ASSERT /homeparams
  line 40: !endif
  The above block will also apply to an ia64 build.
  We don't support that anymore, but I don't know if
  any licensees support it.

src/share/tools/ProjectCreator/BuildConfig.java
  No comments.

src/share/tools/ProjectCreator/WinGammaPlatformVC10.java
  line 373: if(!platformName.equals(Win32)) {
  line 374: addAttr(rv, AdditionalOptions, /homeparams);
  The above block will also apply to an ia64 build.

src/share/tools/ProjectCreator/WinGammaPlatformVC7.java
src/share/tools/ProjectCreator/WinGammaPlatformVC8.java
  Do we need to do anything about the new, but unused
  platform to make lint-like tools not squawk?

Thumbs up.

Dan


On 8/19/14 5:39 PM, Christian Tornqvist wrote:

Hi everyone,

   


This change adds /homeparams
(http://msdn.microsoft.com/en-us/library/6exwh0y6.aspx) to compiler
flags when building fastdebug on Windows x64. This causes the compiler
to generate code to spill the first 4 arguments to the stack (they're
normally only passed in registers), which should make it easier to debug.

   


I also changed the ProjectCreator to enable building with this using
Visual Studio. The size of jvm.dll increases with about 3% (about 504k

increase).
   


Verified that it builds correctly using Visual Studio and JPRT, the
generation of the spill code has been verified by comparing prologue
code for several functions in the JVM with/without using /homeparams.

   


Webrev:

http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.00/

   


Bug:

https://bugs.openjdk.java.net/browse/JDK-8027480

   


Thanks,

Christian

   

RE: RFR(S): 8027480 - Build Windows x64 fastdebug builds using /homeparams

[Christian Tornqvist]

 indicate that we shouldn't need this change in our debug/fastdebug builds.
However, you looked at the generated prologue code before and after turning
on this option right?

Our fastdebug builds are compiled with /O2 which doesn't spill the
parameters onto the stack, our debug builds (with /Od) will do this so
passing /homeparams there is a no-op. 

Here is a look at the prologue code for
ClassFileParser::parse_constant_pool_entries() before /homeparams:

mov dword ptr [rsp+10h],edx
pushrbp
pushrsi

and here is with /homeparams:

mov qword ptr [rsp+18h],r8
mov dword ptr [rsp+10h],edx
mov qword ptr [rsp+8],rcx
pushrbp
pushrsi

Thanks,
Christian

-Original Message-
From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com] 
Sent: Wednesday, August 20, 2014 11:17 AM
To: Christian Tornqvist
Cc: hotspot-...@openjdk.java.net; build-dev@openjdk.java.net
Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds using
/homeparams

On 8/20/14 8:49 AM, Christian Tornqvist wrote:
 Do we have any idea how much of a change? Do we care? I'm presuming 
 the
 increased debuggability is worth it.

 It adds 0-4 additional stack movs in function prologue code which 
 should have a very low impact, it's only on debug/fastdebug builds.

I missed that this was a non-RELEASE build change in my original review.

These two blurbs from the MSDN note:

  However, by default in a release build, the register parameters   will
not be written to the stack, into the space that is already   provided for
the parameters. This makes it difficult to debug an   optimized (release)
build of your program.

and

  In a debug build, the stack is always populated with parameters   passed
in registers.

indicate that we shouldn't need this change in our debug/fastdebug builds.
However, you looked at the generated prologue code before and after turning
on this option right?

Does this mean that the MSDN note is not correct for the way that we do
debug and fastdebug builds? We might have some option enabled that prevents
the default /homeparams behavior from working...

Dan



 The above block will also apply to an ia64 build. We don't support 
 that
 anymore, but I don't know if any licensees support it.

 I've changed the checks in vm.make and WinGammaPlatformVC10.java

 Do we need to do anything about the new, but unused  platform to make
 lint-like tools not squawk?

 I'll open a new bug to clean up the VC7/8/9 files in ProjectCreator.

 New webrev:
 http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.01/

 Thanks,
 Christian

 -Original Message-
 From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com]
 Sent: Wednesday, August 20, 2014 9:48 AM
 To: Christian Tornqvist
 Cc: hotspot-...@openjdk.java.net
 Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds 
 using /homeparams

http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.00/

 General Comment

 The MSDN note says:

/homeparams does imply a performance disadvantage, because it   
 does require a cycle to load the register parameters on to the stack.

 Do we have any idea how much of a change? Do we care? I'm presuming 
 the increased debuggability is worth it.

 make/windows/makefiles/vm.make
   line 38: !else
   line 39: CXX_FLAGS=$(CXX_FLAGS) /D ASSERT /homeparams
   line 40: !endif
   The above block will also apply to an ia64 build.
   We don't support that anymore, but I don't know if
   any licensees support it.

 src/share/tools/ProjectCreator/BuildConfig.java
   No comments.

 src/share/tools/ProjectCreator/WinGammaPlatformVC10.java
   line 373: if(!platformName.equals(Win32)) {
   line 374: addAttr(rv, AdditionalOptions, /homeparams);
   The above block will also apply to an ia64 build.

 src/share/tools/ProjectCreator/WinGammaPlatformVC7.java
 src/share/tools/ProjectCreator/WinGammaPlatformVC8.java
   Do we need to do anything about the new, but unused
   platform to make lint-like tools not squawk?

 Thumbs up.

 Dan


 On 8/19/14 5:39 PM, Christian Tornqvist wrote:
 Hi everyone,



 This change adds /homeparams
 (http://msdn.microsoft.com/en-us/library/6exwh0y6.aspx) to compiler 
 flags when building fastdebug on Windows x64. This causes the 
 compiler to generate code to spill the first 4 arguments to the stack 
 (they're normally only passed in registers), which should make it easier
to debug.



 I also changed the ProjectCreator to enable building with this using 
 Visual Studio. The size of jvm.dll increases with about 3% (about 
 504k
 increase).


 Verified that it builds correctly using Visual Studio and JPRT, the 
 generation of the spill code has been verified by comparing prologue 
 code for several functions in the JVM with/without using /homeparams.



 Webrev:

 http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.00/



 Bug:

 https://bugs.openjdk.java.net/browse/JDK

Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds using /homeparams

[Daniel D. Daugherty]

On 8/20/14 9:48 AM, Christian Tornqvist wrote:

indicate that we shouldn't need this change in our debug/fastdebug builds.

However, you looked at the generated prologue code before and after turning
on this option right?

Our fastdebug builds are compiled with /O2 which doesn't spill the
parameters onto the stack, our debug builds (with /Od) will do this so
passing /homeparams there is a no-op.

Here is a look at the prologue code for
ClassFileParser::parse_constant_pool_entries() before /homeparams:

mov dword ptr [rsp+10h],edx
pushrbp
pushrsi

and here is with /homeparams:

mov qword ptr [rsp+18h],r8
mov dword ptr [rsp+10h],edx
mov qword ptr [rsp+8],rcx
pushrbp
pushrsi


Cool. When we did the Full Debug Symbols (FDS) project, one of the
things we did was try to use the same optimization and debug options
with RELEASE/product builds as with fastdebug builds. Since we're
generating debug info for all builds configs, we thought this was
a really good design goal unless we ran into a performance issue.

During the FDS project, we saw no performance change when we
switched the optimization options and included the various
generate-debug-info options.

A long way of saying:

I think you should enable /homeparams for RELEASE/product builds.

:-)

Dan




Thanks,
Christian

-Original Message-
From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com]
Sent: Wednesday, August 20, 2014 11:17 AM
To: Christian Tornqvist
Cc: hotspot-...@openjdk.java.net; build-dev@openjdk.java.net
Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds using
/homeparams

On 8/20/14 8:49 AM, Christian Tornqvist wrote:

Do we have any idea how much of a change? Do we care? I'm presuming
the

increased debuggability is worth it.

It adds 0-4 additional stack movs in function prologue code which
should have a very low impact, it's only on debug/fastdebug builds.

I missed that this was a non-RELEASE build change in my original review.

These two blurbs from the MSDN note:

   However, by default in a release build, the register parameters   will
not be written to the stack, into the space that is already   provided for
the parameters. This makes it difficult to debug an   optimized (release)
build of your program.

and

   In a debug build, the stack is always populated with parameters   passed
in registers.

indicate that we shouldn't need this change in our debug/fastdebug builds.
However, you looked at the generated prologue code before and after turning
on this option right?

Does this mean that the MSDN note is not correct for the way that we do
debug and fastdebug builds? We might have some option enabled that prevents
the default /homeparams behavior from working...

Dan



The above block will also apply to an ia64 build. We don't support
that

anymore, but I don't know if any licensees support it.

I've changed the checks in vm.make and WinGammaPlatformVC10.java


Do we need to do anything about the new, but unused  platform to make

lint-like tools not squawk?

I'll open a new bug to clean up the VC7/8/9 files in ProjectCreator.

New webrev:
http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.01/

Thanks,
Christian

-Original Message-
From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com]
Sent: Wednesday, August 20, 2014 9:48 AM
To: Christian Tornqvist
Cc: hotspot-...@openjdk.java.net
Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds
using /homeparams

http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.00/

General Comment

The MSDN note says:

/homeparams does imply a performance disadvantage, because it  
does require a cycle to load the register parameters on to the stack.

Do we have any idea how much of a change? Do we care? I'm presuming
the increased debuggability is worth it.

make/windows/makefiles/vm.make
   line 38: !else
   line 39: CXX_FLAGS=$(CXX_FLAGS) /D ASSERT /homeparams
   line 40: !endif
   The above block will also apply to an ia64 build.
   We don't support that anymore, but I don't know if
   any licensees support it.

src/share/tools/ProjectCreator/BuildConfig.java
   No comments.

src/share/tools/ProjectCreator/WinGammaPlatformVC10.java
   line 373: if(!platformName.equals(Win32)) {
   line 374: addAttr(rv, AdditionalOptions, /homeparams);
   The above block will also apply to an ia64 build.

src/share/tools/ProjectCreator/WinGammaPlatformVC7.java
src/share/tools/ProjectCreator/WinGammaPlatformVC8.java
   Do we need to do anything about the new, but unused
   platform to make lint-like tools not squawk?

Thumbs up.

Dan


On 8/19/14 5:39 PM, Christian Tornqvist wrote:

Hi everyone,




This change adds /homeparams
(http://msdn.microsoft.com/en-us/library/6exwh0y6.aspx) to compiler
flags when building fastdebug on Windows x64. This causes the
compiler to generate code to spill the first 4 arguments to the stack
(they're

RE: RFR(S): 8027480 - Build Windows x64 fastdebug builds using /homeparams

[Christian Tornqvist]

I talked to Staffan Friberg (perf tech lead) about enabling this in a
release builds and this is his opinion: 

For /homeparams I think enable it now in fastdebug, let's use it for a
while and understand home much of a benefit it is for debugging. If it is a
huge help let's consider it for release, but that will require a lot of
performance testing before commiting

I agree with Staffan and think we should only enable it in fastdebug builds
for now.

Thanks,
Christian
 
-Original Message-
From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com] 
Sent: Wednesday, August 20, 2014 12:02 PM
To: Christian Tornqvist
Cc: hotspot-...@openjdk.java.net; build-dev@openjdk.java.net
Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds using
/homeparams

On 8/20/14 9:48 AM, Christian Tornqvist wrote:
 indicate that we shouldn't need this change in our debug/fastdebug
builds.
 However, you looked at the generated prologue code before and after 
 turning on this option right?

 Our fastdebug builds are compiled with /O2 which doesn't spill the 
 parameters onto the stack, our debug builds (with /Od) will do this so 
 passing /homeparams there is a no-op.

 Here is a look at the prologue code for
 ClassFileParser::parse_constant_pool_entries() before /homeparams:

 mov dword ptr [rsp+10h],edx
 pushrbp
 pushrsi

 and here is with /homeparams:

 mov qword ptr [rsp+18h],r8
 mov dword ptr [rsp+10h],edx
 mov qword ptr [rsp+8],rcx
 pushrbp
 pushrsi

Cool. When we did the Full Debug Symbols (FDS) project, one of the things we
did was try to use the same optimization and debug options with
RELEASE/product builds as with fastdebug builds. Since we're generating
debug info for all builds configs, we thought this was a really good design
goal unless we ran into a performance issue.

During the FDS project, we saw no performance change when we switched the
optimization options and included the various generate-debug-info options.

A long way of saying:

I think you should enable /homeparams for RELEASE/product builds.

:-)

Dan



 Thanks,
 Christian

 -Original Message-
 From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com]
 Sent: Wednesday, August 20, 2014 11:17 AM
 To: Christian Tornqvist
 Cc: hotspot-...@openjdk.java.net; build-dev@openjdk.java.net
 Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds 
 using /homeparams

 On 8/20/14 8:49 AM, Christian Tornqvist wrote:
 Do we have any idea how much of a change? Do we care? I'm presuming 
 the
 increased debuggability is worth it.

 It adds 0-4 additional stack movs in function prologue code which 
 should have a very low impact, it's only on debug/fastdebug builds.
 I missed that this was a non-RELEASE build change in my original review.

 These two blurbs from the MSDN note:

However, by default in a release build, the register parameters   
 will not be written to the stack, into the space that is already   
 provided for the parameters. This makes it difficult to debug an   
 optimized (release) build of your program.

 and

In a debug build, the stack is always populated with parameters   
 passed in registers.

 indicate that we shouldn't need this change in our debug/fastdebug builds.
 However, you looked at the generated prologue code before and after 
 turning on this option right?

 Does this mean that the MSDN note is not correct for the way that we 
 do debug and fastdebug builds? We might have some option enabled that 
 prevents the default /homeparams behavior from working...

 Dan


 The above block will also apply to an ia64 build. We don't support 
 that
 anymore, but I don't know if any licensees support it.

 I've changed the checks in vm.make and WinGammaPlatformVC10.java

 Do we need to do anything about the new, but unused  platform to 
 make
 lint-like tools not squawk?

 I'll open a new bug to clean up the VC7/8/9 files in ProjectCreator.

 New webrev:
 http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.01/

 Thanks,
 Christian

 -Original Message-
 From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com]
 Sent: Wednesday, August 20, 2014 9:48 AM
 To: Christian Tornqvist
 Cc: hotspot-...@openjdk.java.net
 Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds 
 using /homeparams

 http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.00/

 General Comment

 The MSDN note says:

 /homeparams does imply a performance disadvantage, because it   
 does require a cycle to load the register parameters on to the stack.

 Do we have any idea how much of a change? Do we care? I'm presuming 
 the increased debuggability is worth it.

 make/windows/makefiles/vm.make
line 38: !else
line 39: CXX_FLAGS=$(CXX_FLAGS) /D ASSERT /homeparams
line 40: !endif
The above block will also apply to an ia64 build.
We don't support that anymore, but I don't know if
any licensees

Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds using /homeparams

[Daniel D. Daugherty]

Your call.

Dan


On 8/20/14 11:05 AM, Christian Tornqvist wrote:

I talked to Staffan Friberg (perf tech lead) about enabling this in a
release builds and this is his opinion:

For /homeparams I think enable it now in fastdebug, let's use it for a
while and understand home much of a benefit it is for debugging. If it is a
huge help let's consider it for release, but that will require a lot of
performance testing before commiting

I agree with Staffan and think we should only enable it in fastdebug builds
for now.

Thanks,
Christian
  
-Original Message-

From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com]
Sent: Wednesday, August 20, 2014 12:02 PM
To: Christian Tornqvist
Cc: hotspot-...@openjdk.java.net; build-dev@openjdk.java.net
Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds using
/homeparams

On 8/20/14 9:48 AM, Christian Tornqvist wrote:

indicate that we shouldn't need this change in our debug/fastdebug

builds.

However, you looked at the generated prologue code before and after
turning on this option right?

Our fastdebug builds are compiled with /O2 which doesn't spill the
parameters onto the stack, our debug builds (with /Od) will do this so
passing /homeparams there is a no-op.

Here is a look at the prologue code for
ClassFileParser::parse_constant_pool_entries() before /homeparams:

mov dword ptr [rsp+10h],edx
pushrbp
pushrsi

and here is with /homeparams:

mov qword ptr [rsp+18h],r8
mov dword ptr [rsp+10h],edx
mov qword ptr [rsp+8],rcx
pushrbp
pushrsi

Cool. When we did the Full Debug Symbols (FDS) project, one of the things we
did was try to use the same optimization and debug options with
RELEASE/product builds as with fastdebug builds. Since we're generating
debug info for all builds configs, we thought this was a really good design
goal unless we ran into a performance issue.

During the FDS project, we saw no performance change when we switched the
optimization options and included the various generate-debug-info options.

A long way of saying:

I think you should enable /homeparams for RELEASE/product builds.

:-)

Dan



Thanks,
Christian

-Original Message-
From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com]
Sent: Wednesday, August 20, 2014 11:17 AM
To: Christian Tornqvist
Cc: hotspot-...@openjdk.java.net; build-dev@openjdk.java.net
Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds
using /homeparams

On 8/20/14 8:49 AM, Christian Tornqvist wrote:

Do we have any idea how much of a change? Do we care? I'm presuming
the

increased debuggability is worth it.

It adds 0-4 additional stack movs in function prologue code which
should have a very low impact, it's only on debug/fastdebug builds.

I missed that this was a non-RELEASE build change in my original review.

These two blurbs from the MSDN note:

However, by default in a release build, the register parameters  
will not be written to the stack, into the space that is already  
provided for the parameters. This makes it difficult to debug an  
optimized (release) build of your program.

and

In a debug build, the stack is always populated with parameters  
passed in registers.

indicate that we shouldn't need this change in our debug/fastdebug builds.
However, you looked at the generated prologue code before and after
turning on this option right?

Does this mean that the MSDN note is not correct for the way that we
do debug and fastdebug builds? We might have some option enabled that
prevents the default /homeparams behavior from working...

Dan



The above block will also apply to an ia64 build. We don't support
that

anymore, but I don't know if any licensees support it.

I've changed the checks in vm.make and WinGammaPlatformVC10.java


Do we need to do anything about the new, but unused  platform to
make

lint-like tools not squawk?

I'll open a new bug to clean up the VC7/8/9 files in ProjectCreator.

New webrev:
http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.01/

Thanks,
Christian

-Original Message-
From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com]
Sent: Wednesday, August 20, 2014 9:48 AM
To: Christian Tornqvist
Cc: hotspot-...@openjdk.java.net
Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds
using /homeparams

 http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.00/

General Comment

The MSDN note says:

 /homeparams does imply a performance disadvantage, because it  
does require a cycle to load the register parameters on to the stack.

Do we have any idea how much of a change? Do we care? I'm presuming
the increased debuggability is worth it.

make/windows/makefiles/vm.make
line 38: !else
line 39: CXX_FLAGS=$(CXX_FLAGS) /D ASSERT /homeparams
line 40: !endif
The above block will also apply to an ia64 build.
We don't support that anymore, but I don't know if
any licensees support it.

src

Re: RFR: 8047952 : (s) Remove FORTIFY_SOURCE from fastdebug and slowdebug builds

[pointo1d]

Hiya Mike ,

On 09/08/14 01:23, Mike Duigou wrote:

Hello all;

A previous version of this changeset was focused on disabling FORTIFY_SOURCE 
only for specific files with optimization disabled. This version entirely 
disables FORTIFY_SOURCE for all portions of the build. After additional 
problems; incompatibility of FORTIFY_SOURCE with -O0, lack of consistent 
support for FORTIFY_SOURCE in some distributions of glibc, etc. this seems like 
the best course. Should the separate issues with -Od be resolved then 
FORTIFY_SOURCE could be re-enabled selectively for specific platforms.

webrev: http://cr.openjdk.java.net/~mduigou/JDK-8047952/1/webrev/
jbsbug: https://bugs.openjdk.java.net/browse/JDK-8047952

Mike


FWIW, in addition to the other favourable responses, your changes look 
fine to this non-reviewer.


--
​Dave Pointon FIAP MBCS - Contractor engaged by IBM

Now I saw, tho' too late, the folly of beginning a work before we count the 
cost and before we we judge rightly of our strength to go thro' with it - 
Robinson Crusoe

Re: RFR: 8047952 : (s) Remove FORTIFY_SOURCE from fastdebug and slowdebug builds

[Kim Barrett]

On Aug 11, 2014, at 7:09 PM, Mike Duigou mike.dui...@oracle.com wrote:
 
 You are correct. The release and fastdebug need to be combined into one 
 clause. I will fix this. Bash 4.0 doesn't appear to complain about the 
 fallthrough but better to be compliant.

Other than that, looks good.

Re: RFR: 8047952 : (s) Remove FORTIFY_SOURCE from fastdebug and slowdebug builds

[David Holmes]

On 9/08/2014 10:23 AM, Mike Duigou wrote:

Hello all;

A previous version of this changeset was focused on disabling FORTIFY_SOURCE 
only for specific files with optimization disabled. This version entirely 
disables FORTIFY_SOURCE for all portions of the build. After additional 
problems; incompatibility of FORTIFY_SOURCE with -O0, lack of consistent 
support for FORTIFY_SOURCE in some distributions of glibc, etc. this seems like 
the best course. Should the separate issues with -Od be resolved then 
FORTIFY_SOURCE could be re-enabled selectively for specific platforms.

webrev: http://cr.openjdk.java.net/~mduigou/JDK-8047952/1/webrev/
jbsbug: https://bugs.openjdk.java.net/browse/JDK-8047952


Thanks Mike. Looks like a clean removal to me.

David


Mike

RFR: JDK-8035904: Solaris fastdebug builds are failing

[Erik Joelsson]

JDK-8034199 unfortunately didn't work with a bunch of internal Solaris 
installations. It seems common for Solaris 10 to not have an updated 
bash version and the =~ construct used in that patch requires bash 3.x 
or higher.


Here is a rewrite of that part using grep.

Bug: https://bugs.openjdk.java.net/browse/JDK-8035904

Patch inline:

diff -r 73d85b04c45c common/autoconf/configure
--- a/common/autoconf/configure
+++ b/common/autoconf/configure
@@ -127,10 +127,11 @@
   if [[ -n $1 ]]; then
 # Uses only shell-safe characters?  No quoting needed.
 # '=' is a zsh meta-character, but only in word-initial position.
-if [[ $1 =~ 
^[ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789.:,%/+=_-]+$ 
 ! $1 =~ ^= ]]; then
+if echo $1 | grep -qE 
'^[ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789.:,%/+=_-]+$' 
\

+ ! echo $1 | grep -qE '^='; then
   quoted=$1
 else
-  if [[ $1 =~ [\'!] ]]; then
+  if echo $1 | grep -qE [\'!]; then
 # csh does history expansion within single quotes, but not
 # when backslash-escaped!
 local quoted_quote='\\'' quoted_exclam='\\!'


/Erik

Re: RFR: JDK-8035904: Solaris fastdebug builds are failing

[Tim Bell]

Hi Erik:

JDK-8034199 unfortunately didn't work with a bunch of internal Solaris 
installations. It seems common for Solaris 10 to not have an updated 
bash version and the =~ construct used in that patch requires bash 3.x 
or higher.


Here is a rewrite of that part using grep.



Looks good to me.

Tim


Bug: https://bugs.openjdk.java.net/browse/JDK-8035904

Patch inline:

diff -r 73d85b04c45c common/autoconf/configure
--- a/common/autoconf/configure
+++ b/common/autoconf/configure
@@ -127,10 +127,11 @@
   if [[ -n $1 ]]; then
 # Uses only shell-safe characters?  No quoting needed.
 # '=' is a zsh meta-character, but only in word-initial position.
-if [[ $1 =~ 
^[ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789.:,%/+=_-]+$ 
 ! $1 =~ ^= ]]; then
+if echo $1 | grep -qE 
'^[ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789.:,%/+=_-]+$' 
\

+ ! echo $1 | grep -qE '^='; then
   quoted=$1
 else
-  if [[ $1 =~ [\'!] ]]; then
+  if echo $1 | grep -qE [\'!]; then
 # csh does history expansion within single quotes, but not
 # when backslash-escaped!
 local quoted_quote='\\'' quoted_exclam='\\!'


/Erik

Solaris fastdebug build: gobjcopy?

[Weijun Wang]

Hi All

I'm trying --enable-debug on a Solaris and see this failure

## Starting hotspot
INFO: ENABLE_FULL_DEBUG_SYMBOLS=1
INFO: ALT_OBJCOPY=/usr/sfw/bin/gobjcopy
INFO: no objcopy cmd found so cannot create .debuginfo files.
INFO: ENABLE_FULL_DEBUG_SYMBOLS=0
INFO: ENABLE_FULL_DEBUG_SYMBOLS=1
INFO: ALT_OBJCOPY=/usr/sfw/bin/gobjcopy
INFO: no objcopy cmd found so cannot create .debuginfo files.
INFO: ENABLE_FULL_DEBUG_SYMBOLS=0
Rescanned ../generated/adfiles/solaris_x86_64.ad  but encountered no 
changes.

Making signal interposition lib...
Opening 'libjsig.so' for update
No SHF_ALLOC flags needed to be cleared.
Done with 'libjsig.so'
make381[6]: /usr/sfw/bin/gobjcopy: Command not found
make381[6]: *** [libjsig.so] Error 127
make381[5]: *** [the_vm] Error 2

Where can I install the gobjcopy command? This is a SunOS s11 5.11 
snv_151a machine. I remember I just copy SS12U1 from somewhere else, 
put its bin into $PATH, and start building.


Also, the output above seems to say that gobjcopy is not found so 
ENABLE_FULL_DEBUG_SYMBOLS falls back to 0 and the build could go on. Why 
does it fail again with the same problem?


Thanks
Max

Re: Solaris fastdebug build: gobjcopy?

[David Holmes]

Hi Max,

On 21/10/2013 7:00 PM, Weijun Wang wrote:

Hi All

I'm trying --enable-debug on a Solaris and see this failure

## Starting hotspot
INFO: ENABLE_FULL_DEBUG_SYMBOLS=1
INFO: ALT_OBJCOPY=/usr/sfw/bin/gobjcopy
INFO: no objcopy cmd found so cannot create .debuginfo files.
INFO: ENABLE_FULL_DEBUG_SYMBOLS=0
INFO: ENABLE_FULL_DEBUG_SYMBOLS=1
INFO: ALT_OBJCOPY=/usr/sfw/bin/gobjcopy
INFO: no objcopy cmd found so cannot create .debuginfo files.
INFO: ENABLE_FULL_DEBUG_SYMBOLS=0
Rescanned ../generated/adfiles/solaris_x86_64.ad  but encountered no
changes.
Making signal interposition lib...
Opening 'libjsig.so' for update
No SHF_ALLOC flags needed to be cleared.
Done with 'libjsig.so'
make381[6]: /usr/sfw/bin/gobjcopy: Command not found
make381[6]: *** [libjsig.so] Error 127
make381[5]: *** [the_vm] Error 2

Where can I install the gobjcopy command? This is a SunOS s11 5.11
snv_151a machine. I remember I just copy SS12U1 from somewhere else,
put its bin into $PATH, and start building.

Also, the output above seems to say that gobjcopy is not found so
ENABLE_FULL_DEBUG_SYMBOLS falls back to 0 and the build could go on. Why
does it fail again with the same problem?


Not sure - that seems to be a bug in the hotspot build (hence cc 
hotspot-dev). The FDS logic is processed multiple times but the initial 
pass should disable FDS for the subsequent uses. If gobjcopy is 
installed somewhere then ALT_OBJCOPY can be set to point to it. Was this 
part of a full build? The configure process might also find gobjcopy and 
set the appropriate variables.


David


Thanks
Max

Re: Solaris fastdebug build: gobjcopy?

[Weijun Wang]

This is a full build, but I run configure on one machine and make from 
another. Doing both on the same machine without gobjcopy runs fine.


Thanks
Max

On 10/21/13 7:38 PM, David Holmes wrote:

If gobjcopy is
installed somewhere then ALT_OBJCOPY can be set to point to it. Was this
part of a full build? The configure process might also find gobjcopy and
set the appropriate variables.

David


Thanks
Max

Re: Solaris fastdebug build: gobjcopy?

[Magnus Ihse Bursie]

21 okt 2013 kl. 14:28 skrev Weijun Wang weijun.w...@oracle.com:

 This is a full build, but I run configure on one machine and make from 
 another.

That is not a supported use case. Configure checks the local machine and 
prepares the build.

/Magnus


 Doing both on the same machine without gobjcopy runs fine.
 
 Thanks
 Max
 
 On 10/21/13 7:38 PM, David Holmes wrote:
 If gobjcopy is
 installed somewhere then ALT_OBJCOPY can be set to point to it. Was this
 part of a full build? The configure process might also find gobjcopy and
 set the appropriate variables.
 
 David
 
 Thanks
 Max

Re: Solaris fastdebug build: gobjcopy?

[Daniel D. Daugherty]

Running configure on a machine with gobjcopy and then running the build
on a machine without gobjcopy would explain the failure mode that you're
seeing. You need to file a bug with the build-infra folks. The old build
system wouldn't have had this problem because there was no configure step.
I'm not completely up to speed on the new build-infra stuff so pinging
them is your best bet.

Dan


On 10/21/13 6:28 AM, Weijun Wang wrote:
This is a full build, but I run configure on one machine and make from 
another. Doing both on the same machine without gobjcopy runs fine.


Thanks
Max

On 10/21/13 7:38 PM, David Holmes wrote:

If gobjcopy is
installed somewhere then ALT_OBJCOPY can be set to point to it. Was this
part of a full build? The configure process might also find gobjcopy and
set the appropriate variables.

David


Thanks
Max

Re: Solaris fastdebug build: gobjcopy?

[Weijun Wang]

On 10/21/13 9:18 PM, Magnus Ihse Bursie wrote:

21 okt 2013 kl. 14:28 skrev Weijun Wang weijun.w...@oracle.com:


This is a full build, but I run configure on one machine and make from another.


That is not a supported use case. Configure checks the local machine and 
prepares the build.


Yes I know.

On the other hand, since the build process is able to workaround with 
the missing tool at the earlier stages it should be able to do that later.


If you think it's worth an enhancement I'll file a bug.

Thanks
Max



/Magnus



Doing both on the same machine without gobjcopy runs fine.

Thanks
Max

On 10/21/13 7:38 PM, David Holmes wrote:

If gobjcopy is
installed somewhere then ALT_OBJCOPY can be set to point to it. Was this
part of a full build? The configure process might also find gobjcopy and
set the appropriate variables.

David


Thanks
Max

Re: Solaris fastdebug build: gobjcopy?

[David Holmes]

On 22/10/2013 9:31 AM, Weijun Wang wrote:

On 10/21/13 9:18 PM, Magnus Ihse Bursie wrote:

21 okt 2013 kl. 14:28 skrev Weijun Wang weijun.w...@oracle.com:


This is a full build, but I run configure on one machine and make
from another.


That is not a supported use case. Configure checks the local machine
and prepares the build.


Yes I know.

On the other hand, since the build process is able to workaround with
the missing tool at the earlier stages it should be able to do that later.


The whole point of the configure step is to identify all the build tools 
and dependencies etc and resolve them. It makes no sense to run 
configure on one machine and make on another.


That aside I still think there may be a bug in the hotspot makefiles.

David


If you think it's worth an enhancement I'll file a bug.

Thanks
Max



/Magnus



Doing both on the same machine without gobjcopy runs fine.

Thanks
Max

On 10/21/13 7:38 PM, David Holmes wrote:

If gobjcopy is
installed somewhere then ALT_OBJCOPY can be set to point to it. Was
this
part of a full build? The configure process might also find gobjcopy
and
set the appropriate variables.

David


Thanks
Max

Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so

[Martin Buchholz]

Pushed to jdk8/tl/jdk.
I recommend this be backported to jdk7u.

Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so

[Florian Weimer]

On 02/22/2013 11:03 PM, Martin Buchholz wrote:


I've finally figured out why fastdebug jdk occasionally gives InternalError
in the zip code.


In the distant past, I also saw this with product builds.  Triggering 
conditions involved a JNI DSO calling dlopen(RTLD_GLOBAL) on another DSO 
which eventually triggered loading the system zlib.  This might still be 
relevant on stock OpenJDK 7.


IcedTea avoids this by linking against system zlib.

--
Florian Weimer / Red Hat Product Security Team

Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so

[Martin Buchholz]

On Mon, Mar 4, 2013 at 3:02 AM, Florian Weimer fwei...@redhat.com wrote:

 On 02/22/2013 11:03 PM, Martin Buchholz wrote:

  I've finally figured out why fastdebug jdk occasionally gives
 InternalError
 in the zip code.


 In the distant past, I also saw this with product builds.  Triggering
 conditions involved a JNI DSO calling dlopen(RTLD_GLOBAL) on another DSO
 which eventually triggered loading the system zlib.  This might still be
 relevant on stock OpenJDK 7.

 IcedTea avoids this by linking against system zlib.


It's certainly risky to have two separate system libraries in the same
process using the same symbols.  You are then very dependent on carefully
using the linker (or utilties like objcopy) to produce symbol jails where
one set of symbols cannot affect the other.  Which is not consistent with
traditional linker culture where there is a single global symbol namespace.

Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so

[Alan Bateman]

On 28/02/2013 16:11, Martin Buchholz wrote:



On Thu, Feb 28, 2013 at 6:03 AM, Alan Bateman alan.bate...@oracle.com 
mailto:alan.bate...@oracle.com wrote:


The update to make/java/zip/Makefile looks good to me, we should
have done it a long time ago. I assume you are pushing ahead on
this because you want to push it to jdk7u-dev (as it's not
interesting to jdk8 now because of the new build).


Yes I'm hoping you push this fix to jdk7u.

Also, I'm not sure that the new build system will not try to be 
bug-for-bug compatible with the old one and will reintroduce the problem.
The old build is on life-support, I don't know how long it will before 
it will terminally break or be removed. In any case, I wouldn't expect 
anyone to decide to change it to not use the map files.




The exciting new exception detail change looks okay to me too
although it is hard to read. It wasn't immediately obvious to me
why stddef.h was needed and we'd need to make sure that is okay on
all platforms.


It's hard to find something more standard than stddef.h.

It's hard to find something as non-standard as Windows. My point was we 
just need to double check that this builds okay on Windows, which is does.


So I think you are all set on this one.

-Alan.

Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so

[Kelly O'Hair]

On Mar 1, 2013, at 6:44 AM, Alan Bateman wrote:

 It's hard to find something more standard than stddef.h.
 
 It's hard to find something as non-standard as Windows. My point was we just 
 need to double check that this builds okay on Windows, which is does.

The only issue I have seen with adding includes, even standard ones like these, 
is that they tend
to drag in so many more hidden include files, and there could be a chance at a 
compiler error.
But that's fairly easy to diagnose, either it builds or it doesn't.
What makes it a bit dangerous is that if it creates warning errors that are 
significant, we probably
won't easily see them because we are not using -Werror across the board. :^(

Having said that, my job is to be paranoid, and this does border on paranoia, 
so I say add the stddef.h
and damn the torpedos, full steam ahead.

-kto

Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so

[Alan Bateman]

On 27/02/2013 23:16, Martin Buchholz wrote:

I have another iteration of this change
http://cr.openjdk.java.net/~martin/webrevs/openjdk8/hide-zlib/ 
http://cr.openjdk.java.net/%7Emartin/webrevs/openjdk8/hide-zlib/
that adds exciting new exception detail message for the InternalError 
I was scratching my head about earlier.


-msg = strm-msg;
+msg = ((strm-msg != NULL) ? strm-msg :
+   (ret == Z_VERSION_ERROR) ?
+   zlib returned Z_VERSION_ERROR: 
+   compile time and runtime zlib implementations differ :
+   (ret == Z_STREAM_ERROR) ?
+   inflateInit2 returned Z_STREAM_ERROR :
+   unknown error initializing zlib library);
The update to make/java/zip/Makefile looks good to me, we should have 
done it a long time ago. I assume you are pushing ahead on this because 
you want to push it to jdk7u-dev (as it's not interesting to jdk8 now 
because of the new build).


The exciting new exception detail change looks okay to me too although 
it is hard to read. It wasn't immediately obvious to me why stddef.h was 
needed and we'd need to make sure that is okay on all platforms.


-Alan.

Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so

[Martin Buchholz]

On Thu, Feb 28, 2013 at 6:03 AM, Alan Bateman alan.bate...@oracle.comwrote:

   The update to make/java/zip/Makefile looks good to me, we should have
 done it a long time ago. I assume you are pushing ahead on this because you
 want to push it to jdk7u-dev (as it's not interesting to jdk8 now because
 of the new build).


Yes I'm hoping you push this fix to jdk7u.

Also, I'm not sure that the new build system will not try to be bug-for-bug
compatible with the old one and will reintroduce the problem.


 The exciting new exception detail change looks okay to me too although it
 is hard to read. It wasn't immediately obvious to me why stddef.h was
 needed and we'd need to make sure that is okay on all platforms.


It's hard to find something more standard than stddef.h.

Include What You Use
==
always #include stddef.h in any source file that uses NULL.

Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so

[Martin Buchholz]

I have another iteration of this change
http://cr.openjdk.java.net/~martin/webrevs/openjdk8/hide-zlib/
that adds exciting new exception detail message for the InternalError I was
scratching my head about earlier.

-msg = strm-msg;
+msg = ((strm-msg != NULL) ? strm-msg :
+   (ret == Z_VERSION_ERROR) ?
+   zlib returned Z_VERSION_ERROR: 
+   compile time and runtime zlib implementations differ :
+   (ret == Z_STREAM_ERROR) ?
+   inflateInit2 returned Z_STREAM_ERROR :
+   unknown error initializing zlib library);

Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so

[Alan Bateman]

On 22/02/2013 22:03, Martin Buchholz wrote:

Hi Alan, Xueming, build-ers,

I'd like you to do a code review.

I've finally figured out why fastdebug jdk occasionally gives 
InternalError in the zip code.


Exception in thread main java.lang.InternalError
at java.util.zip.Inflater.init(Native Method)
at java.util.zip.Inflater.init(Inflater.java:101)
at java.util.zip.ZipFile.getInflater(ZipFile.java:448)

It's because:
- jdk changed structure size of z_stream struct
- making jdk zlib incompatible with stock zlib
- as a result of which, it is imperative to keep jdk zlib sequestered 
from system zlib

- so need to not export zlib symbols from libzip.so
- so need to tell makefiles to use linker script unconditionally

http://cr.openjdk.java.net/~martin/webrevs/openjdk8/hide-zlib/ 
http://cr.openjdk.java.net/%7Emartin/webrevs/openjdk8/hide-zlib/


I see Sherman has created a bug for this but it turns out that someone 
else running with fastdebug ran into issues too:


8006319: fastdebug libzip.so is linked with global symbols, allowing 
symbols to be overridden


I see you are proposing to update make/java/zip/Makefile so that is the 
old build. Looking at makefiles/CompileNativeLibraries.gmk (new build) 
then it looks to me that it is using the map file. I haven't checking 
the symbols in a fast debug build to double check so I'm curious if 
you've seen this with a recent (new) build.


-Alan.

Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so

[Martin Buchholz]

I am actually encountering this in openjdk7 with the old build system.
I can repro the problem in openjdk8 with the old build system, but not the
new one.

I don't know if you consider this a bug with the new build system - it's
supposed to generate the same bits, after  all???!!
I'm not sure why - I'd guess something to do with VARIANT or FILES_m

It's highly dubious whether we should have this extremely subtle and
error-prone distinction between release and fastdebug builds at all.
 Consider fixing this more fundamentally in Mapfile-vers.gmk, or at least
document clearly why mapfiles are only on in OPT, and what the consequences
might be.

ifeq ($(PLATFORM), linux)

ifeq ($(VARIANT), OPT)
  # OPT build MUST have a mapfile?
  ifndef FILES_m
FILES_m =mapfile-vers
  endif
endif # VARIANT


Here's a repro recipe on Ubuntu Linux, running javac on any old source file:

env LD_PRELOAD=/usr/lib/x86_64-linux-gnu/libz.so
~/ws/openjdk8/build/linux-amd64-fastdebug/j2sdk-image/bin/javac Main.java
An exception has occurred in the compiler (1.8.0-internal). Please file a
bug at the Java Developer Connection (http://java.sun.com/webapps/bugreport)
 after checking the Bug Parade for duplicates. Include your program and the
following diagnostic in your report.  Thank you.
java.lang.InternalError
at java.util.zip.Inflater.init(Native Method)
at java.util.zip.Inflater.init(Inflater.java:103)
at java.util.zip.ZipFile.getInflater(ZipFile.java:448)
at java.util.zip.ZipFile.getInputStream(ZipFile.java:367)
at java.util.jar.JarFile.getBytes(JarFile.java:379)
at java.util.jar.JarFile.getManifestFromReference(JarFile.java:178)
at java.util.jar.JarFile.getManifest(JarFile.java:165)
at com.sun.tools.javac.file.FSInfo.getJarClassPath(FSInfo.java:69)
at com.sun.tools.javac.file.CacheFSInfo.getJarClassPath(CacheFSInfo.java:94)
at
com.sun.tools.javac.file.Locations$Path.addJarClassPath(Locations.java:304)
at com.sun.tools.javac.file.Locations$Path.addFile(Locations.java:295)
at com.sun.tools.javac.file.Locations$Path.addDirectory(Locations.java:217)
at
com.sun.tools.javac.file.Locations$Path.addDirectories(Locations.java:192)
at
com.sun.tools.javac.file.Locations$BootClassPathLocationHandler.computePath(Locations.java:630)
at
com.sun.tools.javac.file.Locations$BootClassPathLocationHandler.lazy(Locations.java:642)
at
com.sun.tools.javac.file.Locations$BootClassPathLocationHandler.getLocation(Locations.java:576)
at com.sun.tools.javac.file.Locations.getLocation(Locations.java:677)
at
com.sun.tools.javac.file.JavacFileManager.getLocation(JavacFileManager.java:803)
at com.sun.tools.javac.file.JavacFileManager.list(JavacFileManager.java:617)
at com.sun.tools.javac.jvm.ClassReader.fillIn(ClassReader.java:2699)
at com.sun.tools.javac.jvm.ClassReader.complete(ClassReader.java:2395)
at com.sun.tools.javac.code.Symbol.complete(Symbol.java:434)
at com.sun.tools.javac.comp.Enter.visitTopLevel(Enter.java:302)
at com.sun.tools.javac.tree.JCTree$JCCompilationUnit.accept(JCTree.java:519)
at com.sun.tools.javac.comp.Enter.classEnter(Enter.java:262)
at com.sun.tools.javac.comp.Enter.classEnter(Enter.java:276)
at com.sun.tools.javac.comp.Enter.complete(Enter.java:488)
at com.sun.tools.javac.comp.Enter.main(Enter.java:473)
at com.sun.tools.javac.main.JavaCompiler.enterTrees(JavaCompiler.java:990)
at com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:865)
at com.sun.tools.javac.main.Main.compile(Main.java:517)
at com.sun.tools.javac.main.Main.compile(Main.java:376)
at com.sun.tools.javac.main.Main.compile(Main.java:365)
at com.sun.tools.javac.main.Main.compile(Main.java:356)
at com.sun.tools.javac.Main.compile(Main.java:76)
at com.sun.tools.javac.Main.main(Main.java:61)

On Sat, Feb 23, 2013 at 3:51 AM, Alan Bateman alan.bate...@oracle.comwrote:

  On 22/02/2013 22:03, Martin Buchholz wrote:

 Hi Alan, Xueming, build-ers,

  I'd like you to do a code review.

  I've finally figured out why fastdebug jdk occasionally gives
 InternalError in the zip code.

  Exception in thread main java.lang.InternalError
  at java.util.zip.Inflater.init(Native Method)
  at java.util.zip.Inflater.init(Inflater.java:101)
  at java.util.zip.ZipFile.getInflater(ZipFile.java:448)

  It's because:
 - jdk changed structure size of z_stream struct
 - making jdk zlib incompatible with stock zlib
 - as a result of which, it is imperative to keep jdk zlib sequestered from
 system zlib
 - so need to not export zlib symbols from libzip.so
  - so need to tell makefiles to use linker script unconditionally

  http://cr.openjdk.java.net/~martin/webrevs/openjdk8/hide-zlib/

   I see Sherman has created a bug for this but it turns out that someone
 else running with fastdebug ran into issues too:

 8006319: fastdebug libzip.so is linked with global symbols, allowing
 symbols to be overridden

 I see you are proposing to update make/java/zip/Makefile so that is the
 old build. Looking at makefiles/CompileNativeLibraries.gmk (new build) then
 it looks to me

Re: Do not let internal JDK zlib symbols leak out of fastdebug libzip.so

[Alan Bateman]

On 23/02/2013 18:06, Martin Buchholz wrote:

I am actually encountering this in openjdk7 with the old build system.
I can repro the problem in openjdk8 with the old build system, but not 
the new one.


I don't know if you consider this a bug with the new build system - 
it's supposed to generate the same bits, after  all???!!

I'm not sure why - I'd guess something to do with VARIANT or FILES_m

It's highly dubious whether we should have this extremely subtle and 
error-prone distinction between release and fastdebug builds at all.
Thanks for confirming it's old build only, that was my reading too but 
without checking.


Anyway, I do consider it a bug (in the old build), and really hard to 
track down too until you see both libz and the JDK's libzip loaded. 
Kelly might have some insight the product vs. fastdebug difference.


-Alan

fastdebug

[Pete Brunet]

What is the difference between using a target of fastdebug_build and no
target but using the variable FASTDEBUG=true?

Re: Request to mailing list discuss rejected regarding Re: Where have all the fastdebug builds gone?

[Ulf Zibis]

Hi all,

as disc...@openjdk.java.net was in the cc-list of those 3 posts, I just did answer to all, as I 
thought my post would be interesting to all participants.


I think, list moderators should accept such posts from un-subscribed senders, to avoid communication 
thread gaps.


What is the right behaviour here from posters side?

I do not want to overflow my mail account in subscribing to *all* openjdk lists.

Your opinions are appreciated,

-Ulf



Am 09.11.2011 23:21, schrieb discuss-boun...@openjdk.java.net:

Your request to the discuss mailing list

 Posting of your message titled Re: Where have all the fastdebug
builds gone?

has been rejected by the list moderator.  The moderator gave the
following reason for rejecting your request:

No reason given

Any questions or comments should be directed to the list administrator
at:

 discuss-ow...@openjdk.java.net

Re: Request to mailing list discuss rejected regarding Re: Where have all the fastdebug builds gone?

[Dalibor Topic]

On 11/10/11 2:25 PM, Ulf Zibis wrote:
 Hi all,
 
 as disc...@openjdk.java.net was in the cc-list of those 3 posts, I just did 
 answer to all, as I thought my post would be interesting to all 
 participants.

While that is an understandable opinion on the side of any e-mail author, in 
practice it is very rare that any single 
e-mail message is interesting to all participants on any single mailing list, 
not to mention on multiple ones. See
https://secure.wikimedia.org/wikipedia/en/wiki/Crossposting .

 I think, list moderators should accept such posts from un-subscribed senders, 
 to avoid communication thread gaps.

No.

As a list moderator for a couple of lists, I should point out that I almost 
never moderate any posts through, with
the one exception being mercurial commit messages, since missing those confuses 
people about commits.
 
 What is the right behaviour here from posters side?

Subscribe to a list in order to post to it.

cheers,
dalibor topic
-- 
Oracle http://www.oracle.com
Dalibor Topic | Java F/OSS Ambassador
Phone: +494023646738 tel:+494023646738 | Mobile: +491772664192 
tel:+491772664192
Oracle Java Platform Group

ORACLE Deutschland B.V.  Co. KG | Nagelsweg 55 | 20097 Hamburg

ORACLE Deutschland B.V.  Co. KG
Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Niederlande, Nr. 30143697
Geschäftsführer: Jürgen Kunz, Marcel van de Molen, Alexander van der Ven

Green Oracle http://www.oracle.com/commitment Oracle is committed to 
developing practices and products that help protect the environment

Re: Request to mailing list discuss rejected regarding Re: Where have all the fastdebug builds gone?

[Ulf Zibis]

Am 10.11.2011 15:04, schrieb Dalibor Topic:

On 11/10/11 2:25 PM, Ulf Zibis wrote:

Hi all,

as disc...@openjdk.java.net was in the cc-list of those 3 posts, I just did answer 
to all, as I thought my post would be interesting to all participants.

While that is an understandable opinion on the side of any e-mail author, in 
practice it is very rare that any single
e-mail message is interesting to all participants on any single mailing list, 
not to mention on multiple ones. See
https://secure.wikimedia.org/wikipedia/en/wiki/Crossposting .


I think, list moderators should accept such posts from un-subscribed senders, 
to avoid communication thread gaps.

No.

As a list moderator for a couple of lists, I should point out that I almost 
never moderate any posts through, with
the one exception being mercurial commit messages, since missing those confuses 
people about commits.


What is the right behaviour here from posters side?

Subscribe to a list in order to post to it.

I recently found out, that I could disable email delivery to me.
So now, subscribed to the list, I can CC to it without overflow in my email 
account.

Thanks your your clarification Dalibor.

-Ulf



cheers,
dalibor topic

Where have all the fastdebug builds gone?

[Volker Simonis]

Hi,

I could have sworn that

http://jdk6.java.net/download.html and
http://jdk7.java.net/download.html

always contained product AND fastdebug builds, but as I just
verified today there are no more fastdebug builds available neither
for JDK6 nor for JDK7 and JDK8.

Is this intentional?
Is there any other source for debug builds of the original Oracle JDK?

The debug builds have always been very useful to track down problems,
especially because many of the diagnostic VM options are only
available in debug builds.

Regards,
Volker