RFR: JDK-8147388: Add diagnostic commands to attach JVMTI agent.
David Holmes
david.holmes at oracle.com
Tue Jan 19 22:37:09 UTC 2016
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.
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