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

Staffan Larsen staffan.larsen at oracle.com
Wed Jan 20 08:53:06 UTC 2016 

> On 19 jan. 2016, at 23:37, David Holmes <david.holmes at oracle.com> wrote:
> 
> On 19/01/2016 11:19 PM, Yasumasa Suenaga wrote:
>> Hi,
>> 
>> I uploaded a new webrev:
>>   http://cr.openjdk.java.net/~ysuenaga/JDK-8147388/webrev.03/
>> 
>> It is malloc/free version.
>> If NULL returns from malloc(), it calls vm_exit_out_of_memory().
> 
> That seems rather extreme - do other dcmd failures abort the VM? I would have expected some kind of error propagation back to the caller.

I would suggest outputting an error message to output(). Diagnostic Commands should not terminate the VM on error conditions.



> Thanks,
> David
> 
>> All test about this changes work fine.
>> Please review.
>> 
>> Thanks,
>> 
>> Yasumasa
>> 
>> 
>> On 2016/01/19 22:07, Dmitry Samersoff wrote:
>>> David,
>>> 
>>>> 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.
>>> 
>>> I hear your concern.
>>> 
>>> Moreover I revisit my advice and think it's not a good idea to do a
>>> stack allocation based on unverified user input.
>>> 
>>> Yasumasa, sorry for leading in wrong direction. Lets use malloc/free in
>>> this case.
>>> 
>>> Nevertheless, I would like to start broader evaluation of possible use
>>> on-stack allocation (either alloca or VLA) - hotspot may benefit of it
>>> in many places.
>>> 
>>> -Dmitry
>>> 
>>> On 2016-01-19 02:06, David Holmes wrote:
>>>> 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)
>>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>> 
>>>>> 
>>> 
>>>

More information about the serviceability-dev mailing list