[
https://issues.apache.org/jira/browse/GROOVY-12364?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18111981#comment-18111981
]
ASF GitHub Bot commented on GROOVY-12364:
-----------------------------------------
codecov-commenter commented on PR #2888:
URL: https://github.com/apache/groovy/pull/2888#issuecomment-5556811135
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2888?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
Report
:x: Patch coverage is `83.33333%` with `3 lines` in your changes missing
coverage. Please review.
:white_check_mark: Project coverage is 71.1045%. Comparing base
([`0c4dd93`](https://app.codecov.io/gh/apache/groovy/commit/0c4dd93ee000639493bdaf480edbd05f02633fd8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`eb2f136`](https://app.codecov.io/gh/apache/groovy/commit/eb2f1363787fffd1256b35c303494a7aabc042f2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 3 commits behind head on master.
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2888?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Patch % | Lines |
|---|---|---|
|
[...vy/vmplugin/v8/IndyGuardsFiltersAndSignatures.java](https://app.codecov.io/gh/apache/groovy/pull/2888?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fvmplugin%2Fv8%2FIndyGuardsFiltersAndSignatures.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3ZtcGx1Z2luL3Y4L0luZHlHdWFyZHNGaWx0ZXJzQW5kU2lnbmF0dXJlcy5qYXZh)
| 50.0000% | [2 Missing :warning:
](https://app.codecov.io/gh/apache/groovy/pull/2888?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[...java/org/codehaus/groovy/vmplugin/v8/Selector.java](https://app.codecov.io/gh/apache/groovy/pull/2888?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fvmplugin%2Fv8%2FSelector.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3ZtcGx1Z2luL3Y4L1NlbGVjdG9yLmphdmE=)
| 92.8571% | [1 Missing :warning:
](https://app.codecov.io/gh/apache/groovy/pull/2888?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/groovy/pull/2888?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #2888 +/- ##
==================================================
+ Coverage 71.1006% 71.1045% +0.0038%
- Complexity 37348 37352 +4
==================================================
Files 1579 1579
Lines 135477 135495 +18
Branches 25104 25109 +5
==================================================
+ Hits 96325 96343 +18
Misses 30478 30478
Partials 8674 8674
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2888?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[...vmplugin/v8/ColdReflectiveMethodHandleWrapper.java](https://app.codecov.io/gh/apache/groovy/pull/2888?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fvmplugin%2Fv8%2FColdReflectiveMethodHandleWrapper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3ZtcGx1Z2luL3Y4L0NvbGRSZWZsZWN0aXZlTWV0aG9kSGFuZGxlV3JhcHBlci5qYXZh)
| `94.1176% <ø> (ø)` | |
|
[...java/org/codehaus/groovy/vmplugin/v8/Selector.java](https://app.codecov.io/gh/apache/groovy/pull/2888?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fvmplugin%2Fv8%2FSelector.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3ZtcGx1Z2luL3Y4L1NlbGVjdG9yLmphdmE=)
| `81.5000% <92.8571%> (+0.2713%)` | :arrow_up: |
|
[...vy/vmplugin/v8/IndyGuardsFiltersAndSignatures.java](https://app.codecov.io/gh/apache/groovy/pull/2888?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fvmplugin%2Fv8%2FIndyGuardsFiltersAndSignatures.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3ZtcGx1Z2luL3Y4L0luZHlHdWFyZHNGaWx0ZXJzQW5kU2lnbmF0dXJlcy5qYXZh)
| `91.0256% <50.0000%> (-2.2176%)` | :arrow_down: |
... and [208 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/2888/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests,
report on failures, and find test suite problems.
- :package: [JS Bundle
Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save
yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
> 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)
