Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v7]

2022-02-16 Thread Brent Christian
On Wed, 16 Feb 2022 21:39:20 GMT, Tim Prinzing  wrote:

>> JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is 
>> null
>
> Tim Prinzing has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 10 commits:
> 
>  - Merge master
>  - fix test breakage from rename
>  - reformat test summary
>  - reformat test summary
>  - improve test name and remove experimental test code
>  - fix copyright date
>  - More changes from feedback.
>
>The javadoc is improved to reflect InvalidCallerException is thrown with
>a caller that can't be assigned to a ClassLoader as well as a null
>caller frame.  Added a test IsParallelCapableBadCaller that uses
>reflection hackery to create a case where called with an invalid caller
>on the call stack.
>  - Changes from feedback.
>
>- Copyright dates fixed
>- IllegalCallerException thrown for no caller frame, and associated
>javadoc changes
>- test changed to look for IllegalCallerException thrown.
>  - Merge branch 'master' into JDK-8281000
>  - JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is 
> null

Looks fine.

-

Marked as reviewed by bchristi (Reviewer).

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


RFR: 8188073: Add Capstone as backend for hsdis

2022-02-16 Thread Magnus Ihse Bursie
The Capstone library is a simple to use, efficient disassembly library, 
distributed under the MIT license. 

This PR implements an hsdis backend using Capstone. It has been tested on 
Linux, macOS and Windows (x64).

The actual C implementation of the backend was done by @JornVernee. I assume 
there are plenty of room for enhancing this implementation, with options like 
in the binutils backend. Such improvements can be done later, by teams more 
familiar with the requirements of hsdis.

-

Commit messages:
 - Fix whitespace error (thanks, jcheck!)
 - Minor fixes
 - The pandoc-style copyright header looks bad on Github...
 - Turn the README into markdown format. Add documentation on building Capstone.
 - Fix capstone on Windows
 - Add capstone as hsdis backend

Changes: https://git.openjdk.java.net/jdk/pull/7506/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7506=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8188073
  Stats: 517 lines in 9 files changed: 346 ins; 136 del; 35 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7506.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7506/head:pull/7506

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


Re: RFR: 8253757: Add LLVM-based backend for hsdis [v8]

2022-02-16 Thread Magnus Ihse Bursie
On Wed, 16 Feb 2022 14:39:49 GMT, Magnus Ihse Bursie  wrote:

>> This patch expands the newly added system for hsdis backends to include LLVM.
>> 
>> The actual code in hsdis-llvm.cpp is based heavily on the work by @luhenry, 
>> as published in the never integrated PR 
>> https://github.com/openjdk/jdk/pull/392. (I have basically just ripped out 
>> the binutils-based part of it.)
>> 
>> Unfortunately I have not been able to make this work properly on Windows. 
>> With some additional flags I made it compile without complaints, but it 
>> caused hotspot to segfault in `LoadLibrary` (!) in `os::dll_load` when I 
>> tried to load the library. This is somewhat ironic, since the initial 
>> implementation was created by Ludovic for the very purpose of using it on 
>> Windows.
>> 
>> The lack of Windows support in this patch does not mean it is impossible to 
>> get it to work, just that I need to co-operate with someone who has more 
>> experience of compiling LLVM on Windows, and/or are more eager to get this 
>> combination to work.
>
> Magnus Ihse Bursie has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - Add section on using LLVM
>  - Allow override of LLVM location using --with-llvm

This PR has just gotten too messy. :-( I'll close this one for now, focus on 
getting the (much simpler) Capstone backend integrated, and then I can come 
back and revisit this functionality, but in a fresh new PR.

-

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


Withdrawn: 8253757: Add LLVM-based backend for hsdis

2022-02-16 Thread Magnus Ihse Bursie
On Wed, 13 Oct 2021 00:00:22 GMT, Magnus Ihse Bursie  wrote:

> This patch expands the newly added system for hsdis backends to include LLVM.
> 
> The actual code in hsdis-llvm.cpp is based heavily on the work by @luhenry, 
> as published in the never integrated PR 
> https://github.com/openjdk/jdk/pull/392. (I have basically just ripped out 
> the binutils-based part of it.)
> 
> Unfortunately I have not been able to make this work properly on Windows. 
> With some additional flags I made it compile without complaints, but it 
> caused hotspot to segfault in `LoadLibrary` (!) in `os::dll_load` when I 
> tried to load the library. This is somewhat ironic, since the initial 
> implementation was created by Ludovic for the very purpose of using it on 
> Windows.
> 
> The lack of Windows support in this patch does not mean it is impossible to 
> get it to work, just that I need to co-operate with someone who has more 
> experience of compiling LLVM on Windows, and/or are more eager to get this 
> combination to work.

This pull request has been closed without being integrated.

-

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


Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v7]

2022-02-16 Thread Mandy Chung
On Wed, 16 Feb 2022 21:39:20 GMT, Tim Prinzing  wrote:

>> JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is 
>> null
>
> Tim Prinzing has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 10 commits:
> 
>  - Merge master
>  - fix test breakage from rename
>  - reformat test summary
>  - reformat test summary
>  - improve test name and remove experimental test code
>  - fix copyright date
>  - More changes from feedback.
>
>The javadoc is improved to reflect InvalidCallerException is thrown with
>a caller that can't be assigned to a ClassLoader as well as a null
>caller frame.  Added a test IsParallelCapableBadCaller that uses
>reflection hackery to create a case where called with an invalid caller
>on the call stack.
>  - Changes from feedback.
>
>- Copyright dates fixed
>- IllegalCallerException thrown for no caller frame, and associated
>javadoc changes
>- test changed to look for IllegalCallerException thrown.
>  - Merge branch 'master' into JDK-8281000
>  - JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is 
> null

Looks good.

-

Marked as reviewed by mchung (Reviewer).

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


Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v7]

2022-02-16 Thread Tim Prinzing
> JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is 
> null

Tim Prinzing has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains 10 commits:

 - Merge master
 - fix test breakage from rename
 - reformat test summary
 - reformat test summary
 - improve test name and remove experimental test code
 - fix copyright date
 - More changes from feedback.
   
   The javadoc is improved to reflect InvalidCallerException is thrown with
   a caller that can't be assigned to a ClassLoader as well as a null
   caller frame.  Added a test IsParallelCapableBadCaller that uses
   reflection hackery to create a case where called with an invalid caller
   on the call stack.
 - Changes from feedback.
   
   - Copyright dates fixed
   - IllegalCallerException thrown for no caller frame, and associated
   javadoc changes
   - test changed to look for IllegalCallerException thrown.
 - Merge branch 'master' into JDK-8281000
 - JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is 
null

-

Changes: https://git.openjdk.java.net/jdk/pull/7448/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7448=06
  Stats: 216 lines in 5 files changed: 216 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7448.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7448/head:pull/7448

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


Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v6]

2022-02-16 Thread Tim Prinzing
> JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is 
> null

Tim Prinzing has updated the pull request incrementally with one additional 
commit since the last revision:

  fix test breakage from rename

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7448/files
  - new: https://git.openjdk.java.net/jdk/pull/7448/files/a449acdc..977162db

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7448=05
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7448=04-05

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7448.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7448/head:pull/7448

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


Integrated: JDK-8281003 - MethodHandles::lookup throws NPE if caller is null

2022-02-16 Thread Tim Prinzing
On Fri, 11 Feb 2022 20:32:46 GMT, Tim Prinzing  wrote:

> JDK-8281003 - MethodHandles::lookup throws NPE if caller is null

This pull request has now been integrated.

Changeset: 67763df4
Author:Tim Prinzing 
Committer: Mandy Chung 
URL:   
https://git.openjdk.java.net/jdk/commit/67763df4dce387da33da6d93d0f5d80e54cf8e5b
Stats: 160 lines in 4 files changed: 158 ins; 0 del; 2 mod

8281003: MethodHandles::lookup throws NPE if caller is null

Reviewed-by: ihse, mchung, jrose, alanb

-

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


Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v5]

2022-02-16 Thread Tim Prinzing
> JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is 
> null

Tim Prinzing has updated the pull request incrementally with three additional 
commits since the last revision:

 - reformat test summary
 - reformat test summary
 - improve test name and remove experimental test code

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7448/files
  - new: https://git.openjdk.java.net/jdk/pull/7448/files/533a0660..a449acdc

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7448=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7448=03-04

  Stats: 129 lines in 3 files changed: 58 ins; 70 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7448.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7448/head:pull/7448

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


Re: RFR: JDK-8281003 - MethodHandles::lookup throws NPE if caller is null [v5]

2022-02-16 Thread Alan Bateman
On Wed, 16 Feb 2022 17:55:55 GMT, Tim Prinzing  wrote:

>> JDK-8281003 - MethodHandles::lookup throws NPE if caller is null
>
> Tim Prinzing has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fix copyright date

Marked as reviewed by alanb (Reviewer).

-

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


Re: RFR: JDK-8281003 - MethodHandles::lookup throws NPE if caller is null [v5]

2022-02-16 Thread Tim Prinzing
> JDK-8281003 - MethodHandles::lookup throws NPE if caller is null

Tim Prinzing has updated the pull request incrementally with one additional 
commit since the last revision:

  fix copyright date

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7447/files
  - new: https://git.openjdk.java.net/jdk/pull/7447/files/7d1b97fe..ecfd125f

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7447=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7447=03-04

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7447.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7447/head:pull/7447

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


Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v4]

2022-02-16 Thread Tim Prinzing
> JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is 
> null

Tim Prinzing has updated the pull request incrementally with one additional 
commit since the last revision:

  fix copyright date

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7448/files
  - new: https://git.openjdk.java.net/jdk/pull/7448/files/d67bbb16..533a0660

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7448=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7448=02-03

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7448.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7448/head:pull/7448

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


Re: RFR: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null [v3]

2022-02-16 Thread Tim Prinzing
> JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is 
> null

Tim Prinzing has updated the pull request incrementally with one additional 
commit since the last revision:

  More changes from feedback.
  
  The javadoc is improved to reflect InvalidCallerException is thrown with
  a caller that can't be assigned to a ClassLoader as well as a null
  caller frame.  Added a test IsParallelCapableBadCaller that uses
  reflection hackery to create a case where called with an invalid caller
  on the call stack.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7448/files
  - new: https://git.openjdk.java.net/jdk/pull/7448/files/fa66af15..d67bbb16

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7448=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7448=01-02

  Stats: 85 lines in 3 files changed: 74 ins; 5 del; 6 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7448.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7448/head:pull/7448

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


Re: RFR: 8203290: [AIX] Check functionality of JDK-8199712 (Flight Recorder) [v21]

2022-02-16 Thread Tyler Steele
On Mon, 14 Feb 2022 23:44:40 GMT, Tyler Steele  wrote:

>> Just in time for the holidays I have completed an implementation of the JFR 
>> functionality for AIX. As a side note, this is my first submission to 
>> OpenJDK 
>> 
>> ### Implementation notes and alternatives considered
>> 
>> After modifying the build system to allow the --enable-jvm-feature-jfr to 
>> work on AIX, my task was to implement the interfaces from os_perf.hpp. The 
>> os_perf_aix.cpp implementation that existed was, I believe, a copy of the 
>> Linux implementation. A review of the code in that file showed that 
>> NetworkInterface, CPUPerformanceInterface, and SystemProcessInterface would 
>> require modification to work on AIX. Using the Linux implementation as a 
>> guide, I initially expected to use files from the aix procfs like 
>> /proc//psinfo, and /proc//status in a manner similar to the Linux 
>> implementation. However, I ended up using libperfstat for all functionality 
>> required by the interfaces.
>> 
>> ### Testing
>> 
>> Testing for JFR seems to be quite light in the repo both before and after 
>> this change. In addition to performing manual testing, I have added some 
>> basic sanity checks that ensure events can be created and committed (using 
>> jtreg), and performs some basic checks on the results of the interface 
>> member functions (using gtest).
>> 
>> ### More notes
>> 
>> I've sent an email to the JFR group about a TOC overflow warning I 
>> encountered while building (for the release server target). I believe the 
>> fix is to pass -qpic=large when using the xlc toolchain, but my 
>> modifications to flags-cflags.m4 have not been successful in removing this 
>> warning.
>
> Tyler Steele has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains eight commits:
> 
>  - Adds Oracle & IBM copyrights as per guidance from IBM legal team.
>  - Removes incorrect Oracle copyright line from libperfstat_aix.cpp/hpp, and 
> loadlib_aix.hpp
>  - Edit thread_aix.cpp to match thread_linux.cpp in 
> pd_get_top_fram_for_profiling and ...for_signal_handler
>  - Addresses issues from review and other sm fixes
>
>- Adds commenting in regards to memory handling by SystemProcess &
>NetworkInterface classes
>- Replaces explicit initialization and copy of structs with memcpy
>and memset as appropriate
>- Renames internal struct definitions in os_perf_aix
>- Other minor fixes
>  - Changes macoss -> macosx in problem list
>  - Refactors loadlib_aix: Removes redundant c++ class
>  - Merge branch 'master' into JDK-8203290
>  - Implements JFR on AIX
>
>- Implements interfaces from os_perf.hpp in os_perf_aix.cpp
>- Updates libperfstat_aix to contain functionality needed by os_perf_aix
>- Implements missing functionality in loadlib_aix (Fixes failure noted
>by TestNativeLibraies.java)
>- Removes platform checks for --enable-feature-jfr (Now enable-able on
>all platforms)
>- Enables TestNetworkUtilizationEvent.java which now passes on AIX
>- Updates AIX JavaThread::pd_get_top_frame_for_profiling with changes from 
> Linux

Thanks very much to the many reviewers who helped make this PR happen. Your 
support is very much appreciated  

I suggest these we /integrate these changes.

-

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


Re: RFR: 8253757: Add LLVM-based backend for hsdis [v8]

2022-02-16 Thread Magnus Ihse Bursie
> This patch expands the newly added system for hsdis backends to include LLVM.
> 
> The actual code in hsdis-llvm.cpp is based heavily on the work by @luhenry, 
> as published in the never integrated PR 
> https://github.com/openjdk/jdk/pull/392. (I have basically just ripped out 
> the binutils-based part of it.)
> 
> Unfortunately I have not been able to make this work properly on Windows. 
> With some additional flags I made it compile without complaints, but it 
> caused hotspot to segfault in `LoadLibrary` (!) in `os::dll_load` when I 
> tried to load the library. This is somewhat ironic, since the initial 
> implementation was created by Ludovic for the very purpose of using it on 
> Windows.
> 
> The lack of Windows support in this patch does not mean it is impossible to 
> get it to work, just that I need to co-operate with someone who has more 
> experience of compiling LLVM on Windows, and/or are more eager to get this 
> combination to work.

Magnus Ihse Bursie has updated the pull request incrementally with two 
additional commits since the last revision:

 - Add section on using LLVM
 - Allow override of LLVM location using --with-llvm

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5920/files
  - new: https://git.openjdk.java.net/jdk/pull/5920/files/467e1bb1..5ea9bc0d

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5920=07
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5920=06-07

  Stats: 37 lines in 2 files changed: 32 ins; 0 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5920.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5920/head:pull/5920

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


Re: RFR: 8253757: Add LLVM-based backend for hsdis [v7]

2022-02-16 Thread Magnus Ihse Bursie
> This patch expands the newly added system for hsdis backends to include LLVM.
> 
> The actual code in hsdis-llvm.cpp is based heavily on the work by @luhenry, 
> as published in the never integrated PR 
> https://github.com/openjdk/jdk/pull/392. (I have basically just ripped out 
> the binutils-based part of it.)
> 
> Unfortunately I have not been able to make this work properly on Windows. 
> With some additional flags I made it compile without complaints, but it 
> caused hotspot to segfault in `LoadLibrary` (!) in `os::dll_load` when I 
> tried to load the library. This is somewhat ironic, since the initial 
> implementation was created by Ludovic for the very purpose of using it on 
> Windows.
> 
> The lack of Windows support in this patch does not mean it is impossible to 
> get it to work, just that I need to co-operate with someone who has more 
> experience of compiling LLVM on Windows, and/or are more eager to get this 
> combination to work.

Magnus Ihse Bursie has updated the pull request incrementally with one 
additional commit since the last revision:

  Make sure install-hsdis also copies to image

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5920/files
  - new: https://git.openjdk.java.net/jdk/pull/5920/files/47cf9a1b..467e1bb1

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5920=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5920=05-06

  Stats: 7 lines in 1 file changed: 6 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5920.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5920/head:pull/5920

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