RFR: 8307478: Implementation of Prepare to Restrict The Dynamic Loading of Agents [v7]

[Serguei Spitsyn]

On Wed, 31 May 2023 15:01:37 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> This is the implementation for JEP 451. There are two parts to this:
>> 
>> 1. A multi-line warning is printed when a JVM TI or Java agent is loaded into a running VM. For JVM TI, the message is printed to stderr from JvmtiAgent::load. For Java agents, it is printed to System.err (as that may be redirected) in the JPLIS (j.l.instrumentation) implementation. This part includes an update to the JVM TI spec and API docs to require the warning.
>> 
>> 2. If running with -Djdk.instrument.traceUsage or -Djdk.instrument.traceUsage=true, the calls to the Instrumentation API print a trace message and stack trace.
>> 
>> Testing: tier1-6
>
> Alan Bateman has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 17 additional commits since the last revision:
> 
>  - Add impl note to document the XX option
>  - Cleanup
>  - Merge
>  - Allow for warning to be skipped when same agent loaded a second/subsequent time
>  - Merge
>  - Tweak javadoc, update test to use more test infra
>  - Merge
>  - Merge
>  - Refresh package description
>  - Merge
>  - ... and 7 more: https://git.openjdk.org/jdk/compare/381973e4...2d9d5922

The update looks good to me.
Posted one nit though.
Thanks,
Serguei

test/jdk/com/sun/tools/attach/warnings/DynamicLoadWarningTest.java line 67:

> 65: class DynamicLoadWarningTest {
> 66:     private static final String JVMTI_AGENT_WARNING = "WARNING: A JVM TI agent has been dynamically loaded";
> 67:     private static final String JAVA_AGENT_WARNING = "WARNING: A Java agent has been loaded dynamically";

Nit: The warnings can be unified to say "loaded dynamically" or "dynamically loaded" in the same order.
Of course, it also impacts the files: `src/hotspot/share/prims/jvmtiAgent.cpp` and `src/java.instrument/share/classes/sun/instrument/InstrumentationImpl.java`.

-------------

Marked as reviewed by sspitsyn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13899#pullrequestreview-1454785048
PR Review Comment: https://git.openjdk.org/jdk/pull/13899#discussion_r1212717874