JDK-8211740 : [AOT] -XX:AOTLibrary doesn't accept windows path
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 11,12
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: windows
  • Submitted: 2018-10-04
  • Updated: 2020-04-27
  • Resolved: 2018-10-16
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 11 JDK 12
11.0.6Fixed 12 b16Fixed
Description
-XX:AOTLibrary used to specify aot-ed library doesn't accepts windows paths and fails with "error opening file: Can't find dependent libraries"

The error is thrown by open/src/hotspot/os/windows/os_windows.cp:os::dll_load
  1358  void * os::dll_load(const char *name, char *ebuf, int ebuflen) {
  1359    void * result = LoadLibrary(name);
  1360    if (result != NULL) {
  1361      // Recalculate pdb search path if a DLL was loaded successfully.
  1362      SymbolEngine::recalc_search_path();
  1363      return result;
  1364    }
  1365  
  1366    DWORD errcode = GetLastError();
  1367    if (errcode == ERROR_MOD_NOT_FOUND) {
  1368      strncpy(ebuf, "Can't find dependent libraries", ebuflen - 1);
  1369      ebuf[ebuflen - 1] = '\0';
  1370      return NULL;
  1371    }

The cause is that windows LoadLibrary() doesn't accept path.


AOT JEP (http://openjdk.java.net/jeps/295) says:
"-XX:AOTLibrary=<file>
Specify a list of AOT library files. Separate libraries entries with colons (:) or comma (,)."

Also -XX:AOTLibrary works fine with paths on unix systems.

Looks like os_windows.cp:os::dll_load need to be fixed to accepts paths.

Comments
Fix Request (11u) I would like to downport this change. It is a useful and harmless bugfix and makes "JDK-8227439:Turn off AOT by default" apply better. Applies out-of-the-box.
13-11-2019

I updated the jtreg test and the MultipleAOTLibraryTest now passes. I'll send the change out for review. --- a/test/hotspot/jtreg/compiler/aot/cli/MultipleAOTLibraryTest.java +++ b/test/hotspot/jtreg/compiler/aot/cli/MultipleAOTLibraryTest.java @@ -54,6 +54,7 @@ package compiler.aot.cli; import compiler.aot.HelloWorldPrinter; +import java.io.File; import java.util.Arrays; import jdk.test.lib.process.ExitCode; import jdk.test.lib.cli.CommandLineOptionTest; @@ -75,8 +76,11 @@ boolean addTestVMOptions = true; String[] allArgs = Arrays.copyOf(args, args.length + 4); allArgs[args.length] = "-XX:AOTLibrary=" - + "./libMultipleAOTLibraryTest1.so:" - + "./libMultipleAOTLibraryTest2.so"; + + "." + File.separator + + "libMultipleAOTLibraryTest1.so" + + File.pathSeparator + + "." + File.separator + + "libMultipleAOTLibraryTest2.so"; allArgs[args.length + 1] = "-XX:+PrintAOT"; allArgs[args.length + 2] = "-XX:+UseAOT";
15-10-2018

[~bobv] the issue was discovered while working on JDK-8152988 See related changes here: http://cr.openjdk.java.net/~epavlova/8152988/webrev.00/test/TestCommon.gmk.udiff.html
15-10-2018

I made the suggested change but this caused the MultipleAOTLibraryTest to fail since this test is specifying ':' as the path separator, even on Windows. allArgs[args.length] = "-XX:AOTLibrary=" + "./libMultipleAOTLibraryTest1.so:" + "./libMultipleAOTLibraryTest2.so"; What was the original failure mode for this bug? Was it this test? I would not expect that to be the case since there is no drive letter specified in these library names.
15-10-2018

Correct, the : on windows is reserved for drive letter.
15-10-2018

Thanks for the suggested fix Erik. Looks good to me as well. I'll try to get this out for review this week.
15-10-2018

Is it because on windows ':' can be used for drive latter? d:\dir Erik, your suggested fix looks good. Thanks!
13-10-2018

Looking at the source, it already splits on both : and ;. I would suggest making that conditional on OS. Suggested patch: diff -r 4eb1ec86dd02 src/hotspot/share/aot/aotLoader.cpp --- a/src/hotspot/share/aot/aotLoader.cpp Sat Oct 13 03:05:21 2018 +0200 +++ b/src/hotspot/share/aot/aotLoader.cpp Sat Oct 13 19:25:44 2018 +0200 @@ -147,7 +147,12 @@ char* end = cp + len; while (cp < end) { const char* name = cp; - while ((*cp) != '\0' && (*cp) != '\n' && (*cp) != ',' && (*cp) != ':' && (*cp) != ';') cp++; +#ifdef _WINDOWS + char pathSep = ';'; +#else + char pathSep = ':'; +#endif + while ((*cp) != '\0' && (*cp) != '\n' && (*cp) != ',' && (*cp) != pathSep) cp++; cp[0] = '\0'; // Terminate name cp++; load_library(name, true);
13-10-2018

Surely specifying : as separator actually meant that the platform specific path separator (; on Windows)?
13-10-2018