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
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.hol...@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 >>>>>>>>>>>> <yasue...@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) >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>> >>>>>>> >>>>> >>>>> >>> >>>
