On Wed, 9 Apr 2025 17:14:27 GMT, Jiangli Zhou <jian...@openjdk.org> wrote:

>> Please review this PR that changes 
>> `LoadAgentDcmdTest.getLibInstrumentPath()` to not locate `libinstrument` 
>> shared library if running on static JDK, instead just returns 
>> "libinstrument.<ext>" directly. Both test case #1 and #2 in 
>> `LoadAgentDcmdTest.run()` run ok on static JDK with the change:
>> 
>> Commands:
>> Test case #1: `JVMTI.agent_load libinstrument.so agent.jar`
>> Test case #2: `JVMTI.agent_load libinstrument.so "agent.jar=foo=bar"`
>> 
>> Additional notes/considerations:
>> 
>> 1. I notice 
>> [JDKToolFinder.getJDKTool()](https://github.com/openjdk/jdk/blob/80ff7b9c9406c7845ecb3bc40910e92ccdd23ff2/test/lib/jdk/test/lib/JDKToolFinder.java#L41)
>>  is used by the test to find the `jcmd` tool. `JDKToolFinder.getJDKTool()` 
>> looks for the requested tool in both `test.jdk` and `compile.jdk`. When 
>> running the jtreg test on static JDK, it's able to locate the `jcmd` from 
>> `compile.jdk` even though the static JDK binary (`test.jdk`) does not 
>> provide the tool. For tools used by jtreg tests at runtime, how about change 
>> to always do that?
>> 
>> 2. From https://docs.oracle.com/en/java/javase/21/docs/specs/man/jcmd.html:
>> 
>> JVMTI.agent_load [arguments]
>> Loads JVMTI native agent.
>>   Impact: Low
>> arguments:
>>   library path: Absolute path of the JVMTI agent to load. (STRING, no 
>> default value)
>>   agent option: (Optional) Option string to pass the agent. (STRING, no 
>> default value)
>> 
>> The command spec requires the absolute path of the JVMTI agent for the 
>> `library path` argument. On static JDK, if the agent library is built-in 
>> (statically linked), passing the shared library name works and allows the VM 
>> to find the built-in agent. There would be no need to specify the absolute 
>> path. Please see  
>> https://bugs.openjdk.org/browse/JDK-8353938?focusedId=14767737&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14767737
>>  for more details.
>
> Jiangli Zhou has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Address @AlanBateman's comment:
>   - Updated copyright header

Marked as reviewed by alanb (Reviewer).

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

PR Review: https://git.openjdk.org/jdk/pull/24497#pullrequestreview-2754244716

Reply via email to