RFR: JDK-8147388: Add diagnostic commands to attach JVMTI agent.
Dmitry Samersoff
dmitry.samersoff at oracle.com
Wed Jan 20 07:53:51 UTC 2016
David,
> Unfortunately I do see some other implicit aborts due to allocation
> failures, but I have to say this seems very wrong to me.
IMHO,
If the system can't allocate about 100 bytes for valid options, we have
no chance to recover or send a message.
if someone try to feed dcmd with 1Gb of garbage (e.g. because of
corrupted script file) we have no reason to recover.
-Dmitry
On 2016-01-20 08:03, David Holmes wrote:
> On 20/01/2016 9:13 AM, Yasumasa Suenaga wrote:
>> Hi David,
>>
>> ShouldNotReachHere( ) is called at NMTDcmd:
>> http://hg.openjdk.java.net/jdk9/hs-rt/hotspot/file/8fcd5cba7938/src/share/vm/services/nmtDCmd.cpp#l156
>>
>
> Sure but that is more of a debugging check - we never expect to reach
> that code.
>
> Unfortunately I do see some other implicit aborts due to allocation
> failures, but I have to say this seems very wrong to me.
>
> David
> -----
>
>> If target VM is aborted, caller (e.g. jcmd) cannot receive response. I
>> think that the caller show Exception.
>>
>> Thanks,
>>
>> Yasumasa
>>
>> 2016/01/20 7:37 "David Holmes" <david.holmes at oracle.com
>> <mailto:david.holmes at oracle.com>>:
>>
>> 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
>>
>> <mailto: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)
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.
More information about the serviceability-dev
mailing list