[ 
https://issues.apache.org/jira/browse/GROOVY-12364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18111972#comment-18111972
 ] 

ASF GitHub Bot commented on GROOVY-12364:
-----------------------------------------

paulk-asert opened a new pull request, #2888:
URL: https://github.com/apache/groovy/pull/2888

   … AOT-linked sites
   
   Lookup.unreflect returns a caller-bound handle for a @CallerSensitive 
method, which is exactly right on HotSpot but cannot be executed by GraalVM's 
MethodHandle interpreter: a native image aborts with the VM-fatal "Cannot 
invoke method that has a @CallerSensitiveAdapter without an explicit caller" as 
soon as dynamic code calls e.g. Logger.getLogger or MethodHandles.lookup. 
Reflection handles such methods fine there, so an AOT-linked site now builds 
the target handle around Method.invoke instead, shaped like the unreflected 
handle so the guarded chain is unchanged. The observed caller then becomes 
Groovy's runtime rather than the calling class, which is harmless for logger 
and bundle lookups in a single-module image; MethodHandles.lookup() should be 
static-compiled in native code.




> Native image: dynamic call to a @CallerSensitive method (e.g. 
> Logger.getLogger) is fatal under AOT link mode
> ------------------------------------------------------------------------------------------------------------
>
>                 Key: GROOVY-12364
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12364
>             Project: Groovy
>          Issue Type: Bug
>            Reporter: Paul King
>            Assignee: Paul King
>            Priority: Major
>
> In a GraalVM native image, a *dynamic* Groovy call to a JDK method annotated 
> {{@CallerSensitive}} whose JDK implementation has a 
> {{@CallerSensitiveAdapter}} variant (JDK 18+) kills the process with an 
> uncatchable VM error. {{java.util.logging.Logger.getLogger(String)}} is the 
> everyday case, so any dynamic Groovy class with a {{static final Logger LOG = 
> Logger.getLogger(...)}} field cannot even initialise in a native image:
> {noformat}
> Fatal error: Cannot invoke method that has a @CallerSensitiveAdapter without 
> an explicit caller
> {noformat}
> followed by a native crash dump. The Java frames in the dump are (innermost 
> first):
> {noformat}
> com.oracle.svm.shared.util.VMError.shouldNotReachHere(VMError.java:91)
> com.oracle.svm.core.reflect.SubstrateMethodAccessor.methodHandleInvoke(SubstrateMethodAccessor.java:130)
> com.oracle.svm.core.methodhandles.Util_java_lang_invoke_MethodHandle.invokeInternal(Target_java_lang_invoke_MethodHandle.java:263)
> java.lang.invoke.MethodHandle.invokeBasic(MethodHandle.java:119)
> com.oracle.svm.core.methodhandles.MethodHandleIntrinsicImpl.execute(MethodHandleIntrinsicImpl.java:178)
> ... (LambdaForm.interpretWithArguments / invokeBasic repeated for each link 
> of Groovy's guarded chain) ...
> java.lang.invoke.MethodHandleImpl.guardWithCatch(MethodHandleImpl.java:957)
> org.codehaus.groovy.vmplugin.v8.IndyInterface.aotDispatch(IndyInterface.java:604)
> Svc.<clinit>(NativeLogProbe.groovy:6)      // static final Logger JUL = 
> Logger.getLogger('svc')
> {noformat}
> h3. Reproducer
> Precompile and build with the agent-recorded metadata as in the Groovy 6 
> native-image instructions (GraalVM CE 25.2.4, Groovy 6.0.0-SNAPSHOT master):
> {code:groovy}
> class CsNative {
>     static void main(String[] args) {
>         switch (args[0]) {
>             case 'logger':  println 
> java.util.logging.Logger.getLogger('x').name; break   // fatal
>             case 'lookup':  println 
> java.lang.invoke.MethodHandles.lookup().lookupClass(); break   // fatal
>             case 'forname': println Class.forName('java.lang.String'); break  
>  // ok
>             case 'syslog':  println System.getLogger('y').name; break   // ok
>             case 'ok':      println Integer.parseInt('42'); break   // ok
>         }
>     }
> }
> {code}
> Results, same binary:
> ||call||native image||JVM with {{-Dgroovy.indy.aot.link=true}}||
> |{{Logger.getLogger('x')}}|Fatal error|ok|
> |{{MethodHandles.lookup()}}|Fatal error|ok|
> |{{Class.forName(...)}}, {{System.getLogger(...)}}|ok|ok|
> |{{Logger.getMethod('getLogger', String).invoke(null, 'r')}} from 
> {{@CompileStatic}} code|ok|ok|
> |{{MethodHandles.lookup().unreflect(getLogger).invokeWithArguments('m')}} 
> from {{@CompileStatic}} code|ok|ok|
> So neither the method nor reflection nor method handles as such are the 
> problem; only Groovy's *runtime-built* handle for the target is.
> h3. Cause
> {{Selector.MethodSelector.unreflect}} obtains the target handle with 
> {{callSite.getLookup().unreflect(method)}}. For a caller-sensitive method the 
> JDK returns a handle bound to the lookup class (the Groovy caller; 
> {{MethodHandles.Lookup#unreflect}} / {{MethodHandleImpl.bindCaller}}), which 
> on HotSpot gives exactly the right caller semantics. The AOT link mode 
> (GROOVY-12234) then invokes the guarded chain from {{aotDispatch}}. GraalVM 
> cannot intrinsify a handle that is only constructed at run time, so it 
> executes the chain in its MethodHandle interpreter, and when the interpreter 
> reaches the caller-sensitive leaf, 
> {{SubstrateMethodAccessor.methodHandleInvoke}} requires the adapter form with 
> an explicit caller argument, which the bound handle does not supply, and it 
> aborts with {{VMError.shouldNotReachHere}} rather than throwing. The last two 
> rows of the table show the same handle works when GraalVM can see its 
> construction at build time, and that GraalVM's reflective path handles 
> caller-sensitive methods fine.
> {{Class.forName}} and {{System.getLogger}} survive because GraalVM 
> substitutes them directly.
> h3. Proposed fix
> Groovy already knows which targets are caller-sensitive: 
> {{CachedMethod.isCallerSensitive()}} (annotation probe plus the serialization 
> classes, conservative when undecidable), used today to keep such methods off 
> the reflective cold tier on HotSpot, where {{Method.invoke}} would misreport 
> the caller. Under AOT link mode the trade-off is the opposite: the handle 
> path is fatal and the reflective path works. So for an AOT-linked site whose 
> selected method is caller-sensitive, dispatch through {{Method.invoke}} (the 
> reflective wrapper already used as the AOT steady state) instead of building 
> the handle chain; static targets such as {{Logger.getLogger}} need to be 
> admitted to that path, which currently accepts instance methods only. 
> Interface selections should probe the receiver's implementation as the cold 
> tier does.
> Consequence to document: in a native image the observed caller of such a 
> method becomes Groovy's runtime rather than the calling class. For 
> {{Logger.getLogger}}, {{System.getLogger}} and resource bundles that only 
> affects module/loader resolution, which is a single application module in an 
> image anyway; for {{MethodHandles.lookup()}} it means the returned lookup is 
> not the caller's, so that call should be static-compiled or avoided in 
> dynamic native code. Not crashing is still strictly better than the current 
> fatal error.
> Longer term this is worth reporting to GraalVM: a caller-bound handle 
> produced by {{Lookup.unreflect}} should be invocable from the MethodHandle 
> interpreter (the caller is known at bind time), and a VM-fatal error for a 
> user-triggerable condition is disproportionate; an 
> {{UnsupportedOperationException}} would at least let Groovy fall back at run 
> time.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to