Re: RFR: 8283092: JMX subclass permission check redundant with strong encapsulation [v2]

2022-03-18 Thread Kevin Walls
> Removing permission checks which, in the presence of a Security Manager, 
> would check for a RuntimePermission "className.subclass".  This was to 
> prevent subclassing these classes, but is no longer necessary with strong 
> encapsulation from modules.

Kevin Walls has updated the pull request incrementally with one additional 
commit since the last revision:

  Test update

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7827/files
  - new: https://git.openjdk.java.net/jdk/pull/7827/files/a6d05f73..98f1577d

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7827&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7827&range=00-01

  Stats: 71 lines in 1 file changed: 28 ins; 11 del; 32 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7827.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7827/head:pull/7827

PR: https://git.openjdk.java.net/jdk/pull/7827


Re: RFR: 8283092: JMX subclass permission check redundant with strong encapsulation

2022-03-18 Thread Kevin Walls
On Tue, 15 Mar 2022 20:22:16 GMT, Kevin Walls  wrote:

> Removing permission checks which, in the presence of a Security Manager, 
> would check for a RuntimePermission "className.subclass".  This was to 
> prevent subclassing these classes, but is no longer necessary with strong 
> encapsulation from modules.

Added test update.  Checks that with permissive module options we can extend 
sun.management.spi.PlatformMBeanProvider, and that without them we cannot.

Output of the new test is like this:

--System.out:(11/1279)--
---PlatformMBeanProviderConstructorCheck:
---PlatformMBeanProviderConstructorCheck: invoke MyProvider with expectedFail = 
false
---PlatformMBeanProviderConstructorCheck PASSED (1) (expectedFail = false)
---PlatformMBeanProviderConstructorCheck: re-invoke without --add-modules or 
--add-exports
Command line: 
[/tank/kwalls/repos/personal/jdk/open/test/../../build/linux-x86_64-server-release/images/jdk/bin/java
 -cp 
/tank/kwalls/repos/personal/jdk/open/test/JTwork/classes/sun/management/PlatformMBeanProviderConstructorCheck.d:/tank/kwalls/repos/personal/jdk/open/test/jdk/sun/management:/tank/kwalls/repos/personal/jdk/open/test/JTwork/classes/test/lib:/tank/kwalls/repos/personal/jdk/open/test/lib:/opt/jtreg6.1/lib/javatest.jar:/opt/jtreg6.1/lib/jtreg.jar
 PlatformMBeanProviderConstructorCheck --nomoduleargs ]
[2022-03-18T17:19:34.798024163Z] Gathering output for process 10627
[2022-03-18T17:19:34.902532753Z] Waiting for completion for process 10627
[2022-03-18T17:19:34.902807078Z] Waiting for completion finished for process 
10627
Output and diagnostic info for process 10627 was saved into 
'pid-10627-output.log'
[2022-03-18T17:19:34.908368606Z] Waiting for completion for process 10627
[2022-03-18T17:19:34.908503964Z] Waiting for completion finished for process 
10627
--System.err:(9/647)--
 stdout: [---PlatformMBeanProviderConstructorCheck:
---PlatformMBeanProviderConstructorCheck: invoke MyProvider with expectedFail = 
true
---PlatformMBeanProviderConstructorCheck got exception: 
java.lang.IllegalAccessError: superclass access check failed: class 
PlatformMBeanProviderConstructorCheck$MyProvider (in unnamed module 
@0x4795bbf5) cannot access class sun.management.spi.PlatformMBeanProvider (in 
module java.management) because module java.management does not export 
sun.management.spi to unnamed module @0x4795bbf5
---PlatformMBeanProviderConstructorCheck PASSED (2) (expectedFail = true)
];
 stderr: []
 exitValue = 0

STATUS:Passed.

-

PR: https://git.openjdk.java.net/jdk/pull/7827


Re: RFR: 8283092: JMX subclass permission check redundant with strong encapsulation

2022-03-18 Thread Kevin Walls
On Thu, 17 Mar 2022 13:55:22 GMT, Sean Mullan  wrote:

> test/jdk/sun/management/PlatformMBeanProviderConstructorCheck.java

Thank for noticing that Sean - had run various tests but missed this.  I have 
an update, will add it here soon.

-

PR: https://git.openjdk.java.net/jdk/pull/7827


Re: Proposal of a new version of AsyncGetCallTrace

2022-03-18 Thread Remi Forax
Knowing if there is a C stackframe in the middle of the stack while blocking on 
a synchronized is an important feature for a profiler when loom will land.

RĂ©mi

- Original Message -
> From: "Bechberger, Johannes" 
> To: "hotspot-dev" , 
> hotspot-jfr-...@openjdk.java.net, "serviceability-dev"
> 
> Sent: Friday, March 18, 2022 10:43:58 AM
> Subject: Proposal of a new version of AsyncGetCallTrace

> Hi,
> 
> I would like propose to
> 
> 1. Replace duplicated stack walking code with unified API
> 2. Create a new version of AsyncGetCallTrace, tentatively called
> "AsyncGetCallTrace2", with more information on more frames using the unified
> API
> 
> A demo (as well as this text) is available at
> https://github.com/parttimenerd/asgct2-demo
> if you want to see a prototype of this proposal in action.
> 
> Unify Stack Walking
> 
> 
> There are currently multiple implementations of stack walking in JFR and for
> AsyncGetCallTrace.
> They each implement their own extension of vframeStream but with comparable
> features
> and check for problematic frames.
> 
> My proposal is, therefore, to replace the stack walking code with a unified 
> API
> that
> includes all error checking and vframeStream extensions in a single place.
> The prosposed new class is called StackWalker and could be part of
> `jfr/recorder/stacktrace` [1].
> This class also supports getting information on C frames so it can be
> potentially
> used for walking stacks in VMError (used to create hs_err files), further
> reducing the amount of different stack walking code.
> 
> AsyncGetCallTrace2
> 
> 
> The AsyncGetCallTrace call has seen increasing use in recent years
> in profilers like async-profiler.
> But it is not really an API (not exported in any header) and
> the information on frames it returns is pretty limited
> (only the method and bci for Java frames) which makes implementing
> profilers and other tooling harder. Tools like async-profiler
> have to resort to complicated code to partially obtain the information
> that the JVM already has.
> Information that is currently hidden and impossible to obtain is
> 
> - whether a compiled frame is inlined (currently only obtainable for the 
> topmost
> compiled frames)
>  -  although this can be obtained using JFR
> - C frames that are not at the top of the stack
> - compilation level (C1 or C2 compiled)
> 
> This information is helpful when profiling and tuning the VM for
> a given application and also for profiling code that uses
> JNI heavily.
> 
> Using the proposed StackWalker class, implementing a new API
> that returns more information on frames is possible
> as a thin wrapper over the StackWalker API [2].
> This also improves the maintainability as the code used
> in this API is used in multiple places and is therefore
> also better tested than the previous implementation, see
> [1] for the implementation.
> 
> The following describes the proposed API:
> 
> ```cpp
> void AsyncGetCallTrace2(asgct2::CallTrace *trace, jint depth, void* ucontext);
> ```
> 
> The structure of `CallTrace` is the same as the original
> `ASGCT_CallTrace` with the same error codes encoded in <= 0
> values of `num_frames`.
> 
> ```cpp
> typedef struct {
>  JNIEnv *env_id;   // Env where trace was recorded
>  jint num_frames;  // number of frames in this trace
>  CallFrame *frames;// frames
>  void* frame_info; // more information on frames
> } CallTrace;
> ```
> 
> The only difference is that the `frames` array also contains
> information on C frames and the field `frame_info`.
> The `frame_info` is currently null and can later be used
> for extended information on each frame, being an array with
> an element for each frame. But the type of the
> elements in this array is implementation specific.
> This akin to `compile_info` field in JVMTI's CompiledMethodLoad
> [3] and used for extending the information returned by the
> API later.
> 
> Protoype
> 
> 
> Currently `CallFrame` is implemented in the prototype [4] as
> 
> ```cpp
> typedef struct {
>  void *machine_pc;   // program counter, for C and native frames 
> (frames
>  of native methods)
>  uint8_t type;   // frame type (single byte)
>  uint8_t comp_level; // highest compilation level of a method related 
> to
>  a Java frame
>  // information from original CallFrame
>  jint bci;   // bci for Java frames
>  jmethodID method_id;// method ID for Java frames
> } CallFrame;
> ```
> 
> The `FrameTypeId` is based on the frame type in JFRStackFrame:
> 
> ```cpp
> enum FrameTypeId {
>  FRAME_INTERPRETED = 0,
>  FRAME_JIT = 1, // JIT compiled
>  FRAME_INLINE  = 2, // inlined JITed methods
>  FRAME_NATIVE  = 3, // native wrapper to call C methods from Java
>  FRAME_CPP = 4  // c/c++/... frames, stub frames have CompLevel_all
> };
> ```
> 
> The `comp_level` states the compilatio

Proposal of a new version of AsyncGetCallTrace

2022-03-18 Thread Bechberger, Johannes
Hi,

I would like propose to

1. Replace duplicated stack walking code with unified API
2. Create a new version of AsyncGetCallTrace, tentatively called 
"AsyncGetCallTrace2", with more information on more frames using the unified API

A demo (as well as this text) is available at 
https://github.com/parttimenerd/asgct2-demo
if you want to see a prototype of this proposal in action.

Unify Stack Walking


There are currently multiple implementations of stack walking in JFR and for 
AsyncGetCallTrace. 
They each implement their own extension of vframeStream but with comparable 
features
and check for problematic frames.

My proposal is, therefore, to replace the stack walking code with a unified API 
that
includes all error checking and vframeStream extensions in a single place.
The prosposed new class is called StackWalker and could be part of
`jfr/recorder/stacktrace` [1].
This class also supports getting information on C frames so it can be 
potentially
used for walking stacks in VMError (used to create hs_err files), further
reducing the amount of different stack walking code.

AsyncGetCallTrace2


The AsyncGetCallTrace call has seen increasing use in recent years
in profilers like async-profiler.
But it is not really an API (not exported in any header) and
the information on frames it returns is pretty limited 
(only the method and bci for Java frames) which makes implementing
profilers and other tooling harder. Tools like async-profiler
have to resort to complicated code to partially obtain the information
that the JVM already has.
Information that is currently hidden and impossible to obtain is

- whether a compiled frame is inlined (currently only obtainable for the 
topmost compiled frames)
  -  although this can be obtained using JFR 
- C frames that are not at the top of the stack
- compilation level (C1 or C2 compiled)

This information is helpful when profiling and tuning the VM for
a given application and also for profiling code that uses
JNI heavily.

Using the proposed StackWalker class, implementing a new API 
that returns more information on frames is possible 
as a thin wrapper over the StackWalker API [2]. 
This also improves the maintainability as the code used
in this API is used in multiple places and is therefore
also better tested than the previous implementation, see 
[1] for the implementation.

The following describes the proposed API:

```cpp
void AsyncGetCallTrace2(asgct2::CallTrace *trace, jint depth, void* ucontext);
```

The structure of `CallTrace` is the same as the original
`ASGCT_CallTrace` with the same error codes encoded in <= 0
values of `num_frames`.

```cpp
typedef struct {
  JNIEnv *env_id;   // Env where trace was recorded
  jint num_frames;  // number of frames in this trace
  CallFrame *frames;// frames
  void* frame_info; // more information on frames
} CallTrace;
```

The only difference is that the `frames` array also contains
information on C frames and the field `frame_info`.
The `frame_info` is currently null and can later be used
for extended information on each frame, being an array with
an element for each frame. But the type of the
elements in this array is implementation specific.
This akin to `compile_info` field in JVMTI's CompiledMethodLoad 
[3] and used for extending the information returned by the
API later.

Protoype


Currently `CallFrame` is implemented in the prototype [4] as

```cpp
typedef struct {
  void *machine_pc;   // program counter, for C and native frames 
(frames of native methods)
  uint8_t type;   // frame type (single byte)
  uint8_t comp_level; // highest compilation level of a method related 
to a Java frame
  // information from original CallFrame
  jint bci;   // bci for Java frames
  jmethodID method_id;// method ID for Java frames
} CallFrame;
```

The `FrameTypeId` is based on the frame type in JFRStackFrame:

```cpp
enum FrameTypeId {
  FRAME_INTERPRETED = 0, 
  FRAME_JIT = 1, // JIT compiled
  FRAME_INLINE  = 2, // inlined JITed methods
  FRAME_NATIVE  = 3, // native wrapper to call C methods from Java
  FRAME_CPP = 4  // c/c++/... frames, stub frames have CompLevel_all
};
```

The `comp_level` states the compilation level of the method related to the frame
with higher numbers representing "more" compilation. `0` is defined as
interpreted. It is modeled after the `CompLevel` enum in 
`compiler/compilerDefinitions`:

```cpp
// Enumeration to distinguish tiers of compilation
enum CompLevel {
  // ...
  CompLevel_none  = 0, // Interpreter
  CompLevel_simple= 1, // C1
  CompLevel_limited_profile   = 2, // C1, invocation & backedge counters
  CompLevel_full_profile  = 3, // C1, invocation & backedge 
counters + mdo
  CompLevel_full_optimization = 4  // C2 or JVMCI
};
```

The 

Re: RFR: 8282475: SafeFetch should not rely on existence of Thread::current [v7]

2022-03-18 Thread Johannes Bechberger
On Fri, 11 Mar 2022 07:52:16 GMT, Johannes Bechberger  
wrote:

>> The WXMode for the current thread (on MacOS aarch64) is currently stored in 
>> the thread class which is unnecessary as the WXMode is bound to the current 
>> OS thread, not the current instance of the thread class.
>> This pull request moves the storage of the current WXMode into a thread 
>> local global variable in `os` and changes all related code. SafeFetch 
>> depended on the existence of a thread object only because of the WXMode. 
>> This pull request therefore removes the dependency, making SafeFetch usable 
>> in more contexts.
>
> Johannes Bechberger has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Remove two unnecessary lines

This is not the point: It comes down to API design. If we use SafeFetch in 
os::is_first_C_frame (and thereby in frame::link_or_null) and not just in 
ASGCT, then it depends on when the other methods can be called. These methods 
are e.g. used whenever an error happens and a hs_err file is generated. We 
cannot guarantee that a JavaThread is always present there.

-

PR: https://git.openjdk.java.net/jdk/pull/7727