RFR: JDK-8147388: Add diagnostic commands to attach JVMTI agent.

[David Holmes]

On 19/01/2016 7:26 AM, Dmitry Samersoff wrote:
> David,
>
> On 2016-01-18 23:47, David Holmes wrote:
>> On 18/01/2016 11:20 PM, Dmitry Samersoff wrote:
>>> Yasumasa,
>>>
>>>> Can we use VLA (Variable Length Arrays) ?
>>>
>>> Apple LLVM version 6.1.0 (clang-602.0.53) (based on LLVM 3.6.0svn)
>>> Target: x86_64-apple-darwin14.5.0
>>> Thread model: posix
>>>
>>> Compiles it just fine.
>>
>> Are we using variable length arrays anywhere else in the VM yet?
>
> Probably not.
>
> But personally, I see no reason to don't use it for simple cases like
> this one.

Anytime we start to use a language feature for the first time we need to 
be extra diligent to make sure there are no unintended side-effects and 
that all our supported compilers (and probably a few others used in the 
community) do the right thing. A bit of googling seems to indicate that 
variable length arrays are part of C99 but are not allowed in C++ - 
though gcc has an extension that does allow them.

This reports they are not available in Visual Studio C++:

https://msdn.microsoft.com/en-us/library/zb1574zs.aspx

David
-----

>> What
>> are the implications for allocation and in particular allocation failure?
>
> This allocation just reserves some space on the stack[1], so it can
> cause stack overflow if we attempt to allocate two much bytes.



> 1. Listing fragment (extra labels are removed)
>
>     3                    .Ltext0:
>     5                    .LC0:
>
>    14:test.cxx      **** void testme(int n) {
>    15:test.cxx      ****   char m[n];
>    25 0000 4863FF                movslq  %edi, %rdi
>    28 0003 55                    pushq   %rbp
>    37 0004 BE000000              movl    $.LC0, %esi
>    41 0009 4883C70F              addq    $15, %rdi
>    46 000d 31C0                  xorl    %eax, %eax
>    50 000f 4883E7F0              andq    $-16, %rdi
>    54 0013 4889E5                movq    %rsp, %rbp
>    59 0016 4829FC                subq    %rdi, %rsp
>    64 0019 BF010000              movl    $1, %edi
>    65 001e 4889E2                movq    %rsp, %rdx
>    66 0021 E8000000              call    __printf_chk
>    16:test.cxx      ****   printf("%s", m);
>    17:test.cxx      **** }
>
> -Dmitry
>
>
>>
>> David
>> -----
>>
>>> -Dmitry
>>>
>>> On 2016-01-18 16:09, Yasumasa Suenaga wrote:
>>>> Hi Dmitry,
>>>>
>>>>> 1. It might be better to have one jcmd to run both java and native java
>>>>> agent. If agent library name ends with ".jar" we can assume it's java
>>>>> agent.
>>>>
>>>> Okay, I'll try it.
>>>>
>>>>> if (_libpath.value() == NULL) {
>>>>>       error ...
>>>>> }
>>>>
>>>> I will add it.
>>>> However, I note you that _libpath is given mandatory flag.
>>>> http://mail.openjdk.java.net/pipermail/serviceability-dev/2016-January/018661.html
>>>>
>>>>
>>>>
>>>>> char options[option_len];
>>>>
>>>> Can we use VLA (Variable Length Arrays) ?
>>>> I'm worried that several C++ compiler cannot compile this code.
>>>> http://clang.llvm.org/compatibility.html#vla
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> On 2016/01/18 19:38, Dmitry Samersoff wrote:
>>>>> Yasumasa,
>>>>>
>>>>> 1. It might be better to have one jcmd to run both java and native java
>>>>> agent. If agent library name ends with ".jar" we can assume it's java
>>>>> agent.
>>>>>
>>>>> 2. Please get rid of malloc/free and check _libpath.value() for NULL at
>>>>> ll. 295 and below.
>>>>>
>>>>>
>>>>> if (_libpath.value() == NULL) {
>>>>>       error ...
>>>>> }
>>>>>
>>>>> if (_option.value() == NULL) {
>>>>>       JvmtiExport::load_agent_library("instrument",
>>>>>                    "false", _libpath.value(), output());
>>>>>      return;
>>>>> }
>>>>>
>>>>> size_t option_len = \
>>>>>       strlen(_libpath.value()) + strlen(_option.value()) + 1;
>>>>>
>>>>> char options[option_len];
>>>>>
>>>>> ....
>>>>>
>>>>> -Dmitry
>>>>>
>>>>>
>>>>> On 2016-01-15 16:33, Yasumasa Suenaga wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I added permissions and tests in new webrev:
>>>>>>      http://cr.openjdk.java.net/~ysuenaga/JDK-8147388/webrev.01/
>>>>>>
>>>>>> Two tests (LoadAgentDcmdTest, LoadJavaAgentDcmdTest) work fine.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>> On 2016/01/15 17:20, Staffan Larsen wrote:
>>>>>>> This is a good improvement overall.
>>>>>>>
>>>>>>> The new diagnostic commands need to have proper permissions set:
>>>>>>>
>>>>>>>        static const JavaPermission permission() {
>>>>>>>           JavaPermission p =
>>>>>>> {"java.lang.management.ManagementPermission",
>>>>>>>                              “control", NULL};
>>>>>>>          return p;
>>>>>>>       }
>>>>>>>
>>>>>>> And as David said: tests! See hotspot/test/serviceability/dcmd/jvmti.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> /Staffan
>>>>>>>
>>>>>>>> On 14 jan. 2016, at 15:00, Yasumasa Suenaga <yasuenag at gmail.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> We can use Attach API to attach JVMTI agent to live process.
>>>>>>>> However, we have to write Java code for it.
>>>>>>>>
>>>>>>>> If we can attach JVMTI agents through jcmd, it is very useful.
>>>>>>>> So I want to add two new diagnostic commands:
>>>>>>>>
>>>>>>>> * JVMTI.agent_load: Load JVMTI native agent.
>>>>>>>> * JVMTI.javaagent_load: Load JVMTI java agent.
>>>>>>>>
>>>>>>>> I maintain two JVMTI agents - HeapStats [1] and JLivePatcher [2].
>>>>>>>> [1] is native agent, [2] is java agent.
>>>>>>>> They provide a program for attaching to live process.
>>>>>>>>
>>>>>>>> I guess that various JVMTI agents provide own attach mechanism like
>>>>>>>> them.
>>>>>>>> I think that we should provide general way to attach.
>>>>>>>>
>>>>>>>> I've uploaded webrev. Could you review it?
>>>>>>>>      http://cr.openjdk.java.net/~ysuenaga/JDK-8147388/webrev.00/
>>>>>>>>
>>>>>>>> I'm jdk9 committer, however I cannot access JPRT.
>>>>>>>> So I need a sponsor.
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> Yasumasa
>>>>>>>>
>>>>>>>>
>>>>>>>> [1] http://icedtea.classpath.org/wiki/HeapStats
>>>>>>>> [2] https://github.com/YaSuenag/jlivepatcher  (in Japanese)
>>>>>>>
>>>>>
>>>>>
>>>
>>>
>
>