JDK-8210768 : [AOT] jaotc should support a search path for untrusted dependencies
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 11,12
  • Priority: P4
  • Status: Closed
  • Resolution: Won't Fix
  • Submitted: 2018-09-14
  • Updated: 2021-04-27
  • Resolved: 2021-04-27
Related Reports
Relates :  
JDK-8211065 - Private method check in linkResolver is incorrect
Description
Jaotc has no command-line flag of its own to choose a search path for class dependencies.  Instead the user is forced to pass -J-cp, or in the case of untrusted classes, -J-cp and -J-Xverify:all, which forces verification on even trusted system classes.
Comments
JEP 410 removed AOT from JDK.
27-04-2021
Thanks Dean. That's not what I was told but doesn't materially affect things.
05-10-2018
The .class files generated by jasm seem to be version 45.3, not 49.0.
05-10-2018
No I still can't reproduce this kind of failure unless I also add -XX:+RelaxAccessControl. This has really got me stumped. I don't understand how jatoc + jck is trigerring this problem!
04-10-2018
I think I have now determined the missing piece in trying to get this to reproduce outside of JCK. I was not aware that jasm will produce class files with a version 49.0 by default, which causes relaxed access checks to be applied. I think that, in conjunction with the trusted loader, is what enabled the test code to hit the nestmate assertion check.
02-10-2018
I am inclining to extend --search-path functionality.
28-09-2018
> But main issue is that class paths have to be processed (put into system dictionary) before java code I may be missing something, but I think there is a simple code fix, but it will require a doc update (well if we had jaotc docs -- it seems to be missing from the Tools Reference). For example, instead of doing something like this: classLoader = URLClassPath.newInstance(buildUrls(jarFile)); we can do: parent = URLClassPath.newInstance(buildUrls(user-class-path)); classLoader = URLClassPath.newInstance(buildUrls(jarFile), parent); or we can append user-class-path to the jarFile path. Here, user-class-path is for untrusted classes, for example generated from .jasm or .jcod files, like we have in the TCK. Trusted classes can still be added with -J-cp. Another option would be to use the existing --search-path for the user classpath, because right now --search-path seems to be used only for finding jar files.
28-09-2018
The search class path is very complicated code. And it was intentional that we did not duplicated it in JAOTC and rely on Hotspot to process it. But main issue is that class paths have to be processed (put into system dictionary) before java code is executed and we can't do that since JAOTC is java code.
28-09-2018
When jaotc creates the untrusted URLClassloader, it does not specify a parent, so we get the default system classloader as the parent. This is consistent with the language in JEP 295: "jaotc does not resolve referenced classes which are not system classes or part of compiled classes. They have to be added to class path. Otherwise ClassNotFoundException could be thrown during AOT compilation." However, it does mean you need to be careful not to put untrusted class files on java.class.path, just like when running "java". Adding a -cp/-classpath/--class-path option to jaotc would allow us to specify a path for resolving untrusted "user" classes.
28-09-2018
David you bring up a good point. Jaotc first loads the classes it wants to compile using an untrusted classloader. Then it creates a list of methods and compiles those. During compilation, the Graal compiler uses JVMCI to resolve dependencies found in the constant pools of those classes. I know the untrusted classloader doesn't have a search path to find those dependencies, but I would need to investigate further where in the code we choose to fallback to using the system classloader instead.
24-09-2018
I'm confused. By default anything loaded by an untrusted classloader (ie all non-boot classes) is verified. You have to explicitly turn off verification.
24-09-2018
I refreshed by memory, and jaotc does use a separate classloader for classes it compiles (except for modules). I didn't realize before that this crash is not on a class we are compiling, but a dependency. Jaotc has no support for loading dependencies in separate classloader. Instead, the user is forced to pass -J-cp to jaotc. Since the user passed, -cp, the user should also reasonably be expected to pass -Xverify:all if that classpath contains untrusted classes. Using -Xverify:all is a big hammer and will force even trusted classes to be verified, so I'm changing this to an RFE to provide a better way to find dependencies.
24-09-2018
Yes, please file a new bug for the nestmate assert.
24-09-2018
I can trigger the assert by hacking things so that RelaxAccessControlCheck disables all access checking. That confirmed my initial nestmate check was using the wrong class (which is why the this!=k assert failed). Once that is fixed we hit the "only expected a nestmate" assertion failure. To fix that I need to allow for relaxed access checking, but until I can reproduce this in a test I don't know exactly what it is I need to try and check. [~dlong] I assume you will use this bug to change your verification setting? In which case I will file a new bug to correct the nestmate assertion logic. Thanks.
23-09-2018
I created what I thought was the test scenario: Start with: class Super { private static void m() {}} class Sub extends Super { private static void m() {} static void test() { Sub.m(); } } then use jcod file to rename Sub.m so it can't be found. The test throws IllegalAccessError as expected. I can't get the assertion failure. I tried adding the test classes to the bootclasspath so we're in a trusted loader but that still didn't disable the access check! This is very perplexing.
23-09-2018
Yes it's wrong to not verify the bytecode. I plan to fix it with -Xverify:all for now and fie an RFE to use a difference classloader. I don't think it's possible to trigger the assert with javac-generated .class files. You need to access A.m() from A, but not declare m() in A. Javac wouldn't allow that.
20-09-2018
Surely jaotc is not loading the target classes in a trusted classloader though? That seems wrong to me. I still can't reproduce this with a simple direct test.
20-09-2018
> Regarding verification - unless explicitly disabled the default is to verify everything not loaded from the boot classpath. jaotc is using a trusted classloader. So Verifier::relax_access_for(loader) is returning true unless -Xverify:all is used. The place where this matters is here: #0 Verifier::relax_access_for (loader=0x8ae5dab8) at open/src/hotspot/share/classfile/verifier.cpp:103 #1 0x00007ffff60b6c92 in can_relax_access_check_for (accessor=0x100080c70, accessee=0x100080a58, classloader_only=true) at open/src/hotspot/share/runtime/reflection.cpp:462 #2 0x00007ffff60b7ac3 in Reflection::verify_member_access (current_class=0x100080c70, resolved_class=0x100080c70, member_class=0x100080a58, access=..., classloader_only=true, protected_restriction=false, __the_thread__=0x7ffff09ed000) at open/src/hotspot/share/runtime/reflection.cpp:736 #3 0x00007ffff5e15141 in LinkResolver::check_method_accessability (ref_klass=0x100080c70, resolved_klass=0x100080c70, sel_klass=0x100080a58, sel_method=..., __the_thread__=0x7ffff09ed000) at open/src/hotspot/share/interpreter/linkResolver.cpp:594 #4 0x00007ffff5e161e9 in LinkResolver::resolve_method (link_info=..., code=Bytecodes::_invokestatic, __the_thread__=0x7ffff09ed000) at open/src/hotspot/share/interpreter/linkResolver.cpp:786
19-09-2018
ILW = jaotc crash; aot, graal, with single invokestatic test, related to new nest-mates code; -Xverify:all = HLL = P4
18-09-2018
The test is described as "An attempt to invoke from subclass private static method hiding accessible one is made." But where is the accessible method that is supposedly being hidden? A simple test where we make the super class static method private without recompiling the subclass does not show any problems with jaotc. Regarding verification - unless explicitly disabled the default is to verify everything not loaded from the boot classpath.
18-09-2018
Sorry I need to be clearer. Method resolution will search the superclass, but method selection will not. But as I said above, If I am reading the test correctly. The subclass has an invokestatic to a method in the superclass. At runtime that superclass method has been made private. In that case we have: - current class == subclass - resolved class == superclass - method_holder == superclass so the nestmate check between current and resolved should not trigger the this!=k assertion because they are different. I probably need to reconstruct this test in a way that is more easily debuggable.
18-09-2018
> for invokestatic we don't (or shouldn't!) search for the method in the superclass. That is what is happening, and my understanding is that it is correct to do so.
18-09-2018
If I am reading the test correctly. The subclass has an invokestatic to a method in the superclass. At runtime that superclass method has been made private. In that case we have: - current class == subclass - resolved class == superclass - method_holder == superclass so the nestmate check between current and resolved should not trigger the this!=k assertion because they are different. If current class and resolved class are the same (subclass) but the method_holder is the superclass, then that doesn't make sense either because for invokestatic we don't (or shouldn't!) search for the method in the superclass. This really isn't making sense to me.
18-09-2018
For correctness I assume JCK is verifying the class before running the test. But in this case, I am only running jaotc to AOT the class, and jaotc is not running the verifier by default.
18-09-2018
Previous discussion aside an assertion in LinkResolver is wrong. I should be checking current class against method_holder not current against resolved!
18-09-2018
The only access related checks done in the verifier are for protected members of superclasses. As far as I am aware everything else is left to resolution and selection time. Looking at JVMCIEnv::lookup_method I don't see it disabling the access check. Also verifier failures should cause VerifyError to be thrown, which you don't see. The test also expects to fail with IllegalAccessError, so I'm still quite perplexed as to how we are getting past the access check. Outside of AOT testing is the test run with -Xverify:all?
18-09-2018
Aren't accessibility checks for superclass members done in the verifier? That would explain why -Xverify:all makes the problem go away.
18-09-2018
Thanks. If the method is private then the accessibility check should have failed before we got to the nestmate check! Unless this is one of those cases where access checking has been disabled ... in which case the assert needs to check that.
17-09-2018
The current class and resolved class are: javasoft/sqe/tests/vm/instr/invokestatic/invokestatic008/invokestatic00804m1/invokestatic00804m11n The method_holder is: javasoft/sqe/tests/vm/instr/invokestatic/invokestatic008/invokestatic00804m1/_invokestatic00804m11n We are trying resolve the method "mmm(II)I". _invokestatic00804m11n is the superclass of invokestatic00804m11n. These classes come from .jasm files.
17-09-2018
Thanks - that is interesting. Are you able to see what the resolved class and the method_holder class are? And the current class please.
17-09-2018
Here's the stack trace: V [libjvm.so+0x9168b7] report_vm_error(char const*, int, char const*, char const*, ...)+0x152 V [libjvm.so+0xba6afb] InstanceKlass::has_nestmate_access_to(InstanceKlass*, Thread*)+0x6f V [libjvm.so+0xe7d30b] LinkResolver::resolve_method(LinkInfo const&, Bytecodes::Code, Thread*)+0x63d V [libjvm.so+0xe7eb2d] LinkResolver::linktime_resolve_static_method(LinkInfo const&, Thread*)+0x75 V [libjvm.so+0xe7e8a8] LinkResolver::resolve_static_call(CallInfo&, LinkInfo const&, bool, Thread*)+0x4a V [libjvm.so+0xe811e3] LinkResolver::resolve_static_call_or_null(LinkInfo const&)+0x5f V [libjvm.so+0xd8c383] JVMCIEnv::lookup_method(InstanceKlass*, Klass*, Symbol*, Symbol*, Bytecodes::Code, constantTag)+0x13f V [libjvm.so+0xd8c6c7] JVMCIEnv::get_method_by_index_impl(constantPoolHandle const&, int, Bytecodes::Code, InstanceKlass*)+0x22f V [libjvm.so+0xd8c7d9] JVMCIEnv::get_method_by_index(constantPoolHandle const&, int, Bytecodes::Code, InstanceKlass*)+0x43 V [libjvm.so+0xd44d11] c2v_lookupMethodInPool(JNIEnv_*, _jobject*, _jobject*, int, signed char)+0x219 j jdk.vm.ci.hotspot.CompilerToVM.lookupMethodInPool(Ljdk/vm/ci/hotspot/HotSpotConstantPool;IB)Ljdk/vm/ci/hotspot/HotSpotResolvedJavaMethodImpl;+0 jdk.internal.vm.ci@12-internal j jdk.vm.ci.hotspot.HotSpotConstantPool.lookupMethod(II)Ljdk/vm/ci/meta/JavaMethod;+13 jdk.internal.vm.ci@12-internal
17-09-2018
What was the stack for this assertion failure? The expectation is that any code that needs to perform a nestmate check trivially excludes the current class so that you don't waste a call into the VM to check something that is trivially true (every class is a nestmate of itself). However LinkResolver::resolve_method does have a special case for private methods that uses an assertion to check that an unexpectedly found private method belongs to a nestmate of the current class - it's possible that code isn't accounting for all possibilities.
17-09-2018
http://cr.openjdk.java.net/~dlong/8210768/webrev/
17-09-2018
Agree!!!
14-09-2018
If I add -J-Xverify:all to the jaotc command line, the problem goes away. It might be a good idea to always set this flag in the jaotc launcher to avoid unverified bytecodes.
14-09-2018
Steps to reproduce: pushd JCK-runtime-11 jar cf ../aot.jar -C classes javasoft/sqe/tests/vm/instr/invokestatic/invokestatic008/invokestatic00804m1 $DEBUGJDK/bin/jaotc -J-ea -J-esa --info -J-cp -Jclasses --ignore-errors ../aot.jar
14-09-2018