Re: RFR: 8283092: JMX subclass permission check redundant with strong encapsulation [v2]
> 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
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
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
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
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]
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