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) >>>>>>>> >>>>>> >>>>>> >>>> >>>> >> >> -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.