Debug compiler configuration fails to build JDK21 with: Assertion failed at openj9/runtime/compiler/env/VMJ9.cpp:8881: vettedForAOT

[cjjdespres]

Following up on https://github.com/eclipse-openj9/openj9/issues/19849, I tried building JDK21 (xlinux) with a debug compiler configuration using the usual environment variables:

export cflags="-Og -ggdb3 -fno-inline -DDEBUG"
export EXTRA_CMAKE_ARGS="-DJ9JIT_EXTRA_CFLAGS=\"$cflags\" -DJ9JIT_EXTRA_CXXFLAGS=\"$cflags\""

I said in that linked issue that reverting the first few commits of https://github.com/eclipse-openj9/openj9/pull/19514 was sufficient to get the debug JDK to finish building, and while that is usually true, I noticed after trying to build a debug JDK that it will occasionally crash and fail to generate the full image with the following assert:

Compiling up to 4 files for BUILD_JIGSAW_TOOLS
Optimizing the exploded image
Assertion failed at /home/despresc/dev/testing/openj9-openjdk-jdk21/openj9/runtime/compiler/env/VMJ9.cpp:8881: vettedForAOT
VMState: 0x000501ff
    The TR_J9SharedCacheVM version of this method is expected to be called only from isClassLibraryMethod.
Please consider whether you call is vetted for AOT!
compiling java/util/ImmutableCollections$MapN.probe(Ljava/lang/Object;)I at level: warm

The reason this assert is firing is this method:

https://github.com/eclipse-openj9/openj9/blob/27dac39d2c5b8ea2df7c7e005a5ac0d9eb1f5b9c/runtime/compiler/env/VMJ9.cpp#L3927-L3938

introduced in https://github.com/eclipse-openj9/openj9/pull/18243. It calls the virtual method isClassLibraryMethod, which in the failing case is TR_J9SharedCacheVM::isClassLibraryMethod, with the default argument false for vettedForAOT, causing the assertion.

(I should say that I didn't go through the trouble of reverting those commits in https://github.com/eclipse-openj9/openj9/pull/19514 to attempt to get a debug build to finish compiling. I just commented out the few "Too many dependencies" asserts that exist in omr/compiler/x/codegen/OMRRegisterDependency.hpp on the omr side.)

[cjjdespres]

I'm not sure exactly what the requirements for vettedForAOT being true are here. Would you happen to know, @dsouzai?

[dsouzai]

Yeah I have some idea about it. vettedForAOT is specific to non-SVM AOT; the purpose is to allow specific locations in TR code that a developer as "vetted" is safe to call under AOT. The problem I ran into was that it is very non-trivial to determine just based on code location whether it's ok for a query to be called under AOT; that's why we came up with the SVM, which bypassed the need for this "vetting".

If you look at the queries, SVM completely ignores the vettedForAOT param. For non-SVM AOT, the safe thing to do is to pass in false since that basically says "we can't validate the answer of this query for that specific caller".

[dsouzai]

I guess that assert doesn't know about the SVM heh

[cjjdespres]

This looks safe, then? The method is called to prevent the inlining of a java method that has the annotation @ChangesCurrentThread if the caller does not have @ChangesCurrentThread. I think method annotations like these are in the ROM class, so they should be covered in the class/class chain validations, and so a compile time/load time difference in these annotations would be caught elsewhere.

[dsouzai]

Yeah I suppose so; if the annotations are part of the J9ROMClass such that a change would result in a different J9ROMClass than what is stored in the SCC, then this query does seem safe even in non-SVM AOT.