JDK-8230364 : [JVMCI] a number of JVMCI tests are not jtreg enabled
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 11,13,14
  • Priority: P2
  • Status: Closed
  • Resolution: Fixed
  • Submitted: 2019-08-29
  • Updated: 2020-03-23
  • Resolved: 2019-11-07
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 14
11.0.7-oracleFixed 14 b23Fixed
Related Reports
Relates :  
JDK-8233745 - [JVMCI] TranslatedException should serialize classloader and module info
Relates :  
JDK-8220623 - [JVMCI] Update JVMCI to support JVMCI based Compiler compiled into shared library
Description
For example: https://github.com/openjdk/jdk/blob/5ad889b5e0a99863a089ceee215b3f7a7158b78c/test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.hotspot.test/src/jdk/vm/ci/hotspot/test/TestHotSpotSpeculationLog.java#L35

I discovered this while working on a bug in HotSpotSpeculationLog.java (JDK-8230369) and was expecting a change to the test to show the problem. Since it's not jtreg enabled, it obviously didn't.
Comments
OpenJDK 11 updates backport will follow after larger Graal integration, see https://github.com/oracle/graal/issues/2196
11-03-2020
URL: https://hg.openjdk.java.net/jdk/jdk/rev/2b0f2fe82735 User: iignatyev Date: 2019-11-07 21:41:17 +0000
07-11-2019
Excellent - thanks!
07-11-2019
ok, I've filed JDK-8233745 for TranslatedException changes and will exclude TestTranslatedException from execution by jtreg till JDK-8233745 is fixed.
07-11-2019
The purpose of TranslatedException is to serialize/deserialize exceptions across runtimes. The translation should be as information preserving as possible so I would prefer version 1.
06-11-2019
thanks for clarification Doug. what about TranslatedException omitting module name? we can change TranslatedException to encode/decode all new fields added in JDK9 (classloader name, module name and version)[1]; or just module name[2]. [1] diff -r 0ab0e11968fa src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/TranslatedException.java --- a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/TranslatedException.java Tue Nov 05 17:23:17 2019 -0800 +++ b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/TranslatedException.java Wed Nov 06 09:38:01 2019 -0800 @@ -122,7 +122,7 @@ * a single exception is: * * <pre> - * <exception class name> '|' <exception message> '|' <stack size> '|' [<class> '|' <method> '|' <file> '|' <line> '|' ]* + * <exception class name> '|' <exception message> '|' <stack size> '|' [ <classLoader> '|' <module> '|' <moduleVersion> '|' <class> '|' <method> '|' <file> '|' <line> '|' ]* * </pre> * * Each exception is encoded before the exception it causes. @@ -149,8 +149,12 @@ for (int i = 0; i < stackTrace.length; i++) { StackTraceElement frame = stackTrace[i]; if (frame != null) { - enc.format("%s|%s|%s|%d|", frame.getClassName(), frame.getMethodName(), - encodedString(frame.getFileName()), frame.getLineNumber()); + String moduleName = frame.getModuleName(); + String className = moduleName == null ? frame.getClassName() : moduleName + '/' + frame.getClassName(); + enc.format("%s|%s|%s|%s|%s|%s|%d|", encodedString(frame.getClassLoaderName()), + ecodeString(frame.getModuleName()), encodeString(frame.getModuleVersnio()) + frame.getClassName(), frame.getMethodName(), + encodedString(frame.getFileName()), frame.getLineNumber()); } } } @@ -206,14 +210,26 @@ StackTraceElement[] suffix = getStackTraceSuffix(); StackTraceElement[] stackTrace = new StackTraceElement[stackTraceDepth + suffix.length]; for (int j = 0; j < stackTraceDepth; j++) { + String classLoaderName = parts[i++]; + String moduleName = parts[i++]; + String moduleVersion = parts[i++]; String className = parts[i++]; String methodName = parts[i++]; String fileName = parts[i++]; int lineNumber = Integer.parseInt(parts[i++]); + if (classLoaderName.isEmpty()) { + classLoaderName = null; + } + if (moduleName.isEmpty()) { + moduleName = null; + } + if (moduleVersion.isEmpty()) { + moduleVersion = null; + } if (fileName.isEmpty()) { fileName = null; } - stackTrace[j] = new StackTraceElement(className, methodName, fileName, lineNumber); + stackTrace[j] = new StackTraceElement(classLoaderName, moduleName, moduleVersion, className, methodName, fileName, lineNumber); } System.arraycopy(suffix, 0, stackTrace, stackTraceDepth, suffix.length); throwable.setStackTrace(stackTrace); [2] diff -r ba99c5a4491d src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/TranslatedException.java --- a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/TranslatedException.java Tue Nov 05 08:57:36 2019 -0800 +++ b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/TranslatedException.java Wed Nov 06 09:52:10 2019 -0800 @@ -117,12 +117,19 @@ return Objects.toString(value, "").replace('|', '_'); } + private static String encodeClassName(String moduleName, String className) { + if (moduleName == null) { + return className; + } + return moduleName + '/' + className; + } + /** * Encodes {@code throwable} including its stack and causes as a string. The encoding format of * a single exception is: * * <pre> - * <exception class name> '|' <exception message> '|' <stack size> '|' [<class> '|' <method> '|' <file> '|' <line> '|' ]* + * <exception class name> '|' <exception message> '|' <stack size> '|' [[<module> '/' ] <class> '|' <method> '|' <file> '|' <line> '|' ]* * </pre> * * Each exception is encoded before the exception it causes. @@ -149,8 +156,8 @@ for (int i = 0; i < stackTrace.length; i++) { StackTraceElement frame = stackTrace[i]; if (frame != null) { - enc.format("%s|%s|%s|%d|", frame.getClassName(), frame.getMethodName(), - encodedString(frame.getFileName()), frame.getLineNumber()); + enc.format("%s|%s|%s|%d|", encodeClassName(frame.getModuleName(), frame.getClassName()), + frame.getMethodName(), encodedString(frame.getFileName()), frame.getLineNumber()); } } } @@ -206,6 +213,7 @@ StackTraceElement[] suffix = getStackTraceSuffix(); StackTraceElement[] stackTrace = new StackTraceElement[stackTraceDepth + suffix.length]; for (int j = 0; j < stackTraceDepth; j++) { + String moduleAndClassNames = parts[i++] String className = parts[i++]; String methodName = parts[i++]; String fileName = parts[i++]; @@ -213,7 +221,21 @@ if (fileName.isEmpty()) { fileName = null; } - stackTrace[j] = new StackTraceElement(className, methodName, fileName, lineNumber); + String moduleName; + String className; + // decode moduleAndClassNames := [<module> '/'] <class> + { + int index = moduleAndClassNames.indexOf('/'); + if (index == -1) { + moduleName = null; + className = moduleAndClassNames; + } else { + assert index < moduleAndClassNames.length(); + moduleName = moduleAndClassNames.substring(0, index); + className = moduleAndClassNames.substring(index + 1); + } + } + stackTrace[j] = new StackTraceElement(/*classLoaderName=*/ null, moduleName, /*moduleVersion=/* null, className, methodName, fileName, lineNumber); } System.arraycopy(suffix, 0, stackTrace, stackTraceDepth, suffix.length); throwable.setStackTrace(stackTrace);
06-11-2019
> is jdk/jdk the main repo for jdk.vm.ci.hotspot.test tests? or is it graal-jvmci-8? Since JVMCI is JDK specific (unlike Graal is which shared acorss multiple JDK versions), the tests can differ in each repo. That said, the majority of tests are the same in each JDK version (modulo jtreg annotations).
06-11-2019
TestHotSpotJVMCIRuntime.java compiles w/ the correct @modules, @library values. tTe test uses ext. loader which got removed in jdk9, for now, I made the test to use platform loader instead of ext. loader if 'sun.misc.Launcher' class doesn't exist so the test can work w/ pre-9 and 9+, this, however, can be made simpler if the changes aren't to be backported to graal-jvmci-8. http://cr.openjdk.java.net/~iignatyev//8230364/webrev.01/index.html
06-11-2019
http://cr.openjdk.java.net/~iignatyev//8230364/webrev.00/index.html adds test descriptions to all tests in /test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.hotspot.test and replaces junit ceremonies w/ testng. a few things need to be sorted out, before RFR - compilation of TestHotSpotJVMCIRuntime.java leads to javac crash for me; if it can't be worked around, the test will have to be ignored till javac is fixed - TestTranslatedException failed b/c of the absence of module name (namely java.base) in decoded exception; for now, I removed module name from the expected string.
06-11-2019
[~kvn]/[~dnsimon], is jdk/jdk the main repo for jdk.vm.ci.hotspot.test tests? or is it graal-jvmci-8?
06-11-2019
I've tried to add TEST.properties w/ 'TestNG.dir=.' (and a few other properties) so jtreg will treat all files in test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.hotspot.test/ as testng tests, but since their execution also require a couple of JVM flags (and jtreg doesn't support and I'd argue shouldn't support flag specification in TEST.properties files), we have to add jtreg test description into each test.
05-11-2019
ILW = Tests are not executed because JTREG header is missing, several JVMCI tests, no workaround = HLH = P2 Test was added by JDK-8220623.
30-08-2019