Hi David, Dmitry,

This reports they are not available in Visual Studio C++:

https://msdn.microsoft.com/en-us/library/zb1574zs.aspx

Thanks David,
Should I use malloc/free at this patch?
(I think that we should NOT use VLA because VS2015 does not support it.)


Thanks,

Yasumasa


On 2016/01/19 8: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)







Reply via email to