RFR: JDK-8147388: Add diagnostic commands to attach JVMTI agent.
David Holmes
david.holmes at oracle.com
Wed Jan 20 05:03:26 UTC 2016
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)
>
>
>
>
>
>
>
>
>
>
More information about the serviceability-dev
mailing list