Re: RFR(L) : 8199370: [TESTBUG] Open source vm testbase GC tests

2018-05-07 Thread Erik Joelsson

Build change looks good.

/Erik


On 2018-05-07 15:35, Igor Ignatyev wrote:

http://cr.openjdk.java.net/~iignatyev/8199370/webrev.00/index.html

45710 lines changed: 45710 ins; 0 del; 0 mod;

Hi all,

could you please review the patch which open sources GC tests from vm testbase? 
it introduces the following test groups:
- vmTestbase_vm_g1classunloading
- vmTestbase_vm_gc_compact
- vmTestbase_vm_gc_concurrent
- vmTestbase_vm_gc_container
- vmTestbase_vm_gc_juggle
- vmTestbase_vm_gc_locker
- vmTestbase_vm_gc_ref
- vmTestbase_vm_gc_misc

besides these test groups, which split tests by functionality under test, the 
patch also adds test groups used only for test selection purposes:
- vmTestbase_vm_gc which includes all vmTestbase_vm_gc_* test groups
- vmTestbase_vm_gc_quick -- "quick" tests from vmTestbase_vm_gc test group
- vmTestbase_largepages which consists of tests which are believed to be good 
candidate to test largepage. this group is used in our testing w/ external vm 
flags and/or on pre-configured hosts.

As usually w/ VM testbase code, these tests are old, they have been run in 
hotspot testing for a long period of time. Originally, these tests were run by 
a test harness different from jtreg and had different build and execution 
schemes, some parts couldn't be easily translated to jtreg, so tests might have 
actions or pieces of code which look weird. In a long term, we are planning to 
rework them.

JBS: https://bugs.openjdk.java.net/browse/JDK-8199370
webrev: http://cr.openjdk.java.net/~iignatyev/8199370/webrev.00/index.html

Thanks,
-- Igor




RFR(L) : 8199370: [TESTBUG] Open source vm testbase GC tests

2018-05-07 Thread Igor Ignatyev
http://cr.openjdk.java.net/~iignatyev/8199370/webrev.00/index.html
> 45710 lines changed: 45710 ins; 0 del; 0 mod;

Hi all,

could you please review the patch which open sources GC tests from vm testbase? 
it introduces the following test groups:
- vmTestbase_vm_g1classunloading
- vmTestbase_vm_gc_compact
- vmTestbase_vm_gc_concurrent
- vmTestbase_vm_gc_container
- vmTestbase_vm_gc_juggle
- vmTestbase_vm_gc_locker
- vmTestbase_vm_gc_ref
- vmTestbase_vm_gc_misc

besides these test groups, which split tests by functionality under test, the 
patch also adds test groups used only for test selection purposes:
- vmTestbase_vm_gc which includes all vmTestbase_vm_gc_* test groups
- vmTestbase_vm_gc_quick -- "quick" tests from vmTestbase_vm_gc test group
- vmTestbase_largepages which consists of tests which are believed to be good 
candidate to test largepage. this group is used in our testing w/ external vm 
flags and/or on pre-configured hosts. 

As usually w/ VM testbase code, these tests are old, they have been run in 
hotspot testing for a long period of time. Originally, these tests were run by 
a test harness different from jtreg and had different build and execution 
schemes, some parts couldn't be easily translated to jtreg, so tests might have 
actions or pieces of code which look weird. In a long term, we are planning to 
rework them.

JBS: https://bugs.openjdk.java.net/browse/JDK-8199370
webrev: http://cr.openjdk.java.net/~iignatyev/8199370/webrev.00/index.html

Thanks,
-- Igor

RFR: JDK-8202557: OpenJDK fails to start in Windows 7 and 8.1 after upgrading compiler to VC 2017

2018-05-07 Thread Erik Joelsson
With the new VS2017 toolchain Microsoft has changed how the C++ 
libraries work. In addition to the old msvcr* and msvcp* dll files, we 
now have a big lot of UCRT dlls as well. These files are also 
redistributable but are found in the Windows Kit rather than the Visual 
Studio installation. On modern and updated Windows systems these files 
should be present on the system, but to support older and non updated 
OSes, you can bundle them with your app, just like we already do with 
the old msvc* files.


This patch does the following:

* Adds the UCRT dll files to the windows devkit
* Adds logic in configure for finding the correct UCRT redist dir if the 
toolchain version needs it

* Makes the build copy the files into java.base
* Makes sure any of these files are filtered out of any imported module 
to avoid conflicts


Bug: https://bugs.openjdk.java.net/browse/JDK-8202557

Webrev: http://cr.openjdk.java.net/~erikj/8202557/webrev.01/index.html

/Erik



Re: RFR 8202683: Minimal VM should build cleanly on 64-bit platforms

2018-05-07 Thread David Holmes

On 8/05/2018 1:40 AM, Erik Joelsson wrote:

Looks good to me.

Note for future, I would like it even more if we got rid of the 
pre-defined jvm.cfg altogether and just always generated it. The build 


That was supposed to be happening ... see (non-public sorry) 
JDK-8179985. Don't know what the current status is.


David

itself shouldn't be dictating artificial limitations on build 
parameters. If we want to enforce limitations those should be explicit 
instead.


/Erik


On 2018-05-05 04:26, Aleksey Shipilev wrote:

RFE:
   https://bugs.openjdk.java.net/browse/JDK-8202683

Fix:
   http://cr.openjdk.java.net/~shade/8202683/webrev.01/

Minimal VM is targeted to 32-bit only, but hear me out. Recent build 
system changes, notably
conditional GC compilation, requires build/testing with some GCs 
disabled. Ultimately, we want
minimal VM to be buildable at all times. Today, we need to 
cross-compile to x86_32 to verify that,
but with this change, we can build minimal on x86_64, thus simplifying 
our day-to-day jobs. The
change itself generates jvm.cfg for minimal VM properly on both 
bitnesses.


Testing: x86_64 server/minimal builds, x86_32 server/minimal builds

Thanks,
-Aleksey





Re: Windows: problem with msvxxx.dll locations containing spaces

2018-05-07 Thread Thomas Stüfe
Great, thanks!

On Mon, May 7, 2018 at 5:41 PM, Erik Joelsson  wrote:
> Yes, sure, I'm working on a different fix in the same area now anyway so can
> sneak this fix in there. Will get done this week.
>
> /Erik
>
>
>
> On 2018-05-07 07:37, Thomas Stüfe wrote:
>>
>> Hi Erik,
>>
>> since your proposal worked, will you do a fix?
>>
>> Thanks, Thomas
>>
>> On Fri, May 4, 2018 at 8:30 AM, Thomas Stüfe 
>> wrote:
>>>
>>> Hi Erik,
>>>
>>> that worked on both machines for all builds.
>>>
>>> Thanks, Thomas
>>>
>>> On Thu, May 3, 2018 at 10:13 PM, Erik Joelsson 
>>> wrote:

 Hello,

 In my case, VCINSTALLDIR is in short form already. Could you try
 changing
 line 543 from BASIC_WINDOWS_REWRITE_AS_UNIX_PATH to BASIC_FIXUP_PATH?

 /Erik



 On 2018-05-03 11:17, Thomas Stüfe wrote:
>
> Hi Erik,
>
> I see the error on two very different machines. One is my private
> machine - windows 7, VS2013 and VS2017 installed. The second one is my
> corporate Laptop, Windows 10 and only VS2013.
>
> In both cases I use a very recent cygwin64 installation.
>
> Both cases run in TOOLCHAIN_SETUP_MSVC_DLL into the "# Probe: Using
> well-known location from Visual Studio 12.0 and older" case, and in
> the for loop at line 559 attempt to tokenize the POSSIBLE_MSVC_DLL
> variable content. Here the 64bit build on my corporate laptop:
>
> checking if fixpath.exe works... yes
> POSSIBLE_MSVC_DLL /cygdrive/c/Program
> POSSIBLE_MSVC_DLL Files
> POSSIBLE_MSVC_DLL (x86)/Microsoft
> POSSIBLE_MSVC_DLL Visual
> POSSIBLE_MSVC_DLL Studio
> POSSIBLE_MSVC_DLL 12.0/VC/redist/x64/Microsoft.VC120.CRT/msvcr120.dll
> configure: Found msvcr120.dll at
> /cygdrive/c/WINDOWS/system32/msvcr120.dll using well-known location in
> SYSTEMROOT
> checking found msvcr120.dll architecture... ok
> checking for msvcr120.dll... /cygdrive/c/WINDOWS/system32/msvcr120.dll
> POSSIBLE_MSVC_DLL /cygdrive/c/Program
> POSSIBLE_MSVC_DLL Files
> POSSIBLE_MSVC_DLL (x86)/Microsoft
> POSSIBLE_MSVC_DLL Visual
> POSSIBLE_MSVC_DLL Studio
> POSSIBLE_MSVC_DLL 12.0/VC/redist/x64/Microsoft.VC120.CRT/msvcp120.dll
> configure: Found msvcp120.dll at
> /cygdrive/c/WINDOWS/system32/msvcp120.dll using well-known location in
> SYSTEMROOT
>
> As you can see, we fall back to the default in windows/system32, which
> happens to be working for 64bit. On 32bit, we get this error:
>
> checking if fixpath.exe works... yes
> POSSIBLE_MSVC_DLL /cygdrive/c/Program
> POSSIBLE_MSVC_DLL Files
> POSSIBLE_MSVC_DLL (x86)/Microsoft
> POSSIBLE_MSVC_DLL Visual
> POSSIBLE_MSVC_DLL Studio
> POSSIBLE_MSVC_DLL 12.0/VC/redist/x86/Microsoft.VC120.CRT/msvcr120.dll
> configure: Found msvcr120.dll at
> /cygdrive/c/WINDOWS/system32/msvcr120.dll using well-known location in
> SYSTEMROOT
> checking found msvcr120.dll architecture... incorrect, ignoring
> configure: The file type of the located msvcr120.dll is PE32+
> executable (DLL) (GUI) x86-64, for MS Windows
> configure: Found msvcr120.dll at /cygdrive/c/Program Files
> (x86)/Microsoft Visual Studio
> 12.0/VC/redist/arm/Microsoft.VC120.CRT/msvcr120.dll using search of
> VCINSTALLDIR
> checking found msvcr120.dll architecture... incorrect, ignoring
> configure: The file type of the located msvcr120.dll is PE32
> executable (DLL) (GUI) ARMv7 Thumb, for MS Windows
> checking for msvcr120.dll... no
> configure: error: Could not find msvcr120.dll. Please specify using
> --with-msvcr-dll.
> configure exiting with result code 1
>
> Best Regards, Thomas
>
> On Thu, May 3, 2018 at 7:18 PM, Erik Joelsson
> 
> wrote:
>>
>> Also, on my windows system, if I try using my local Visual Studio,
>> both
>> MSVC
>> dll's are found in the Visual Studio dir, and the spaces are all
>> cleaned
>> up.
>> I wonder what's different about your setup, Thomas.
>>
>> /Erik
>>
>>
>>
>> On 2018-05-03 10:13, Erik Joelsson wrote:
>>>
>>> Hello,
>>>
>>> On 2018-05-03 03:41, Magnus Ihse Bursie wrote:

 I think the main issue here is that BASIC_FIXUP_PATH should be
 called
 for
 the located msvcr*.dll files. I don't have time to look at it now,
 but
 see
 if you can do a rewrite using BASIC_FIXUP_PATH when the potential
 file
 is
 located. This should remove all spaces from the path.

>>> No, that is not a good solution. I intentionally removed that
>>> BASIC_FIXUP_PATH because in VS2017, one of the files has a file name
>>> longer
>>> than 8 characters and BASIC_FIXUP_PATH rewrites that filename to
>>> short

Re: RFR 8202683: Minimal VM should build cleanly on 64-bit platforms

2018-05-07 Thread Erik Joelsson

On 2018-05-07 08:40, Erik Joelsson wrote:

Looks good to me.

Note for future, I would like it even more if we got rid of the 
pre-defined jvm.cfg altogether and just always generated it. The build 
itself shouldn't be dictating artificial limitations on build 
parameters. If we want to enforce limitations those should be explicit 
instead.



Just to clarify, I mean implicit artificial limitations.

/Erik

/Erik


On 2018-05-05 04:26, Aleksey Shipilev wrote:

RFE:
   https://bugs.openjdk.java.net/browse/JDK-8202683

Fix:
   http://cr.openjdk.java.net/~shade/8202683/webrev.01/

Minimal VM is targeted to 32-bit only, but hear me out. Recent build 
system changes, notably
conditional GC compilation, requires build/testing with some GCs 
disabled. Ultimately, we want
minimal VM to be buildable at all times. Today, we need to 
cross-compile to x86_32 to verify that,
but with this change, we can build minimal on x86_64, thus 
simplifying our day-to-day jobs. The
change itself generates jvm.cfg for minimal VM properly on both 
bitnesses.


Testing: x86_64 server/minimal builds, x86_32 server/minimal builds

Thanks,
-Aleksey







Re: Windows: problem with msvxxx.dll locations containing spaces

2018-05-07 Thread Erik Joelsson
Yes, sure, I'm working on a different fix in the same area now anyway so 
can sneak this fix in there. Will get done this week.


/Erik


On 2018-05-07 07:37, Thomas Stüfe wrote:

Hi Erik,

since your proposal worked, will you do a fix?

Thanks, Thomas

On Fri, May 4, 2018 at 8:30 AM, Thomas Stüfe  wrote:

Hi Erik,

that worked on both machines for all builds.

Thanks, Thomas

On Thu, May 3, 2018 at 10:13 PM, Erik Joelsson  wrote:

Hello,

In my case, VCINSTALLDIR is in short form already. Could you try changing
line 543 from BASIC_WINDOWS_REWRITE_AS_UNIX_PATH to BASIC_FIXUP_PATH?

/Erik



On 2018-05-03 11:17, Thomas Stüfe wrote:

Hi Erik,

I see the error on two very different machines. One is my private
machine - windows 7, VS2013 and VS2017 installed. The second one is my
corporate Laptop, Windows 10 and only VS2013.

In both cases I use a very recent cygwin64 installation.

Both cases run in TOOLCHAIN_SETUP_MSVC_DLL into the "# Probe: Using
well-known location from Visual Studio 12.0 and older" case, and in
the for loop at line 559 attempt to tokenize the POSSIBLE_MSVC_DLL
variable content. Here the 64bit build on my corporate laptop:

checking if fixpath.exe works... yes
POSSIBLE_MSVC_DLL /cygdrive/c/Program
POSSIBLE_MSVC_DLL Files
POSSIBLE_MSVC_DLL (x86)/Microsoft
POSSIBLE_MSVC_DLL Visual
POSSIBLE_MSVC_DLL Studio
POSSIBLE_MSVC_DLL 12.0/VC/redist/x64/Microsoft.VC120.CRT/msvcr120.dll
configure: Found msvcr120.dll at
/cygdrive/c/WINDOWS/system32/msvcr120.dll using well-known location in
SYSTEMROOT
checking found msvcr120.dll architecture... ok
checking for msvcr120.dll... /cygdrive/c/WINDOWS/system32/msvcr120.dll
POSSIBLE_MSVC_DLL /cygdrive/c/Program
POSSIBLE_MSVC_DLL Files
POSSIBLE_MSVC_DLL (x86)/Microsoft
POSSIBLE_MSVC_DLL Visual
POSSIBLE_MSVC_DLL Studio
POSSIBLE_MSVC_DLL 12.0/VC/redist/x64/Microsoft.VC120.CRT/msvcp120.dll
configure: Found msvcp120.dll at
/cygdrive/c/WINDOWS/system32/msvcp120.dll using well-known location in
SYSTEMROOT

As you can see, we fall back to the default in windows/system32, which
happens to be working for 64bit. On 32bit, we get this error:

checking if fixpath.exe works... yes
POSSIBLE_MSVC_DLL /cygdrive/c/Program
POSSIBLE_MSVC_DLL Files
POSSIBLE_MSVC_DLL (x86)/Microsoft
POSSIBLE_MSVC_DLL Visual
POSSIBLE_MSVC_DLL Studio
POSSIBLE_MSVC_DLL 12.0/VC/redist/x86/Microsoft.VC120.CRT/msvcr120.dll
configure: Found msvcr120.dll at
/cygdrive/c/WINDOWS/system32/msvcr120.dll using well-known location in
SYSTEMROOT
checking found msvcr120.dll architecture... incorrect, ignoring
configure: The file type of the located msvcr120.dll is PE32+
executable (DLL) (GUI) x86-64, for MS Windows
configure: Found msvcr120.dll at /cygdrive/c/Program Files
(x86)/Microsoft Visual Studio
12.0/VC/redist/arm/Microsoft.VC120.CRT/msvcr120.dll using search of
VCINSTALLDIR
checking found msvcr120.dll architecture... incorrect, ignoring
configure: The file type of the located msvcr120.dll is PE32
executable (DLL) (GUI) ARMv7 Thumb, for MS Windows
checking for msvcr120.dll... no
configure: error: Could not find msvcr120.dll. Please specify using
--with-msvcr-dll.
configure exiting with result code 1

Best Regards, Thomas

On Thu, May 3, 2018 at 7:18 PM, Erik Joelsson 
wrote:

Also, on my windows system, if I try using my local Visual Studio, both
MSVC
dll's are found in the Visual Studio dir, and the spaces are all cleaned
up.
I wonder what's different about your setup, Thomas.

/Erik



On 2018-05-03 10:13, Erik Joelsson wrote:

Hello,

On 2018-05-03 03:41, Magnus Ihse Bursie wrote:

I think the main issue here is that BASIC_FIXUP_PATH should be called
for
the located msvcr*.dll files. I don't have time to look at it now, but
see
if you can do a rewrite using BASIC_FIXUP_PATH when the potential file
is
located. This should remove all spaces from the path.


No, that is not a good solution. I intentionally removed that
BASIC_FIXUP_PATH because in VS2017, one of the files has a file name
longer
than 8 characters and BASIC_FIXUP_PATH rewrites that filename to short
style. This in turn has weird consequences in make when we copy that
file
using generated copy rules. The target file then gets the short style
name
literally.

When I made that change, I looked at all the calls for potential
locations
and thought all of them had the directory prepared properly already. It
seems I was wrong. I think the correct solution is to either get rid of
spaces earlier for all the input directories we search, or maybe tweak
BASIC_FIXUP_PATH to not touch the actual filename.

/Erik

/Magnus

On 2018-05-02 20:46, Thomas Stüfe wrote:

Hi,

My 32bit builds on Windows were failing since quite a while and I
finally had some minutes to look into that.

See prior discussion here:

http://mail.openjdk.java.net/pipermail/build-dev/2018-March/021150.html

My output used to look like this:

checking if fixpath.exe works... yes
POSSIBLE_MSVC_DLL /cygdrive/c/Program

Re: RFR 8202683: Minimal VM should build cleanly on 64-bit platforms

2018-05-07 Thread Erik Joelsson

Looks good to me.

Note for future, I would like it even more if we got rid of the 
pre-defined jvm.cfg altogether and just always generated it. The build 
itself shouldn't be dictating artificial limitations on build 
parameters. If we want to enforce limitations those should be explicit 
instead.


/Erik


On 2018-05-05 04:26, Aleksey Shipilev wrote:

RFE:
   https://bugs.openjdk.java.net/browse/JDK-8202683

Fix:
   http://cr.openjdk.java.net/~shade/8202683/webrev.01/

Minimal VM is targeted to 32-bit only, but hear me out. Recent build system 
changes, notably
conditional GC compilation, requires build/testing with some GCs disabled. 
Ultimately, we want
minimal VM to be buildable at all times. Today, we need to cross-compile to 
x86_32 to verify that,
but with this change, we can build minimal on x86_64, thus simplifying our 
day-to-day jobs. The
change itself generates jvm.cfg for minimal VM properly on both bitnesses.

Testing: x86_64 server/minimal builds, x86_32 server/minimal builds

Thanks,
-Aleksey





Re: RFR: 8179887 - Build failure with glibc >= 2.24: error: 'int readdir_r(DIR*, dirent*, dirent**)' is deprecated

2018-05-07 Thread B. Blaser
On 7 May 2018 at 14:19, B. Blaser  wrote:
> On 6 May 2018 at 18:35, B. Blaser  wrote:
>> On 5 May 2018 at 22:26, Kim Barrett  wrote:
 On May 5, 2018, at 8:03 AM, B. Blaser  wrote:

 On 4 May 2018 at 17:42, Alan Bateman  wrote:
> On 04/05/2018 16:31, B. Blaser wrote:
>>
>> :
>>
>> I'll create a new JBS issue (unless you want to) since the fix is
>> mostly located in core-libs and only intended to make the build
>> complete.
>> But I'll still need someone to review and push the fix, would you be
>> interested in doing this?
>>
> I think someone needs to explore the behavior on other platforms too as 
> the
> #ifndef __linux__ patches are ugly.

 Yes sure but I cannot test it elsewhere myself... and we can easily
 remove them once POSIX officially deprecates 'readdir_r' or if someone
 validates the fix on other systems.
>>>
>>> I'm not sure anyone has the ability to test on all supported.  That
>>> doesn't (and really can't) prevent making changes across platforms to
>>> platform-specific code.  It just means one needs to get help with
>>> testing.  Make the change across platforms, test on those platforms
>>> one has access to, and ask here for help testing on others.
>>
>> OK, I'll provide a cross-platform fix to be evaluated on other systems too.
>
> Here it is, tier1 is successful on Linux with glibc=2.26.
> Any feedback on other systems would be very helpful.

Some more cleanup...

Does this look better?

Thanks,
Bernard


diff -r e81481fea884 src/java.base/unix/native/libjava/TimeZone_md.c
--- a/src/java.base/unix/native/libjava/TimeZone_md.cMon May 07
08:56:35 2018 +0200
+++ b/src/java.base/unix/native/libjava/TimeZone_md.cMon May 07
15:14:26 2018 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -122,32 +122,18 @@
 DIR *dirp = NULL;
 struct stat statbuf;
 struct dirent64 *dp = NULL;
-struct dirent64 *entry = NULL;
 char *pathname = NULL;
 int fd = -1;
 char *dbuf = NULL;
 char *tz = NULL;
 int res;
-long name_max = 0;

 dirp = opendir(dir);
 if (dirp == NULL) {
 return NULL;
 }

-name_max = pathconf(dir, _PC_NAME_MAX);
-// If pathconf did not work, fall back to a mimimum buffer size.
-if (name_max < 1024) {
-name_max = 1024;
-}
-
-entry = (struct dirent64 *)malloc(offsetof(struct dirent64,
d_name) + name_max + 1);
-if (entry == NULL) {
-(void) closedir(dirp);
-return NULL;
-}
-
-while (readdir64_r(dirp, entry, ) == 0 && dp != NULL) {
+while ((dp = readdir64(dirp)) != NULL) {
 /*
  * Skip '.' and '..' (and possibly other .* files)
  */
@@ -214,9 +200,6 @@
 pathname = NULL;
 }

-if (entry != NULL) {
-free((void *) entry);
-}
 if (dirp != NULL) {
 (void) closedir(dirp);
 }
diff -r e81481fea884 src/java.base/unix/native/libjava/UnixFileSystem_md.c
--- a/src/java.base/unix/native/libjava/UnixFileSystem_md.cMon May
07 08:56:35 2018 +0200
+++ b/src/java.base/unix/native/libjava/UnixFileSystem_md.cMon May
07 15:14:26 2018 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -312,7 +312,6 @@
 {
 DIR *dir = NULL;
 struct dirent64 *ptr;
-struct dirent64 *result;
 int len, maxlen;
 jobjectArray rv, old;
 jclass str_class;
@@ -325,13 +324,6 @@
 } END_PLATFORM_STRING(env, path);
 if (dir == NULL) return NULL;

-ptr = malloc(sizeof(struct dirent64) + (PATH_MAX + 1));
-if (ptr == NULL) {
-JNU_ThrowOutOfMemoryError(env, "heap allocation failed");
-closedir(dir);
-return NULL;
-}
-
 /* Allocate an initial String array */
 len = 0;
 maxlen = 16;
@@ -339,7 +331,7 @@
 if (rv == NULL) goto error;

 /* Scan the directory */
-while ((readdir64_r(dir, ptr, ) == 0)  && (result != NULL)) {
+while ((ptr = readdir64(dir)) != NULL) {
 jstring name;
 if (!strcmp(ptr->d_name, ".") || !strcmp(ptr->d_name, ".."))
 continue;
@@ -360,7 +352,6 @@
 (*env)->DeleteLocalRef(env, name);
 }
 closedir(dir);
-free(ptr);

 /* Copy the final results into an appropriately-sized array */
 old = rv;
@@ -375,7 +366,6 @@

  error:
 closedir(dir);
-free(ptr);
   

Re: Windows: problem with msvxxx.dll locations containing spaces

2018-05-07 Thread Thomas Stüfe
Hi Erik,

since your proposal worked, will you do a fix?

Thanks, Thomas

On Fri, May 4, 2018 at 8:30 AM, Thomas Stüfe  wrote:
> Hi Erik,
>
> that worked on both machines for all builds.
>
> Thanks, Thomas
>
> On Thu, May 3, 2018 at 10:13 PM, Erik Joelsson  
> wrote:
>> Hello,
>>
>> In my case, VCINSTALLDIR is in short form already. Could you try changing
>> line 543 from BASIC_WINDOWS_REWRITE_AS_UNIX_PATH to BASIC_FIXUP_PATH?
>>
>> /Erik
>>
>>
>>
>> On 2018-05-03 11:17, Thomas Stüfe wrote:
>>>
>>> Hi Erik,
>>>
>>> I see the error on two very different machines. One is my private
>>> machine - windows 7, VS2013 and VS2017 installed. The second one is my
>>> corporate Laptop, Windows 10 and only VS2013.
>>>
>>> In both cases I use a very recent cygwin64 installation.
>>>
>>> Both cases run in TOOLCHAIN_SETUP_MSVC_DLL into the "# Probe: Using
>>> well-known location from Visual Studio 12.0 and older" case, and in
>>> the for loop at line 559 attempt to tokenize the POSSIBLE_MSVC_DLL
>>> variable content. Here the 64bit build on my corporate laptop:
>>>
>>> checking if fixpath.exe works... yes
>>> POSSIBLE_MSVC_DLL /cygdrive/c/Program
>>> POSSIBLE_MSVC_DLL Files
>>> POSSIBLE_MSVC_DLL (x86)/Microsoft
>>> POSSIBLE_MSVC_DLL Visual
>>> POSSIBLE_MSVC_DLL Studio
>>> POSSIBLE_MSVC_DLL 12.0/VC/redist/x64/Microsoft.VC120.CRT/msvcr120.dll
>>> configure: Found msvcr120.dll at
>>> /cygdrive/c/WINDOWS/system32/msvcr120.dll using well-known location in
>>> SYSTEMROOT
>>> checking found msvcr120.dll architecture... ok
>>> checking for msvcr120.dll... /cygdrive/c/WINDOWS/system32/msvcr120.dll
>>> POSSIBLE_MSVC_DLL /cygdrive/c/Program
>>> POSSIBLE_MSVC_DLL Files
>>> POSSIBLE_MSVC_DLL (x86)/Microsoft
>>> POSSIBLE_MSVC_DLL Visual
>>> POSSIBLE_MSVC_DLL Studio
>>> POSSIBLE_MSVC_DLL 12.0/VC/redist/x64/Microsoft.VC120.CRT/msvcp120.dll
>>> configure: Found msvcp120.dll at
>>> /cygdrive/c/WINDOWS/system32/msvcp120.dll using well-known location in
>>> SYSTEMROOT
>>>
>>> As you can see, we fall back to the default in windows/system32, which
>>> happens to be working for 64bit. On 32bit, we get this error:
>>>
>>> checking if fixpath.exe works... yes
>>> POSSIBLE_MSVC_DLL /cygdrive/c/Program
>>> POSSIBLE_MSVC_DLL Files
>>> POSSIBLE_MSVC_DLL (x86)/Microsoft
>>> POSSIBLE_MSVC_DLL Visual
>>> POSSIBLE_MSVC_DLL Studio
>>> POSSIBLE_MSVC_DLL 12.0/VC/redist/x86/Microsoft.VC120.CRT/msvcr120.dll
>>> configure: Found msvcr120.dll at
>>> /cygdrive/c/WINDOWS/system32/msvcr120.dll using well-known location in
>>> SYSTEMROOT
>>> checking found msvcr120.dll architecture... incorrect, ignoring
>>> configure: The file type of the located msvcr120.dll is PE32+
>>> executable (DLL) (GUI) x86-64, for MS Windows
>>> configure: Found msvcr120.dll at /cygdrive/c/Program Files
>>> (x86)/Microsoft Visual Studio
>>> 12.0/VC/redist/arm/Microsoft.VC120.CRT/msvcr120.dll using search of
>>> VCINSTALLDIR
>>> checking found msvcr120.dll architecture... incorrect, ignoring
>>> configure: The file type of the located msvcr120.dll is PE32
>>> executable (DLL) (GUI) ARMv7 Thumb, for MS Windows
>>> checking for msvcr120.dll... no
>>> configure: error: Could not find msvcr120.dll. Please specify using
>>> --with-msvcr-dll.
>>> configure exiting with result code 1
>>>
>>> Best Regards, Thomas
>>>
>>> On Thu, May 3, 2018 at 7:18 PM, Erik Joelsson 
>>> wrote:

 Also, on my windows system, if I try using my local Visual Studio, both
 MSVC
 dll's are found in the Visual Studio dir, and the spaces are all cleaned
 up.
 I wonder what's different about your setup, Thomas.

 /Erik



 On 2018-05-03 10:13, Erik Joelsson wrote:
>
> Hello,
>
> On 2018-05-03 03:41, Magnus Ihse Bursie wrote:
>>
>> I think the main issue here is that BASIC_FIXUP_PATH should be called
>> for
>> the located msvcr*.dll files. I don't have time to look at it now, but
>> see
>> if you can do a rewrite using BASIC_FIXUP_PATH when the potential file
>> is
>> located. This should remove all spaces from the path.
>>
> No, that is not a good solution. I intentionally removed that
> BASIC_FIXUP_PATH because in VS2017, one of the files has a file name
> longer
> than 8 characters and BASIC_FIXUP_PATH rewrites that filename to short
> style. This in turn has weird consequences in make when we copy that
> file
> using generated copy rules. The target file then gets the short style
> name
> literally.
>
> When I made that change, I looked at all the calls for potential
> locations
> and thought all of them had the directory prepared properly already. It
> seems I was wrong. I think the correct solution is to either get rid of
> spaces earlier for all the input directories we search, or maybe tweak
> BASIC_FIXUP_PATH to not touch the actual filename.
>
> /Erik
>>
>> 

Re: RFR: 8179887 - Build failure with glibc >= 2.24: error: 'int readdir_r(DIR*, dirent*, dirent**)' is deprecated

2018-05-07 Thread B. Blaser
On 6 May 2018 at 18:35, B. Blaser  wrote:
> On 5 May 2018 at 22:26, Kim Barrett  wrote:
>>> On May 5, 2018, at 8:03 AM, B. Blaser  wrote:
>>>
>>> On 4 May 2018 at 17:42, Alan Bateman  wrote:
 On 04/05/2018 16:31, B. Blaser wrote:
>
> :
>
> I'll create a new JBS issue (unless you want to) since the fix is
> mostly located in core-libs and only intended to make the build
> complete.
> But I'll still need someone to review and push the fix, would you be
> interested in doing this?
>
 I think someone needs to explore the behavior on other platforms too as the
 #ifndef __linux__ patches are ugly.
>>>
>>> Yes sure but I cannot test it elsewhere myself... and we can easily
>>> remove them once POSIX officially deprecates 'readdir_r' or if someone
>>> validates the fix on other systems.
>>
>> I'm not sure anyone has the ability to test on all supported.  That
>> doesn't (and really can't) prevent making changes across platforms to
>> platform-specific code.  It just means one needs to get help with
>> testing.  Make the change across platforms, test on those platforms
>> one has access to, and ask here for help testing on others.
>
> OK, I'll provide a cross-platform fix to be evaluated on other systems too.

Here it is, tier1 is successful on Linux with glibc=2.26.
Any feedback on other systems would be very helpful.
Does this look better?

Thanks,
Bernard


diff -r e81481fea884 src/java.base/unix/native/libjava/TimeZone_md.c
--- a/src/java.base/unix/native/libjava/TimeZone_md.cMon May 07
08:56:35 2018 +0200
+++ b/src/java.base/unix/native/libjava/TimeZone_md.cMon May 07
12:38:42 2018 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -122,32 +122,18 @@
 DIR *dirp = NULL;
 struct stat statbuf;
 struct dirent64 *dp = NULL;
-struct dirent64 *entry = NULL;
 char *pathname = NULL;
 int fd = -1;
 char *dbuf = NULL;
 char *tz = NULL;
 int res;
-long name_max = 0;

 dirp = opendir(dir);
 if (dirp == NULL) {
 return NULL;
 }

-name_max = pathconf(dir, _PC_NAME_MAX);
-// If pathconf did not work, fall back to a mimimum buffer size.
-if (name_max < 1024) {
-name_max = 1024;
-}
-
-entry = (struct dirent64 *)malloc(offsetof(struct dirent64,
d_name) + name_max + 1);
-if (entry == NULL) {
-(void) closedir(dirp);
-return NULL;
-}
-
-while (readdir64_r(dirp, entry, ) == 0 && dp != NULL) {
+while ((dp = readdir64(dirp)) != NULL) {
 /*
  * Skip '.' and '..' (and possibly other .* files)
  */
@@ -214,9 +200,6 @@
 pathname = NULL;
 }

-if (entry != NULL) {
-free((void *) entry);
-}
 if (dirp != NULL) {
 (void) closedir(dirp);
 }
diff -r e81481fea884 src/java.base/unix/native/libjava/UnixFileSystem_md.c
--- a/src/java.base/unix/native/libjava/UnixFileSystem_md.cMon May
07 08:56:35 2018 +0200
+++ b/src/java.base/unix/native/libjava/UnixFileSystem_md.cMon May
07 12:38:42 2018 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -312,7 +312,6 @@
 {
 DIR *dir = NULL;
 struct dirent64 *ptr;
-struct dirent64 *result;
 int len, maxlen;
 jobjectArray rv, old;
 jclass str_class;
@@ -325,13 +324,6 @@
 } END_PLATFORM_STRING(env, path);
 if (dir == NULL) return NULL;

-ptr = malloc(sizeof(struct dirent64) + (PATH_MAX + 1));
-if (ptr == NULL) {
-JNU_ThrowOutOfMemoryError(env, "heap allocation failed");
-closedir(dir);
-return NULL;
-}
-
 /* Allocate an initial String array */
 len = 0;
 maxlen = 16;
@@ -339,7 +331,7 @@
 if (rv == NULL) goto error;

 /* Scan the directory */
-while ((readdir64_r(dir, ptr, ) == 0)  && (result != NULL)) {
+while ((ptr = readdir64(dir)) != NULL) {
 jstring name;
 if (!strcmp(ptr->d_name, ".") || !strcmp(ptr->d_name, ".."))
 continue;
@@ -360,7 +352,6 @@
 (*env)->DeleteLocalRef(env, name);
 }
 closedir(dir);
-free(ptr);

 /* Copy the final results into an appropriately-sized array */
 old = rv;
@@ -375,7 +366,6 @@

  error:
 closedir(dir);
-free(ptr);
 return NULL;
 }

diff -r e81481fea884 src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c
--- 

Re: License and Usage Terms of generated API documentation

2018-05-07 Thread mark . reinhold
2018/5/3 13:16:11 +0100, volker.simo...@gmail.com:
> On Thu, May 3, 2018 at 12:21 PM, magnus.ihse.bur...@oracle.com wrote:
>> ...
>> 
>> Until a consensus of a better solution for hosting the generated
>> documentation is reached, I'd like to avoid changing the build code. That
>> will just open for an uneccessary split in what constitutes the proper
>> documentation URL.
> 
> The JAVADOC_BASE_URL is the minor issue here and I agree that it would
> be not easy to create a community driven documentation of the same or
> at least similar quality as we have today from Oracle.

We should publish the API Javadoc somewhere on OpenJDK at some point,
but that’s not something that we can do today -- if nothing else, we’d
have to set up a CDN to front these very popular pages.

>Nevertheless I
> don't see why we shouldn't be able to make this URL configurable at
> configure time.

Agreed.

> The main issue are the two links to "LICENSE_URL" and
> "REDISTRIBUTION_URL" which clearly refer to the API-docs published by
> Oracle. I just want to be able to publish these API-docs myself
> without having to refer to these restrictive licenses. The change
> itself, to make these URLs configurable at configure time, would be
> trivial (and again, I'm not against leaving the default "as-is" if
> this is important for you).

Making these things configurable is a fine idea, but I agree with Magnus
that it’d be simpler for now to retain the current values as defaults.

- Mark


Re: RFR 8202683: Minimal VM should build cleanly on 64-bit platforms

2018-05-07 Thread David Holmes

On 7/05/2018 6:20 PM, Aleksey Shipilev wrote:

On 05/06/2018 09:20 AM, David Holmes wrote:

On 5/05/2018 9:26 PM, Aleksey Shipilev wrote:

RFE:
    https://bugs.openjdk.java.net/browse/JDK-8202683

Fix:
    http://cr.openjdk.java.net/~shade/8202683/webrev.01/

Minimal VM is targeted to 32-bit only, but hear me out. Recent build system 
changes, notably
conditional GC compilation, requires build/testing with some GCs disabled. 
Ultimately, we want
minimal VM to be buildable at all times. Today, we need to cross-compile to 
x86_32 to verify that,
but with this change, we can build minimal on x86_64, thus simplifying our 
day-to-day jobs. The
change itself generates jvm.cfg for minimal VM properly on both bitnesses.


Is 64-bit client VM now able to be built routinely?


As far as I understand, the culprit before was x86_64 C1, which was fixed to 
allow Tiered
compilation. Anyhow, with the build fix above, Minimal VM seems to build, 
bootcycle, and run fine on
x86_64.


Okay. I don't know how well stand-alone 64-bit C1 is supported.




I'm not sure we should be expanding the ability to build the Minimal VM.


I am thinking about the patch as removing the 32-bit special case from the 
jvm.cfg generation. If we
want to forbid some JVM variants from building on some arches, we should be 
doing that explicitly
somewhere else.


Perhaps. But until that happens this may give an illusion of being able 
to do more than you really can.


Anyway my unease has been noted. I don't object enough to make an issue 
of it.


Cheers,
David


-Aleksey



Re: RFR 8202683: Minimal VM should build cleanly on 64-bit platforms

2018-05-07 Thread Aleksey Shipilev
On 05/06/2018 09:20 AM, David Holmes wrote:
> On 5/05/2018 9:26 PM, Aleksey Shipilev wrote:
>> RFE:
>>    https://bugs.openjdk.java.net/browse/JDK-8202683
>>
>> Fix:
>>    http://cr.openjdk.java.net/~shade/8202683/webrev.01/
>>
>> Minimal VM is targeted to 32-bit only, but hear me out. Recent build system 
>> changes, notably
>> conditional GC compilation, requires build/testing with some GCs disabled. 
>> Ultimately, we want
>> minimal VM to be buildable at all times. Today, we need to cross-compile to 
>> x86_32 to verify that,
>> but with this change, we can build minimal on x86_64, thus simplifying our 
>> day-to-day jobs. The
>> change itself generates jvm.cfg for minimal VM properly on both bitnesses.
> 
> Is 64-bit client VM now able to be built routinely?

As far as I understand, the culprit before was x86_64 C1, which was fixed to 
allow Tiered
compilation. Anyhow, with the build fix above, Minimal VM seems to build, 
bootcycle, and run fine on
x86_64.

> I'm not sure we should be expanding the ability to build the Minimal VM.

I am thinking about the patch as removing the 32-bit special case from the 
jvm.cfg generation. If we
want to forbid some JVM variants from building on some arches, we should be 
doing that explicitly
somewhere else.

-Aleksey