[OpenJDK 2D-Dev] Integrated: 8261107: ArrayIndexOutOfBoundsException in the ICC_Profile.getInstance(InputStream)
On Thu, 4 Feb 2021 01:56:57 GMT, Sergey Bylokhov wrote: > Some broken data in the InputStream may cause the wrong exception to be > thrown - ArrayIndexOutOfBoundsException instead of IllegalArgumentException, > if the data is too small and we try to validate it. This pull request has now been integrated. Changeset: 06b33a0a Author:Sergey Bylokhov URL: https://git.openjdk.java.net/jdk/commit/06b33a0a Stats: 56 lines in 2 files changed: 54 ins; 0 del; 2 mod 8261107: ArrayIndexOutOfBoundsException in the ICC_Profile.getInstance(InputStream) Reviewed-by: azvegint, psadhukhan - PR: https://git.openjdk.java.net/jdk/pull/2394
Re: [OpenJDK 2D-Dev] RFR: 8261107: ArrayIndexOutOfBoundsException in the ICC_Profile.getInstance(InputStream)
On Thu, 4 Feb 2021 01:56:57 GMT, Sergey Bylokhov wrote: > Some broken data in the InputStream may cause the wrong exception to be > thrown - ArrayIndexOutOfBoundsException instead of IllegalArgumentException, > if the data is too small and we try to validate it. Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2394
[OpenJDK 2D-Dev] Integrated: 6436374: Graphics.setColor(null) is not documented
On Wed, 3 Feb 2021 07:04:22 GMT, Prasanta Sadhukhan wrote: > Document setting null setColor similar to way setFont(null) is documented. This pull request has now been integrated. Changeset: 60f440de Author:Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/60f440de Stats: 48 lines in 2 files changed: 48 ins; 0 del; 0 mod 6436374: Graphics.setColor(null) is not documented Reviewed-by: serb, pbansal - PR: https://git.openjdk.java.net/jdk/pull/2371
Re: [OpenJDK 2D-Dev] RFR: 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified [v2]
> Method createStrokedShape(Shape) for java.awt.BasicStroke class throws > NullPointerException when passed a null object reference for a input > parameter but it's not specified in the spec. > Updated spec to illustrate this. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Doc change to @throws - Changes: - all: https://git.openjdk.java.net/jdk/pull/2377/files - new: https://git.openjdk.java.net/jdk/pull/2377/files/899868c0..f7d6e8fe Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2377&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2377&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2377.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2377/head:pull/2377 PR: https://git.openjdk.java.net/jdk/pull/2377
Re: [OpenJDK 2D-Dev] RFR: 8261107: ArrayIndexOutOfBoundsException in the ICC_Profile.getInstance(InputStream)
On Thu, 4 Feb 2021 01:56:57 GMT, Sergey Bylokhov wrote: > Some broken data in the InputStream may cause the wrong exception to be > thrown - ArrayIndexOutOfBoundsException instead of IllegalArgumentException, > if the data is too small and we try to validate it. Marked as reviewed by azvegint (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2394
[OpenJDK 2D-Dev] RFR: 8261107: ArrayIndexOutOfBoundsException in the ICC_Profile.getInstance(InputStream)
Some broken data in the InputStream may cause the wrong exception to be thrown - ArrayIndexOutOfBoundsException instead of IllegalArgumentException, if the data is too small and we try to validate it. - Commit messages: - Initial fix Changes: https://git.openjdk.java.net/jdk/pull/2394/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2394&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8261107 Stats: 56 lines in 2 files changed: 54 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2394.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2394/head:pull/2394 PR: https://git.openjdk.java.net/jdk/pull/2394
[OpenJDK 2D-Dev] Integrated: 8260616: Removing remaining JNF dependencies in the java.desktop module
On Fri, 29 Jan 2021 00:30:21 GMT, Phil Race wrote: > This completes the desktop module JNF removal > > * remove -framework JavaNativeFoundation from make files > > * remove #import from all > source files. If needed add import of JNIUtilities.h to get jni.h definitions > - better anyway since then it gets the current JDK ones not the ones from the > O/S > > * replace JNFNSToJavaString with NSStringToJavaString and JNFJavaToNSString > with JavaStringToNSString > > * replace JNFNormalizedNSStringForPath with > NormalizedPathNSStringFromJavaString and JNFNormalizedJavaStringForPath with > NormalizedPathJavaStringFromNSString > > * replace JNFGet/ReleaseStringUTF16UniChars with direct calls to JNI > > * Map all JNFRunLoop perform* calls to the ThreadUtilities versions (the vast > majority already did this) > > * Redo the ThreadUtilities calls to JNFRunLoop to directly invoke NSObject > perform* methods. > > * define new javaRunLoopMode in ThreadUtilities to replace the JNF one and > use where needed. > > * Remove the single usage of JNFPerformEnvBlock > > * replace JNFJavaToNSNumber in single A11Y file with local replacement > > * replace single usage of JNFNSTimeIntervalToJavaMillis in ScreenMenu.m with > local replacement > > * remove un-needed JNFRunLoopDidStartNotification from NSApplicationAWT.m > > * misc. remaining cleanup (eg missed JNF_CHECK_AND_RETHROW_EXCEPTION) This pull request has now been integrated. Changeset: 8760688d Author:Phil Race URL: https://git.openjdk.java.net/jdk/commit/8760688d Stats: 580 lines in 74 files changed: 243 ins; 99 del; 238 mod 8260616: Removing remaining JNF dependencies in the java.desktop module Reviewed-by: gziemski, ihse, serb - PR: https://git.openjdk.java.net/jdk/pull/2305
Re: [OpenJDK 2D-Dev] RFR: 8260616: Removing remaining JNF dependencies in the java.desktop module [v5]
> This completes the desktop module JNF removal > > * remove -framework JavaNativeFoundation from make files > > * remove #import from all > source files. If needed add import of JNIUtilities.h to get jni.h definitions > - better anyway since then it gets the current JDK ones not the ones from the > O/S > > * replace JNFNSToJavaString with NSStringToJavaString and JNFJavaToNSString > with JavaStringToNSString > > * replace JNFNormalizedNSStringForPath with > NormalizedPathNSStringFromJavaString and JNFNormalizedJavaStringForPath with > NormalizedPathJavaStringFromNSString > > * replace JNFGet/ReleaseStringUTF16UniChars with direct calls to JNI > > * Map all JNFRunLoop perform* calls to the ThreadUtilities versions (the vast > majority already did this) > > * Redo the ThreadUtilities calls to JNFRunLoop to directly invoke NSObject > perform* methods. > > * define new javaRunLoopMode in ThreadUtilities to replace the JNF one and > use where needed. > > * Remove the single usage of JNFPerformEnvBlock > > * replace JNFJavaToNSNumber in single A11Y file with local replacement > > * replace single usage of JNFNSTimeIntervalToJavaMillis in ScreenMenu.m with > local replacement > > * remove un-needed JNFRunLoopDidStartNotification from NSApplicationAWT.m > > * misc. remaining cleanup (eg missed JNF_CHECK_AND_RETHROW_EXCEPTION) Phil Race has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision: - Merge branch 'master' into jnf_string - 8260616: Removing remaining JNF dependencies in the java.desktop module - 8260616: Removing remaining JNF dependencies in the java.desktop module - Merge branch 'master' into jnf_string - 8260616: Removing remaining JNF dependencies in the java.desktop module - 8260616: Removing remaining JNF dependencies in the java.desktop modul - 8260616: Removing remaining JNF dependencies in the java.desktop module - Changes: - all: https://git.openjdk.java.net/jdk/pull/2305/files - new: https://git.openjdk.java.net/jdk/pull/2305/files/8a014fa6..43880e5f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2305&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2305&range=03-04 Stats: 3151 lines in 249 files changed: 1263 ins; 952 del; 936 mod Patch: https://git.openjdk.java.net/jdk/pull/2305.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2305/head:pull/2305 PR: https://git.openjdk.java.net/jdk/pull/2305
Re: [OpenJDK 2D-Dev] RFR: 8260616: Removing remaining JNF dependencies in the java.desktop module [v4]
On Tue, 2 Feb 2021 23:22:12 GMT, Phil Race wrote: >> This completes the desktop module JNF removal >> >> * remove -framework JavaNativeFoundation from make files >> >> * remove #import from all >> source files. If needed add import of JNIUtilities.h to get jni.h >> definitions - better anyway since then it gets the current JDK ones not the >> ones from the O/S >> >> * replace JNFNSToJavaString with NSStringToJavaString and JNFJavaToNSString >> with JavaStringToNSString >> >> * replace JNFNormalizedNSStringForPath with >> NormalizedPathNSStringFromJavaString and JNFNormalizedJavaStringForPath with >> NormalizedPathJavaStringFromNSString >> >> * replace JNFGet/ReleaseStringUTF16UniChars with direct calls to JNI >> >> * Map all JNFRunLoop perform* calls to the ThreadUtilities versions (the >> vast majority already did this) >> >> * Redo the ThreadUtilities calls to JNFRunLoop to directly invoke NSObject >> perform* methods. >> >> * define new javaRunLoopMode in ThreadUtilities to replace the JNF one and >> use where needed. >> >> * Remove the single usage of JNFPerformEnvBlock >> >> * replace JNFJavaToNSNumber in single A11Y file with local replacement >> >> * replace single usage of JNFNSTimeIntervalToJavaMillis in ScreenMenu.m with >> local replacement >> >> * remove un-needed JNFRunLoopDidStartNotification from NSApplicationAWT.m >> >> * misc. remaining cleanup (eg missed JNF_CHECK_AND_RETHROW_EXCEPTION) > > Phil Race has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains six additional commits since > the last revision: > > - 8260616: Removing remaining JNF dependencies in the java.desktop module > - 8260616: Removing remaining JNF dependencies in the java.desktop module > - Merge branch 'master' into jnf_string > - 8260616: Removing remaining JNF dependencies in the java.desktop module > - 8260616: Removing remaining JNF dependencies in the java.desktop modul > - 8260616: Removing remaining JNF dependencies in the java.desktop module Marked as reviewed by serb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2305
Re: [OpenJDK 2D-Dev] RFR: 8216358: [accessibility] [macos] The focus is invisible when tab to "Image Radio Buttons" and "Image CheckBoxes"
On Wed, 3 Feb 2021 23:50:16 GMT, Sergey Bylokhov wrote:
>> 8216358: [accessibility] [macos] The focus is invisible when tab to "Image
>> Radio Buttons" and "Image CheckBoxes"
>
> test/jdk/javax/swing/JCheckBox/ImageCheckboxFocus/ImageCheckboxTest.java line
> 68:
>
>> 66: public void performTest() throws Exception {
>> 67: try {
>> 68: BufferedImage imageFocus1 = null;
>
> Probably the usage of Robot/Frame could be removed? Could you try to create
> the buffered image and button then paint the button to the graphics via
> button.paint(Graphics)? I guess in this case you can check the content of the
> buffered image directly w/o the robot.
How it will work if the button was too small say w=4 pixels?
-
PR: https://git.openjdk.java.net/jdk/pull/2384
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Wed, 3 Feb 2021 22:48:33 GMT, Gerard Ziemski wrote: >> I don't like the idea of using masks on architectures that do not require >> them. How about something like this? >> >> `#if defined(__APPLE__)` >> ` // lldb (gdb) installs both standard BSD signal handlers, and mach >> exception` >> ` // handlers. By replacing the existing task exception handler, we disable >> lldb's mach` >> ` // exception handling, while leaving the standard BSD signal handlers >> functional.` >> ` //` >> ` // EXC_MASK_BAD_ACCESS needed by all architectures for NULL ptr checking` >> ` // EXC_MASK_ARITHMETIC needed by i386` >> ` // EXC_MASK_BAD_INSTRUCTION needed by aarch64 to initiate deoptimization` >> ` kern_return_t kr;` >> ` kr = task_set_exception_ports(mach_task_self(),` >> `EXC_MASK_BAD_ACCESS` >> `NOT_LP64(| EXC_MASK_ARITHMETIC)` >> `AARCH64_ONLY(| EXC_MASK_BAD_INSTRUCTION),` >> `MACH_PORT_NULL,` >> `EXCEPTION_STATE_IDENTITY,` >> `MACHINE_THREAD_STATE);` >> ` ` >> ` assert(kr == KERN_SUCCESS, "could not set mach task signal handler");` >> `#endif` >> >> If I just knew why i386 needs `EXC_MASK_ARITHMETIC` and add that to the >> comment I would be personally happy with that chunk of code. > > No idea how to insert spaces and make text align :-( using ` ```c ` https://docs.github.com/en/github/writing-on-github/creating-and-highlighting-code-blocks I was wrong about `SIGFPE` / `EXC_MASK_ARITHMETIC`, it's used on i386, x86_64: https://github.com/openjdk/jdk/blob/2be60e37e0e433141b2e3d3e32f8e638a4888e3a/src/hotspot/os_cpu/bsd_x86/os_bsd_x86.cpp#L467-L524 and aarch64: https://github.com/AntonKozlov/jdk/blob/80827176cbc5f0dd26003cf234a8076f3f557928/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp#L309-L323 (What happened with the formatting here, ugh?) Your suggestion sounds good otherwise. @AntonKozlov, do you mind to integrate that? - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Wed, 3 Feb 2021 22:44:18 GMT, Gerard Ziemski wrote: >> Thanks for your questions Gerard. >> >>> Part of the comment said This work-around is not necessary for 10.5+, as >>> CrashReporter no longer intercedes on caught fatal signals. >> >> That comment can probably be deleted since minversion is anyway 10.9 (and >> soon 10.12 https://github.com/openjdk/jdk/pull/2268 ). >> >>> Do you know if this also apply to lldb or is it gdb only specific? How do >>> you run gdb on macOS nowadays anyhow? >> >> `lldb` is shipped with Xcode, `gdb` isn't. You would need to build and sign >> it yourself, I haven't tried that in a while. So, we should update that >> comment to talk about `lldb` 🙂 >> >>> a) why we need `EXC_MASK_ARITHMETIC` ? >> >> I _believe_ this dates back to i386. As far as I can tell this isn't needed >> for x86_64 or aarch64. I guess we can remove it, the worst case is that it >> makes the debugging experience of the runtime a little bit worse. OTOH it >> doesn't hurt either to have it here. >> >>> b) we hit signal SIGSEGV in debugger even with the code in place, any way >>> to avoid that? >> >> The equivalent for `handle SIGSEGV nostop noprint` (gdb) in lldb is `process >> handle -n false -p true -s false SIGSEGV`. >> >>> c) does `BSD aarch6` need only `EXC_MASK_BAD_INSTRUCTION` or does it need >>> `EXC_MASK_BAD_ACCESS` as well? >> >> aarch64 needs `EXC_MASK_BAD_ACCESS` at least for implicit null checking, >> there might be other cases. >> >>> d) can we `#ifdef` the `EXC_MASK_BAD_INSTRUCTION` part of the mask only to >>> apply to `aarch64`? >> >> Maybe. I don't see any value in it though, except making the code more >> complicated to read 🙂 > > I don't like the idea of using masks on architectures that do not require > them. How about something like this? > > `#if defined(__APPLE__)` > ` // lldb (gdb) installs both standard BSD signal handlers, and mach > exception` > ` // handlers. By replacing the existing task exception handler, we disable > lldb's mach` > ` // exception handling, while leaving the standard BSD signal handlers > functional.` > ` //` > ` // EXC_MASK_BAD_ACCESS needed by all architectures for NULL ptr checking` > ` // EXC_MASK_ARITHMETIC needed by i386` > ` // EXC_MASK_BAD_INSTRUCTION needed by aarch64 to initiate deoptimization` > ` kern_return_t kr;` > ` kr = task_set_exception_ports(mach_task_self(),` > `EXC_MASK_BAD_ACCESS` > `NOT_LP64(| EXC_MASK_ARITHMETIC)` > `AARCH64_ONLY(| EXC_MASK_BAD_INSTRUCTION),` > `MACH_PORT_NULL,` > `EXCEPTION_STATE_IDENTITY,` > `MACHINE_THREAD_STATE);` > ` ` > ` assert(kr == KERN_SUCCESS, "could not set mach task signal handler");` > `#endif` > > If I just knew why i386 needs `EXC_MASK_ARITHMETIC` and add that to the > comment I would be personally happy with that chunk of code. No idea how to insert spaces and make text align :-( - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8216358: [accessibility] [macos] The focus is invisible when tab to "Image Radio Buttons" and "Image CheckBoxes"
On Wed, 3 Feb 2021 18:50:34 GMT, Alexander Zuev wrote:
> 8216358: [accessibility] [macos] The focus is invisible when tab to "Image
> Radio Buttons" and "Image CheckBoxes"
test/jdk/javax/swing/JCheckBox/ImageCheckboxFocus/ImageCheckboxTest.java line
68:
> 66: public void performTest() throws Exception {
> 67: try {
> 68: BufferedImage imageFocus1 = null;
Probably the usage of Robot/Frame could be removed? Could you try to create the
buffered image and button then paint the button to the graphics via
button.paint(Graphics)? I guess in this case you can check the content of the
buffered image directly w/o the robot.
src/java.desktop/macosx/classes/com/apple/laf/AquaButtonLabeledUI.java line 178:
> 176: }
> 177:
> 178: altIcon.paintIcon(c, g, iconRect.x - offset, iconRect.y -
> offset);
Is it possible that due to offset on the left/top we will draw the icon outside
the button bounds on the right/bottom?
-
PR: https://git.openjdk.java.net/jdk/pull/2384
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Wed, 3 Feb 2021 23:13:12 GMT, Bernhard Urban-Forster wrote: >> No idea how to insert spaces and make text align :-( > > using ` ```c ` > https://docs.github.com/en/github/writing-on-github/creating-and-highlighting-code-blocks > > I was wrong about `SIGFPE` / `EXC_MASK_ARITHMETIC`, it's used on i386, x86_64: > https://github.com/openjdk/jdk/blob/2be60e37e0e433141b2e3d3e32f8e638a4888e3a/src/hotspot/os_cpu/bsd_x86/os_bsd_x86.cpp#L467-L524 > and aarch64: > https://github.com/AntonKozlov/jdk/blob/80827176cbc5f0dd26003cf234a8076f3f557928/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp#L309-L323 > (What happened with the formatting here, ugh?) > > Your suggestion sounds good otherwise. @AntonKozlov, do you mind to integrate > that? So it should be: #if defined(__APPLE__) // lldb (gdb) installs both standard BSD signal handlers, and mach exception // handlers. By replacing the existing task exception handler, we disable lldb's mach // exception handling, while leaving the standard BSD signal handlers functional. // // EXC_MASK_BAD_ACCESS needed by all architectures for NULL ptr checking // EXC_MASK_ARITHMETIC needed by all architectures for div by 0 checking // EXC_MASK_BAD_INSTRUCTION needed by aarch64 to initiate deoptimization kern_return_t kr; kr = task_set_exception_ports(mach_task_self(), EXC_MASK_BAD_ACCESS | EXC_MASK_ARITHMETIC AARCH64_ONLY(| EXC_MASK_BAD_INSTRUCTION), MACH_PORT_NULL, EXCEPTION_STATE_IDENTITY, MACHINE_THREAD_STATE); assert(kr == KERN_SUCCESS, "could not set mach task signal handler"); #endif - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Wed, 3 Feb 2021 22:17:02 GMT, Bernhard Urban-Forster wrote: >> To answer my own question, it seems that code is still needed on `x86_64` >> for `lldb` with `EXC_MASK_BAD_ACCESS` or we keep tripping over >> `EXC_BAD_ACCESS` >> >> Remaining questions: >> >> a) why we need `EXC_MASK_ARITHMETIC` ? >> b) we hit `signal SIGSEGV` in debugger even with the code in place, any way >> to avoid that? >> c) does `BSD aarch64` need only `EXC_MASK_BAD_INSTRUCTION` or does it need >> `EXC_MASK_BAD_ACCESS` as well? >> d) can we `#ifdef` the `EXC_MASK_BAD_INSTRUCTION` part of the mask only to >> apply to `aarch64`? > > Thanks for your questions Gerard. > >> Part of the comment said This work-around is not necessary for 10.5+, as >> CrashReporter no longer intercedes on caught fatal signals. > > That comment can probably be deleted since minversion is anyway 10.9 (and > soon 10.12 https://github.com/openjdk/jdk/pull/2268 ). > >> Do you know if this also apply to lldb or is it gdb only specific? How do >> you run gdb on macOS nowadays anyhow? > > `lldb` is shipped with Xcode, `gdb` isn't. You would need to build and sign > it yourself, I haven't tried that in a while. So, we should update that > comment to talk about `lldb` 🙂 > >> a) why we need `EXC_MASK_ARITHMETIC` ? > > I _believe_ this dates back to i386. As far as I can tell this isn't needed > for x86_64 or aarch64. I guess we can remove it, the worst case is that it > makes the debugging experience of the runtime a little bit worse. OTOH it > doesn't hurt either to have it here. > >> b) we hit signal SIGSEGV in debugger even with the code in place, any way to >> avoid that? > > The equivalent for `handle SIGSEGV nostop noprint` (gdb) in lldb is `process > handle -n false -p true -s false SIGSEGV`. > >> c) does `BSD aarch6` need only `EXC_MASK_BAD_INSTRUCTION` or does it need >> `EXC_MASK_BAD_ACCESS` as well? > > aarch64 needs `EXC_MASK_BAD_ACCESS` at least for implicit null checking, > there might be other cases. > >> d) can we `#ifdef` the `EXC_MASK_BAD_INSTRUCTION` part of the mask only to >> apply to `aarch64`? > > Maybe. I don't see any value in it though, except making the code more > complicated to read 🙂 I don't like the idea of using masks on architectures that do not require them. How about something like this? `#if defined(__APPLE__)` ` // lldb (gdb) installs both standard BSD signal handlers, and mach exception` ` // handlers. By replacing the existing task exception handler, we disable lldb's mach` ` // exception handling, while leaving the standard BSD signal handlers functional.` ` //` ` // EXC_MASK_BAD_ACCESS needed by all architectures for NULL ptr checking` ` // EXC_MASK_ARITHMETIC needed by i386` ` // EXC_MASK_BAD_INSTRUCTION needed by aarch64 to initiate deoptimization` ` kern_return_t kr;` ` kr = task_set_exception_ports(mach_task_self(),` `EXC_MASK_BAD_ACCESS` `NOT_LP64(| EXC_MASK_ARITHMETIC)` `AARCH64_ONLY(| EXC_MASK_BAD_INSTRUCTION),` `MACH_PORT_NULL,` `EXCEPTION_STATE_IDENTITY,` `MACHINE_THREAD_STATE);` ` ` ` assert(kr == KERN_SUCCESS, "could not set mach task signal handler");` `#endif` If I just knew why i386 needs `EXC_MASK_ARITHMETIC` and add that to the comment I would be personally happy with that chunk of code. - PR: https://git.openjdk.java.net/jdk/pull/2200
[OpenJDK 2D-Dev] Integrated: 8261010: Delete the Netbeans "default" license header
On Wed, 3 Feb 2021 04:01:51 GMT, Sergey Bylokhov wrote: > Trivial cleanup, the "default" license header is removed in a few components. This pull request has now been integrated. Changeset: f279ff9d Author:Sergey Bylokhov URL: https://git.openjdk.java.net/jdk/commit/f279ff9d Stats: 14 lines in 3 files changed: 0 ins; 14 del; 0 mod 8261010: Delete the Netbeans "default" license header Reviewed-by: iris, psadhukhan - PR: https://git.openjdk.java.net/jdk/pull/2368
Re: [OpenJDK 2D-Dev] RFR: 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified
On Wed, 3 Feb 2021 12:11:40 GMT, Prasanta Sadhukhan
wrote:
> Method createStrokedShape(Shape) for java.awt.BasicStroke class throws
> NullPointerException when passed a null object reference for a input
> parameter but it's not specified in the spec.
> Updated spec to illustrate this.
src/java.desktop/share/classes/java/awt/BasicStroke.java line 297:
> 295: * @param s the {@code Shape} boundary be stroked
> 296: * @return the {@code Shape} of the stroked outline.
> 297: * @exception NullPointerException if {@code Shape} is {@code null}
We use "@throws" in the new code.
-
PR: https://git.openjdk.java.net/jdk/pull/2377
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Wed, 3 Feb 2021 20:29:48 GMT, Gerard Ziemski wrote: >> Part of the comment said `This work-around is not necessary for 10.5+, as >> CrashReporter no longer intercedes on caught fatal signals.` so I thought it >> was no longer needed, but it sounds like the part about `gdb` still applies >> then. >> >> We should update the comment to just say the `gdb` relevant part perhaps >> (and evaluate which of the EXC_MASK_BAD_ACCESS | EXC_MASK_BAD_INSTRUCTION | >> EXC_MASK_ARITHMETIC) we actually need for gdb: >> >> `// gdb installs both standard BSD signal handlers, and mach exception` >> `// handlers. By replacing the existing task exception handler, we disable >> gdb's mach` >> `// exception handling, while leaving the standard BSD signal handlers >> functional.` >> >> Do you know if this also apply to `lldb` or is it `gdb` only specific? How >> do you run `gdb` on macOS nowadays anyhow? > > To answer my own question, it seems that code is still needed on `x86_64` for > `lldb` with `EXC_MASK_BAD_ACCESS` or we keep tripping over `EXC_BAD_ACCESS` > > Remaining questions: > > a) why we need `EXC_MASK_ARITHMETIC` ? > b) we hit `signal SIGSEGV` in debugger even with the code in place, any way > to avoid that? > c) does `BSD aarch64` need only `EXC_MASK_BAD_INSTRUCTION` or does it need > `EXC_MASK_BAD_ACCESS` as well? > d) can we `#ifdef` the `EXC_MASK_BAD_INSTRUCTION` part of the mask only to > apply to `aarch64`? Thanks for your questions Gerard. > Part of the comment said This work-around is not necessary for 10.5+, as > CrashReporter no longer intercedes on caught fatal signals. That comment can probably be deleted since minversion is anyway 10.9 (and soon 10.12 https://github.com/openjdk/jdk/pull/2268 ). > Do you know if this also apply to lldb or is it gdb only specific? How do you > run gdb on macOS nowadays anyhow? `lldb` is shipped with Xcode, `gdb` isn't. You would need to build and sign it yourself, I haven't tried that in a while. So, we should update that comment to talk about `lldb` 🙂 > a) why we need `EXC_MASK_ARITHMETIC` ? I _believe_ this dates back to i386. As far as I can tell this isn't needed for x86_64 or aarch64. I guess we can remove it, the worst case is that it makes the debugging experience of the runtime a little bit worse. OTOH it doesn't hurt either to have it here. > b) we hit signal SIGSEGV in debugger even with the code in place, any way to > avoid that? The equivalent for `handle SIGSEGV nostop noprint` (gdb) in lldb is `process handle -n false -p true -s false SIGSEGV`. > c) does `BSD aarch6` need only `EXC_MASK_BAD_INSTRUCTION` or does it need > `EXC_MASK_BAD_ACCESS` as well? aarch64 needs `EXC_MASK_BAD_ACCESS` at least for implicit null checking, there might be other cases. > d) can we `#ifdef` the `EXC_MASK_BAD_INSTRUCTION` part of the mask only to > apply to `aarch64`? Maybe. I don't see any value in it though, except making the code more complicated to read 🙂 - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Wed, 3 Feb 2021 20:04:18 GMT, Gerard Ziemski wrote: >> See comment above about `gdb`, the same applies to `lldb` today. The AArch64 >> backend uses `SIGILL` (~= `EXC_MASK_BAD_INSTRUCTION`) to initiate a >> deoptimization. Without this change you cannot continue debugging once you >> the debuggee receives `SIGILL`. This wasn't needed before as x86 doesn't use >> `SIGILL`. > > Part of the comment said `This work-around is not necessary for 10.5+, as > CrashReporter no longer intercedes on caught fatal signals.` so I thought it > was no longer needed, but it sounds like the part about `gdb` still applies > then. > > We should update the comment to just say the `gdb` relevant part perhaps (and > evaluate which of the EXC_MASK_BAD_ACCESS | EXC_MASK_BAD_INSTRUCTION | > EXC_MASK_ARITHMETIC) we actually need for gdb: > > `// gdb installs both standard BSD signal handlers, and mach exception` > `// handlers. By replacing the existing task exception handler, we disable > gdb's mach` > `// exception handling, while leaving the standard BSD signal handlers > functional.` > > Do you know if this also apply to `lldb` or is it `gdb` only specific? How do > you run `gdb` on macOS nowadays anyhow? To answer my own question, it seems that code is still needed on `x86_64` for `lldb` with `EXC_MASK_BAD_ACCESS` or we keep tripping over `EXC_BAD_ACCESS` Remaining questions: a) why we need `EXC_MASK_ARITHMETIC` ? b) we hit `signal SIGSEGV` in debugger even with the code in place, any way to avoid that? c) does `BSD aarch64` need only `EXC_MASK_BAD_INSTRUCTION` or does it need `EXC_MASK_BAD_ACCESS` as well? d) can we `#ifdef` the `EXC_MASK_BAD_INSTRUCTION` part of the mask only to apply to `aarch64`? - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Wed, 3 Feb 2021 09:14:24 GMT, Andrew Haley wrote:
>> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5271:
>>
>>> 5269: //
>>> 5270: void MacroAssembler::get_thread(Register dst) {
>>> 5271: RegSet saved_regs = RegSet::range(r0, r1) +
>>> BSD_ONLY(RegSet::range(r2, r17)) + lr - dst;
>>
>> The comment needs to be updated, since on BSD we also seem to clobber r2,r17
>> ?
>
> Surely this should be
>
> saved_regs = RegSet::range(r0, r1) BSD_ONLY(+ RegSet::range(r2, r17)) + lr -
> dst;```
>
> Shouldn't it?
Interesting, I wonder why it has built successfully on Linux. I'm going to fix
this under as JDK-8261072
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Tue, 2 Feb 2021 23:03:45 GMT, Daniel D. Daugherty wrote: >> Anton Kozlov has updated the pull request incrementally with one additional >> commit since the last revision: >> >> support macos_aarch64 in hsdis > > src/hotspot/share/runtime/thread.cpp line 3991: > >> 3989: JavaThread* thread = JavaThread::current(); >> 3990: ThreadToNativeFromVM ttn(thread); >> 3991: Thread::WXExecFromWriteSetter wx_exec; > > I saw somewhere in this review a comment about why this new > WXExecFromWriteSetter helper isn't folded into ThreadToNativeFromVM > and I understand that not every current ThreadToNativeFromVM needs > the new helper. If the vast majority of the ThreadToNativeFromVM > locations need WXExecFromWriteSetter, then perhaps those locations > that currently have a ThreadToNativeFromVM followed by the new > WXExecFromWriteSetter should use a new helper: > > ThreadToNativeWithWXExecFromVM > > so we'll see changes from: > > ThreadToNativeFromVM -> ThreadToNativeWithWXExecFromVM > > where we need them and hopefully a short comment can be added > at the same time to explain the need for WXExec. This will allow us > to easily distinguish ThreadToNativeFromVM locations that DO NOT > need WXExec from those that DO need it. With a small overhead for tracking the current W^X state, I avoided WXExecFromWriteSetter near ThreadToNativeFromVM at all. New ThreadWXEnable(WXExec) is used only to call a generated function. More common ThreadWXEnable(WXWrite) is tied to JVM entry functions. I added comments for functions that are not clear to be entries, although they are. Thank you for the suggestion! - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Tue, 2 Feb 2021 18:00:06 GMT, Gerard Ziemski wrote: >> Anton Kozlov has updated the pull request incrementally with one additional >> commit since the last revision: >> >> support macos_aarch64 in hsdis > > src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp line 390: > >> 388: store_and_inc(_to, from_obj, NativeStack::intSpace); >> 389: >> 390: _num_int_args++; > > `pass_byte()` and `pass_short()` use only one `_num_int_args++;` after the > `if else` but other methods use 2 of them inside `if else` branches. > > We should be consistent. Agree. I'm going to do as much refactoring as needed before this patch under JDK-8261071 - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Tue, 26 Jan 2021 12:50:22 GMT, Anton Kozlov wrote:
>> Yes, that's why I thought it should be added to the classes
>> ThreadInVMfromNative, etc like:
>> class ThreadInVMfromNative : public ThreadStateTransition {
>> ResetNoHandleMark __rnhm;
>>
>> We can look at it with cleaning up the thread transitions RFE or as a
>> follow-on. If every line of ThreadInVMfromNative has to have one of these
>> Thread::WXWriteVerifier __wx_write; people are going to miss them when
>> adding the former.
>
> Not every ThreadInVMfromNative needs this, for example JIT goes to Native
> state without changing of W^X state. But from some experience of maintaining
> this patch, I actually had a duty to add missing W^X transitions after assert
> failures. A possible solution is actually to make W^X transition a part of
> ThreadInVMfromNative (and similar), but controlled by an optional constructor
> parameter with possible values "do exec->write", "verify write"...;. So in a
> common case ThreadInVMfromNative would implicitly do the transition and still
> would allow something uncommon like verification of the state for the JIT. I
> have to think about this.
I've dropped this transition here and in similar places after state tracking
always available. As a benefit, there are few places really using the setter
and all of them are tied to VM_ENTRY macro or similar one. I expect we don't
need to do W^X management near every java thread state change.
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Tue, 2 Feb 2021 19:23:16 GMT, Bernhard Urban-Forster wrote: >> src/hotspot/os/posix/signals_posix.cpp line 1297: >> >>> 1295: kern_return_t kr; >>> 1296: kr = task_set_exception_ports(mach_task_self(), >>> 1297: EXC_MASK_BAD_ACCESS | >>> EXC_MASK_BAD_INSTRUCTION | EXC_MASK_ARITHMETIC, >> >> Could someone elaborate on why we need to add `EXC_MASK_BAD_INSTRUCTION` to >> the mask here? > > See comment above about `gdb`, the same applies to `lldb` today. The AArch64 > backend uses `SIGILL` (~= `EXC_MASK_BAD_INSTRUCTION`) to initiate a > deoptimization. Without this change you cannot continue debugging once you > the debuggee receives `SIGILL`. This wasn't needed before as x86 doesn't use > `SIGILL`. Part of the comment said `This work-around is not necessary for 10.5+, as CrashReporter no longer intercedes on caught fatal signals.` so I thought it was no longer needed, but it sounds like the part about `gdb` still applies then. We should update the comment to just say the `gdb` relevant part perhaps (and evaluate which of the EXC_MASK_BAD_ACCESS | EXC_MASK_BAD_INSTRUCTION | EXC_MASK_ARITHMETIC) we actually need for gdb: `// gdb installs both standard BSD signal handlers, and mach exception` `// handlers. By replacing the existing task exception handler, we disable gdb's mach` `// exception handling, while leaving the standard BSD signal handlers functional.` Do you know if this also apply to `lldb` or is it `gdb` only specific? How do you run `gdb` on macOS nowadays anyhow? - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v2]
On Tue, 26 Jan 2021 12:01:30 GMT, Coleen Phillimore wrote: >> I assume a WXVerifier class that tracks W^X mode in debug mode and does >> nothing in release mode. I've considered to do this, it's relates to small >> inefficiencies, while this patch brings zero overhead (in release) for a >> platform that does not need W^X. >> * We don't need thread instance in release to call >> `os::current_thread_enable_wx`. Having WXVerifier a part of the Thread will >> require calling `Thread::current()` first and we could only hope for >> compiler to optimize this out, not sure if it will happen at all. In some >> contexts the Thread instance is available, in some it's not. >> * An instance of the empty class (as WXVerifier will be in the release) will >> occupy non-zero space in the Thread instance. >> >> If such costs are negligible, I can do as suggested. > > I really just want the minimal number of lines of code and hooks in > thread.hpp. You can still access it through the thread, just like these > lines currently. Look at HandshakeState as an example. Please take a look at the recent changes. Changes in thread.hpp were reduced: https://github.com/openjdk/jdk/pull/2200/files#diff-abdc409967d04172ecc20e3747aa55a79e755584d570b57c4d58902a9813d188 thread.inline.hpp provides definitions of accessors (non-trivial): https://github.com/openjdk/jdk/pull/2200/files#diff-3a29f7f952bf2bd936f49e97cb3b86a7324851133e879c142dec724455310b50 And new threadWXSetters.hpp defines RAII context setter: https://github.com/openjdk/jdk/pull/2200/files#diff-6424782ec43941031282f079e89adaa76d341ce340a3b78b0e9657358ec16278 - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Wed, 3 Feb 2021 20:05:29 GMT, Anton Kozlov wrote:
>> Thank you all for your comments regarding W^X implementation. I've made a
>> change that reduces the footprint of the implementation, also addressing
>> most of the comments. I'll revisit them individually to make sure nothing is
>> forgotten.
>>
>> The basic principle has not changed: when we execute JVM code (owned by
>> libjvm.so, starting from JVM entry function), we switch to Write state. When
>> we leave JVM to execute generated or JNI code, we switch to Executable
>> state. I would like to highlight that JVM code does not mean the VM state of
>> the java thread. After @stefank's suggestion, I could also drop a few W^X
>> state switches, so now it should be more clear that switches are tied to JVM
>> entry functions.
>
>> I wonder if this is the right choice
>> ...
>> ```
>> OopStorageParIterPerf::~OopStorageParIterPerf() {
>> ...
>> ```
>>
>
> The transition in OopStorageParIterPerf was made for gtest setup to execute
> in WXWrite context. For tests themselves, defining macro set WXWrite.
>
> I've simplified the scheme and now we switch to WXWrite once at the gtest
> launcher. So this transition was dropped.
>
> I've also refreshed my memory and tried to switch to WXWrite as close as
> possible to each place where we'll be writing executable memory. There are a
> lot of such places! As you correctly noted, code cache contains objects, not
> plain data. For example, CodeCache memory management structures,
> CompiledMethod, ... are there, so we need more WXWrite switches than we have
> in the current approach. I had a comparable amount of them just to run
> -version, but certainly not enough to run tier1 tests.
>
> Following your advice, I don't require a known "from" state anymore. So a few
> W^X transitions were dropped, e.g. when the JVM code calls a JNI entry
> function, which expects to be called from the native code. I had to switch to
> WXExec just only to satisfy the expectations. After the update, we don't need
> this anymore.
>
> W^X switches are mostly hidden by VM_ENTRY and similar macros. Some JVM
> functions are not marked as entries for some reason, although they are called
> directly from e.g. interpreter. I added W^X management to such functions.
Out of curiosity, have you looked at the performance of the W^X state
transition? In particular I'm wondering if the cost is effectively negligible
so doing it unconditionally on JVM entry is a no-brainer and just
easier/cleaner than the alternatives, or if there are reasons to look at only
doing the transition if/when needed (perhaps do it lazily and revert back to X
when leaving the JVM?).
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Wed, 3 Feb 2021 19:57:24 GMT, Anton Kozlov wrote:
>> For platform files that were copied from other ports to this port, if the
>> file wasn't
>> changed I presume the copyright years are left alone. If the file required
>> changes
>> for this port, I expect the year to be updated to 2021. How are you
>> verifying that
>> these copyright years are being properly managed on the new files?
>>
>> For the new W^X helpers, e.g., WXWriteFromExecSetter, a short comment
>> explaining why one was landed in a particular place would help reviewers.
>> Also see my comment about creating a new ThreadToNativeWithWXExecFromVM
>> helper.
>>
>> I'm stopping my review with all the src/hotspot files done for now.
>
> Thank you all for your comments regarding W^X implementation. I've made a
> change that reduces the footprint of the implementation, also addressing most
> of the comments. I'll revisit them individually to make sure nothing is
> forgotten.
>
> The basic principle has not changed: when we execute JVM code (owned by
> libjvm.so, starting from JVM entry function), we switch to Write state. When
> we leave JVM to execute generated or JNI code, we switch to Executable state.
> I would like to highlight that JVM code does not mean the VM state of the
> java thread. After @stefank's suggestion, I could also drop a few W^X state
> switches, so now it should be more clear that switches are tied to JVM entry
> functions.
> I wonder if this is the right choice
> ...
> ```
> OopStorageParIterPerf::~OopStorageParIterPerf() {
> ...
> ```
>
The transition in OopStorageParIterPerf was made for gtest setup to execute in
WXWrite context. For tests themselves, defining macro set WXWrite.
I've simplified the scheme and now we switch to WXWrite once at the gtest
launcher. So this transition was dropped.
I've also refreshed my memory and tried to switch to WXWrite as close as
possible to each place where we'll be writing executable memory. There are a
lot of such places! As you correctly noted, code cache contains objects, not
plain data. For example, CodeCache memory management structures,
CompiledMethod, ... are there, so we need more WXWrite switches than we have
in the current approach. I had a comparable amount of them just to run
-version, but certainly not enough to run tier1 tests.
Following your advice, I don't require a known "from" state anymore. So a few
W^X transitions were dropped, e.g. when the JVM code calls a JNI entry
function, which expects to be called from the native code. I had to switch to
WXExec just only to satisfy the expectations. After the update, we don't need
this anymore.
W^X switches are mostly hidden by VM_ENTRY and similar macros. Some JVM
functions are not marked as entries for some reason, although they are called
directly from e.g. interpreter. I added W^X management to such functions.
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Tue, 2 Feb 2021 23:10:17 GMT, Daniel D. Daugherty wrote: >> Anton Kozlov has updated the pull request incrementally with one additional >> commit since the last revision: >> >> support macos_aarch64 in hsdis > > For platform files that were copied from other ports to this port, if the > file wasn't > changed I presume the copyright years are left alone. If the file required > changes > for this port, I expect the year to be updated to 2021. How are you verifying > that > these copyright years are being properly managed on the new files? > > For the new W^X helpers, e.g., WXWriteFromExecSetter, a short comment > explaining why one was landed in a particular place would help reviewers. > Also see my comment about creating a new ThreadToNativeWithWXExecFromVM > helper. > > I'm stopping my review with all the src/hotspot files done for now. Thank you all for your comments regarding W^X implementation. I've made a change that reduces the footprint of the implementation, also addressing most of the comments. I'll revisit them individually to make sure nothing is forgotten. The basic principle has not changed: when we execute JVM code (owned by libjvm.so, starting from JVM entry function), we switch to Write state. When we leave JVM to execute generated or JNI code, we switch to Executable state. I would like to highlight that JVM code does not mean the VM state of the java thread. After @stefank's suggestion, I could also drop a few W^X state switches, so now it should be more clear that switches are tied to JVM entry functions. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
> Please review the implementation of JEP 391: macOS/AArch64 Port. > > It's heavily based on existing ports to linux/aarch64, macos/x86_64, and > windows/aarch64. > > Major changes are in: > * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks > JDK-8253817, JDK-8253818) > * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with necessary > adjustments (JDK-8253819) > * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), > required on macOS/AArch64 platform. It's implemented with > pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a > thread, so W^X mode change relates to the java thread state change (for java > threads). In most cases, JVM executes in write-only mode, except when calling > a generated stub like SafeFetch, which requires a temporary switch to > execute-only mode. The same execute-only mode is enabled when a java thread > executes in java or native states. This approach of managing W^X mode turned > out to be simple and efficient enough. > * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941) Anton Kozlov has updated the pull request incrementally with six additional commits since the last revision: - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos - Add comments to WX transitions + minor change of placements - Use macro conditionals instead of empty functions - Add W^X to tests - Do not require known W^X state - Revert w^x in gtests - Changes: - all: https://git.openjdk.java.net/jdk/pull/2200/files - new: https://git.openjdk.java.net/jdk/pull/2200/files/3c705ae5..80827176 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2200&range=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2200&range=08-09 Stats: 444 lines in 64 files changed: 112 ins; 278 del; 54 mod Patch: https://git.openjdk.java.net/jdk/pull/2200.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2200/head:pull/2200 PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8216358: [accessibility] [macos] The focus is invisible when tab to "Image Radio Buttons" and "Image CheckBoxes"
On Wed, 3 Feb 2021 18:50:34 GMT, Alexander Zuev wrote: > 8216358: [accessibility] [macos] The focus is invisible when tab to "Image > Radio Buttons" and "Image CheckBoxes" This is how radio button group with custom icons looks without the fix. The focus is on the middle radio button. https://user-images.githubusercontent.com/69642324/106796383-1e5f3d00-6610-11eb-9e28-44927d2eed5c.png";> This is how the same widget looks with the fix. Presence of the focus is clearly visible. https://user-images.githubusercontent.com/69642324/106796565-62ead880-6610-11eb-9113-2e873375f44a.png";> - PR: https://git.openjdk.java.net/jdk/pull/2384
[OpenJDK 2D-Dev] RFR: 8216358: [accessibility] [macos] The focus is invisible when tab to "Image Radio Buttons" and "Image CheckBoxes"
8216358: [accessibility] [macos] The focus is invisible when tab to "Image Radio Buttons" and "Image CheckBoxes" - Commit messages: - 8216358: [accessibility] [macos] The focus is invisible when tab to "Image Radio Buttons" and "Image CheckBoxes" Changes: https://git.openjdk.java.net/jdk/pull/2384/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2384&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8216358 Stats: 164 lines in 2 files changed: 162 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2384.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2384/head:pull/2384 PR: https://git.openjdk.java.net/jdk/pull/2384
[OpenJDK 2D-Dev] RFR: 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified
Method createStrokedShape(Shape) for java.awt.BasicStroke class throws NullPointerException when passed a null object reference for a input parameter but it's not specified in the spec. Updated spec to illustrate this. - Commit messages: - 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified - 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified Changes: https://git.openjdk.java.net/jdk/pull/2377/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2377&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-6211257 Stats: 47 lines in 2 files changed: 47 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2377.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2377/head:pull/2377 PR: https://git.openjdk.java.net/jdk/pull/2377
Re: [OpenJDK 2D-Dev] RFR: 6436374: Graphics.setColor(null) is not documented [v2]
On Wed, 3 Feb 2021 08:28:05 GMT, Prasanta Sadhukhan wrote: >> Document setting null setColor similar to way setFont(null) is documented. > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > Add test Marked as reviewed by serb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2371
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Tue, 2 Feb 2021 18:03:50 GMT, Gerard Ziemski wrote:
>> Anton Kozlov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> support macos_aarch64 in hsdis
>
> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5271:
>
>> 5269: //
>> 5270: void MacroAssembler::get_thread(Register dst) {
>> 5271: RegSet saved_regs = RegSet::range(r0, r1) +
>> BSD_ONLY(RegSet::range(r2, r17)) + lr - dst;
>
> The comment needs to be updated, since on BSD we also seem to clobber r2,r17 ?
Surely this should be
saved_regs = RegSet::range(r0, r1) BSD_ONLY(+ RegSet::range(r2, r17)) + lr -
dst;```
Shouldn't it?
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Tue, 2 Feb 2021 21:49:36 GMT, Daniel D. Daugherty wrote:
>> Anton Kozlov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> support macos_aarch64 in hsdis
>
> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 323:
>
>> 321: str(zr, Address(rthread, JavaThread::last_Java_pc_offset()));
>> 322:
>> 323: str(zr, Address(rthread, JavaFrameAnchor::saved_fp_address_offset()));
>
> I don't think this switch from `JavaThread::saved_fp_address_offset()`
> to `JavaFrameAnchor::saved_fp_address_offset()` is correct since
> `rthread` is still used and is a JavaThread*. The new code will give you:
>
> `rthread` + offset of the `saved_fp_address` field in a JavaFrameAnchor
>
> The old code gave you:
>
> `rthread` + offset of the `saved_fp_address` field in the JavaFrameAnchor
> field in the JavaThread
>
> Those are not the same things.
I agree, I don't understand why this change was made.
> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.hpp line 45:
>
>> 43: // Atomically copy 64 bits of data
>> 44: static void atomic_copy64(const volatile void *src, volatile void
>> *dst) {
>> 45: *(jlong *) dst = *(const jlong *) src;
>
> Is this construct actually atomic on aarch64?
Yes.
> src/hotspot/os_cpu/windows_aarch64/os_windows_aarch64.hpp line 37:
>
>> 35:
>> 36: private:
>> 37:
>
> 'private' usually has one space in front of it.
> Also, why the blank line after it?
It reads better with the blank line, and it's not in violation of HotSpot
conventions.
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] Making the OpenGL-Queue-Flusher work concurrently with AWT threads possible? (... the future of the opengl pipeline)
Sergey, I tested clemens patch and sometimes, jvm hangs if I close the xwindow where the opengl surface is still rendering... Could you explain us or do you have design documents (html, pdf, wiki...) describing the awt locking scheme and its relations with x11 / opengl and other native backends ? Thanks, Laurent Le lun. 18 janv. 2021 à 23:52, Sergey Bylokhov a écrit : > On 18.01.2021 04:41, Clemens Eisserer wrote: > > So sure, for GPU limited cases this won't help a lot - however, typical > java2d is usually CPU limited with tons of very small primitives and many > state changes in between. > > It does not affect so much the xrender which do the same CPU related > steps, but this speeds up so !much! OGL if executed in parallel. > I think the benchmark is cheating here, it does not call XSync nor glflush > for each operation, so it just checks the speed of pushing the data to the > buffer, and not measured the actual rendering for both pipelines, unlike > the software-based rendering. > > > -- > Best regards, Sergey. >
Re: [OpenJDK 2D-Dev] RFR: 6436374: Graphics.setColor(null) is not documented [v2]
> Document setting null setColor similar to way setFont(null) is documented. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Add test - Changes: - all: https://git.openjdk.java.net/jdk/pull/2371/files - new: https://git.openjdk.java.net/jdk/pull/2371/files/dc4fe3c5..a2c6c93e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2371&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2371&range=00-01 Stats: 47 lines in 1 file changed: 47 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2371.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2371/head:pull/2371 PR: https://git.openjdk.java.net/jdk/pull/2371
Re: [OpenJDK 2D-Dev] RFR: 6436374: Graphics.setColor(null) is not documented
On Wed, 3 Feb 2021 07:59:15 GMT, Prasanta Sadhukhan wrote: >> While CSR is under review, maybe we can add a small test? > > What will be the objective of the test...It's just documentation change..no > code is changed...setting null will be ignored before this and after this > change... This change added a statement to the specification, which could not be tested before. So this new test will be the first one that passes a null to this method, how we will know that it is actually ignored? - PR: https://git.openjdk.java.net/jdk/pull/2371
Re: [OpenJDK 2D-Dev] RFR: 6436374: Graphics.setColor(null) is not documented
On Wed, 3 Feb 2021 07:57:17 GMT, Sergey Bylokhov wrote: >>> >>> >>> @mrserb has indicated that a [compatibility and >>> specification](https://wiki.openjdk.java.net/display/csr/Main) (CSR) >>> request is needed for this pull request. >>> @prsadhuk please create a CSR request and add link to it in >>> [JDK-6436374](https://bugs.openjdk.java.net/browse/JDK-6436374). This pull >>> request cannot be integrated until the CSR request is approved. >> >> Done..JDK-8261018...Please review... > > While CSR is under review, maybe we can add a small test? What will be the objective of the test...It's just documentation change..no code is changed...setting null will be ignored before this and after this change... - PR: https://git.openjdk.java.net/jdk/pull/2371
Re: [OpenJDK 2D-Dev] RFR: 6436374: Graphics.setColor(null) is not documented
On Wed, 3 Feb 2021 07:39:54 GMT, Prasanta Sadhukhan wrote: >> Marked as reviewed by pbansal (Reviewer). > >> >> >> @mrserb has indicated that a [compatibility and >> specification](https://wiki.openjdk.java.net/display/csr/Main) (CSR) request >> is needed for this pull request. >> @prsadhuk please create a CSR request and add link to it in >> [JDK-6436374](https://bugs.openjdk.java.net/browse/JDK-6436374). This pull >> request cannot be integrated until the CSR request is approved. > > Done..JDK-8261018...Please review... While CSR is under review, maybe we can add a small test? - PR: https://git.openjdk.java.net/jdk/pull/2371
