Re: Running jaotc with an external Graal
> On Feb 14, 2017, at 4:38 AM, Andrew Haley wrote: > > On 14/02/17 14:37, Alan Bateman wrote: >> --patch-module can be used to patch any module in the boot layer. So if >> you are looking to override or add classes then --patch-module should work. > > Aha! Thanks, Does it?
Re: RFR: 8177845: Need a mechanism to load Graal
I lost track a bit but why are we exporting jdk.vm.ci.services to the world now?
module jdk.internal.vm.ci {
-exports jdk.vm.ci.services to jdk.internal.vm.compiler;
+exports jdk.vm.ci.services;
> On Apr 18, 2017, at 9:16 PM, Doug Simon wrote:
>
> (adding hotspot-compiler-dev)
>
> Please review these changes that make jdk.internal.vm.compiler an upgradable
> compiler.
> The primary requirement for this is to remove all usage of JDK internals from
> Graal.
> While this most involves changes in Graal, there remain a few capabilities
> that must
> be exposed via JVMCI. Namely:
>
> 1. Graal needs a copy of jdk.internal.misc.VM.savedProps so that it can Graal
> initialize
> can use system properties that are guaranteed not to have been modified by
> application
> code (Graal initialization is lazy and may occur after application code has
> been run).
>
> 2. Truffle needs to be able to trigger JVMCI initialization so that it can
> select the Graal
> optimized implementation of the Truffle runtime.
>
> These capabilities have been added to jdk.vm.ci.services.Services. A comment
> has
> also been added to this class to denote the current methods to be removed
> in a subsequent bug to update the JDK copy of Graal with the upstream version
> which
> no longer uses the methods. The methods destined for removal are:
>
> exportJVMCITo(Class requestor)
> load(Class service)
> loadSingle(Class service, boolean required)
>
> The first one is no longer needed as JVMCI exports itself to Graal service
> providers
> it loads via private API and Graal re-exports[1] JVMCI to any module the
> extends Graal.
> The latter 2 are no longer required as Graal uses the standard ServiceLoader.
> This API
> is only useful for JVMCI-8 where a hidden JVMCI class loader is used.
>
> These changes have been tested against upstream Graal. To make the Graal
> tests pass, 2 extra
> minor changes were required:
>
> 1. In JDK-8177673, HotSpotMemoryAccessProviderImpl::checkRead was added and
> throws IllegalArgumentException
> on all error paths except one which throws AssertionError. The latter was a
> mistake and has been
> fixed to also throw IllegalArgumentException.
> 2. An invalid assertion has been removed from
> HotSpotResolvedObjectTypeImpl::isDefinitelyResolvedWithRespectTo.
> Up to JDK 8, it was true that all classes in java.* packages were loaded by
> the boot class loader.
> This is no longer true in JDK 9 with classes like java.sql being loaded by
> platform class loader.
>
> As hinted above, a subsequent bug will be required to pull in the latest
> upstream Graal and
> remove use of JDK internal API from the jdk.aot module. The qualified exports
> to
> jdk.vm.internal.compiler and jdk.aot can then be removed.
>
> -Doug
>
> https://bugs.openjdk.java.net/browse/JDK-8177845
> http://cr.openjdk.java.net/~dnsimon/8177845/
>
> [1]
> http://openjdk.java.net/projects/jigsaw/spec/issues/#IndirectQualifiedReflectiveAccess
>
>
>
Re: RFR: 8177845: Need a mechanism to load Graal
> On Apr 19, 2017, at 8:38 AM, Mandy Chung wrote:
>
> Since jdk.internal.vm.compiler becomes an upgradeable module, it is not
> hashed with java.base to allow it to be upgraded and there is no integrity
> check. Such qualified export will be granted to any module named
> jdk.internal.vm.compiler at runtime. The goal is for upgradeable modules not
> to use any internal APIs and eliminate the qualified exports.
>
> The main thing is that jdk.vm.ci.services API would need to be guarded if
> it’s used by non-Graal modules.
This all makes sense but where is the restriction that only
jdk.internal.vm.compiler can use jdk.vm.ci.services? After all, this is a
security issue.
>
> Mandy
>
>> On Apr 19, 2017, at 11:24 AM, Christian Thalinger
>> wrote:
>>
>> I lost track a bit but why are we exporting jdk.vm.ci.services to the world
>> now?
>>
>> module jdk.internal.vm.ci {
>> -exports jdk.vm.ci.services to jdk.internal.vm.compiler;
>> +exports jdk.vm.ci.services;
>>
>>> On Apr 18, 2017, at 9:16 PM, Doug Simon wrote:
>>>
>>> (adding hotspot-compiler-dev)
>>>
>>> Please review these changes that make jdk.internal.vm.compiler an
>>> upgradable compiler.
>>> The primary requirement for this is to remove all usage of JDK internals
>>> from Graal.
>>> While this most involves changes in Graal, there remain a few capabilities
>>> that must
>>> be exposed via JVMCI. Namely:
>>>
>>> 1. Graal needs a copy of jdk.internal.misc.VM.savedProps so that it can
>>> Graal initialize
>>> can use system properties that are guaranteed not to have been modified by
>>> application
>>> code (Graal initialization is lazy and may occur after application code has
>>> been run).
>>>
>>> 2. Truffle needs to be able to trigger JVMCI initialization so that it can
>>> select the Graal
>>> optimized implementation of the Truffle runtime.
>>>
>>> These capabilities have been added to jdk.vm.ci.services.Services. A
>>> comment has
>>> also been added to this class to denote the current methods to be removed
>>> in a subsequent bug to update the JDK copy of Graal with the upstream
>>> version which
>>> no longer uses the methods. The methods destined for removal are:
>>>
>>> exportJVMCITo(Class requestor)
>>> load(Class service)
>>> loadSingle(Class service, boolean required)
>>>
>>> The first one is no longer needed as JVMCI exports itself to Graal service
>>> providers
>>> it loads via private API and Graal re-exports[1] JVMCI to any module the
>>> extends Graal.
>>> The latter 2 are no longer required as Graal uses the standard
>>> ServiceLoader. This API
>>> is only useful for JVMCI-8 where a hidden JVMCI class loader is used.
>>>
>>> These changes have been tested against upstream Graal. To make the Graal
>>> tests pass, 2 extra
>>> minor changes were required:
>>>
>>> 1. In JDK-8177673, HotSpotMemoryAccessProviderImpl::checkRead was added and
>>> throws IllegalArgumentException
>>> on all error paths except one which throws AssertionError. The latter was a
>>> mistake and has been
>>> fixed to also throw IllegalArgumentException.
>>> 2. An invalid assertion has been removed from
>>> HotSpotResolvedObjectTypeImpl::isDefinitelyResolvedWithRespectTo.
>>> Up to JDK 8, it was true that all classes in java.* packages were loaded by
>>> the boot class loader.
>>> This is no longer true in JDK 9 with classes like java.sql being loaded by
>>> platform class loader.
>>>
>>> As hinted above, a subsequent bug will be required to pull in the latest
>>> upstream Graal and
>>> remove use of JDK internal API from the jdk.aot module. The qualified
>>> exports to
>>> jdk.vm.internal.compiler and jdk.aot can then be removed.
>>>
>>> -Doug
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8177845
>>> http://cr.openjdk.java.net/~dnsimon/8177845/
>>>
>>> [1]
>>> http://openjdk.java.net/projects/jigsaw/spec/issues/#IndirectQualifiedReflectiveAccess
>>>
>>>
>>>
>>
>
Re: RFR: 8177845: Need a mechanism to load Graal
> On Apr 19, 2017, at 9:27 AM, Doug Simon wrote: > >> >> On 19 Apr 2017, at 21:04, Mandy Chung wrote: >> >> >>> On Apr 19, 2017, at 11:55 AM, Christian Thalinger >>> wrote: >>> >>> >>>> On Apr 19, 2017, at 8:38 AM, Mandy Chung wrote: >>>> >>>> Since jdk.internal.vm.compiler becomes an upgradeable module, it is not >>>> hashed with java.base to allow it to be upgraded and there is no integrity >>>> check. Such qualified export will be granted to any module named >>>> jdk.internal.vm.compiler at runtime. The goal is for upgradeable modules >>>> not to use any internal APIs and eliminate the qualified exports. >>>> >>>> The main thing is that jdk.vm.ci.services API would need to be guarded if >>>> it’s used by non-Graal modules. >>> >>> This all makes sense but where is the restriction that only >>> jdk.internal.vm.compiler can use jdk.vm.ci.services? >> >> It's unqualified and no restriction in this change. > > The public methods currently in jdk.vm.ci.services are: > > 1. JVMCIServiceLocator.getProvider(Class) > 2. JVMCIServiceLocator.getProviders(Class) > 3. Services.initializeJVMCI() > 4. Services.getSavedProperties() > 5. Services.exportJVMCITo(Class) > 6. Services.load(Class) > 7. Services.loadSingle(Class, boolean) > > 1 should be made protected. I'll update the webrev with this change. Good. > > 2 should check for JVMCIPermission. I'll update the webrev with this change. Good. > > 3 is harmless from a security perspective in my opinion. Would be good if one of Oracle’s security engineers could take a quick look just to be sure. > > 4 checks for JVMCIPermission. Ok. > > 5, 6 and 7 will be removed in a follow bug that updates Graal from upstream > (and removes its usage of these methods). About this, will this Graal update happen for JDK 9? It’s awfully late in the cycle... > > Does this address the security concerns? Mostly, yes. Thanks. > > -Doug
Re: RFR: 8177845: Need a mechanism to load Graal
sing separator. Stop. >make/Main.gmk:232: recipe for target 'jdk.aot-launchers' failed. > > The latest webrev has been tested against upstream Graal, the closed AOT > tests and jprt. > > -Doug > > [1] http://cr.openjdk.java.net/~dnsimon/8177845.02 > <http://cr.openjdk.java.net/~dnsimon/8177845.02> > [2] http://cr.openjdk.java.net/~dnsimon/8177845 > <http://cr.openjdk.java.net/~dnsimon/8177845> > [3] > http://cr.openjdk.java.net/~dnsimon/8177845.02/hotspot/src/share/vm/runtime/arguments.cpp.udiff.html > > <http://cr.openjdk.java.net/~dnsimon/8177845.02/hotspot/src/share/vm/runtime/arguments.cpp.udiff.html> > [4] > http://cr.openjdk.java.net/~dnsimon/8177845.02/jdk/make/launcher/Launcher-jdk.aot.gmk.udiff.html > > <http://cr.openjdk.java.net/~dnsimon/8177845.02/jdk/make/launcher/Launcher-jdk.aot.gmk.udiff.html> > >> >>> On 20 Apr 2017, at 20:50, Doug Simon wrote: >>> >>> I've had to update the webrev again to support selection of a "null" >>> compiler (i.e. one that raises an >>> exception upon a compilation request) and added -Djvmci.Compiler=null to a >>> large number of JVMCI jtreg >>> tests to prevent Graal being selected and initialized by the JVMCI compiler >>> auto-selection mechanism. >>> Initializing Graal will currently fail with errors (see stack trace below) >>> until Graal is updated to >>> the version compatible with the JVMCI API changes. >>> >>> In addition to resolving the compatibility issue, explicitly selecting the >>> "null" compiler for these >>> tests better isolates them from parts of the runtime they are not aiming to >>> test. >>> >>> org.graalvm.compiler.debug.GraalError: java.lang.ClassCastException: >>> java.base/java.util.ImmutableCollections$MapN cannot be cast to >>> java.base/java.util.Properties >>> at >>> jdk.internal.vm.compiler/org.graalvm.compiler.hotspot.HotSpotGraalCompilerFactory.getSavedProperties(HotSpotGraalCompilerFactory.java:217) >>> at >>> jdk.internal.vm.compiler/org.graalvm.compiler.hotspot.HotSpotGraalCompilerFactory.initializeOptions(HotSpotGraalCompilerFactory.java:138) >>> at >>> jdk.internal.vm.compiler/org.graalvm.compiler.hotspot.HotSpotGraalCompilerFactory.onSelection(HotSpotGraalCompilerFactory.java:95) >>> at >>> jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotJVMCICompilerConfig.getCompilerFactory(HotSpotJVMCICompilerConfig.java:104) >>> at >>> jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotJVMCIRuntime.(HotSpotJVMCIRuntime.java:290) >>> at >>> jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotJVMCIRuntime.(HotSpotJVMCIRuntime.java:65) >>> at >>> jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotJVMCIRuntime$DelayedInit.(HotSpotJVMCIRuntime.java:73) >>> at >>> jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotJVMCIRuntime.runtime(HotSpotJVMCIRuntime.java:83) >>> at jdk.internal.vm.ci/jdk.vm.ci.runtime.JVMCI.initializeRuntime(Native >>> Method) >>> at jdk.internal.vm.ci/jdk.vm.ci.runtime.JVMCI.(JVMCI.java:58) >>> at >>> jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotJVMCIRuntime.runtime(HotSpotJVMCIRuntime.java:82) >>> at >>> jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotVMConfig.config(HotSpotVMConfig.java:41) >>> at >>> jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotResolvedJavaMethodImpl.getHolder(HotSpotResolvedJavaMethodImpl.java:92) >>> at >>> jdk.internal.vm.ci/jdk.vm.ci.hotspot.HotSpotResolvedJavaMethodImpl.fromMetaspace(HotSpotResolvedJavaMethodImpl.java:110) >>> at >>> jdk.internal.vm.ci/jdk.vm.ci.hotspot.CompilerToVM.asResolvedJavaMethod(Native >>> Method) >>> at >>> jdk.internal.vm.ci/jdk.vm.ci.hotspot.CompilerToVMHelper.asResolvedJavaMethod(CompilerToVMHelper.java:185) >>> at >>> compiler.jvmci.common.CTVMUtilities.getResolvedMethod(CTVMUtilities.java:59) >>> at >>> compiler.jvmci.common.CTVMUtilities.getResolvedMethod(CTVMUtilities.java:64) >>> at >>> compiler.jvmci.compilerToVM.AllocateCompileIdTest.runSanityCorrectTest(AllocateCompileIdTest.java:125) >>> at java.base/java.util.ArrayList.forEach(ArrayList.java:1378) >>> at >>> compiler.jvmci.compilerToVM.AllocateCompileIdTest.main(AllocateCompileIdTest.java:71) >>> at >>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native >>> Method) >>> at >>> java.base
Re: RFR: 8177845: Need a mechanism to load Graal
> On Apr 27, 2017, at 11:51 AM, Doug Simon wrote:
>
>>
>> On 27 Apr 2017, at 18:44, Christian Thalinger wrote:
>>
>> Thanks for doing this. I have a hard time following what’s happening :-)
>>
>> src/jdk.internal.vm.compiler/.mx.graal/suite.py
>>
>> + "jdklibraries" : {
>> +"JVMCI_SERVICES" : {
>> + "path" : "lib/jvmci-services.jar",
>> + "sourcePath" : "lib/jvmci-services.src.zip",
>> + "optional" : False,
>> + "jdkStandardizedSince" : "9",
>> + "module" : "jdk.internal.vm.ci"
>> +},
>> +"JVMCI_API" : {
>> + "path" : "lib/jvmci/jvmci-api.jar",
>> + "sourcePath" : "lib/jvmci/jvmci-api.src.zip",
>> + "dependencies" : [
>> +"JVMCI_SERVICES",
>> + ],
>> + "optional" : False,
>> + "jdkStandardizedSince" : "9",
>> + "module" : "jdk.internal.vm.ci"
>> +},
>> +"JVMCI_HOTSPOT" : {
>> + "path" : "lib/jvmci/jvmci-hotspot.jar",
>> + "sourcePath" : "lib/jvmci/jvmci-hotspot.src.zip",
>> + "dependencies" : [
>> +"JVMCI_API",
>> + ],
>> + "optional" : False,
>> + "jdkStandardizedSince" : "9",
>> + "module" : "jdk.internal.vm.ci"
>> +},
>> + },
>>
>> Can you explain to me why we need this? I don’t think these files are built
>> in 9.
>
> This simply allows `mx eclipseinit` to work when wanting to edit the JDK
> Graal sources in Eclipse. As you state, this is completely by-passed by the
> JDK make system.
Got it.
>
>>
>> src/jdk.internal.vm.ci/share/classes/module-info.java
>>
>> Why can we remove the exports to jdk.internal.vm.compiler? Because of this
>> in JVMCIServiceLocator:
>>
>> +ReflectionAccessJDK.openJVMCITo(getClass());
>
> Yes. And the --add-exports in CompileJavaModules.gmk[1] and
> Launcher-jdk.aot.gmk[2]. It also means these VM options are required in
> upstream Graal when running the Graal junit tests.
Ok, thanks.
>
> -Doug
>
> [1]
> http://cr.openjdk.java.net/~dnsimon/8177845.02/make/make/CompileJavaModules.gmk.udiff.html
>
> <http://cr.openjdk.java.net/~dnsimon/8177845.02/make/make/CompileJavaModules.gmk.udiff.html>
> [2]
> http://cr.openjdk.java.net/~dnsimon/8177845.02/jdk/make/launcher/Launcher-jdk.aot.gmk.udiff.html
>
> <http://cr.openjdk.java.net/~dnsimon/8177845.02/jdk/make/launcher/Launcher-jdk.aot.gmk.udiff.html>
>
>>
>> ?
>>
>>> On Apr 27, 2017, at 7:47 AM, Doug Simon wrote:
>>>
>>>>
>>>> On 21 Apr 2017, at 13:46, Doug Simon wrote:
>>>>
>>>> There has been some discussion about whether we want to update Graal in
>>>> the JDK at this late stage. The main (only?) risk is a regression in the
>>>> AOT tool.
>>>>
>>>> If we don't update Graal from upstream, then the qualified exports from
>>>> JVMCI to jdk.internal.vm.compiler cannot be removed in JDK 9. Note that in
>>>> addition to updating Graal to remove the qualified exports, there would
>>>> also need to be changes in the relevant make files to add --add-exports
>>>> options when compiling Graal and jaotc as they use the dynamically
>>>> exported JVMCI packages.
>>>>
>>>> I have an updated hotspot patch that adapts Graal to the JVMCI API changes:
>>>>
>>>> http://cr.openjdk.java.net/~dnsimon/8177845/hotspot.02/
>>>>
>>>> Note that this patch does not include the changes removing use of JDK
>>>> internal API from Graal. Cherry picking those upstream Graal changes would
>>>> be more work than simply doing a complete update from upstream Graal.
>>>>
>>>> As I see it, there are 2 options here:
>>>>
>>>> 1. Go with the current webrev (including hotspot.02 patch) and live with
>>>> the qualified exports.
>>>> 2. Go with the current webrev (including hotspot.02 patch) and create a
>>>> follow up bug to update Graal from upstream, perform the relevant make
>>>> file changes and remove the qualified exports.
>>>
>>> I made a new webrev[1] that implem
Re: RFR: JDK-8080511 - Refresh of jimage support
> On Jul 24, 2015, at 10:10 AM, Sergey Bylokhov
> wrote:
>
> After the latest merge I got some issue, I assume related to this fix.
> Can somebody take a look to this code:
>
> import java.net.URI;
> import java.nio.file.*;
> import java.nio.file.attribute.BasicFileAttributes;
>
> public class JRTFileSystemError {
>
>public static void main(final String[] args) throws Exception {
>FileSystem fs = FileSystems.getFileSystem(URI.create("jrt:/"));
>Files.walkFileTree(fs.getPath("/java.desktop"), new
> SimpleFileVisitor() {
>@Override
>public FileVisitResult visitFile(Path file,
> BasicFileAttributes attrs) {
>return FileVisitResult.CONTINUE;
>}
>});
>}
> }
>
> It fails with "Exception in thread "main" java.nio.file.NoSuchFileException:
> /java.desktop”.
After the refresh all modules are under /modules. Try “/modules/java.desktop”.
> Bur previously it works correctly. Can somebody confirm this is an issue in
> the test(and how to change it) or in jdk?
> It seems that IntelliJ also broken in some way so it cannot take classes from
> the jdk9 after this fix. probably they should update jrt-fs.jar?
>
>
> On 23.06.15 19:08, Jean-Francois Denise wrote:
>> Hi,
>> an updated webrev that takes into account reviews. In addition the hotspot
>> tests have been reworked following Christian Tornqvist suggestions.
>>
>> top: http://cr.openjdk.java.net/~jfdenise/hs-rt-jimage.3/webrev-top/
>> langtools:
>> http://cr.openjdk.java.net/~jfdenise/hs-rt-jimage.3/webrev-langtools/
>> hotspot: http://cr.openjdk.java.net/~jfdenise/hs-rt-jimage.3/webrev-hotspot/
>> jdk: http://cr.openjdk.java.net/~jfdenise/hs-rt-jimage.3/webrev-jdk/
>>
>> Testing:
>> JCK(s) tests are passing (ran only on Mac OS)
>> Hotspot tests are passing (All platforms)
>> Java PIT tests are passing (All platforms). Ran once on
>> https://bugs.openjdk.java.net/browse/JDK-8129592
>>
>> Thanks.
>> JF
>>
>> On 23 Jun 2015, at 14:10, Jean-Francois Denise
>> wrote:
>>
>>> Hi Paul,
>>>
>>> On 19 Jun 2015, at 16:39, Paul Sandoz wrote:
>>>
On Jun 18, 2015, at 2:08 AM, Jim Laskey (Oracle)
wrote:
> https://bugs.openjdk.java.net/browse/JDK-8080511
>
> This is an long overdue refresh of the jimage support in the JDK9-dev
> repo. This includes native support for reading jimage files, improved
> jrt-fs (java runtime file system) support for retrieving modules and
> packages from the runtime, and improved performance for langtools in the
> presence of jrt-fs.
>
> http://cr.openjdk.java.net/~jlaskey/hs-rt-jimage/webrev-top
>
> http://cr.openjdk.java.net/~jlaskey/hs-rt-jimage/webrev-jdk
>
I eyeballed the JDK changes. Just two thing:
make/src/classes/build/tools/module/ModuleArchive.java
90 @Override
91 public Stream entries() {
92 List entries = new ArrayList<>();
93 try {
94 if (classes != null)
95 Files.walk(classes)
96 .sorted()
97 .filter(p -> !Files.isDirectory(p)
98 &&
!classes.relativize(p).toString().startsWith("_the.")
99 &&
!classes.relativize(p).toString().equals("javac_state"))
100 .map(p -> toEntry(p, classes,
EntryType.CLASS_OR_RESOURCE))
101 .forEach(entries::add);
102 if (cmds != null)
103 Files.walk(cmds)
104 .filter(p -> !Files.isDirectory(p))
105 .map(p -> toEntry(p, cmds,
EntryType.NATIVE_CMD))
106 .forEach(entries::add);
107 if (libs != null)
108 Files.walk(libs)
109 .filter(p -> !Files.isDirectory(p))
110 .map(p -> toEntry(p, libs,
EntryType.NATIVE_LIB))
111 .forEach(entries::add);
112 if (configs != null)
113 Files.walk(configs)
114 .filter(p -> !Files.isDirectory(p))
115 .map(p -> toEntry(p, configs,
EntryType.CONFIG))
116 .forEach(entries::add);
117 } catch (IOException ioe) {
118 throw new UncheckedIOException(ioe);
119 }
120 return entries.stream();
121 }
You can use collect(toList())
>>> ==> OK collect used. In addition, filter first then sort, tryWithResource
>>> for 4 Files stream.
>>>
In general the contract for Archive.entries p
