From 54c613acd7a7f051e65d73a1318346cb4f5a234a Mon Sep 17 00:00:00 2001 From: Zixian Cai Date: Tue, 24 Oct 2023 14:36:52 +0000 Subject: [PATCH 01/19] 8318693: Fix rendering for code blocks nested under list items in building.md Reviewed-by: erikj, ccleary --- doc/building.html | 101 ++++++++++++++++++++++++++++------------------ doc/building.md | 21 +++++++--- 2 files changed, 78 insertions(+), 44 deletions(-) diff --git a/doc/building.html b/doc/building.html index feb699fd24d62..2348c398a534b 100644 --- a/doc/building.html +++ b/doc/building.html @@ -819,11 +819,11 @@

Running Configure

Some command line examples:

Common Configure Arguments

Here follows some of the most common and important @@ -1331,14 +1331,12 @@

ALSA

libasound2-dev packages for your target system. Download them to /tmp.

  • Install the libraries into the cross-compilation toolchain. For -instance:

    • - +instance:

    cd /tools/gcc-linaro-arm-linux-gnueabihf-raspbian-2012.09-20120921_linux/arm-linux-gnueabihf/libc
 dpkg-deb -x /tmp/libasound2_1.0.25-4_armhf.deb .
-dpkg-deb -x /tmp/libasound2-dev_1.0.25-4_armhf.deb .
    -

    X11

    You will need X11 libraries suitable for your target system. @@ -1372,21 +1370,18 @@

    X11

  • Install the libraries into the cross-compilation toolchain. For instance:

    -
        cd /tools/gcc-linaro-arm-linux-gnueabihf-raspbian-2012.09-20120921_linux/arm-linux-gnueabihf/libc/usr
-    mkdir X11R6
-    cd X11R6
-    for deb in /tmp/target-x11/*.deb ; do dpkg-deb -x $deb . ; done
-    mv usr/* .
-    cd lib
-    cp arm-linux-gnueabihf/* .
-    ```
-
-You can ignore the following messages. These libraries are not needed to
-successfully complete a full JDK build.
    -

    cp: cannot stat -arm-linux-gnueabihf/libICE.so': No such file or directory cp: cannot statarm-linux-gnueabihf/libSM.so': -No such file or directory cp: cannot stat -`arm-linux-gnueabihf/libXt.so': No such file or directory ```

    • +
    cd /tools/gcc-linaro-arm-linux-gnueabihf-raspbian-2012.09-20120921_linux/arm-linux-gnueabihf/libc/usr
+mkdir X11R6
+cd X11R6
+for deb in /tmp/target-x11/*.deb ; do dpkg-deb -x $deb . ; done
+mv usr/* .
+cd lib
+cp arm-linux-gnueabihf/* .
    +

    You can ignore the following messages. These libraries are not needed +to successfully complete a full JDK build.

    +
    cp: cannot stat `arm-linux-gnueabihf/libICE.so': No such file or directory
+cp: cannot stat `arm-linux-gnueabihf/libSM.so': No such file or directory
+cp: cannot stat `arm-linux-gnueabihf/libXt.so': No such file or directory

  • If the X11 libraries are not properly detected by configure, you can point them out by --with-x.

    • @@ -1404,17 +1399,30 @@

    Cross compiling with

    For example, cross-compiling to AArch64 from x86_64 could be done like this:

    The build does not create new files in that chroot, so it can be reused for multiple builds without additional cleanup.

    @@ -1566,12 +1574,27 @@

    Building for RISC-V

    placeholder <toolchain-installed-path> shown below is the path where you want to install the toolchain.

    Building for musl

    Just like it's possible to cross-compile for a different CPU, it's diff --git a/doc/building.md b/doc/building.md index 415454cf6c36c..ac79cb314b6bb 100644 --- a/doc/building.md +++ b/doc/building.md @@ -626,11 +626,13 @@ automatically, it will exit and inform you about the problem. Some command line examples: * Create a 32-bit build for Windows with FreeType2 in `C:\freetype-i586`: + ``` bash configure --with-freetype=/cygdrive/c/freetype-i586 --with-target-bits=32 ``` * Create a debug build with the `server` JVM and DTrace enabled: + ``` bash configure --enable-debug --with-jvm-variants=server --enable-dtrace ``` @@ -1100,11 +1102,12 @@ Note that alsa is needed even if you only want to build a headless JDK. system. Download them to /tmp. * Install the libraries into the cross-compilation toolchain. For instance: -``` -cd /tools/gcc-linaro-arm-linux-gnueabihf-raspbian-2012.09-20120921_linux/arm-linux-gnueabihf/libc -dpkg-deb -x /tmp/libasound2_1.0.25-4_armhf.deb . -dpkg-deb -x /tmp/libasound2-dev_1.0.25-4_armhf.deb . -``` + + ``` + cd /tools/gcc-linaro-arm-linux-gnueabihf-raspbian-2012.09-20120921_linux/arm-linux-gnueabihf/libc + dpkg-deb -x /tmp/libasound2_1.0.25-4_armhf.deb . + dpkg-deb -x /tmp/libasound2-dev_1.0.25-4_armhf.deb . + ``` * If alsa is not properly detected by `configure`, you can point it out by `--with-alsa`. @@ -1140,6 +1143,7 @@ Note that X11 is needed even if you only want to build a headless JDK. * libxext-dev * Install the libraries into the cross-compilation toolchain. For instance: + ``` cd /tools/gcc-linaro-arm-linux-gnueabihf-raspbian-2012.09-20120921_linux/arm-linux-gnueabihf/libc/usr mkdir X11R6 @@ -1173,11 +1177,13 @@ for foreign architectures with native compilation speed. For example, cross-compiling to AArch64 from x86_64 could be done like this: * Install cross-compiler on the *build* system: + ``` apt install g++-aarch64-linux-gnu gcc-aarch64-linux-gnu ``` * Create chroot on the *build* system, configuring it for *target* system: + ``` sudo debootstrap \ --arch=arm64 \ @@ -1192,11 +1198,13 @@ For example, cross-compiling to AArch64 from x86_64 could be done like this: ``` * Make sure the symlinks inside the newly created chroot point to proper locations: + ``` sudo chroot ~/sysroot-arm64 symlinks -cr . ``` * Configure and build with newly created chroot as sysroot/toolchain-path: + ``` sh ./configure \ --openjdk-target=aarch64-linux-gnu \ @@ -1255,6 +1263,7 @@ complicate the building process. The placeholder `` shown below is the path where you want to install the toolchain. * Install the RISC-V GNU compiler toolchain: + ``` git clone --recursive https://github.com/riscv-collab/riscv-gnu-toolchain cd riscv-gnu-toolchain @@ -1264,6 +1273,7 @@ shown below is the path where you want to install the toolchain. ``` * Cross-compile all the required libraries: + ``` # An example for libffi git clone https://github.com/libffi/libffi @@ -1274,6 +1284,7 @@ shown below is the path where you want to install the toolchain. ``` * Configure and build OpenJDK: + ``` bash configure \ --with-boot-jdk=$BOOT_JDK \ From e2720987b921b95fd8010cea60d2d6e436e5ebaa Mon Sep 17 00:00:00 2001 From: Vicente Romero Date: Tue, 24 Oct 2023 14:45:10 +0000 Subject: [PATCH 02/19] 8318160: javac does not reject private method reference with type-variable receiver Reviewed-by: mcimadamore --- .../com/sun/tools/javac/comp/Resolve.java | 12 +++++++ ...PrivateMethodReferenceWithTypeVarTest.java | 34 +++++++++++++++++++ .../PrivateMethodReferenceWithTypeVarTest.out | 4 +++ 3 files changed, 50 insertions(+) create mode 100644 test/langtools/tools/javac/lambda/methodReference/PrivateMethodReferenceWithTypeVarTest.java create mode 100644 test/langtools/tools/javac/lambda/methodReference/PrivateMethodReferenceWithTypeVarTest.out diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java index 295bb192a4288..ab84d372bd72d 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java @@ -3605,6 +3605,18 @@ ReferenceKind referenceKind(Symbol sym) { ReferenceKind.BOUND; } } + + @Override + Symbol access(Env env, DiagnosticPosition pos, Symbol location, Symbol sym) { + if (originalSite.hasTag(TYPEVAR) && sym.kind == MTH) { + sym = (sym.flags() & Flags.PRIVATE) != 0 ? + new AccessError(env, site, sym) : + sym; + return accessBase(sym, pos, location, originalSite, name, true); + } else { + return super.access(env, pos, location, sym); + } + } } /** diff --git a/test/langtools/tools/javac/lambda/methodReference/PrivateMethodReferenceWithTypeVarTest.java b/test/langtools/tools/javac/lambda/methodReference/PrivateMethodReferenceWithTypeVarTest.java new file mode 100644 index 0000000000000..b87783416a383 --- /dev/null +++ b/test/langtools/tools/javac/lambda/methodReference/PrivateMethodReferenceWithTypeVarTest.java @@ -0,0 +1,34 @@ +/* + * @test /nodynamiccopyright/ + * @bug 8318160 + * @summary javac does not reject private method reference with type-variable receiver + * @compile/fail/ref=PrivateMethodReferenceWithTypeVarTest.out -XDrawDiagnostics PrivateMethodReferenceWithTypeVarTest.java + */ + +import java.util.function.*; + +class PrivateMethodReferenceWithTypeVarTest { + class Foo { + X get() { return null; } + } + + private String asString() { + return "bar"; + } + + private String asString2(Object o) { + return "bar"; + } + + static Function m1() { + return T::asString; + } + + static Function m2(T t) { + return t::asString2; + } + + static Function m2(Foo foo) { + return foo.get()::asString2; + } +} diff --git a/test/langtools/tools/javac/lambda/methodReference/PrivateMethodReferenceWithTypeVarTest.out b/test/langtools/tools/javac/lambda/methodReference/PrivateMethodReferenceWithTypeVarTest.out new file mode 100644 index 0000000000000..b4dac12a9ae59 --- /dev/null +++ b/test/langtools/tools/javac/lambda/methodReference/PrivateMethodReferenceWithTypeVarTest.out @@ -0,0 +1,4 @@ +PrivateMethodReferenceWithTypeVarTest.java:24:16: compiler.err.report.access: asString(), private, PrivateMethodReferenceWithTypeVarTest +PrivateMethodReferenceWithTypeVarTest.java:28:16: compiler.err.report.access: asString2(java.lang.Object), private, PrivateMethodReferenceWithTypeVarTest +PrivateMethodReferenceWithTypeVarTest.java:32:16: compiler.err.report.access: asString2(java.lang.Object), private, PrivateMethodReferenceWithTypeVarTest +3 errors From 6f352740cb5e7c47d226fd4039cfb977c0622488 Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Tue, 24 Oct 2023 14:49:06 +0000 Subject: [PATCH 03/19] 8318702: G1: Fix nonstandard indentation in g1HeapTransition.cpp Reviewed-by: iwalulya --- src/hotspot/share/gc/g1/g1HeapTransition.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/hotspot/share/gc/g1/g1HeapTransition.cpp b/src/hotspot/share/gc/g1/g1HeapTransition.cpp index 49ba19dfdba6d..a2097041b2093 100644 --- a/src/hotspot/share/gc/g1/g1HeapTransition.cpp +++ b/src/hotspot/share/gc/g1/g1HeapTransition.cpp @@ -143,11 +143,11 @@ void G1HeapTransition::print() { usage = blk._usage; assert(usage._eden_region_count == 0, "Expected no eden regions, but got " SIZE_FORMAT, usage._eden_region_count); assert(usage._survivor_region_count == after._survivor_length, "Expected survivors to be " SIZE_FORMAT " but was " SIZE_FORMAT, - after._survivor_length, usage._survivor_region_count); + after._survivor_length, usage._survivor_region_count); assert(usage._old_region_count == after._old_length, "Expected old to be " SIZE_FORMAT " but was " SIZE_FORMAT, - after._old_length, usage._old_region_count); + after._old_length, usage._old_region_count); assert(usage._humongous_region_count == after._humongous_length, "Expected humongous to be " SIZE_FORMAT " but was " SIZE_FORMAT, - after._humongous_length, usage._humongous_region_count); + after._humongous_length, usage._humongous_region_count); } log_regions("Eden", _before._eden_length, after._eden_length, eden_capacity_length_after_gc, @@ -157,17 +157,17 @@ void G1HeapTransition::print() { log_regions("Survivor", _before._survivor_length, after._survivor_length, survivor_capacity_length_before_gc, _before._survivor_length_per_node, after._survivor_length_per_node); log_trace(gc, heap)(" Used: " SIZE_FORMAT "K, Waste: " SIZE_FORMAT "K", - usage._survivor_used / K, ((after._survivor_length * HeapRegion::GrainBytes) - usage._survivor_used) / K); + usage._survivor_used / K, ((after._survivor_length * HeapRegion::GrainBytes) - usage._survivor_used) / K); log_info(gc, heap)("Old regions: " SIZE_FORMAT "->" SIZE_FORMAT, _before._old_length, after._old_length); log_trace(gc, heap)(" Used: " SIZE_FORMAT "K, Waste: " SIZE_FORMAT "K", - usage._old_used / K, ((after._old_length * HeapRegion::GrainBytes) - usage._old_used) / K); + usage._old_used / K, ((after._old_length * HeapRegion::GrainBytes) - usage._old_used) / K); log_info(gc, heap)("Humongous regions: " SIZE_FORMAT "->" SIZE_FORMAT, _before._humongous_length, after._humongous_length); log_trace(gc, heap)(" Used: " SIZE_FORMAT "K, Waste: " SIZE_FORMAT "K", - usage._humongous_used / K, ((after._humongous_length * HeapRegion::GrainBytes) - usage._humongous_used) / K); + usage._humongous_used / K, ((after._humongous_length * HeapRegion::GrainBytes) - usage._humongous_used) / K); MetaspaceUtils::print_metaspace_change(_before._meta_sizes); } From 116503754c4c4bdb91685955ef4456bc76f751c4 Mon Sep 17 00:00:00 2001 From: Naoto Sato Date: Tue, 24 Oct 2023 16:54:57 +0000 Subject: [PATCH 04/19] 8318569: Add getter methods for Locale and Patterns in ListFormat Reviewed-by: joehw, rriggs, iris, mli --- .../share/classes/java/text/ListFormat.java | 20 +++++++++ .../Format/ListFormat/TestListFormat.java | 41 ++++++++++++++++++- 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/src/java.base/share/classes/java/text/ListFormat.java b/src/java.base/share/classes/java/text/ListFormat.java index d0b1b2aaf2aec..920cd9a076214 100644 --- a/src/java.base/share/classes/java/text/ListFormat.java +++ b/src/java.base/share/classes/java/text/ListFormat.java @@ -322,6 +322,26 @@ public static ListFormat getInstance(String[] patterns) { return new ListFormat(Locale.ROOT, Arrays.copyOf(patterns, PATTERN_ARRAY_LENGTH)); } + /** + * {@return the {@code Locale} of this ListFormat} + * + * The {@code locale} is defined by {@link #getInstance(Locale, Type, Style)} or + * {@link #getInstance(String[])}. + */ + public Locale getLocale() { + return locale; + } + + /** + * {@return the patterns used in this ListFormat} + * + * The {@code patterns} are defined by {@link #getInstance(Locale, Type, Style)} or + * {@link #getInstance(String[])}. + */ + public String[] getPatterns() { + return Arrays.copyOf(patterns, patterns.length); + } + /** * {@return the string that consists of the input strings, concatenated with the * patterns of this {@code ListFormat}} diff --git a/test/jdk/java/text/Format/ListFormat/TestListFormat.java b/test/jdk/java/text/Format/ListFormat/TestListFormat.java index 2d260c9a94caf..eee6f6dac83ea 100644 --- a/test/jdk/java/text/Format/ListFormat/TestListFormat.java +++ b/test/jdk/java/text/Format/ListFormat/TestListFormat.java @@ -23,7 +23,7 @@ /* * @test - * @bug 8041488 8316974 + * @bug 8041488 8316974 8318569 * @summary Tests for ListFormat class * @run junit TestListFormat */ @@ -200,6 +200,7 @@ static Arguments[] parseObject_parsePos() { arguments(CUSTOM_PATTERNS_MINIMAL, SAMPLE4), }; } + static Arguments[] getInstance_3Arg_InheritPatterns() { return new Arguments[] { arguments(ListFormat.Type.STANDARD, ListFormat.Style.FULL), @@ -213,6 +214,17 @@ static Arguments[] getInstance_3Arg_InheritPatterns() { arguments(ListFormat.Type.UNIT, ListFormat.Style.NARROW), }; } + + static Arguments[] getLocale_localeDependent() { + return new Arguments[] { + arguments(Locale.ROOT), + arguments(Locale.US), + arguments(Locale.GERMANY), + arguments(Locale.JAPAN), + arguments(Locale.SIMPLIFIED_CHINESE), + }; + } + @ParameterizedTest @MethodSource void getInstance_1Arg(String[] patterns, List input, String expected) throws ParseException { @@ -235,6 +247,33 @@ void getInstance_3Arg(Locale l, ListFormat.Type type, ListFormat.Style style, St compareResult(f, SAMPLE3, expected, roundTrip); } + @Test + void getLocale_invariant() { + var f = ListFormat.getInstance(CUSTOM_PATTERNS_FULL); + assertEquals(Locale.ROOT, f.getLocale()); + } + + @Test + void getLocale_default() { + var f = ListFormat.getInstance(); + assertEquals(Locale.getDefault(Locale.Category.FORMAT), f.getLocale()); + } + + @ParameterizedTest + @MethodSource + void getLocale_localeDependent(Locale l) { + var f = ListFormat.getInstance(l, ListFormat.Type.STANDARD, ListFormat.Style.FULL); + assertEquals(l, f.getLocale()); + } + + @Test + void getPatterns_immutability() { + var f = ListFormat.getInstance(CUSTOM_PATTERNS_FULL); + var p = f.getPatterns(); + p[0] = null; + assertArrayEquals(CUSTOM_PATTERNS_FULL, f.getPatterns()); + } + @Test void format_3Arg() { var f = ListFormat.getInstance(); From 1f2a80b78a6378b5b03f08a1e61614b8db40654c Mon Sep 17 00:00:00 2001 From: vamsi-parasa Date: Tue, 24 Oct 2023 18:31:33 +0000 Subject: [PATCH 05/19] 8318306: java/util/Arrays/Sorting.java fails with "Array is not sorted at 8228-th position: 8251.0 and 8153.0" Reviewed-by: thartmann, jbhateja --- src/hotspot/share/opto/library_call.cpp | 74 ++++++++++--------- .../intrinsics/SortingDeoptimizationTest.java | 72 ++++++++++++++++++ 2 files changed, 112 insertions(+), 34 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/intrinsics/SortingDeoptimizationTest.java diff --git a/src/hotspot/share/opto/library_call.cpp b/src/hotspot/share/opto/library_call.cpp index 4a9d7fb161667..62d785809531e 100644 --- a/src/hotspot/share/opto/library_call.cpp +++ b/src/hotspot/share/opto/library_call.cpp @@ -5367,8 +5367,6 @@ void LibraryCallKit::create_new_uncommon_trap(CallStaticJavaNode* uncommon_trap_ //------------------------------inline_array_partition----------------------- bool LibraryCallKit::inline_array_partition() { - const char *stubName = "array_partition_stub"; - Node* elementType = null_check(argument(0)); Node* obj = argument(1); Node* offset = argument(2); @@ -5377,38 +5375,48 @@ bool LibraryCallKit::inline_array_partition() { Node* indexPivot1 = argument(6); Node* indexPivot2 = argument(7); - const TypeInstPtr* elem_klass = gvn().type(elementType)->isa_instptr(); - ciType* elem_type = elem_klass->const_oop()->as_instance()->java_mirror_type(); - BasicType bt = elem_type->basic_type(); - address stubAddr = nullptr; - stubAddr = StubRoutines::select_array_partition_function(); - // stub not loaded - if (stubAddr == nullptr) { - return false; - } - // get the address of the array - const TypeAryPtr* obj_t = _gvn.type(obj)->isa_aryptr(); - if (obj_t == nullptr || obj_t->elem() == Type::BOTTOM ) { - return false; // failed input validation - } - Node* obj_adr = make_unsafe_address(obj, offset); + Node* pivotIndices = nullptr; - // create the pivotIndices array of type int and size = 2 - Node* size = intcon(2); - Node* klass_node = makecon(TypeKlassPtr::make(ciTypeArrayKlass::make(T_INT))); - Node* pivotIndices = new_array(klass_node, size, 0); // no arguments to push - AllocateArrayNode* alloc = tightly_coupled_allocation(pivotIndices); - guarantee(alloc != nullptr, "created above"); - Node* pivotIndices_adr = basic_plus_adr(pivotIndices, arrayOopDesc::base_offset_in_bytes(T_INT)); + // Set the original stack and the reexecute bit for the interpreter to reexecute + // the bytecode that invokes DualPivotQuicksort.partition() if deoptimization happens. + { PreserveReexecuteState preexecs(this); + jvms()->set_should_reexecute(true); - // pass the basic type enum to the stub - Node* elemType = intcon(bt); + const TypeInstPtr* elem_klass = gvn().type(elementType)->isa_instptr(); + ciType* elem_type = elem_klass->const_oop()->as_instance()->java_mirror_type(); + BasicType bt = elem_type->basic_type(); + address stubAddr = nullptr; + stubAddr = StubRoutines::select_array_partition_function(); + // stub not loaded + if (stubAddr == nullptr) { + return false; + } + // get the address of the array + const TypeAryPtr* obj_t = _gvn.type(obj)->isa_aryptr(); + if (obj_t == nullptr || obj_t->elem() == Type::BOTTOM ) { + return false; // failed input validation + } + Node* obj_adr = make_unsafe_address(obj, offset); - // Call the stub - make_runtime_call(RC_LEAF|RC_NO_FP, OptoRuntime::array_partition_Type(), - stubAddr, stubName, TypePtr::BOTTOM, - obj_adr, elemType, fromIndex, toIndex, pivotIndices_adr, - indexPivot1, indexPivot2); + // create the pivotIndices array of type int and size = 2 + Node* size = intcon(2); + Node* klass_node = makecon(TypeKlassPtr::make(ciTypeArrayKlass::make(T_INT))); + pivotIndices = new_array(klass_node, size, 0); // no arguments to push + AllocateArrayNode* alloc = tightly_coupled_allocation(pivotIndices); + guarantee(alloc != nullptr, "created above"); + Node* pivotIndices_adr = basic_plus_adr(pivotIndices, arrayOopDesc::base_offset_in_bytes(T_INT)); + + // pass the basic type enum to the stub + Node* elemType = intcon(bt); + + // Call the stub + const char *stubName = "array_partition_stub"; + make_runtime_call(RC_LEAF|RC_NO_FP, OptoRuntime::array_partition_Type(), + stubAddr, stubName, TypePtr::BOTTOM, + obj_adr, elemType, fromIndex, toIndex, pivotIndices_adr, + indexPivot1, indexPivot2); + + } // original reexecute is set back here if (!stopped()) { set_result(pivotIndices); @@ -5421,9 +5429,6 @@ bool LibraryCallKit::inline_array_partition() { //------------------------------inline_array_sort----------------------- bool LibraryCallKit::inline_array_sort() { - const char *stubName; - stubName = "arraysort_stub"; - Node* elementType = null_check(argument(0)); Node* obj = argument(1); Node* offset = argument(2); @@ -5451,6 +5456,7 @@ bool LibraryCallKit::inline_array_sort() { Node* elemType = intcon(bt); // Call the stub. + const char *stubName = "arraysort_stub"; make_runtime_call(RC_LEAF|RC_NO_FP, OptoRuntime::array_sort_Type(), stubAddr, stubName, TypePtr::BOTTOM, obj_adr, elemType, fromIndex, toIndex); diff --git a/test/hotspot/jtreg/compiler/intrinsics/SortingDeoptimizationTest.java b/test/hotspot/jtreg/compiler/intrinsics/SortingDeoptimizationTest.java new file mode 100644 index 0000000000000..f9fbcfefb9953 --- /dev/null +++ b/test/hotspot/jtreg/compiler/intrinsics/SortingDeoptimizationTest.java @@ -0,0 +1,72 @@ +/* + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8318306 + * @run main/othervm/timeout=200 -XX:+IgnoreUnrecognizedVMOptions -Xcomp -ea -esa -XX:CompileThreshold=100 -XX:+UnlockExperimentalVMOptions -server -XX:-TieredCompilation -XX:+DeoptimizeALot SortingDeoptimizationTest 1e-2 100 50 + * @summary Exercise Arrays.parallelSort when -XX:+DeoptimizeALot is enabled + * + */ + +import java.io.PrintStream; +import java.util.Arrays; +import java.util.Random; + +public class SortingDeoptimizationTest { + + private static final PrintStream err = System.err; + private static final PrintStream out = System.out; + + public static void main(String[] args) { + int MAX = 2147483647; // 2^32 - 1 + float fraction = Float.parseFloat(args[0]); + int size = (int) (fraction * MAX); // size is a fraction of the MAX size + int iters = Integer.parseInt(args[1]); // number of iterations + int max = args.length > 2 ? Integer.parseInt(args[2]) : -1 ; // max value for the array elements + long seed = 0xC0FFEE; + Random rand = new Random(seed); + + for (int i = 0; i < iters; i++) { + boolean isSorted = runSort(size, max, rand); + out.println("Iteration " + i + ": is sorted? -> "+ isSorted); + if (!isSorted) fail("Array is not correctly sorted."); + } + } + + private static void fail(String message) { + err.format("\n*** TEST FAILED ***\n\n%s\n\n", message); + throw new RuntimeException("Test failed"); + } + + private static boolean runSort(int size, int max, Random rand) { + int[] a = new int[size]; + for (int i = 0; i < a.length; i++) a[i] = max > 0 ? rand.nextInt(max) : rand.nextInt(); + // call parallel sort + Arrays.parallelSort(a); + // check if sorted + boolean isSorted = true; + for (int i = 0; i < (a.length -1); i++) isSorted = isSorted && (a[i] <= a[i+1]); + return isSorted; + } +} From 1ddf826aea7fd18209336dce550821638d5ef89c Mon Sep 17 00:00:00 2001 From: Weijun Wang Date: Tue, 24 Oct 2023 18:32:01 +0000 Subject: [PATCH 06/19] 8316964: Security tools should not call System.exit Reviewed-by: valeriep --- .../sun/security/tools/keytool/Main.java | 27 ++- .../security/krb5/internal/tools/Kinit.java | 78 +++---- .../krb5/internal/tools/KinitOptions.java | 7 +- .../security/krb5/internal/tools/Klist.java | 190 +++++++++--------- .../security/krb5/internal/tools/Ktab.java | 89 +++++--- .../sun/security/tools/jarsigner/Main.java | 62 +++--- .../sun/security/krb5/tools/ExitOrNot.java | 61 ++++++ .../security/tools/jarsigner/ExitOrNot.java | 52 +++++ 8 files changed, 358 insertions(+), 208 deletions(-) create mode 100644 test/jdk/sun/security/krb5/tools/ExitOrNot.java create mode 100644 test/jdk/sun/security/tools/jarsigner/ExitOrNot.java diff --git a/src/java.base/share/classes/sun/security/tools/keytool/Main.java b/src/java.base/share/classes/sun/security/tools/keytool/Main.java index 2effa3425d71a..b094acc51fe96 100644 --- a/src/java.base/share/classes/sun/security/tools/keytool/Main.java +++ b/src/java.base/share/classes/sun/security/tools/keytool/Main.java @@ -405,26 +405,38 @@ public String toString() { collator.setStrength(Collator.PRIMARY); } - private Main() { } - public static void main(String[] args) throws Exception { Main kt = new Main(); - kt.run(args, System.out); + int exitCode = kt.run(args, System.out); + if (exitCode != 0) { + System.exit(exitCode); + } + } + + private static class ExitException extends RuntimeException { + @java.io.Serial + static final long serialVersionUID = 0L; + private final int errorCode; + public ExitException(int errorCode) { + this.errorCode = errorCode; + } } - private void run(String[] args, PrintStream out) throws Exception { + public int run(String[] args, PrintStream out) throws Exception { try { - args = parseArgs(args); + parseArgs(args); if (command != null) { doCommands(out); } + } catch (ExitException ee) { + return ee.errorCode; } catch (Exception e) { System.out.println(rb.getString("keytool.error.") + e); if (verbose) { e.printStackTrace(System.out); } if (!debug) { - System.exit(1); + return 1; } else { throw e; } @@ -441,6 +453,7 @@ private void run(String[] args, PrintStream out) throws Exception { ksStream.close(); } } + return 0; } /** @@ -5247,7 +5260,7 @@ private void tinyHelp() { if (debug) { throw new RuntimeException("NO BIG ERROR, SORRY"); } else { - System.exit(1); + throw new ExitException(1); } } diff --git a/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Kinit.java b/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Kinit.java index 813939643c3cf..a14ece6ee0e02 100644 --- a/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Kinit.java +++ b/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Kinit.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2023, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -92,14 +92,44 @@ public class Kinit { */ public static void main(String[] args) { - try { - Kinit self = new Kinit(args); + Kinit kinit = new Kinit(); + int exitCode = kinit.run(args); + if (exitCode != 0) { + System.exit(exitCode); } - catch (Exception e) { - String msg = null; - if (e instanceof KrbException) { - msg = ((KrbException)e).krbErrorMessage() + " " + - ((KrbException)e).returnCodeMessage(); + } + + /** + * Run the Kinit command. + * @param args array of ticket request options. + * Available options are: -f, -p, -c, principal, password. + * @return the exit code + */ + public int run(String[] args) { + try { + if (args == null || args.length == 0) { + options = new KinitOptions(); + } else { + options = new KinitOptions(args); + } + switch (options.action) { + case 0: + // Help, already displayed in new KinitOptions(). + break; + case 1: + acquire(); + break; + case 2: + renew(); + break; + default: + throw new KrbException("kinit does not support action " + + options.action); + } + } catch (Exception e) { + String msg; + if (e instanceof KrbException ke) { + msg = ke.krbErrorMessage() + " " + ke.returnCodeMessage(); } else { msg = e.getMessage(); } @@ -109,37 +139,9 @@ public static void main(String[] args) { System.out.println("Exception: " + e); } e.printStackTrace(); - System.exit(-1); - } - return; - } - - /** - * Constructs a new Kinit object. - * @param args array of ticket request options. - * Available options are: -f, -p, -c, principal, password. - * @exception IOException if an I/O error occurs. - * @exception RealmException if the Realm could not be instantiated. - * @exception KrbException if error occurs during Kerberos operation. - */ - private Kinit(String[] args) - throws IOException, RealmException, KrbException { - if (args == null || args.length == 0) { - options = new KinitOptions(); - } else { - options = new KinitOptions(args); - } - switch (options.action) { - case 1: - acquire(); - break; - case 2: - renew(); - break; - default: - throw new KrbException("kinit does not support action " - + options.action); + return -1; } + return 0; } private void renew() diff --git a/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/KinitOptions.java b/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/KinitOptions.java index 5cfc574d95acf..445b806bb50cf 100644 --- a/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/KinitOptions.java +++ b/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/KinitOptions.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2018, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2023, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -46,7 +46,7 @@ */ class KinitOptions { - // 1. acquire, 2. renew, 3. validate + // 0. Help, 1. acquire, 2. renew, 3. validate public int action = 1; // forwardable and proxiable flags have two states: @@ -143,7 +143,8 @@ public KinitOptions(String[] args) // -help: legacy. args[i].equalsIgnoreCase("-help")) { printHelp(); - System.exit(0); + action = 0; + return; } else if (p == null) { // Haven't yet processed a "principal" p = args[i]; try { diff --git a/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Klist.java b/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Klist.java index 5a0c3bee0776e..16c9fd99ec406 100644 --- a/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Klist.java +++ b/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Klist.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2023, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -78,115 +78,111 @@ public class Klist { */ public static void main(String[] args) { Klist klist = new Klist(); + int exitCode = klist.run(args); + if (exitCode != 0) { + System.exit(exitCode); + } + } + + public int run(String[] args) { if ((args == null) || (args.length == 0)) { - klist.action = 'c'; // default will list default credentials cache. + action = 'c'; // default will list default credentials cache. } else { - klist.processArgs(args); + Character arg; + for (int i = 0; i < args.length; i++) { + if (args[i].equals("-?") || + args[i].equals("-h") || + args[i].equals("--help")) { + printHelp(); + return 0; + } + if ((args[i].length() >= 2) && (args[i].startsWith("-"))) { + arg = Character.valueOf(args[i].charAt(1)); + switch (arg.charValue()) { + case 'c': + action = 'c'; + break; + case 'k': + action = 'k'; + break; + case 'a': + options[2] = 'a'; + break; + case 'n': + options[3] = 'n'; + break; + case 'f': + options[1] = 'f'; + break; + case 'e': + options[0] = 'e'; + break; + case 'K': + options[1] = 'K'; + break; + case 't': + options[2] = 't'; + break; + default: + System.out.println("Invalid argument: " + args[i]); + printHelp(); + return -1; + } + } else { + if (!args[i].startsWith("-") && (i == args.length - 1)) { + // the argument is the last one. + name = args[i]; + } else { + System.out.println("Invalid argument: " + args[i]); + printHelp(); // incorrect input format. + return -1; + } + } + } } - switch (klist.action) { + switch (action) { case 'c': - if (klist.name == null) { - klist.target = CredentialsCache.getInstance(); - klist.name = CredentialsCache.cacheName(); + if (name == null) { + target = CredentialsCache.getInstance(); + name = CredentialsCache.cacheName(); } else - klist.target = CredentialsCache.getInstance(klist.name); + target = CredentialsCache.getInstance(name); - if (klist.target != null) { - klist.displayCache(); + if (target != null) { + return displayCache(); } else { - klist.displayMessage("Credentials cache"); - System.exit(-1); + return displayError("Credentials cache"); } - break; case 'k': - KeyTab ktab = KeyTab.getInstance(klist.name); + KeyTab ktab = KeyTab.getInstance(name); if (ktab.isMissing()) { - System.out.println("KeyTab " + klist.name + " not found."); - System.exit(-1); + System.out.println("KeyTab " + name + " not found."); + return -1; } else if (!ktab.isValid()) { - System.out.println("KeyTab " + klist.name + System.out.println("KeyTab " + name + " format not supported."); - System.exit(-1); + return -1; } - klist.target = ktab; - klist.name = ktab.tabName(); - klist.displayTab(); - break; + target = ktab; + name = ktab.tabName(); + return displayTab(); default: - if (klist.name != null) { - klist.printHelp(); - System.exit(-1); - } else { - klist.target = CredentialsCache.getInstance(); - klist.name = CredentialsCache.cacheName(); - if (klist.target != null) { - klist.displayCache(); - } else { - klist.displayMessage("Credentials cache"); - System.exit(-1); - } - } - } - } - - /** - * Parses the command line arguments. - */ - void processArgs(String[] args) { - Character arg; - for (int i = 0; i < args.length; i++) { - if (args[i].equals("-?") || - args[i].equals("-h") || - args[i].equals("--help")) { + if (name != null) { printHelp(); - System.exit(0); - } - if ((args[i].length() >= 2) && (args[i].startsWith("-"))) { - arg = Character.valueOf(args[i].charAt(1)); - switch (arg.charValue()) { - case 'c': - action = 'c'; - break; - case 'k': - action = 'k'; - break; - case 'a': - options[2] = 'a'; - break; - case 'n': - options[3] = 'n'; - break; - case 'f': - options[1] = 'f'; - break; - case 'e': - options[0] = 'e'; - break; - case 'K': - options[1] = 'K'; - break; - case 't': - options[2] = 't'; - break; - default: - printHelp(); - System.exit(-1); - } - + return -1; } else { - if (!args[i].startsWith("-") && (i == args.length - 1)) { - // the argument is the last one. - name = args[i]; - arg = null; + target = CredentialsCache.getInstance(); + name = CredentialsCache.cacheName(); + if (target != null) { + return displayCache(); } else { - printHelp(); // incorrect input format. - System.exit(-1); + return displayError("Credentials cache"); } } } } - void displayTab() { + int displayTab() { KeyTab table = (KeyTab)target; KeyTabEntry[] entries = table.getEntries(); if (entries.length == 0) { @@ -201,7 +197,7 @@ void displayTab() { entries.length + " entries found.\n"); for (int i = 0; i < entries.length; i++) { System.out.println("[" + (i + 1) + "] " + - "Service principal: " + + "Service principal: " + entries[i].getService().toString()); System.out.println("\t KVNO: " + entries[i].getKey().getKeyVersionNumber()); @@ -221,18 +217,19 @@ void displayTab() { } } } + return 0; } - void displayCache() { + int displayCache() { CredentialsCache cache = (CredentialsCache)target; sun.security.krb5.internal.ccache.Credentials[] creds = cache.getCredsList(); if (creds == null) { System.out.println ("No credentials available in the cache " + name); - System.exit(-1); + return -1; } - System.out.println("\nCredentials cache: " + name); + System.out.println("\nCredentials cache: " + name); String defaultPrincipal = cache.getPrimaryPrincipal().toString(); int num = creds.length; @@ -327,7 +324,7 @@ void displayCache() { if (DEBUG) { e.printStackTrace(); } - System.exit(-1); + return -1; } } } else { @@ -342,14 +339,17 @@ void displayCache() { System.out.println(" " + e); } } + + return 0; } - void displayMessage(String target) { + int displayError(String target) { if (name == null) { System.out.println("Default " + target + " not found."); } else { System.out.println(target + " " + name + " not found."); } + return -1; } /** * Reformats the date from the form - @@ -359,7 +359,7 @@ void displayMessage(String target) { * the day, mm is the minute within the hour, * ss is the second within the minute, zzz is the time zone, * and yyyy is the year. - * @param date the string form of Date object. + * @param kt the string form of Date object. */ private String format(KerberosTime kt) { String date = kt.toDate().toString(); diff --git a/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Ktab.java b/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Ktab.java index df12b2d6c11dd..21002f3369a03 100644 --- a/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Ktab.java +++ b/src/java.security.jgss/windows/classes/sun/security/krb5/internal/tools/Ktab.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2023, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -73,52 +73,76 @@ public class Ktab { */ public static void main(String[] args) { Ktab ktab = new Ktab(); + int exitCode = ktab.run(args); + if (exitCode != 0) { + System.exit(exitCode); + } + } + + private static class ExitException extends RuntimeException { + @java.io.Serial + static final long serialVersionUID = 0L; + private final int errorCode; + public ExitException(int errorCode) { + this.errorCode = errorCode; + } + } + + public int run(String[] args) { + try { + run0(args); + return 0; + } catch (ExitException ee) { + return ee.errorCode; + } + } + + private void run0(String[] args) throws ExitException { if ((args.length == 1) && ((args[0].equalsIgnoreCase("-?")) || (args[0].equalsIgnoreCase("-h")) || (args[0].equalsIgnoreCase("--help")) || // -help: legacy. (args[0].equalsIgnoreCase("-help")))) { - ktab.printHelp(); - System.exit(0); + printHelp(); return; } else if ((args == null) || (args.length == 0)) { - ktab.action = 'l'; + action = 'l'; } else { - ktab.processArgs(args); + processArgs(args); } - ktab.table = KeyTab.getInstance(ktab.name); - if (ktab.table.isMissing() && ktab.action != 'a') { - if (ktab.name == null) { + table = KeyTab.getInstance(name); + if (table.isMissing() && action != 'a') { + if (name == null) { System.out.println("No default key table exists."); } else { System.out.println("Key table " + - ktab.name + " does not exist."); + name + " does not exist."); } - System.exit(-1); + throw new ExitException(-1); } - if (!ktab.table.isValid()) { - if (ktab.name == null) { + if (!table.isValid()) { + if (name == null) { System.out.println("The format of the default key table " + " is incorrect."); } else { System.out.println("The format of key table " + - ktab.name + " is incorrect."); + name + " is incorrect."); } - System.exit(-1); + throw new ExitException(-1); } - switch (ktab.action) { + switch (action) { case 'l': - ktab.listKt(); + listKt(); break; case 'a': - ktab.addEntry(); + addEntry(); break; case 'd': - ktab.deleteEntry(); + deleteEntry(); break; default: - ktab.error("A command must be provided"); + error("A command must be provided"); } } @@ -267,7 +291,7 @@ void processArgs(String[] args) { void addEntry() { if (salt != null && fopt) { System.err.println("-s and -f cannot coexist when adding a keytab entry."); - System.exit(-1); + throw new ExitException(-1); } PrincipalName pname = null; try { @@ -276,7 +300,7 @@ void addEntry() { System.err.println("Failed to add " + principal + " to keytab."); e.printStackTrace(); - System.exit(-1); + throw new ExitException(-1); } if (password == null) { try { @@ -288,7 +312,7 @@ void addEntry() { } catch (IOException e) { System.err.println("Failed to read the password."); e.printStackTrace(); - System.exit(-1); + throw new ExitException(-1); } } @@ -313,11 +337,11 @@ void addEntry() { } catch (KrbException e) { System.err.println("Failed to add " + principal + " to keytab."); e.printStackTrace(); - System.exit(-1); + throw new ExitException(-1); } catch (IOException e) { System.err.println("Failed to save new entry."); e.printStackTrace(); - System.exit(-1); + throw new ExitException(-1); } } @@ -399,22 +423,23 @@ void deleteEntry() { System.out.flush(); answer = cis.readLine(); if (answer.equalsIgnoreCase("Y") || - answer.equalsIgnoreCase("Yes")); - else { + answer.equalsIgnoreCase("Yes")) { + ; + } else { // no error, the user did not want to delete the entry - System.exit(0); + return; } } } catch (KrbException e) { System.err.println("Error occurred while deleting the entry. "+ "Deletion failed."); e.printStackTrace(); - System.exit(-1); + throw new ExitException(-1); } catch (IOException e) { System.err.println("Error occurred while deleting the entry. "+ " Deletion failed."); e.printStackTrace(); - System.exit(-1); + throw new ExitException(-1); } int count = table.deleteEntries(pname, etype, vDel); @@ -422,7 +447,7 @@ void deleteEntry() { if (count == 0) { System.err.println("No matched entry in the keytab. " + "Deletion fails."); - System.exit(-1); + throw new ExitException(-1); } else { try { table.save(); @@ -430,7 +455,7 @@ void deleteEntry() { System.err.println("Error occurs while saving the keytab. " + "Deletion fails."); e.printStackTrace(); - System.exit(-1); + throw new ExitException(-1); } System.out.println("Done! " + count + " entries removed."); } @@ -441,7 +466,7 @@ void error(String... errors) { System.out.println("Error: " + error + "."); } printHelp(); - System.exit(-1); + throw new ExitException(-1); } /** diff --git a/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java b/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java index 117fea688c71b..ed2c9be854310 100644 --- a/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java +++ b/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java @@ -135,7 +135,19 @@ public class Main { // This is the entry that get launched by the security tool jarsigner. public static void main(String args[]) throws Exception { Main js = new Main(); - js.run(args); + int exitCode = js.run(args); + if (exitCode != 0) { + System.exit(exitCode); + } + } + + private static class ExitException extends RuntimeException { + @java.io.Serial + static final long serialVersionUID = 0L; + private final int errorCode; + public ExitException(int errorCode) { + this.errorCode = errorCode; + } } X509Certificate[] certChain; // signer's cert chain (when composing) @@ -230,13 +242,13 @@ public static void main(String args[]) throws Exception { PKIXBuilderParameters pkixParameters; Set trustedCerts = new HashSet<>(); - public void run(String args[]) { + public int run(String args[]) { try { - args = parseArgs(args); + parseArgs(args); // Try to load and install the specified providers if (providers != null) { - for (String provName: providers) { + for (String provName : providers) { try { KeyStoreUtil.loadProviderByName(provName, providerArgs.get(provName)); @@ -263,7 +275,7 @@ public void run(String args[]) { } else { cl = ClassLoader.getSystemClassLoader(); } - for (String provClass: providerClasses) { + for (String provClass : providerClasses) { try { KeyStoreUtil.loadProviderByClass(provClass, providerArgs.get(provClass), cl); @@ -285,19 +297,9 @@ public void run(String args[]) { loadKeyStore(keystore, false); } catch (Exception e) { if ((keystore != null) || (storepass != null)) { - System.out.println(rb.getString("jarsigner.error.") + - e.getMessage()); - if (debug) { - e.printStackTrace(); - } - System.exit(1); + throw e; } } - /* if (debug) { - SignatureFileVerifier.setDebug(true); - ManifestEntryVerifier.setDebug(true); - } - */ verifyJar(jarfile); } else { loadKeyStore(keystore, true); @@ -305,12 +307,14 @@ public void run(String args[]) { signJar(jarfile, alias); } + } catch (ExitException ee) { + return ee.errorCode; } catch (Exception e) { System.out.println(rb.getString("jarsigner.error.") + e); if (debug) { e.printStackTrace(); } - System.exit(1); + return 1; } finally { // zero-out private key password if (keypass != null) { @@ -343,10 +347,10 @@ public void run(String args[]) { if (tsaChainNotValidated) { exitCode |= 64; } - if (exitCode != 0) { - System.exit(exitCode); - } + return exitCode; } + + return 0; } /* @@ -612,12 +616,12 @@ static void usageNoArg() { static void usage() { System.out.println(); System.out.println(rb.getString("Please.type.jarsigner.help.for.usage")); - System.exit(1); + throw new ExitException(1); } static void doPrintVersion() { System.out.println("jarsigner " + System.getProperty("java.version")); - System.exit(0); + throw new ExitException(0); } static void fullusage() { @@ -719,7 +723,7 @@ static void fullusage() { (".print.this.help.message")); System.out.println(); - System.exit(0); + throw new ExitException(0); } void verifyJar(String jarName) @@ -1105,19 +1109,11 @@ void verifyJar(String jarName) } else { displayMessagesAndResult(false); } - return; - } catch (Exception e) { - System.out.println(rb.getString("jarsigner.") + e); - if (debug) { - e.printStackTrace(); - } } finally { // close the resource if (jf != null) { jf.close(); } } - - System.exit(1); } private void displayMessagesAndResult(boolean isSigning) { @@ -2469,7 +2465,7 @@ void getAliasInfo(String alias) throws Exception { void error(String message) { System.out.println(rb.getString("jarsigner.")+message); - System.exit(1); + throw new ExitException(1); } @@ -2478,7 +2474,7 @@ void error(String message, Throwable e) { if (debug) { e.printStackTrace(); } - System.exit(1); + throw new ExitException(1); } /** diff --git a/test/jdk/sun/security/krb5/tools/ExitOrNot.java b/test/jdk/sun/security/krb5/tools/ExitOrNot.java new file mode 100644 index 0000000000000..9f3000dcb75ae --- /dev/null +++ b/test/jdk/sun/security/krb5/tools/ExitOrNot.java @@ -0,0 +1,61 @@ +/* + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8316964 + * @summary check exit code in kinit, klist, and ktab + * @requires os.family == "windows" + * @library /test/lib + * @modules java.security.jgss/sun.security.krb5.internal.tools + */ + +import jdk.test.lib.Asserts; +import jdk.test.lib.Platform; +import jdk.test.lib.SecurityTools; + +public class ExitOrNot { + + private static final int BAD = Platform.isWindows() ? -1 : 255; + + public static void main(String[] args) throws Exception { + + // launching the tool still exits + SecurityTools.kinit("u@R p1 p2") + .shouldHaveExitValue(BAD); + + SecurityTools.klist("-x") + .shouldHaveExitValue(BAD); + + SecurityTools.ktab("-x") + .shouldHaveExitValue(BAD); + + // calling the run() methods returns the exit code + Asserts.assertEQ(new sun.security.krb5.internal.tools.Kinit() + .run("u@R p1 p2".split(" ")), -1); + Asserts.assertEQ(new sun.security.krb5.internal.tools.Klist() + .run("-x".split(" ")), -1); + Asserts.assertEQ(new sun.security.krb5.internal.tools.Ktab() + .run("-x".split(" ")), -1); + } +} diff --git a/test/jdk/sun/security/tools/jarsigner/ExitOrNot.java b/test/jdk/sun/security/tools/jarsigner/ExitOrNot.java new file mode 100644 index 0000000000000..ae55074f3a50c --- /dev/null +++ b/test/jdk/sun/security/tools/jarsigner/ExitOrNot.java @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8316964 + * @summary check exit code in jarsigner and keytool + * @library /test/lib + * @modules java.base/sun.security.tools.keytool + * jdk.jartool/sun.security.tools.jarsigner + */ + +import jdk.test.lib.Asserts; +import jdk.test.lib.SecurityTools; + +public class ExitOrNot { + public static void main(String[] args) throws Exception { + + // launching the tool still exits + SecurityTools.jarsigner("1 2 3") + .shouldHaveExitValue(1); + SecurityTools.keytool("-x") + .shouldHaveExitValue(1); + + // calling the run() methods no longer + Asserts.assertEQ(new sun.security.tools.jarsigner.Main() + .run("1 2 3".split(" ")), 1); + + Asserts.assertEQ(new sun.security.tools.keytool.Main() + .run("-x".split(" "), System.out), 1); + } +} From 9c819fd3b7e564b53514185573f4ffe28368b46b Mon Sep 17 00:00:00 2001 From: Eamonn McManus Date: Tue, 24 Oct 2023 18:32:33 +0000 Subject: [PATCH 07/19] 8318051: Duration.between uses exceptions for control flow Reviewed-by: rriggs --- .../share/classes/java/time/Duration.java | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/java.base/share/classes/java/time/Duration.java b/src/java.base/share/classes/java/time/Duration.java index c1894e2db5a7c..ab46159efd6a2 100644 --- a/src/java.base/share/classes/java/time/Duration.java +++ b/src/java.base/share/classes/java/time/Duration.java @@ -486,23 +486,26 @@ private static Duration create(boolean negate, long daysAsSecs, long hoursAsSecs * @throws ArithmeticException if the calculation exceeds the capacity of {@code Duration} */ public static Duration between(Temporal startInclusive, Temporal endExclusive) { - try { + long secs = startInclusive.until(endExclusive, SECONDS); + if (secs == 0) { + // We don't know which Temporal is earlier, so the adjustment below would not work. + // But we do know that there's no danger of until(NANOS) overflowing in that case. return ofNanos(startInclusive.until(endExclusive, NANOS)); - } catch (DateTimeException | ArithmeticException ex) { - long secs = startInclusive.until(endExclusive, SECONDS); - long nanos; - try { - nanos = endExclusive.getLong(NANO_OF_SECOND) - startInclusive.getLong(NANO_OF_SECOND); - if (secs > 0 && nanos < 0) { - secs++; - } else if (secs < 0 && nanos > 0) { - secs--; - } - } catch (DateTimeException ex2) { - nanos = 0; - } - return ofSeconds(secs, nanos); } + long nanos; + try { + nanos = endExclusive.getLong(NANO_OF_SECOND) - startInclusive.getLong(NANO_OF_SECOND); + } catch (DateTimeException ex2) { + nanos = 0; + } + if (nanos < 0 && secs > 0) { + // ofSeconds will subtract one even though until(SECONDS) already gave the correct + // number of seconds. So compensate. Similarly for the secs < 0 case below. + secs++; + } else if (nanos > 0 && secs < 0) { + secs--; + } + return ofSeconds(secs, nanos); } //----------------------------------------------------------------------- From f1dfdc1a79f3a16eae58d15d1945541a08f7e145 Mon Sep 17 00:00:00 2001 From: Alexander Matveev Date: Tue, 24 Oct 2023 21:41:20 +0000 Subject: [PATCH 08/19] 8311877: [macos] Add CLI options to provide signing identity directly to codesign and productbuild Reviewed-by: asemenyuk --- .../jdk/jpackage/internal/MacAppBundler.java | 26 +++- .../jpackage/internal/MacAppImageBuilder.java | 50 ++++++-- .../internal/MacBaseInstallerBundler.java | 7 ++ .../jdk/jpackage/internal/MacPkgBundler.java | 54 ++++++-- .../resources/MacResources.properties | 3 +- .../resources/MacResources_de.properties | 3 +- .../resources/MacResources_ja.properties | 3 +- .../resources/MacResources_zh_CN.properties | 3 +- .../jdk/jpackage/internal/Arguments.java | 24 ++++ .../jpackage/internal/BundlerParamInfo.java | 28 +++-- .../jdk/jpackage/internal/ValidOptions.java | 6 +- .../resources/HelpResources.properties | 16 ++- .../resources/MainResources.properties | 5 +- .../resources/MainResources_de.properties | 3 +- .../resources/MainResources_ja.properties | 3 +- .../resources/MainResources_zh_CN.properties | 3 +- .../jpackage/macosx/SigningAppImageTest.java | 26 ++-- .../macosx/SigningAppImageTwoStepsTest.java | 34 ++++-- .../jpackage/macosx/SigningOptionsTest.java | 115 ++++++++++++++++++ ...SigningPackageFromTwoStepAppImageTest.java | 53 ++++++-- .../jpackage/macosx/SigningPackageTest.java | 86 +++++++++++-- .../macosx/SigningPackageTwoStepTest.java | 51 ++++++-- .../jpackage/macosx/base/SigningBase.java | 32 +++-- .../tests/PredefinedAppImageErrorTest.java | 2 +- 24 files changed, 531 insertions(+), 105 deletions(-) create mode 100644 test/jdk/tools/jpackage/macosx/SigningOptionsTest.java diff --git a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacAppBundler.java b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacAppBundler.java index cf031cb47a5ee..06796db98847a 100644 --- a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacAppBundler.java +++ b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacAppBundler.java @@ -85,6 +85,13 @@ public MacAppBundler() { }, (s, p) -> s); + public static final BundlerParamInfo APP_IMAGE_SIGN_IDENTITY = + new StandardBundlerParam<>( + Arguments.CLIOptions.MAC_APP_IMAGE_SIGN_IDENTITY.getId(), + String.class, + params -> "", + null); + public static final BundlerParamInfo BUNDLE_ID_SIGNING_PREFIX = new StandardBundlerParam<>( Arguments.CLIOptions.MAC_BUNDLE_SIGNING_PREFIX.getId(), @@ -127,14 +134,21 @@ private static void doValidate(Map params) // reject explicitly set sign to true and no valid signature key if (Optional.ofNullable( SIGN_BUNDLE.fetchFrom(params)).orElse(Boolean.FALSE)) { - String signingIdentity = - DEVELOPER_ID_APP_SIGNING_KEY.fetchFrom(params); - if (signingIdentity == null) { - throw new ConfigException( - I18N.getString("error.explicit-sign-no-cert"), - I18N.getString("error.explicit-sign-no-cert.advice")); + // Validate DEVELOPER_ID_APP_SIGNING_KEY only if user provided + // SIGNING_KEY_USER. + if (!SIGNING_KEY_USER.getIsDefaultValue(params)) { // --mac-signing-key-user-name + String signingIdentity = + DEVELOPER_ID_APP_SIGNING_KEY.fetchFrom(params); + if (signingIdentity == null) { + throw new ConfigException( + I18N.getString("error.explicit-sign-no-cert"), + I18N.getString("error.explicit-sign-no-cert.advice")); + } } + // No need to validate --mac-app-image-sign-identity, since it is + // pass through option. + // Signing will not work without Xcode with command line developer tools try { ProcessBuilder pb = new ProcessBuilder("/usr/bin/xcrun", "--help"); diff --git a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacAppImageBuilder.java b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacAppImageBuilder.java index d9250ae147409..ea86c41d98128 100644 --- a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacAppImageBuilder.java +++ b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacAppImageBuilder.java @@ -25,8 +25,10 @@ package jdk.jpackage.internal; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; +import java.io.PrintStream; import java.io.Writer; import java.nio.file.Files; import java.nio.file.Path; @@ -53,7 +55,10 @@ import jdk.internal.util.OSVersion; import static jdk.jpackage.internal.MacAppBundler.BUNDLE_ID_SIGNING_PREFIX; import static jdk.jpackage.internal.MacAppBundler.DEVELOPER_ID_APP_SIGNING_KEY; +import static jdk.jpackage.internal.MacAppBundler.APP_IMAGE_SIGN_IDENTITY; import static jdk.jpackage.internal.MacBaseInstallerBundler.SIGNING_KEYCHAIN; +import static jdk.jpackage.internal.MacBaseInstallerBundler.SIGNING_KEY_USER; +import static jdk.jpackage.internal.MacBaseInstallerBundler.INSTALLER_SIGN_IDENTITY; import static jdk.jpackage.internal.OverridableResource.createResource; import static jdk.jpackage.internal.StandardBundlerParam.APP_NAME; import static jdk.jpackage.internal.StandardBundlerParam.CONFIG_ROOT; @@ -395,12 +400,25 @@ private void doSigning(Map params) } catch (InterruptedException e) { Log.error(e.getMessage()); } - String signingIdentity = - DEVELOPER_ID_APP_SIGNING_KEY.fetchFrom(params); + String signingIdentity = null; + // Try --mac-app-image-sign-identity first if set + if (!APP_IMAGE_SIGN_IDENTITY.getIsDefaultValue(params)) { + signingIdentity = APP_IMAGE_SIGN_IDENTITY.fetchFrom(params); + } else { + // Check if INSTALLER_SIGN_IDENTITY is set and if it is set + // then do not sign app image, otherwise use --mac-signing-key-user-name + if (INSTALLER_SIGN_IDENTITY.getIsDefaultValue(params)) { + // --mac-sign and/or --mac-signing-key-user-name case + signingIdentity = DEVELOPER_ID_APP_SIGNING_KEY.fetchFrom(params); + } + } if (signingIdentity != null) { signAppBundle(params, root, signingIdentity, BUNDLE_ID_SIGNING_PREFIX.fetchFrom(params), ENTITLEMENTS.fetchFrom(params)); + } else { + // Case when user requested to sign installer only + signAppBundle(params, root, "-", null, null); } restoreKeychainList(params); } else if (OperatingSystem.isMacOS()) { @@ -715,6 +733,25 @@ private static List getCodesignArgs( return args; } + private static void runCodesign(ProcessBuilder pb, boolean quiet) + throws IOException { + try (ByteArrayOutputStream baos = new ByteArrayOutputStream(); + PrintStream ps = new PrintStream(baos)) { + try { + IOUtils.exec(pb, false, ps, false, + Executor.INFINITE_TIMEOUT, quiet); + } catch (IOException ioe) { + // Log output of "codesign" in case of + // error. It should help user to diagnose + // issue when using --mac-app-image-sign-identity + Log.info(MessageFormat.format(I18N.getString( + "error.tool.failed.with.output"), "codesign")); + Log.info(baos.toString().strip()); + throw ioe; + } + } + } + static void signAppBundle( Map params, Path appLocation, String signingIdentity, String identifierPrefix, Path entitlements) @@ -781,8 +818,7 @@ static void signAppBundle( p.toFile().setWritable(true, true); ProcessBuilder pb = new ProcessBuilder(args); // run quietly - IOUtils.exec(pb, false, null, false, - Executor.INFINITE_TIMEOUT, true); + runCodesign(pb, true); Files.setPosixFilePermissions(p, oldPermissions); } catch (IOException ioe) { toThrow.set(ioe); @@ -810,8 +846,7 @@ static void signAppBundle( List args = getCodesignArgs(true, path, signingIdentity, identifierPrefix, entitlements, keyChain); ProcessBuilder pb = new ProcessBuilder(args); - - IOUtils.exec(pb); + runCodesign(pb, false); } catch (IOException e) { toThrow.set(e); } @@ -842,8 +877,7 @@ static void signAppBundle( List args = getCodesignArgs(true, appLocation, signingIdentity, identifierPrefix, entitlements, keyChain); ProcessBuilder pb = new ProcessBuilder(args); - - IOUtils.exec(pb); + runCodesign(pb, false); } private static String extractBundleIdentifier(Map params) { diff --git a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacBaseInstallerBundler.java b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacBaseInstallerBundler.java index b4f63d66f5722..8d9db0a007783 100644 --- a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacBaseInstallerBundler.java +++ b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacBaseInstallerBundler.java @@ -79,6 +79,13 @@ public abstract class MacBaseInstallerBundler extends AbstractBundler { params -> "", null); + public static final BundlerParamInfo INSTALLER_SIGN_IDENTITY = + new StandardBundlerParam<>( + Arguments.CLIOptions.MAC_INSTALLER_SIGN_IDENTITY.getId(), + String.class, + params -> "", + null); + public static final BundlerParamInfo MAC_INSTALLER_NAME = new StandardBundlerParam<> ( "mac.installerName", diff --git a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacPkgBundler.java b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacPkgBundler.java index 60cd11b6f06f8..6ac84975451b4 100644 --- a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacPkgBundler.java +++ b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacPkgBundler.java @@ -28,7 +28,9 @@ import jdk.internal.util.Architecture; import jdk.internal.util.OSVersion; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.PrintStream; import java.io.PrintWriter; import java.net.URI; import java.net.URISyntaxException; @@ -54,6 +56,8 @@ import static jdk.jpackage.internal.StandardBundlerParam.SIGN_BUNDLE; import static jdk.jpackage.internal.MacBaseInstallerBundler.SIGNING_KEYCHAIN; import static jdk.jpackage.internal.MacBaseInstallerBundler.SIGNING_KEY_USER; +import static jdk.jpackage.internal.MacBaseInstallerBundler.INSTALLER_SIGN_IDENTITY; +import static jdk.jpackage.internal.MacAppBundler.APP_IMAGE_SIGN_IDENTITY; import static jdk.jpackage.internal.StandardBundlerParam.APP_STORE; import static jdk.jpackage.internal.MacAppImageBuilder.MAC_CF_BUNDLE_IDENTIFIER; import static jdk.jpackage.internal.OverridableResource.createResource; @@ -605,8 +609,19 @@ private Path createPKG(Map params, Log.verbose(I18N.getString("message.signing.pkg")); } - String signingIdentity = - DEVELOPER_ID_INSTALLER_SIGNING_KEY.fetchFrom(params); + String signingIdentity = null; + // --mac-installer-sign-identity + if (!INSTALLER_SIGN_IDENTITY.getIsDefaultValue(params)) { + signingIdentity = INSTALLER_SIGN_IDENTITY.fetchFrom(params); + } else { + // Use --mac-signing-key-user-name if user did not request + // to sign just app image using --mac-app-image-sign-identity + if (APP_IMAGE_SIGN_IDENTITY.getIsDefaultValue(params)) { + // --mac-signing-key-user-name + signingIdentity = DEVELOPER_ID_INSTALLER_SIGNING_KEY.fetchFrom(params); + } + } + if (signingIdentity != null) { commandLine.add("--sign"); commandLine.add(signingIdentity); @@ -638,7 +653,21 @@ private Path createPKG(Map params, commandLine.add(finalPKG.toAbsolutePath().toString()); pb = new ProcessBuilder(commandLine); - IOUtils.exec(pb, false, null, true, Executor.INFINITE_TIMEOUT); + + try (ByteArrayOutputStream baos = new ByteArrayOutputStream(); + PrintStream ps = new PrintStream(baos)) { + try { + IOUtils.exec(pb, false, ps, true, Executor.INFINITE_TIMEOUT); + } catch (IOException ioe) { + // Log output of "productbuild" in case of + // error. It should help user to diagnose + // issue when using --mac-installer-sign-identity + Log.info(MessageFormat.format(I18N.getString( + "error.tool.failed.with.output"), "productbuild")); + Log.info(baos.toString().strip()); + throw ioe; + } + } return finalPKG; } catch (Exception ignored) { @@ -702,14 +731,19 @@ public boolean validate(Map params) // reject explicitly set sign to true and no valid signature key if (Optional.ofNullable( SIGN_BUNDLE.fetchFrom(params)).orElse(Boolean.FALSE)) { - String signingIdentity = - DEVELOPER_ID_INSTALLER_SIGNING_KEY.fetchFrom(params); - if (signingIdentity == null) { - throw new ConfigException( - I18N.getString("error.explicit-sign-no-cert"), - I18N.getString( - "error.explicit-sign-no-cert.advice")); + if (!SIGNING_KEY_USER.getIsDefaultValue(params)) { + String signingIdentity = + DEVELOPER_ID_INSTALLER_SIGNING_KEY.fetchFrom(params); + if (signingIdentity == null) { + throw new ConfigException( + I18N.getString("error.explicit-sign-no-cert"), + I18N.getString( + "error.explicit-sign-no-cert.advice")); + } } + + // No need to validate --mac-installer-sign-identity, since it is + // pass through option. } // hdiutil is always available so there's no need diff --git a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources.properties b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources.properties index c575df2494e02..de4e7157b2ae3 100644 --- a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources.properties +++ b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources.properties @@ -1,5 +1,5 @@ # -# Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # # This code is free software; you can redistribute it and/or modify it @@ -44,6 +44,7 @@ error.no.xcode.signing.advice=Install Xcode with command line developer tools. error.cert.not.found=No certificate found matching [{0}] using keychain [{1}] error.multiple.certs.found=WARNING: Multiple certificates found matching [{0}] using keychain [{1}], using first one error.app-image.mac-sign.required=Error: --mac-sign option is required with predefined application image and with type [app-image] +error.tool.failed.with.output=Error: "{0}" failed with following output: resource.bundle-config-file=Bundle config file resource.app-info-plist=Application Info.plist resource.runtime-info-plist=Java Runtime Info.plist diff --git a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources_de.properties b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources_de.properties index 6dd037c9bce05..8586041d417f9 100644 --- a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources_de.properties +++ b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources_de.properties @@ -1,5 +1,5 @@ # -# Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # # This code is free software; you can redistribute it and/or modify it @@ -44,6 +44,7 @@ error.no.xcode.signing.advice=Installieren Sie Xcode mit Befehlszeilen-Entwickle error.cert.not.found=Kein Zertifikat gefunden, das [{0}] mit Schlüsselbund [{1}] entspricht error.multiple.certs.found=WARNUNG: Mehrere Zertifikate gefunden, die [{0}] mit Schlüsselbund [{1}] entsprechen. Es wird das erste Zertifikat verwendet error.app-image.mac-sign.required=Fehler: Die Option "--mac-sign" ist mit einem vordefinierten Anwendungsimage und Typ [app-image] erforderlich +error.tool.failed.with.output=Error: "{0}" failed with following output: resource.bundle-config-file=Bundle-Konfigurationsdatei resource.app-info-plist=Info.plist der Anwendung resource.runtime-info-plist=Info.plist von Java Runtime diff --git a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources_ja.properties b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources_ja.properties index b75d1dc465f36..518e3e45f97e9 100644 --- a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources_ja.properties +++ b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources_ja.properties @@ -1,5 +1,5 @@ # -# Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # # This code is free software; you can redistribute it and/or modify it @@ -44,6 +44,7 @@ error.no.xcode.signing.advice=Xcodeとコマンドライン・デベロッパ・ error.cert.not.found=キーチェーン[{1}]を使用する[{0}]と一致する証明書が見つかりません error.multiple.certs.found=警告: キーチェーン[{1}]を使用する[{0}]と一致する複数の証明書が見つかりました。最初のものを使用します error.app-image.mac-sign.required=エラー: --mac-signオプションは、事前定義済アプリケーション・イメージおよびタイプ[app-image]で必要です +error.tool.failed.with.output=Error: "{0}" failed with following output: resource.bundle-config-file=バンドル構成ファイル resource.app-info-plist=アプリケーションのInfo.plist resource.runtime-info-plist=JavaランタイムのInfo.plist diff --git a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources_zh_CN.properties b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources_zh_CN.properties index dc1f592852dd9..c4e4bd2293950 100644 --- a/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources_zh_CN.properties +++ b/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/resources/MacResources_zh_CN.properties @@ -1,5 +1,5 @@ # -# Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # # This code is free software; you can redistribute it and/or modify it @@ -44,6 +44,7 @@ error.no.xcode.signing.advice=安装带命令行开发人员工具的 Xcode。 error.cert.not.found=使用密钥链 [{1}] 找不到与 [{0}] 匹配的证书 error.multiple.certs.found=警告：使用密钥链 [{1}] 找到多个与 [{0}] 匹配的证书，将使用第一个证书 error.app-image.mac-sign.required=错误：预定义的应用程序映像和类型 [app image] 需要 --mac-sign 选项 +error.tool.failed.with.output=Error: "{0}" failed with following output: resource.bundle-config-file=包配置文件 resource.app-info-plist=应用程序 Info.plist resource.runtime-info-plist=Java 运行时 Info.plist diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/Arguments.java b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/Arguments.java index b4a28e7fb3c4e..16cd89d9d52d0 100644 --- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/Arguments.java +++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/Arguments.java @@ -338,6 +338,12 @@ public enum CLIOptions { MAC_SIGNING_KEY_NAME ("mac-signing-key-user-name", OptionCategories.PLATFORM_MAC), + MAC_APP_IMAGE_SIGN_IDENTITY ("mac-app-image-sign-identity", + OptionCategories.PLATFORM_MAC), + + MAC_INSTALLER_SIGN_IDENTITY ("mac-installer-sign-identity", + OptionCategories.PLATFORM_MAC), + MAC_SIGNING_KEYCHAIN ("mac-signing-keychain", OptionCategories.PLATFORM_MAC), @@ -631,6 +637,24 @@ private void validateArguments() throws PackagerException { CLIOptions.JLINK_OPTIONS.getIdWithPrefix()); } } + if (allOptions.contains(CLIOptions.MAC_SIGNING_KEY_NAME) && + allOptions.contains(CLIOptions.MAC_APP_IMAGE_SIGN_IDENTITY)) { + throw new PackagerException("ERR_MutuallyExclusiveOptions", + CLIOptions.MAC_SIGNING_KEY_NAME.getIdWithPrefix(), + CLIOptions.MAC_APP_IMAGE_SIGN_IDENTITY.getIdWithPrefix()); + } + if (allOptions.contains(CLIOptions.MAC_SIGNING_KEY_NAME) && + allOptions.contains(CLIOptions.MAC_INSTALLER_SIGN_IDENTITY)) { + throw new PackagerException("ERR_MutuallyExclusiveOptions", + CLIOptions.MAC_SIGNING_KEY_NAME.getIdWithPrefix(), + CLIOptions.MAC_INSTALLER_SIGN_IDENTITY.getIdWithPrefix()); + } + if (isMac && (imageOnly || "dmg".equals(type)) && + allOptions.contains(CLIOptions.MAC_INSTALLER_SIGN_IDENTITY)) { + throw new PackagerException("ERR_InvalidTypeOption", + CLIOptions.MAC_INSTALLER_SIGN_IDENTITY.getIdWithPrefix(), + type); + } if (allOptions.contains(CLIOptions.DMG_CONTENT) && !("dmg".equals(type))) { throw new PackagerException("ERR_InvalidTypeOption", diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/BundlerParamInfo.java b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/BundlerParamInfo.java index 4173980002bb8..fa99073ff7f79 100644 --- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/BundlerParamInfo.java +++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/BundlerParamInfo.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 2023, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -46,11 +46,6 @@ class BundlerParamInfo { */ Class valueType; - /** - * Indicates if value was set using default value function - */ - boolean isDefaultValue; - /** * If the value is not set, and no fallback value is found, * the parameter uses the value returned by the producer. @@ -70,8 +65,24 @@ Class getValueType() { return valueType; } - boolean getIsDefaultValue() { - return isDefaultValue; + /** + * Returns true if value was not provided on command line for this + * parameter. + * + * @param params - params from which value will be fetch + * @return true if value was not provided on command line, false otherwise + */ + boolean getIsDefaultValue(Map params) { + Object o = params.get(getID()); + if (o != null) { + return false; // We have user provided value + } + + if (params.containsKey(getID())) { + return false; // explicit nulls are allowed for provided value + } + + return true; } Function, T> getDefaultValueFunction() { @@ -114,7 +125,6 @@ final T fetchFrom(Map params, T result = getDefaultValueFunction().apply(params); if (result != null) { params.put(getID(), result); - isDefaultValue = true; } return result; } diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/ValidOptions.java b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/ValidOptions.java index 4d3e163cdc194..89a2e4b56fed7 100644 --- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/ValidOptions.java +++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/ValidOptions.java @@ -27,6 +27,7 @@ import java.util.EnumSet; import java.util.HashMap; + import jdk.internal.util.OperatingSystem; import jdk.jpackage.internal.Arguments.CLIOptions; @@ -62,7 +63,6 @@ enum USE { private static final HashMap> options = new HashMap<>(); - // initializing list of mandatory arguments static { put(CLIOptions.NAME.getId(), USE.ALL); @@ -130,6 +130,10 @@ enum USE { EnumSet.of(USE.ALL, USE.SIGN)); put(CLIOptions.MAC_SIGNING_KEY_NAME.getId(), EnumSet.of(USE.ALL, USE.SIGN)); + put(CLIOptions.MAC_APP_IMAGE_SIGN_IDENTITY.getId(), + EnumSet.of(USE.ALL, USE.SIGN)); + put(CLIOptions.MAC_INSTALLER_SIGN_IDENTITY.getId(), + EnumSet.of(USE.INSTALL, USE.SIGN)); put(CLIOptions.MAC_SIGNING_KEYCHAIN.getId(), EnumSet.of(USE.ALL, USE.SIGN)); put(CLIOptions.MAC_APP_STORE.getId(), USE.ALL); diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources.properties b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources.properties index 6c92cf3e55ca6..c21daab30d030 100644 --- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources.properties +++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/HelpResources.properties @@ -1,5 +1,5 @@ # -# Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # # This code is free software; you can redistribute it and/or modify it @@ -265,7 +265,19 @@ MSG_Help_mac_launcher=\ \ Name of the keychain to search for the signing identity\n\ \ If not specified, the standard keychains are used.\n\ \ --mac-signing-key-user-name \n\ -\ Team or user name portion of Apple signing identities.\n\ +\ Team or user name portion of Apple signing identities. For direct\n\ +\ control of the signing identity used to sign application images or\n\ +\ installers use --mac-app-image-sign-identity and/or\n\ +\ --mac-installer-sign-identity. This option cannot be combined with\n\ +\ --mac-app-image-sign-identity or --mac-installer-sign-identity.\n\ +\ --mac-app-image-sign-identity \n\ +\ Identity used to sign application image. This value will be passed\n\ +\ directly to --sign option of "codesign" tool. This option cannot\n\ +\ be combined with --mac-signing-key-user-name.\n\ +\ --mac-installer-sign-identity \n\ +\ Identity used to sign "pkg" installer. This value will be passed\n\ +\ directly to --sign option of "productbuild" tool. This option\n\ +\ cannot be combined with --mac-signing-key-user-name.\n\ \ --mac-app-store\n\ \ Indicates that the jpackage output is intended for the\n\ \ Mac App Store.\n\ diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources.properties b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources.properties index 5297ef92d3414..4c5d51d5bcc65 100644 --- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources.properties +++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources.properties @@ -1,5 +1,5 @@ # -# Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # # This code is free software; you can redistribute it and/or modify it @@ -92,10 +92,11 @@ ERR_NoMainClass=Error: Main application class is missing ERR_UnsupportedOption=Error: Option [{0}] is not valid on this platform ERR_InvalidTypeOption=Error: Option [{0}] is not valid with type [{1}] ERR_NoInstallerEntryPoint=Error: Option [{0}] is not valid without --module or --main-jar entry point option -ERR_MutuallyExclusiveOptions="Error: Mutually exclusive options [{0}] and [{1}] +ERR_MutuallyExclusiveOptions=Error: Mutually exclusive options [{0}] and [{1}] ERR_InvalidOptionWithAppImageSigning=Error: Option [{0}] is not valid when signing application image ERR_MissingArgument=Error: Missing argument: {0} +ERR_MissingRequiredArgument=Error: {0} argument requires at least one of [{1}] argument(s) ERR_MissingAppResources=Error: No application jars found ERR_AppImageNotExist=Error: App image directory "{0}" does not exist ERR_NoAddLauncherName=Error: --add-launcher option requires a name and a file path (--add-launcher =) diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_de.properties b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_de.properties index 732c8a339f9d9..e505262ea28c5 100644 --- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_de.properties +++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_de.properties @@ -1,5 +1,5 @@ # -# Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # # This code is free software; you can redistribute it and/or modify it @@ -95,6 +95,7 @@ ERR_MutuallyExclusiveOptions="Fehler: Optionen [{0}] und [{1}] schließen sich g ERR_InvalidOptionWithAppImageSigning=Fehler: Option [{0}] ist nicht gültig beim Signieren eines Anwendungsimages ERR_MissingArgument=Fehler: Fehlendes Argument: {0} +ERR_MissingRequiredArgument=Error: {0} argument requires at least one of [{1}] argument(s) ERR_MissingAppResources=Fehler: Keine Anwendungs-JAR-Dateien gefunden ERR_AppImageNotExist=Fehler: Anwendungsimageverzeichnis "{0}" ist nicht vorhanden ERR_NoAddLauncherName=Fehler: Für Option --add-launcher müssen ein Name und ein Dateipfad angegeben werden (--add-launcher =) diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_ja.properties b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_ja.properties index 8b4c457e61cb8..6cc9c23793c29 100644 --- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_ja.properties +++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_ja.properties @@ -1,5 +1,5 @@ # -# Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # # This code is free software; you can redistribute it and/or modify it @@ -95,6 +95,7 @@ ERR_MutuallyExclusiveOptions="エラー: 相互排他的なオプション[{0}] ERR_InvalidOptionWithAppImageSigning=エラー: アプリケーション・イメージへの署名時にオプション[{0}]が有効ではありません ERR_MissingArgument=エラー: 引数がありません: {0} +ERR_MissingRequiredArgument=Error: {0} argument requires at least one of [{1}] argument(s) ERR_MissingAppResources=エラー: アプリケーションjarが見つかりませんでした ERR_AppImageNotExist=エラー: アプリケーション・イメージ・ディレクトリ"{0}"は存在しません ERR_NoAddLauncherName=エラー: --add-launcherオプションには名前およびファイル・パスが必要です(--add-launcher =) diff --git a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_zh_CN.properties b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_zh_CN.properties index 4a37c781ba4e1..7eab865568c81 100644 --- a/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_zh_CN.properties +++ b/src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_zh_CN.properties @@ -1,5 +1,5 @@ # -# Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved. # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. # # This code is free software; you can redistribute it and/or modify it @@ -95,6 +95,7 @@ ERR_MutuallyExclusiveOptions="错误：选项 [{0}] 和 [{1}] 相互排斥 ERR_InvalidOptionWithAppImageSigning=错误：对应用程序映像签名时，选项 [{0}] 无效 ERR_MissingArgument=错误: 缺少参数: {0} +ERR_MissingRequiredArgument=Error: {0} argument requires at least one of [{1}] argument(s) ERR_MissingAppResources=错误: 找不到应用程序 jar ERR_AppImageNotExist=错误：应用程序映像目录 "{0}" 不存在 ERR_NoAddLauncherName=错误：--add-launcher 选项需要一个名称和一个文件路径 (--add-launcher =) diff --git a/test/jdk/tools/jpackage/macosx/SigningAppImageTest.java b/test/jdk/tools/jpackage/macosx/SigningAppImageTest.java index 6cea5ebd33250..65793baa09672 100644 --- a/test/jdk/tools/jpackage/macosx/SigningAppImageTest.java +++ b/test/jdk/tools/jpackage/macosx/SigningAppImageTest.java @@ -55,28 +55,40 @@ * @build SigningAppImageTest * @modules jdk.jpackage/jdk.jpackage.internal * @requires (os.family == "mac") - * @run main/othervm -Xmx512m jdk.jpackage.test.Main + * @run main/othervm/timeout=720 -Xmx512m jdk.jpackage.test.Main * --jpt-run=SigningAppImageTest */ public class SigningAppImageTest { @Test - @Parameter({"true", "0"}) // ({"sign or not", "certificate index"}) - @Parameter({"true", "1"}) - @Parameter({"false", "-1"}) + // ({"sign or not", "signing-key or sign-identity", "certificate index"}) + // Sign, signing-key and ASCII certificate + @Parameter({"true", "true", SigningBase.ASCII_INDEX}) + // Sign, signing-key and UNICODE certificate + @Parameter({"true", "true", SigningBase.UNICODE_INDEX}) + // Sign, signing-indentity and UNICODE certificate + @Parameter({"true", "false", SigningBase.UNICODE_INDEX}) + // Unsigned + @Parameter({"false", "true", "-1"}) public void test(String... testArgs) throws Exception { boolean doSign = Boolean.parseBoolean(testArgs[0]); - int certIndex = Integer.parseInt(testArgs[1]); + boolean signingKey = Boolean.parseBoolean(testArgs[1]); + int certIndex = Integer.parseInt(testArgs[2]); SigningCheck.checkCertificates(certIndex); JPackageCommand cmd = JPackageCommand.helloAppImage(); if (doSign) { cmd.addArguments("--mac-sign", - "--mac-signing-key-user-name", - SigningBase.getDevName(certIndex), "--mac-signing-keychain", SigningBase.getKeyChain()); + if (signingKey) { + cmd.addArguments("--mac-signing-key-user-name", + SigningBase.getDevName(certIndex)); + } else { + cmd.addArguments("--mac-app-image-sign-identity", + SigningBase.getAppCert(certIndex)); + } } AdditionalLauncher testAL = new AdditionalLauncher("testAL"); testAL.applyTo(cmd); diff --git a/test/jdk/tools/jpackage/macosx/SigningAppImageTwoStepsTest.java b/test/jdk/tools/jpackage/macosx/SigningAppImageTwoStepsTest.java index b5d1030ab95d9..cf479d9ba945b 100644 --- a/test/jdk/tools/jpackage/macosx/SigningAppImageTwoStepsTest.java +++ b/test/jdk/tools/jpackage/macosx/SigningAppImageTwoStepsTest.java @@ -56,15 +56,23 @@ * @build SigningAppImageTwoStepsTest * @modules jdk.jpackage/jdk.jpackage.internal * @requires (os.family == "mac") - * @run main/othervm/timeout=360 -Xmx512m jdk.jpackage.test.Main + * @run main/othervm/timeout=720 -Xmx512m jdk.jpackage.test.Main * --jpt-run=SigningAppImageTwoStepsTest */ public class SigningAppImageTwoStepsTest { @Test - @Parameter("true") - @Parameter("false") - public void test(boolean signAppImage) throws Exception { + // ({"sign or not", "signing-key or sign-identity"}) + // Sign and signing-key + @Parameter({"true", "true"}) + // Sign and sign-identity + @Parameter({"true", "false"}) + // Unsigned + @Parameter({"false", "true"}) + public void test(String... testArgs) throws Exception { + boolean signAppImage = Boolean.parseBoolean(testArgs[0]); + boolean signingKey = Boolean.parseBoolean(testArgs[1]); + SigningCheck.checkCertificates(SigningBase.DEFAULT_INDEX); Path appimageOutput = TKit.createTempDirectory("appimage"); @@ -76,10 +84,15 @@ public void test(boolean signAppImage) throws Exception { .setArgumentValue("--dest", appimageOutput); if (signAppImage) { appImageCmd.addArguments("--mac-sign", - "--mac-signing-key-user-name", - SigningBase.getDevName(SigningBase.DEFAULT_INDEX), "--mac-signing-keychain", SigningBase.getKeyChain()); + if (signingKey) { + appImageCmd.addArguments("--mac-signing-key-user-name", + SigningBase.getDevName(SigningBase.DEFAULT_INDEX)); + } else { + appImageCmd.addArguments("--mac-app-image-sign-identity", + SigningBase.getAppCert(SigningBase.DEFAULT_INDEX)); + } } // Add addtional launcher @@ -97,9 +110,14 @@ public void test(boolean signAppImage) throws Exception { cmd.setPackageType(PackageType.IMAGE) .addArguments("--app-image", appImageCmd.outputBundle().toAbsolutePath()) .addArguments("--mac-sign") - .addArguments("--mac-signing-key-user-name", - SigningBase.getDevName(SigningBase.DEFAULT_INDEX)) .addArguments("--mac-signing-keychain", SigningBase.getKeyChain()); + if (signingKey) { + cmd.addArguments("--mac-signing-key-user-name", + SigningBase.getDevName(SigningBase.DEFAULT_INDEX)); + } else { + cmd.addArguments("--mac-app-image-sign-identity", + SigningBase.getAppCert(SigningBase.DEFAULT_INDEX)); + } cmd.executeAndAssertImageCreated(); // Should be signed app image diff --git a/test/jdk/tools/jpackage/macosx/SigningOptionsTest.java b/test/jdk/tools/jpackage/macosx/SigningOptionsTest.java new file mode 100644 index 0000000000000..10b2dd6c8d258 --- /dev/null +++ b/test/jdk/tools/jpackage/macosx/SigningOptionsTest.java @@ -0,0 +1,115 @@ +/* + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import java.util.Collection; +import java.util.List; +import jdk.jpackage.test.Annotations.Parameters; +import jdk.jpackage.test.Annotations.Test; +import jdk.jpackage.test.JPackageCommand; +import jdk.jpackage.test.TKit; + +/* + * @test + * @summary Test jpackage signing options errors + * @library ../helpers + * @build SigningOptionsTest + * @modules jdk.jpackage/jdk.jpackage.internal + * @requires (os.family == "mac") + * @run main/othervm/timeout=360 -Xmx512m jdk.jpackage.test.Main + * --jpt-run=SigningOptionsTest + * --jpt-before-run=jdk.jpackage.test.JPackageCommand.useExecutableByDefault + */ + +/* + * @test + * @summary Test jpackage signing options errors + * @library ../helpers + * @build SigningOptionsTest + * @modules jdk.jpackage/jdk.jpackage.internal + * @requires (os.family == "mac") + * @run main/othervm/timeout=360 -Xmx512m jdk.jpackage.test.Main + * --jpt-run=SigningOptionsTest + * --jpt-before-run=jdk.jpackage.test.JPackageCommand.useToolProviderByDefault + */ + +public final class SigningOptionsTest { + + private final String expectedError; + private final JPackageCommand cmd; + + @Parameters + public static Collection input() { + return List.of(new Object[][]{ + // --mac-signing-key-user-name and --mac-app-image-sign-identity + {"Hello", + new String[]{"--mac-sign", + "--mac-signing-key-user-name", "test-key", + "--mac-app-image-sign-identity", "test-identity"}, + null, + "Mutually exclusive options"}, + // --mac-signing-key-user-name and --mac-installer-sign-identity + {"Hello", + new String[]{"--mac-sign", + "--mac-signing-key-user-name", "test-key", + "--mac-installer-sign-identity", "test-identity"}, + null, + "Mutually exclusive options"}, + // --mac-installer-sign-identity and --type app-image + {"Hello", + new String[]{"--mac-sign", + "--mac-installer-sign-identity", "test-identity"}, + null, + "Option [--mac-installer-sign-identity] is not valid with type"}, + // --mac-installer-sign-identity and --type dmg + {"Hello", + new String[]{"--type", "dmg", + "--mac-sign", + "--mac-installer-sign-identity", "test-identity"}, + new String[]{"--type"}, + "Option [--mac-installer-sign-identity] is not valid with type"}, + }); + } + + public SigningOptionsTest(String javaAppDesc, String[] jpackageArgs, + String[] removeArgs, String expectedError) { + this.expectedError = expectedError; + + cmd = JPackageCommand.helloAppImage(javaAppDesc) + .saveConsoleOutput(true).dumpOutput(true); + if (jpackageArgs != null) { + cmd.addArguments(jpackageArgs); + } if (removeArgs != null) { + for (String arg : removeArgs) { + cmd.removeArgumentWithValue(arg); + } + } + } + + @Test + public void test() { + List output = cmd.execute(1).getOutput(); + TKit.assertNotNull(output, "output is null"); + TKit.assertTextStream(expectedError).apply(output.stream()); + } + +} diff --git a/test/jdk/tools/jpackage/macosx/SigningPackageFromTwoStepAppImageTest.java b/test/jdk/tools/jpackage/macosx/SigningPackageFromTwoStepAppImageTest.java index 861bcecfe0934..48f5dca898640 100644 --- a/test/jdk/tools/jpackage/macosx/SigningPackageFromTwoStepAppImageTest.java +++ b/test/jdk/tools/jpackage/macosx/SigningPackageFromTwoStepAppImageTest.java @@ -72,7 +72,7 @@ private static void verifyPKG(JPackageCommand cmd) { } Path outputBundle = cmd.outputBundle(); - SigningBase.verifyPkgutil(outputBundle, SigningBase.DEFAULT_INDEX); + SigningBase.verifyPkgutil(outputBundle, true, SigningBase.DEFAULT_INDEX); SigningBase.verifySpctl(outputBundle, "install", SigningBase.DEFAULT_INDEX); } @@ -97,9 +97,17 @@ private static void verifyAppImageInDMG(JPackageCommand cmd) { } @Test - @Parameter("true") - @Parameter("false") - public static void test(boolean signAppImage) throws Exception { + // ({"sign or not", "signing-key or sign-identity"}) + // Sign and signing-key + @Parameter({"true", "true"}) + // Sign and sign-identity + @Parameter({"true", "false"}) + // Unsigned + @Parameter({"false", "true"}) + public void test(String... testArgs) throws Exception { + boolean signAppImage = Boolean.parseBoolean(testArgs[0]); + boolean signingKey = Boolean.parseBoolean(testArgs[1]); + SigningCheck.checkCertificates(SigningBase.DEFAULT_INDEX); Path appimageOutput = TKit.createTempDirectory("appimage"); @@ -110,9 +118,15 @@ public static void test(boolean signAppImage) throws Exception { JPackageCommand appImageCmd = JPackageCommand.helloAppImage() .setArgumentValue("--dest", appimageOutput); if (signAppImage) { - appImageCmd.addArguments("--mac-sign", "--mac-signing-key-user-name", - SigningBase.getDevName(SigningBase.DEFAULT_INDEX), + appImageCmd.addArguments("--mac-sign", "--mac-signing-keychain", SigningBase.getKeyChain()); + if (signingKey) { + appImageCmd.addArguments("--mac-signing-key-user-name", + SigningBase.getDevName(SigningBase.DEFAULT_INDEX)); + } else { + appImageCmd.addArguments("--mac-app-image-sign-identity", + SigningBase.getAppCert(SigningBase.DEFAULT_INDEX)); + } } // Generate app image @@ -126,9 +140,14 @@ public static void test(boolean signAppImage) throws Exception { appImageSignedCmd.setPackageType(PackageType.IMAGE) .addArguments("--app-image", appImageCmd.outputBundle().toAbsolutePath()) .addArguments("--mac-sign") - .addArguments("--mac-signing-key-user-name", - SigningBase.getDevName(SigningBase.DEFAULT_INDEX)) .addArguments("--mac-signing-keychain", SigningBase.getKeyChain()); + if (signingKey) { + appImageSignedCmd.addArguments("--mac-signing-key-user-name", + SigningBase.getDevName(SigningBase.DEFAULT_INDEX)); + } else { + appImageSignedCmd.addArguments("--mac-app-image-sign-identity", + SigningBase.getAppCert(SigningBase.DEFAULT_INDEX)); + } appImageSignedCmd.executeAndAssertImageCreated(); // Should be signed app image @@ -141,16 +160,30 @@ public static void test(boolean signAppImage) throws Exception { cmd.removeArgumentWithValue("--input"); if (signAppImage) { cmd.addArguments("--mac-sign", - "--mac-signing-key-user-name", - SigningBase.getDevName(SigningBase.DEFAULT_INDEX), "--mac-signing-keychain", SigningBase.getKeyChain()); + if (signingKey) { + cmd.addArguments("--mac-signing-key-user-name", + SigningBase.getDevName(SigningBase.DEFAULT_INDEX)); + } else { + cmd.addArguments("--mac-installer-sign-identity", + SigningBase.getInstallerCert(SigningBase.DEFAULT_INDEX)); + } } }) .forTypes(PackageType.MAC_PKG) .addBundleVerifier( SigningPackageFromTwoStepAppImageTest::verifyPKG) .forTypes(PackageType.MAC_DMG) + .addInitializer(cmd -> { + if (signAppImage && !signingKey) { + // jpackage throws expected error with + // --mac-installer-sign-identity and DMG type + cmd.removeArgument("--mac-sign"); + cmd.removeArgumentWithValue("--mac-signing-keychain"); + cmd.removeArgumentWithValue("--mac-installer-sign-identity"); + } + }) .addBundleVerifier( SigningPackageFromTwoStepAppImageTest::verifyDMG) .addBundleVerifier( diff --git a/test/jdk/tools/jpackage/macosx/SigningPackageTest.java b/test/jdk/tools/jpackage/macosx/SigningPackageTest.java index d7a2395db2adf..c555b0386cde3 100644 --- a/test/jdk/tools/jpackage/macosx/SigningPackageTest.java +++ b/test/jdk/tools/jpackage/macosx/SigningPackageTest.java @@ -59,15 +59,27 @@ * @build SigningPackageTest * @modules jdk.jpackage/jdk.jpackage.internal * @requires (os.family == "mac") - * @run main/othervm/timeout=360 -Xmx512m jdk.jpackage.test.Main + * @run main/othervm/timeout=720 -Xmx512m jdk.jpackage.test.Main * --jpt-run=SigningPackageTest */ public class SigningPackageTest { + private static boolean isAppImageSigned(JPackageCommand cmd) { + return cmd.hasArgument("--mac-signing-key-user-name") || + cmd.hasArgument("--mac-app-image-sign-identity"); + } + + private static boolean isPKGSigned(JPackageCommand cmd) { + return cmd.hasArgument("--mac-signing-key-user-name") || + cmd.hasArgument("--mac-installer-sign-identity"); + } + private static void verifyPKG(JPackageCommand cmd) { Path outputBundle = cmd.outputBundle(); - SigningBase.verifyPkgutil(outputBundle, getCertIndex(cmd)); - SigningBase.verifySpctl(outputBundle, "install", getCertIndex(cmd)); + SigningBase.verifyPkgutil(outputBundle, isPKGSigned(cmd), getCertIndex(cmd)); + if (isPKGSigned(cmd)) { + SigningBase.verifySpctl(outputBundle, "install", getCertIndex(cmd)); + } } private static void verifyDMG(JPackageCommand cmd) { @@ -82,22 +94,45 @@ private static void verifyAppImageInDMG(JPackageCommand cmd) { // We will be called with all folders in DMG since JDK-8263155, but // we only need to verify app. if (dmgImage.endsWith(cmd.name() + ".app")) { - SigningBase.verifyCodesign(launcherPath, true, getCertIndex(cmd)); - SigningBase.verifyCodesign(dmgImage, true, getCertIndex(cmd)); - SigningBase.verifySpctl(dmgImage, "exec", getCertIndex(cmd)); + SigningBase.verifyCodesign(launcherPath, isAppImageSigned(cmd), + getCertIndex(cmd)); + SigningBase.verifyCodesign(dmgImage, isAppImageSigned(cmd), + getCertIndex(cmd)); + if (isAppImageSigned(cmd)) { + SigningBase.verifySpctl(dmgImage, "exec", getCertIndex(cmd)); + } } }); } private static int getCertIndex(JPackageCommand cmd) { - String devName = cmd.getArgumentValue("--mac-signing-key-user-name"); - return SigningBase.getDevNameIndex(devName); + if (cmd.hasArgument("--mac-signing-key-user-name")) { + String devName = cmd.getArgumentValue("--mac-signing-key-user-name"); + return SigningBase.getDevNameIndex(devName); + } else { + // Signing-indentity + return Integer.valueOf(SigningBase.UNICODE_INDEX); + } } @Test - @Parameter("0") - @Parameter("1") - public static void test(int certIndex) throws Exception { + // ("signing-key or sign-identity", "sign app-image", "sign pkg", "certificate index"}) + // Signing-key and ASCII certificate + @Parameter({"true", "true", "true", SigningBase.ASCII_INDEX}) + // Signing-key and UNICODE certificate + @Parameter({"true", "true", "true", SigningBase.UNICODE_INDEX}) + // Signing-indentity and UNICODE certificate + @Parameter({"false", "true", "true", SigningBase.UNICODE_INDEX}) + // Signing-indentity, but sign app-image only and UNICODE certificate + @Parameter({"false", "true", "false", SigningBase.UNICODE_INDEX}) + // Signing-indentity, but sign pkg only and UNICODE certificate + @Parameter({"false", "false", "true", SigningBase.UNICODE_INDEX}) + public static void test(String... testArgs) throws Exception { + boolean signingKey = Boolean.parseBoolean(testArgs[0]); + boolean signAppImage = Boolean.parseBoolean(testArgs[1]); + boolean signPKG = Boolean.parseBoolean(testArgs[2]); + int certIndex = Integer.parseInt(testArgs[3]); + SigningCheck.checkCertificates(certIndex); new PackageTest() @@ -105,12 +140,39 @@ public static void test(int certIndex) throws Exception { .forTypes(PackageType.MAC) .addInitializer(cmd -> { cmd.addArguments("--mac-sign", - "--mac-signing-key-user-name", SigningBase.getDevName(certIndex), "--mac-signing-keychain", SigningBase.getKeyChain()); + if (signingKey) { + cmd.addArguments("--mac-signing-key-user-name", + SigningBase.getDevName(certIndex)); + } else { + if (signAppImage) { + cmd.addArguments("--mac-app-image-sign-identity", + SigningBase.getAppCert(certIndex)); + } + if (signPKG) { + cmd.addArguments("--mac-installer-sign-identity", + SigningBase.getInstallerCert(certIndex)); + } + } }) .forTypes(PackageType.MAC_PKG) .addBundleVerifier(SigningPackageTest::verifyPKG) .forTypes(PackageType.MAC_DMG) + .addInitializer(cmd -> { + if (!signingKey) { + // jpackage throws expected error with + // --mac-installer-sign-identity and DMG type + cmd.removeArgumentWithValue("--mac-installer-sign-identity"); + // In case of not signing app image and DMG we need to + // remove signing completely, otherwise we will default + // to --mac-signing-key-user-name once + // --mac-installer-sign-identity is removed. + if (!signAppImage) { + cmd.removeArgumentWithValue("--mac-signing-keychain"); + cmd.removeArgument("--mac-sign"); + } + } + }) .addBundleVerifier(SigningPackageTest::verifyDMG) .addBundleVerifier(SigningPackageTest::verifyAppImageInDMG) .run(); diff --git a/test/jdk/tools/jpackage/macosx/SigningPackageTwoStepTest.java b/test/jdk/tools/jpackage/macosx/SigningPackageTwoStepTest.java index 66145a9793bb6..a34f7921b9126 100644 --- a/test/jdk/tools/jpackage/macosx/SigningPackageTwoStepTest.java +++ b/test/jdk/tools/jpackage/macosx/SigningPackageTwoStepTest.java @@ -73,7 +73,7 @@ private static void verifyPKG(JPackageCommand cmd) { } Path outputBundle = cmd.outputBundle(); - SigningBase.verifyPkgutil(outputBundle, SigningBase.DEFAULT_INDEX); + SigningBase.verifyPkgutil(outputBundle, true, SigningBase.DEFAULT_INDEX); SigningBase.verifySpctl(outputBundle, "install", SigningBase.DEFAULT_INDEX); } @@ -101,10 +101,18 @@ private static void verifyAppImageInDMG(JPackageCommand cmd) { } @Test - @Parameter("true") - @Parameter("false") - public static void test(boolean signAppImage) throws Exception { - SigningCheck.checkCertificates(0); + // (Signed, "signing-key or sign-identity"}) + // Signed and signing-key + @Parameter({"true", "true"}) + // Signed and signing-identity + @Parameter({"true", "false"}) + // Unsigned + @Parameter({"false", "true"}) + public static void test(String... testArgs) throws Exception { + boolean signAppImage = Boolean.parseBoolean(testArgs[0]); + boolean signingKey = Boolean.parseBoolean(testArgs[1]); + + SigningCheck.checkCertificates(SigningBase.DEFAULT_INDEX); Path appimageOutput = TKit.createTempDirectory("appimage"); @@ -112,10 +120,15 @@ public static void test(boolean signAppImage) throws Exception { .setArgumentValue("--dest", appimageOutput); if (signAppImage) { appImageCmd.addArguments("--mac-sign") - .addArguments("--mac-signing-key-user-name", - SigningBase.getDevName(0)) - .addArguments("--mac-signing-keychain", - SigningBase.getKeyChain()); + .addArguments("--mac-signing-keychain", + SigningBase.getKeyChain()); + if (signingKey) { + appImageCmd.addArguments("--mac-signing-key-user-name", + SigningBase.getDevName(SigningBase.DEFAULT_INDEX)); + } else { + appImageCmd.addArguments("--mac-app-image-sign-identity", + SigningBase.getAppCert(SigningBase.DEFAULT_INDEX)); + } } new PackageTest() @@ -126,15 +139,31 @@ public static void test(boolean signAppImage) throws Exception { cmd.removeArgumentWithValue("--input"); if (signAppImage) { cmd.addArguments("--mac-sign", - "--mac-signing-key-user-name", - SigningBase.getDevName(0), "--mac-signing-keychain", SigningBase.getKeyChain()); + if (signingKey) { + cmd.addArguments("--mac-signing-key-user-name", + SigningBase.getDevName(SigningBase.DEFAULT_INDEX)); + } else { + cmd.addArguments("--mac-installer-sign-identity", + SigningBase.getInstallerCert(SigningBase.DEFAULT_INDEX)); + } } }) .forTypes(PackageType.MAC_PKG) .addBundleVerifier(SigningPackageTwoStepTest::verifyPKG) .forTypes(PackageType.MAC_DMG) + .addInitializer(cmd -> { + if (signAppImage && !signingKey) { + // jpackage throws expected error with + // --mac-installer-sign-identity and DMG type + cmd.removeArgumentWithValue("--mac-installer-sign-identity"); + // It will do nothing, but it signals test that app + // image itself is signed for verification. + cmd.addArguments("--mac-app-image-sign-identity", + SigningBase.getAppCert(SigningBase.DEFAULT_INDEX)); + } + }) .addBundleVerifier(SigningPackageTwoStepTest::verifyDMG) .addBundleVerifier(SigningPackageTwoStepTest::verifyAppImageInDMG) .run(); diff --git a/test/jdk/tools/jpackage/macosx/base/SigningBase.java b/test/jdk/tools/jpackage/macosx/base/SigningBase.java index 702e7b1f7ae85..15031bc42be57 100644 --- a/test/jdk/tools/jpackage/macosx/base/SigningBase.java +++ b/test/jdk/tools/jpackage/macosx/base/SigningBase.java @@ -33,6 +33,8 @@ public class SigningBase { public static int DEFAULT_INDEX = 0; + public static final String ASCII_INDEX = "0"; + public static final String UNICODE_INDEX = "0"; private static String [] DEV_NAMES = { "jpackage.openjdk.java.net", "jpackage.openjdk.java.net (ö)", @@ -182,22 +184,30 @@ private static void verifySpctlResult(List output, Path target, checkString(output, lookupString); } - private static List pkgutilResult(Path target) { + private static List pkgutilResult(Path target, boolean signed) { List result = new Executor() .setExecutable("/usr/sbin/pkgutil") .addArguments("--check-signature", target.toString()) - .executeAndGetOutput(); + .saveOutput() + .execute(signed ? 0 : 1) + .getOutput(); return result; } - private static void verifyPkgutilResult(List result, int certIndex) { + private static void verifyPkgutilResult(List result, boolean signed, + int certIndex) { result.stream().forEachOrdered(TKit::trace); - String lookupString = "Status: signed by"; - checkString(result, lookupString); - lookupString = "1. " + getInstallerCert(certIndex); - checkString(result, lookupString); + if (signed) { + String lookupString = "Status: signed by"; + checkString(result, lookupString); + lookupString = "1. " + getInstallerCert(certIndex); + checkString(result, lookupString); + } else { + String lookupString = "Status: no signature"; + checkString(result, lookupString); + } } public static void verifyCodesign(Path target, boolean signed, int certIndex) { @@ -228,9 +238,9 @@ public static void verifySpctl(Path target, String type, int certIndex) { verifySpctlResult(output, target, type, result.getExitCode(), certIndex); } - public static void verifyPkgutil(Path target, int certIndex) { - List result = pkgutilResult(target); - verifyPkgutilResult(result, certIndex); + public static void verifyPkgutil(Path target, boolean signed, int certIndex) { + List result = pkgutilResult(target, signed); + verifyPkgutilResult(result, signed, certIndex); } public static void verifyAppImageSignature(JPackageCommand appImageCmd, @@ -247,7 +257,7 @@ public static void verifyAppImageSignature(JPackageCommand appImageCmd, Path appImage = appImageCmd.outputBundle(); SigningBase.verifyCodesign(appImage, isSigned, SigningBase.DEFAULT_INDEX); if (isSigned) { - SigningBase.verifySpctl(appImage, "exec", 0); + SigningBase.verifySpctl(appImage, "exec", SigningBase.DEFAULT_INDEX); } } diff --git a/test/jdk/tools/jpackage/share/jdk/jpackage/tests/PredefinedAppImageErrorTest.java b/test/jdk/tools/jpackage/share/jdk/jpackage/tests/PredefinedAppImageErrorTest.java index 8fb3e60244971..d03c7cf28df96 100644 --- a/test/jdk/tools/jpackage/share/jdk/jpackage/tests/PredefinedAppImageErrorTest.java +++ b/test/jdk/tools/jpackage/share/jdk/jpackage/tests/PredefinedAppImageErrorTest.java @@ -69,7 +69,7 @@ public static Collection input() throws IOException { }, // --mac-app-store is required {"Hello", - new String[]{"--mac-sign", "--mac-app-store"}, + new String[]{"--mac-sign", "--mac-app-store", "--mac-app-image-sign-identity", "test"}, new String[]{"--input", "--dest", "--name", "--main-jar", "--main-class"}, TKit.isOSX() ? "Option [--mac-app-store] is not valid" : From 5ce718eb175dd0855983577d41b0af57422f4a0e Mon Sep 17 00:00:00 2001 From: Jayathirth D V Date: Wed, 25 Oct 2023 04:01:59 +0000 Subject: [PATCH 09/19] 8318100: Remove redundant check for Metal support Reviewed-by: prr, dnguyen --- .../macosx/classes/sun/java2d/MacOSFlags.java | 11 +---------- .../sun/java2d/metal/MTLGraphicsConfig.java | 14 -------------- .../java2d/metal/MTLGraphicsConfig.m | 16 ---------------- 3 files changed, 1 insertion(+), 40 deletions(-) diff --git a/src/java.desktop/macosx/classes/sun/java2d/MacOSFlags.java b/src/java.desktop/macosx/classes/sun/java2d/MacOSFlags.java index 5e09d39692b9c..37b573a339b25 100644 --- a/src/java.desktop/macosx/classes/sun/java2d/MacOSFlags.java +++ b/src/java.desktop/macosx/classes/sun/java2d/MacOSFlags.java @@ -125,16 +125,7 @@ private static void initJavaFlags() { System.out.println("Could not enable OpenGL pipeline (CGL not available)"); } oglEnabled = false; - metalEnabled = MTLGraphicsConfig.isMetalAvailable(); - } - } else if (metalEnabled && !oglEnabled) { - // Check whether Metal framework is available - if (!MTLGraphicsConfig.isMetalAvailable()) { - if (metalVerbose) { - System.out.println("Could not enable Metal pipeline (Metal framework not available)"); - } - metalEnabled = false; - oglEnabled = CGLGraphicsConfig.isCGLAvailable(); + metalEnabled = true; } } diff --git a/src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java b/src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java index 64117b8999e19..8a992361b9628 100644 --- a/src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java +++ b/src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java @@ -72,7 +72,6 @@ public final class MTLGraphicsConfig extends CGraphicsConfig implements AccelGraphicsConfig, SurfaceManager.ProxiedGraphicsConfig { - private static boolean mtlAvailable; private static ImageCapabilities imageCaps = new MTLImageCaps(); @SuppressWarnings("removal") @@ -89,7 +88,6 @@ public final class MTLGraphicsConfig extends CGraphicsConfig private final Object disposerReferent = new Object(); private final int maxTextureSize; - private static native boolean isMetalFrameworkAvailable(); private static native boolean tryLoadMetalLibrary(int displayID, String shaderLib); private static native long getMTLConfigInfo(int displayID, String mtlShadersLib); @@ -99,10 +97,6 @@ public final class MTLGraphicsConfig extends CGraphicsConfig */ private static native int nativeGetMaxTextureSize(); - static { - mtlAvailable = isMetalFrameworkAvailable(); - } - private MTLGraphicsConfig(CGraphicsDevice device, long configInfo, int maxTextureSize, ContextCapabilities mtlCaps) { @@ -133,10 +127,6 @@ public SurfaceData createManagedSurface(int w, int h, int transparency) { public static MTLGraphicsConfig getConfig(CGraphicsDevice device, int displayID) { - if (!mtlAvailable) { - return null; - } - if (!tryLoadMetalLibrary(displayID, mtlShadersLib)) { return null; } @@ -171,10 +161,6 @@ public static MTLGraphicsConfig getConfig(CGraphicsDevice device, return new MTLGraphicsConfig(device, cfginfo, textureSize, caps); } - public static boolean isMetalAvailable() { - return mtlAvailable; - } - /** * Returns true if the provided capability bit is present for this config. * See MTLContext.java for a list of supported capabilities. diff --git a/src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLGraphicsConfig.m b/src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLGraphicsConfig.m index af6662cecf2ab..7b165a668b3a2 100644 --- a/src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLGraphicsConfig.m +++ b/src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLGraphicsConfig.m @@ -54,22 +54,6 @@ free(mtlinfo); } -JNIEXPORT jboolean JNICALL -Java_sun_java2d_metal_MTLGraphicsConfig_isMetalFrameworkAvailable - (JNIEnv *env, jclass mtlgc) -{ - jboolean metalSupported = JNI_FALSE; - - // It is guaranteed that metal supported GPU is available since macOS 10.14 - if (@available(macOS 10.14, *)) { - metalSupported = JNI_TRUE; - } - - J2dRlsTraceLn1(J2D_TRACE_INFO, "MTLGraphicsConfig_isMetalFrameworkAvailable : %d", metalSupported); - - return metalSupported; -} - JNIEXPORT jboolean JNICALL Java_sun_java2d_metal_MTLGraphicsConfig_tryLoadMetalLibrary (JNIEnv *env, jclass mtlgc, jint displayID, jstring shadersLibName) From d7205e690fe92464caee9122e11a88b4cc5c2c2d Mon Sep 17 00:00:00 2001 From: Prasanta Sadhukhan Date: Wed, 25 Oct 2023 07:22:04 +0000 Subject: [PATCH 10/19] 8318102: macos10.14 check in CSystemColors can be removed. Reviewed-by: prr, aivanov --- .../macosx/native/libawt_lwawt/awt/CSystemColors.m | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/java.desktop/macosx/native/libawt_lwawt/awt/CSystemColors.m b/src/java.desktop/macosx/native/libawt_lwawt/awt/CSystemColors.m index 40ef6e67d0173..1bd767c320b52 100644 --- a/src/java.desktop/macosx/native/libawt_lwawt/awt/CSystemColors.m +++ b/src/java.desktop/macosx/native/libawt_lwawt/awt/CSystemColors.m @@ -96,11 +96,7 @@ + (void)reloadColors { sColors[java_awt_SystemColor_TEXT_INACTIVE_TEXT] = [NSColor disabledControlTextColor]; sColors[java_awt_SystemColor_CONTROL] = [NSColor controlColor]; sColors[java_awt_SystemColor_CONTROL_TEXT] = [NSColor controlTextColor]; - if (@available(macOS 10.14, *)) { - sColors[java_awt_SystemColor_CONTROL_HIGHLIGHT] = [NSColor selectedContentBackgroundColor]; - } else { - sColors[java_awt_SystemColor_CONTROL_HIGHLIGHT] = [NSColor alternateSelectedControlColor]; - } + sColors[java_awt_SystemColor_CONTROL_HIGHLIGHT] = [NSColor selectedContentBackgroundColor]; sColors[java_awt_SystemColor_CONTROL_LT_HIGHLIGHT] = [NSColor alternateSelectedControlTextColor]; sColors[java_awt_SystemColor_CONTROL_SHADOW] = [NSColor controlShadowColor]; sColors[java_awt_SystemColor_CONTROL_DK_SHADOW] = [NSColor controlDarkShadowColor]; @@ -121,11 +117,7 @@ + (void)reloadColors { } // added for JTable Focus Ring - if (@available(macOS 10.14, *)) { - appleColors[sun_lwawt_macosx_LWCToolkit_CELL_HIGHLIGHT_COLOR] = [NSColor controlAccentColor]; - } else { - appleColors[sun_lwawt_macosx_LWCToolkit_CELL_HIGHLIGHT_COLOR] = [NSColor keyboardFocusIndicatorColor]; - } + appleColors[sun_lwawt_macosx_LWCToolkit_CELL_HIGHLIGHT_COLOR] = [NSColor controlAccentColor]; appleColors[sun_lwawt_macosx_LWCToolkit_KEYBOARD_FOCUS_COLOR] = [NSColor keyboardFocusIndicatorColor]; appleColors[sun_lwawt_macosx_LWCToolkit_INACTIVE_SELECTION_BACKGROUND_COLOR] = [NSColor secondarySelectedControlColor]; appleColors[sun_lwawt_macosx_LWCToolkit_INACTIVE_SELECTION_FOREGROUND_COLOR] = [NSColor controlDarkShadowColor]; From ba7d08b8199172058bd369d880d2d6a9f9649319 Mon Sep 17 00:00:00 2001 From: Aleksey Shipilev Date: Wed, 25 Oct 2023 08:29:58 +0000 Subject: [PATCH 11/19] 8316961: Fallback implementations for 64-bit Atomic::{add,xchg} on 32-bit platforms Reviewed-by: eosterlund, dholmes, kbarrett, simonis --- src/hotspot/os_cpu/bsd_x86/atomic_bsd_x86.hpp | 8 +++ .../os_cpu/linux_arm/atomic_linux_arm.hpp | 7 ++ .../os_cpu/linux_x86/atomic_linux_x86.hpp | 8 +++ src/hotspot/share/runtime/atomic.hpp | 65 ++++++++++++++++++- test/hotspot/gtest/runtime/test_atomic.cpp | 23 ++++--- 5 files changed, 100 insertions(+), 11 deletions(-) diff --git a/src/hotspot/os_cpu/bsd_x86/atomic_bsd_x86.hpp b/src/hotspot/os_cpu/bsd_x86/atomic_bsd_x86.hpp index 77104194b0b78..9ba246f553d88 100644 --- a/src/hotspot/os_cpu/bsd_x86/atomic_bsd_x86.hpp +++ b/src/hotspot/os_cpu/bsd_x86/atomic_bsd_x86.hpp @@ -153,6 +153,14 @@ inline T Atomic::PlatformCmpxchg<8>::operator()(T volatile* dest, return cmpxchg_using_helper(_Atomic_cmpxchg_long, dest, compare_value, exchange_value); } +// No direct support for 8-byte xchg; emulate using cmpxchg. +template<> +struct Atomic::PlatformXchg<8> : Atomic::XchgUsingCmpxchg<8> {}; + +// No direct support for 8-byte add; emulate using cmpxchg. +template<> +struct Atomic::PlatformAdd<8> : Atomic::AddUsingCmpxchg<8> {}; + template<> template inline T Atomic::PlatformLoad<8>::operator()(T const volatile* src) const { diff --git a/src/hotspot/os_cpu/linux_arm/atomic_linux_arm.hpp b/src/hotspot/os_cpu/linux_arm/atomic_linux_arm.hpp index 814dbd9aab501..513217649e633 100644 --- a/src/hotspot/os_cpu/linux_arm/atomic_linux_arm.hpp +++ b/src/hotspot/os_cpu/linux_arm/atomic_linux_arm.hpp @@ -128,6 +128,13 @@ inline T Atomic::PlatformXchg<4>::operator()(T volatile* dest, return xchg_using_helper(ARMAtomicFuncs::_xchg_func, dest, exchange_value); } +// No direct support for 8-byte xchg; emulate using cmpxchg. +template<> +struct Atomic::PlatformXchg<8> : Atomic::XchgUsingCmpxchg<8> {}; + +// No direct support for 8-byte add; emulate using cmpxchg. +template<> +struct Atomic::PlatformAdd<8> : Atomic::AddUsingCmpxchg<8> {}; // The memory_order parameter is ignored - we always provide the strongest/most-conservative ordering diff --git a/src/hotspot/os_cpu/linux_x86/atomic_linux_x86.hpp b/src/hotspot/os_cpu/linux_x86/atomic_linux_x86.hpp index 2e472a020683a..0156546ba9b77 100644 --- a/src/hotspot/os_cpu/linux_x86/atomic_linux_x86.hpp +++ b/src/hotspot/os_cpu/linux_x86/atomic_linux_x86.hpp @@ -153,6 +153,14 @@ inline T Atomic::PlatformCmpxchg<8>::operator()(T volatile* dest, return cmpxchg_using_helper(_Atomic_cmpxchg_long, dest, compare_value, exchange_value); } +// No direct support for 8-byte xchg; emulate using cmpxchg. +template<> +struct Atomic::PlatformXchg<8> : Atomic::XchgUsingCmpxchg<8> {}; + +// No direct support for 8-byte add; emulate using cmpxchg. +template<> +struct Atomic::PlatformAdd<8> : Atomic::AddUsingCmpxchg<8> {}; + template<> template inline T Atomic::PlatformLoad<8>::operator()(T const volatile* src) const { diff --git a/src/hotspot/share/runtime/atomic.hpp b/src/hotspot/share/runtime/atomic.hpp index c85bf9055ab39..ac0ce49d26e56 100644 --- a/src/hotspot/share/runtime/atomic.hpp +++ b/src/hotspot/share/runtime/atomic.hpp @@ -398,11 +398,15 @@ class Atomic : AllStatic { T compare_value, T exchange_value); - // Support platforms that do not provide Read-Modify-Write - // byte-level atomic access. To use, derive PlatformCmpxchg<1> from - // this class. + // Support platforms that do not provide Read-Modify-Write atomic + // accesses for 1-byte and 8-byte widths. To use, derive PlatformCmpxchg<1>, + // PlatformAdd, PlatformXchg from these classes. public: // Temporary, can't be private: C++03 11.4/2. Fixed by C++11. struct CmpxchgByteUsingInt; + template + struct XchgUsingCmpxchg; + template + class AddUsingCmpxchg; private: // Dispatch handler for xchg. Provides type-based validity @@ -677,6 +681,47 @@ struct Atomic::CmpxchgByteUsingInt { atomic_memory_order order) const; }; +// Define the class before including platform file, which may use this +// as a base class, requiring it be complete. The definition is later +// in this file, near the other definitions related to xchg. +template +struct Atomic::XchgUsingCmpxchg { + template + T operator()(T volatile* dest, + T exchange_value, + atomic_memory_order order) const; +}; + +// Define the class before including platform file, which may use this +// as a base class, requiring it be complete. +template +class Atomic::AddUsingCmpxchg { +public: + template + static inline D add_then_fetch(D volatile* dest, + I add_value, + atomic_memory_order order) { + D addend = add_value; + return fetch_then_add(dest, add_value, order) + add_value; + } + + template + static inline D fetch_then_add(D volatile* dest, + I add_value, + atomic_memory_order order) { + STATIC_ASSERT(byte_size == sizeof(I)); + STATIC_ASSERT(byte_size == sizeof(D)); + + D old_value; + D new_value; + do { + old_value = Atomic::load(dest); + new_value = old_value + add_value; + } while (old_value != Atomic::cmpxchg(dest, old_value, new_value, order)); + return old_value; + } +}; + // Define the class before including platform file, which may specialize // the operator definition. No generic definition of specializations // of the operator template are provided, nor are there any generic @@ -1170,4 +1215,18 @@ inline D Atomic::xchg(volatile D* dest, T exchange_value, atomic_memory_order or return XchgImpl()(dest, exchange_value, order); } +template +template +inline T Atomic::XchgUsingCmpxchg::operator()(T volatile* dest, + T exchange_value, + atomic_memory_order order) const { + STATIC_ASSERT(byte_size == sizeof(T)); + + T old_value; + do { + old_value = Atomic::load(dest); + } while (old_value != Atomic::cmpxchg(dest, old_value, exchange_value, order)); + return old_value; +} + #endif // SHARE_RUNTIME_ATOMIC_HPP diff --git a/test/hotspot/gtest/runtime/test_atomic.cpp b/test/hotspot/gtest/runtime/test_atomic.cpp index e7c6f9e3f2b59..744714c6f7fa5 100644 --- a/test/hotspot/gtest/runtime/test_atomic.cpp +++ b/test/hotspot/gtest/runtime/test_atomic.cpp @@ -59,14 +59,14 @@ TEST_VM(AtomicAddTest, int32) { Support().test_fetch_add(); } -// 64bit Atomic::add is only supported on 64bit platforms. -#ifdef _LP64 TEST_VM(AtomicAddTest, int64) { + // Check if 64-bit atomics are available on the machine. + if (!VM_Version::supports_cx8()) return; + using Support = AtomicAddTestSupport; Support().test_add(); Support().test_fetch_add(); } -#endif // _LP64 TEST_VM(AtomicAddTest, ptr) { uint _test_values[10] = {}; @@ -108,13 +108,13 @@ TEST_VM(AtomicXchgTest, int32) { Support().test(); } -// 64bit Atomic::xchg is only supported on 64bit platforms. -#ifdef _LP64 TEST_VM(AtomicXchgTest, int64) { + // Check if 64-bit atomics are available on the machine. + if (!VM_Version::supports_cx8()) return; + using Support = AtomicXchgTestSupport; Support().test(); } -#endif // _LP64 template struct AtomicCmpxchgTestSupport { @@ -142,6 +142,9 @@ TEST_VM(AtomicCmpxchgTest, int32) { } TEST_VM(AtomicCmpxchgTest, int64) { + // Check if 64-bit atomics are available on the machine. + if (!VM_Version::supports_cx8()) return; + using Support = AtomicCmpxchgTestSupport; Support().test(); } @@ -345,12 +348,16 @@ TEST_VM(AtomicBitopsTest, uint32) { AtomicBitopsTestSupport()(); } -#ifdef _LP64 TEST_VM(AtomicBitopsTest, int64) { + // Check if 64-bit atomics are available on the machine. + if (!VM_Version::supports_cx8()) return; + AtomicBitopsTestSupport()(); } TEST_VM(AtomicBitopsTest, uint64) { + // Check if 64-bit atomics are available on the machine. + if (!VM_Version::supports_cx8()) return; + AtomicBitopsTestSupport()(); } -#endif // _LP64 From c3cdfe2a328c59213b614a2b723184582550f8c7 Mon Sep 17 00:00:00 2001 From: Zixian Cai Date: Wed, 25 Oct 2023 08:33:10 +0000 Subject: [PATCH 12/19] 8318692: Add instructions for creating Ubuntu-based sysroot for cross compilation Reviewed-by: erikj, shade --- doc/building.html | 11 +++++++++++ doc/building.md | 15 +++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/doc/building.html b/doc/building.html index 2348c398a534b..3601fa8af16c4 100644 --- a/doc/building.html +++ b/doc/building.html @@ -1413,6 +1413,17 @@

    Cross compiling with http://httpredir.debian.org/debian/ # If the target architecture is `riscv64`, # the path should be `debian-ports` instead of `debian`. +

  • To create a Ubuntu-based chroot:

    +
    sudo debootstrap \
+  --arch=arm64 \
+  --verbose \
+  --components=main,universe \
+  --include=fakeroot,symlinks,build-essential,libx11-dev,libxext-dev,libxrender-dev,libxrandr-dev,libxtst-dev,libxt-dev,libcups2-dev,libfontconfig1-dev,libasound2-dev,libfreetype6-dev,libpng-dev,libffi-dev \
+  --resolve-deps \
+  jammy \
+  ~/sysroot-arm64 \
+  http://ports.ubuntu.com/ubuntu-ports/
+# symlinks is in the universe repository

  • Make sure the symlinks inside the newly created chroot point to proper locations:

    sudo chroot ~/sysroot-arm64 symlinks -cr .
    • diff --git a/doc/building.md b/doc/building.md index ac79cb314b6bb..68c65bf0517a4 100644 --- a/doc/building.md +++ b/doc/building.md @@ -1197,6 +1197,21 @@ For example, cross-compiling to AArch64 from x86_64 could be done like this: # the path should be `debian-ports` instead of `debian`. ``` + * To create a Ubuntu-based chroot: + + ``` + sudo debootstrap \ + --arch=arm64 \ + --verbose \ + --components=main,universe \ + --include=fakeroot,symlinks,build-essential,libx11-dev,libxext-dev,libxrender-dev,libxrandr-dev,libxtst-dev,libxt-dev,libcups2-dev,libfontconfig1-dev,libasound2-dev,libfreetype6-dev,libpng-dev,libffi-dev \ + --resolve-deps \ + jammy \ + ~/sysroot-arm64 \ + http://ports.ubuntu.com/ubuntu-ports/ + # symlinks is in the universe repository + ``` + * Make sure the symlinks inside the newly created chroot point to proper locations: ``` From d2d1592dd94e897fae6fc4098e43b4fffb6d6750 Mon Sep 17 00:00:00 2001 From: Albert Mingkun Yang Date: Wed, 25 Oct 2023 08:43:54 +0000 Subject: [PATCH 13/19] 8318713: G1: Use more accurate age in predict_eden_copy_time_ms Reviewed-by: tschatzl, iwalulya --- src/hotspot/share/gc/g1/g1Policy.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/gc/g1/g1Policy.cpp b/src/hotspot/share/gc/g1/g1Policy.cpp index 13736b891e208..1bc1d44691bbb 100644 --- a/src/hotspot/share/gc/g1/g1Policy.cpp +++ b/src/hotspot/share/gc/g1/g1Policy.cpp @@ -1099,7 +1099,7 @@ double G1Policy::predict_eden_copy_time_ms(uint count, size_t* bytes_to_copy) co if (count == 0) { return 0.0; } - size_t const expected_bytes = _eden_surv_rate_group->accum_surv_rate_pred(count) * HeapRegion::GrainBytes; + size_t const expected_bytes = _eden_surv_rate_group->accum_surv_rate_pred(count - 1) * HeapRegion::GrainBytes; if (bytes_to_copy != nullptr) { *bytes_to_copy = expected_bytes; } From 14090ef6039ff2f3064f397a75219b2bc715cc27 Mon Sep 17 00:00:00 2001 From: Alexey Ivanov Date: Wed, 25 Oct 2023 11:17:00 +0000 Subject: [PATCH 14/19] 8294158: HTML formatting for PassFailJFrame instructions Reviewed-by: azvegint, prr --- .../awt/regtesthelpers/PassFailJFrame.java | 45 ++++++++++++++++--- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/test/jdk/java/awt/regtesthelpers/PassFailJFrame.java b/test/jdk/java/awt/regtesthelpers/PassFailJFrame.java index 92ff937193341..4d8f3c6659c22 100644 --- a/test/jdk/java/awt/regtesthelpers/PassFailJFrame.java +++ b/test/jdk/java/awt/regtesthelpers/PassFailJFrame.java @@ -51,6 +51,7 @@ import javax.swing.JComboBox; import javax.swing.JComponent; import javax.swing.JDialog; +import javax.swing.JEditorPane; import javax.swing.JFrame; import javax.swing.JLabel; import javax.swing.JOptionPane; @@ -58,6 +59,9 @@ import javax.swing.JScrollPane; import javax.swing.JTextArea; import javax.swing.Timer; +import javax.swing.text.JTextComponent; +import javax.swing.text.html.HTMLEditorKit; +import javax.swing.text.html.StyleSheet; import static javax.swing.SwingUtilities.invokeAndWait; import static javax.swing.SwingUtilities.isEventDispatchThread; @@ -81,8 +85,11 @@ public class PassFailJFrame { private static volatile boolean failed; private static volatile boolean timeout; private static volatile String testFailedReason; + private static final AtomicInteger imgCounter = new AtomicInteger(0); + private static JFrame frame; + private static Robot robot; public enum Position {HORIZONTAL, VERTICAL, TOP_LEFT_CORNER} @@ -169,7 +176,7 @@ public PassFailJFrame(String title, String instructions, long testTimeOut, InvocationTargetException { if (isEventDispatchThread()) { createUI(title, instructions, testTimeOut, rows, columns, - enableScreenCapture); + enableScreenCapture); } else { invokeAndWait(() -> createUI(title, instructions, testTimeOut, rows, columns, enableScreenCapture)); @@ -187,9 +194,11 @@ private static void createUI(String title, String instructions, boolean enableScreenCapture) { frame = new JFrame(title); frame.setLayout(new BorderLayout()); - JTextArea instructionsText = new JTextArea(instructions, rows, columns); - instructionsText.setEditable(false); - instructionsText.setLineWrap(true); + + JTextComponent text = instructions.startsWith("") + ? configureHTML(instructions, rows, columns) + : configurePlainText(instructions, rows, columns); + text.setEditable(false); long tTimeout = TimeUnit.MINUTES.toMillis(testTimeOut); @@ -210,7 +219,7 @@ private static void createUI(String title, String instructions, }); timer.start(); frame.add(testTimeoutLabel, BorderLayout.NORTH); - frame.add(new JScrollPane(instructionsText), BorderLayout.CENTER); + frame.add(new JScrollPane(text), BorderLayout.CENTER); JButton btnPass = new JButton("Pass"); btnPass.addActionListener((e) -> { @@ -249,6 +258,32 @@ public void windowClosing(WindowEvent e) { windowList.add(frame); } + private static JTextComponent configurePlainText(String instructions, + int rows, int columns) { + JTextArea text = new JTextArea(instructions, rows, columns); + text.setLineWrap(true); + text.setWrapStyleWord(true); + return text; + } + + private static JTextComponent configureHTML(String instructions, + int rows, int columns) { + JEditorPane text = new JEditorPane("text/html", instructions); + text.putClientProperty(JEditorPane.HONOR_DISPLAY_PROPERTIES, + Boolean.TRUE); + // Set preferred size as if it were JTextArea + text.setPreferredSize(new JTextArea(rows, columns).getPreferredSize()); + + HTMLEditorKit kit = (HTMLEditorKit) text.getEditorKit(); + StyleSheet styles = kit.getStyleSheet(); + // Reduce the default margins + styles.addRule("ol, ul { margin-left-ltr: 20; margin-left-rtl: 20 }"); + // Make the size of code blocks the same as other text + styles.addRule("code { font-size: inherit }"); + + return text; + } + private static JComponent createCapturePanel() { JComboBox screenShortType = new JComboBox<>(CaptureType.values()); From 42b9ac8a07b540f4d7955a778923d24a876451cc Mon Sep 17 00:00:00 2001 From: Alexey Ivanov Date: Wed, 25 Oct 2023 11:31:44 +0000 Subject: [PATCH 15/19] 8294156: Allow PassFailJFrame.Builder to create test UI Reviewed-by: azvegint, prr --- .../awt/regtesthelpers/PassFailJFrame.java | 481 +++++++++++++++--- 1 file changed, 404 insertions(+), 77 deletions(-) diff --git a/test/jdk/java/awt/regtesthelpers/PassFailJFrame.java b/test/jdk/java/awt/regtesthelpers/PassFailJFrame.java index 4d8f3c6659c22..d2e650b213427 100644 --- a/test/jdk/java/awt/regtesthelpers/PassFailJFrame.java +++ b/test/jdk/java/awt/regtesthelpers/PassFailJFrame.java @@ -29,19 +29,23 @@ import java.awt.GraphicsEnvironment; import java.awt.Image; import java.awt.Insets; +import java.awt.Point; import java.awt.Rectangle; import java.awt.Robot; import java.awt.Toolkit; import java.awt.Window; import java.awt.event.WindowAdapter; import java.awt.event.WindowEvent; +import java.awt.event.WindowListener; import java.awt.image.RenderedImage; import java.io.File; import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.List; +import java.util.Objects; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -63,9 +67,83 @@ import javax.swing.text.html.HTMLEditorKit; import javax.swing.text.html.StyleSheet; +import static java.util.Collections.unmodifiableList; import static javax.swing.SwingUtilities.invokeAndWait; import static javax.swing.SwingUtilities.isEventDispatchThread; +/** + * Provides a framework for manual tests to display test instructions and + * Pass/Fail buttons. + *

    + * Instructions for the user can be either plain text or HTML as supported + * by Swing. If the instructions start with {@code }, the + * instructions are displayed as HTML. + *

    + * A simple test would look like this: + * 

    {@code
+ * public class SampleManualTest {
+ *     private static final String INSTRUCTIONS =
+ *             "Click Pass, or click Fail if the test failed.";
+ *
+ *     public static void main(String[] args) throws Exception {
+ *         PassFailJFrame.builder()
+ *                       .instructions(INSTRUCTIONS)
+ *                       .testUI(() -> createTestUI())
+ *                       .build()
+ *                       .awaitAndCheck();
+ *     }
+ *
+ *     private static List createTestUI() {
+ *         JFrame testUI = new JFrame("Test UI");
+ *         testUI.setSize(250, 150);
+ *         return List.of(testUI);
+ *     }
+ * }
+ * }
    + *

    + * The above example uses the {@link Builder Builder} to set the parameters of + * the instruction frame. It is the recommended way. + *

    + * The framework will create instruction UI, it will call + * the provided {@code createTestUI} on the Event Dispatch Thread (EDT), + * and it will automatically position the test UI and make it visible. + *

    + * Alternatively, use one of the {@code PassFailJFrame} constructors to + * create an object, then create secondary test UI, register it + * with {@code PassFailJFrame}, position it and make it visible. + * The following sample demonstrates it: + * 

    {@code
+ * public class SampleOldManualTest {
+ *     private static final String INSTRUCTIONS =
+ *             "Click Pass, or click Fail if the test failed.";
+ *
+ *     public static void main(String[] args) throws Exception {
+ *         PassFailJFrame passFail = new PassFailJFrame(INSTRUCTIONS);
+ *
+ *         SwingUtilities.invokeAndWait(() -> createTestUI());
+ *
+ *         passFail.awaitAndCheck();
+ *     }
+ *
+ *     private static void createTestUI() {
+ *         JFrame testUI = new JFrame("Test UI");
+ *         testUI.setSize(250, 150);
+ *         PassFailJFrame.addTestWindow(testUI);
+ *         PassFailJFrame.positionTestWindow(testUI, PassFailJFrame.Position.HORIZONTAL);
+ *         testUI.setVisible(true);
+ *     }
+ * }
+ * }
    + *

    + * Use methods of the {@code Builder} class or constructors of the + * {@code PassFailJFrame} class to control other parameters: + *

      + *
    • the title of the instruction UI,
      • + *
    • the timeout of the test,
      • + *
    • the size of the instruction UI via rows and columns, and
      • + *
    • to enable screenshots.
      • + *
    + */ public class PassFailJFrame { private static final String TITLE = "Test Instruction Frame"; @@ -172,21 +250,67 @@ public PassFailJFrame(String title, String instructions, long testTimeOut, */ public PassFailJFrame(String title, String instructions, long testTimeOut, int rows, int columns, - boolean enableScreenCapture) throws InterruptedException, - InvocationTargetException { - if (isEventDispatchThread()) { - createUI(title, instructions, testTimeOut, rows, columns, - enableScreenCapture); - } else { - invokeAndWait(() -> createUI(title, instructions, testTimeOut, - rows, columns, enableScreenCapture)); - } + boolean enableScreenCapture) + throws InterruptedException, InvocationTargetException { + invokeOnEDT(() -> createUI(title, instructions, + testTimeOut, + rows, columns, + enableScreenCapture)); } private PassFailJFrame(Builder builder) throws InterruptedException, InvocationTargetException { this(builder.title, builder.instructions, builder.testTimeOut, - builder.rows, builder.columns, builder.screenCapture); + builder.rows, builder.columns, builder.screenCapture); + + if (builder.windowCreator != null) { + invokeOnEDT(() -> + builder.testWindows = builder.windowCreator.createTestUI()); + } + + if (builder.testWindows != null) { + addTestWindow(builder.testWindows); + builder.testWindows + .forEach(w -> w.addWindowListener(windowClosingHandler)); + + if (builder.positionWindows != null) { + positionInstructionFrame(builder.position); + invokeOnEDT(() -> { + builder.positionWindows + .positionTestWindows(unmodifiableList(builder.testWindows), + builder.instructionUIHandler); + + windowList.forEach(w -> w.setVisible(true)); + }); + } else if (builder.testWindows.size() == 1) { + Window window = builder.testWindows.get(0); + positionTestWindow(window, builder.position); + window.setVisible(true); + } else { + positionTestWindow(null, builder.position); + } + } + } + + /** + * Performs an operation on EDT. If called on EDT, invokes {@code run} + * directly, otherwise wraps into {@code invokeAndWait}. + * + * @param doRun an operation to run on EDT + * @throws InterruptedException if we're interrupted while waiting for + * the event dispatching thread to finish executing + * {@code doRun.run()} + * @throws InvocationTargetException if an exception is thrown while + * running {@code doRun} + * @see javax.swing.SwingUtilities#invokeAndWait(Runnable) + */ + private static void invokeOnEDT(Runnable doRun) + throws InterruptedException, InvocationTargetException { + if (isEventDispatchThread()) { + doRun.run(); + } else { + invokeAndWait(doRun); + } } private static void createUI(String title, String instructions, @@ -241,16 +365,7 @@ private static void createUI(String title, String instructions, buttonsPanel.add(createCapturePanel()); } - frame.addWindowListener(new WindowAdapter() { - @Override - public void windowClosing(WindowEvent e) { - super.windowClosing(e); - testFailedReason = FAILURE_REASON - + "User closed the instruction Frame"; - failed = true; - latch.countDown(); - } - }); + frame.addWindowListener(windowClosingHandler); frame.add(buttonsPanel, BorderLayout.SOUTH); frame.pack(); @@ -284,6 +399,101 @@ private static JTextComponent configureHTML(String instructions, return text; } + + /** + * Creates one or more windows for test UI. + */ + @FunctionalInterface + public interface WindowCreator { + /** + * Creates one or more windows for test UI. + * This method is called by the framework on the EDT. + * @return a list of windows. + */ + List createTestUI(); + } + + /** + * Positions test UI windows. + */ + @FunctionalInterface + public interface PositionWindows { + /** + * Positions test UI windows. + * This method is called by the framework on the EDT after + * the instruction UI frame was positioned on the screen. + *

    + * The list of the test windows contains the windows + * that were passed to the framework via + * {@link Builder#testUI(WindowCreator) testUI} method. + * + * @param testWindows the list of test windows + * @param instructionUI information about the instruction frame + */ + void positionTestWindows(List testWindows, + InstructionUI instructionUI); + } + + /** + * Provides information about the instruction frame. + */ + public interface InstructionUI { + /** + * {@return the location of the instruction frame} + */ + Point getLocation(); + + /** + * {@return the size of the instruction frame} + */ + Dimension getSize(); + + /** + * {@return the bounds of the instruction frame} + */ + Rectangle getBounds(); + + /** + * Allows to change the location of the instruction frame. + * + * @param location the new location of the instruction frame + */ + void setLocation(Point location); + + /** + * Allows to change the location of the instruction frame. + * + * @param x the x coordinate of the new location + * @param y the y coordinate of the new location + */ + void setLocation(int x, int y); + + /** + * Returns the specified position that was used to set + * the initial location of the instruction frame. + * + * @return the specified position + * + * @see Position + */ + Position getPosition(); + } + + + private static final class WindowClosingHandler extends WindowAdapter { + @Override + public void windowClosing(WindowEvent e) { + testFailedReason = FAILURE_REASON + + "User closed a window"; + failed = true; + latch.countDown(); + } + } + + private static final WindowListener windowClosingHandler = + new WindowClosingHandler(); + + private static JComponent createCapturePanel() { JComboBox screenShortType = new JComboBox<>(CaptureType.values()); @@ -409,13 +619,11 @@ public void awaitAndCheck() throws InterruptedException, InvocationTargetExcepti } /** - * Dispose all the window(s) i,e both the test instruction frame and - * the window(s) that is added via addTestWindow(Window testWindow) + * Disposes of all the windows. It disposes of the test instruction frame + * and all other windows added via {@link #addTestWindow(Window)}. */ private static synchronized void disposeWindows() { - for (Window win : windowList) { - win.dispose(); - } + windowList.forEach(Window::dispose); } /** @@ -450,6 +658,36 @@ private static void getFailureReason() { latch.countDown(); } + private static void positionInstructionFrame(final Position position) { + Dimension screenSize = Toolkit.getDefaultToolkit().getScreenSize(); + + // Get the screen insets to position the frame by taking into + // account the location of taskbar or menu bar on screen. + GraphicsConfiguration gc = GraphicsEnvironment.getLocalGraphicsEnvironment() + .getDefaultScreenDevice() + .getDefaultConfiguration(); + Insets screenInsets = Toolkit.getDefaultToolkit().getScreenInsets(gc); + + switch (position) { + case HORIZONTAL: + int newX = ((screenSize.width / 2) - frame.getWidth()); + frame.setLocation((newX + screenInsets.left), + (frame.getY() + screenInsets.top)); + break; + + case VERTICAL: + int newY = ((screenSize.height / 2) - frame.getHeight()); + frame.setLocation((frame.getX() + screenInsets.left), + (newY + screenInsets.top)); + break; + + case TOP_LEFT_CORNER: + frame.setLocation(screenInsets.left, screenInsets.top); + break; + } + syncLocationToWindowManager(); + } + /** * Approximately positions the instruction frame relative to the test * window as specified by the {@code position} parameter. If {@code testWindow} @@ -480,40 +718,23 @@ private static void getFailureReason() { * */ public static void positionTestWindow(Window testWindow, Position position) { - Dimension screenSize = Toolkit.getDefaultToolkit().getScreenSize(); - - // Get the screen insets to position the frame by taking into - // account the location of taskbar/menubars on screen. - GraphicsConfiguration gc = GraphicsEnvironment.getLocalGraphicsEnvironment() - .getDefaultScreenDevice().getDefaultConfiguration(); - Insets screenInsets = Toolkit.getDefaultToolkit().getScreenInsets(gc); - - if (position.equals(Position.HORIZONTAL)) { - int newX = ((screenSize.width / 2) - frame.getWidth()); - frame.setLocation((newX + screenInsets.left), - (frame.getY() + screenInsets.top)); - syncLocationToWindowManager(); - if (testWindow != null) { - testWindow.setLocation((frame.getX() + frame.getWidth() + 5), - frame.getY()); - } - } else if (position.equals(Position.VERTICAL)) { - int newY = ((screenSize.height / 2) - frame.getHeight()); - frame.setLocation((frame.getX() + screenInsets.left), - (newY + screenInsets.top)); - syncLocationToWindowManager(); - if (testWindow != null) { - testWindow.setLocation(frame.getX(), - (frame.getY() + frame.getHeight() + 5)); - } - } else if (position.equals(Position.TOP_LEFT_CORNER)) { - frame.setLocation(screenInsets.left, screenInsets.top); - syncLocationToWindowManager(); - if (testWindow != null) { - testWindow.setLocation((frame.getX() + frame.getWidth() + 5), - frame.getY()); + positionInstructionFrame(position); + + if (testWindow != null) { + switch (position) { + case HORIZONTAL: + case TOP_LEFT_CORNER: + testWindow.setLocation((frame.getX() + frame.getWidth() + 5), + frame.getY()); + break; + + case VERTICAL: + testWindow.setLocation(frame.getX(), + (frame.getY() + frame.getHeight() + 5)); + break; } } + // make instruction frame visible after updating // frame & window positions frame.setVisible(true); @@ -553,13 +774,7 @@ public static Rectangle getInstructionFrameBounds() throws InterruptedException, InvocationTargetException { final Rectangle[] bounds = {null}; - if (isEventDispatchThread()) { - bounds[0] = frame != null ? frame.getBounds() : null; - } else { - invokeAndWait(() -> { - bounds[0] = frame != null ? frame.getBounds() : null; - }); - } + invokeOnEDT(() -> bounds[0] = frame != null ? frame.getBounds() : null); return bounds[0]; } @@ -574,6 +789,16 @@ public static synchronized void addTestWindow(Window testWindow) { windowList.add(testWindow); } + /** + * Adds a collection of test windows to the windowList to be disposed of + * when the test completes. + * + * @param testWindows the collection of test windows to be disposed of + */ + public static synchronized void addTestWindow(Collection testWindows) { + windowList.addAll(testWindows); + } + /** * Forcibly pass the test. *

    The sample usage: @@ -607,13 +832,20 @@ public static void forceFail(String reason) { latch.countDown(); } - public static class Builder { + public static final class Builder { private String title; private String instructions; private long testTimeOut; private int rows; private int columns; - private boolean screenCapture = false; + private boolean screenCapture; + + private List testWindows; + private WindowCreator windowCreator; + private PositionWindows positionWindows; + private InstructionUI instructionUIHandler; + + private Position position; public Builder title(String title) { this.title = title; @@ -645,6 +877,51 @@ public Builder screenCapture() { return this; } + public Builder testUI(Window window) { + return testUI(List.of(window)); + } + + public Builder testUI(Window... windows) { + return testUI(List.of(windows)); + } + + public Builder testUI(List windows) { + if (windows == null) { + throw new IllegalArgumentException("The list of windows can't be null"); + } + if (windows.stream() + .anyMatch(Objects::isNull)) { + throw new IllegalArgumentException("The windows list can't contain null"); + } + + if (windowCreator != null) { + throw new IllegalStateException("windowCreator is already set"); + } + this.testWindows = windows; + return this; + } + + public Builder testUI(WindowCreator windowCreator) { + if (windowCreator == null) { + throw new IllegalArgumentException("The window creator can't be null"); + } + if (testWindows != null) { + throw new IllegalStateException("testWindows are already set"); + } + this.windowCreator = windowCreator; + return this; + } + + public Builder positionTestUI(PositionWindows positionWindows) { + this.positionWindows = positionWindows; + return this; + } + + public Builder position(Position position) { + this.position = position; + return this; + } + public PassFailJFrame build() throws InterruptedException, InvocationTargetException { validate(); @@ -652,26 +929,76 @@ public PassFailJFrame build() throws InterruptedException, } private void validate() { - if (this.title == null) { - this.title = TITLE; + if (title == null) { + title = TITLE; } - if (this.instructions == null || this.instructions.length() == 0) { - throw new RuntimeException("Please provide the test " + - "instruction for this manual test"); + if (instructions == null || instructions.isEmpty()) { + throw new IllegalStateException("Please provide the test " + + "instructions for this manual test"); } - if (this.testTimeOut == 0L) { - this.testTimeOut = TEST_TIMEOUT; + if (testTimeOut == 0L) { + testTimeOut = TEST_TIMEOUT; } - if (this.rows == 0) { - this.rows = ROWS; + if (rows == 0) { + rows = ROWS; } - if (this.columns == 0) { - this.columns = COLUMNS; + if (columns == 0) { + columns = COLUMNS; + } + + if (position == null + && (testWindows != null || windowCreator != null)) { + + position = Position.HORIZONTAL; + } + + if (positionWindows != null) { + if (testWindows == null && windowCreator == null) { + throw new IllegalStateException("To position windows, " + + "provide an a list of windows to the builder"); + } + instructionUIHandler = new InstructionUIHandler(); } } + + private final class InstructionUIHandler implements InstructionUI { + @Override + public Point getLocation() { + return frame.getLocation(); + } + + @Override + public Dimension getSize() { + return frame.getSize(); + } + + @Override + public Rectangle getBounds() { + return frame.getBounds(); + } + + @Override + public void setLocation(Point location) { + setLocation(location.x, location.y); + } + + @Override + public void setLocation(int x, int y) { + frame.setLocation(x, y); + } + + @Override + public Position getPosition() { + return position; + } + } + } + + public static Builder builder() { + return new Builder(); } } From c587211bf8c60a7a1f6cc63770c38ede6cb4e173 Mon Sep 17 00:00:00 2001 From: Alexey Ivanov Date: Wed, 25 Oct 2023 11:33:47 +0000 Subject: [PATCH 16/19] 8316003: Update FileChooserSymLinkTest.java to HTML instructions Reviewed-by: prr --- .../JFileChooser/FileChooserSymLinkTest.java | 73 +++++++++++-------- 1 file changed, 43 insertions(+), 30 deletions(-) diff --git a/test/jdk/javax/swing/JFileChooser/FileChooserSymLinkTest.java b/test/jdk/javax/swing/JFileChooser/FileChooserSymLinkTest.java index a161a94d90cfa..ef29b708536b0 100644 --- a/test/jdk/javax/swing/JFileChooser/FileChooserSymLinkTest.java +++ b/test/jdk/javax/swing/JFileChooser/FileChooserSymLinkTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -74,48 +74,61 @@ public void run() { static void initialize() throws InterruptedException, InvocationTargetException { //Initialize the components final String INSTRUCTIONS = """ + Instructions to Test: - 1. Open an elevated Command Prompt. - 2. Paste the following commands: - cd /d C:\\ +

      +
    1. Open an elevated Command Prompt. +
    2. Paste the following commands: + 
      cd /d C:\\
                 mkdir FileChooserTest
                 cd FileChooserTest
                 mkdir target
-                mklink /d link target
+                mklink /d link target
      - 3. Navigate to C:\\FileChooserTest in the JFileChooser. - 4. Use "Enable Multi-Selection" checkbox to enable/disable - MultiSelection Mode - 5. Single-selection: - Click "link" directory, the absolute path of the symbolic - link should be displayed. If it's null, click FAIL. - Click "target" directory, its absolute path should be - displayed. - - Enable multiple selection by clicking the checkbox. - Multi-selection: - Click "link", press Ctrl and then click "target". - Both should be selected and their absolute paths should be - displayed. - - If "link" can't be selected or if its absolute path is null, - click FAIL. - - If "link" can be selected in both single- and multi-selection modes, - click PASS. - 6. When done with testing, paste the following commands to - remove the 'FileChooserTest' directory: - cd \\ - rmdir /s /q C:\\FileChooserTest +
    3. Navigate to C:\\FileChooserTest in + the JFileChooser. +
    4. Perform testing in single- and multi-selection modes: +
        +
      • Single-selection: +
          +
        1. Ensure Enable multi-selection is cleared + (the default state). +
        2. Click link directory, + the absolute path of the symbolic + link should be displayed.
          + If it's null, click Fail. +
        3. Click target directory, + its absolute path should be displayed. +
        +
      • Multi-selection: +
          +
        1. Select Enable multi-selection. +
        2. Click link, +
        3. Press Ctrl and + then click target. +
        4. Both should be selected and + their absolute paths should be displayed. +
        5. If link can't be selected or + if its absolute path is null, + click Fail. +
        +
      +

      If link can be selected in both + single- and multi-selection modes, click Pass.

      +
    5. When done with testing, paste the following commands to + remove the FileChooserTest directory: + 
      cd \\
+                rmdir /s /q C:\\FileChooserTest
      or use File Explorer to clean it up. +
    """; frame = new JFrame("JFileChooser Symbolic Link test"); panel = new JPanel(new BorderLayout()); multiSelection = new JCheckBox("Enable Multi-Selection"); pathList = new JTextArea(10, 50); jfc = new JFileChooser(new File("C:\\")); - passFailJFrame = new PassFailJFrame("Test Instructions", INSTRUCTIONS, 5L, 35, 40); + passFailJFrame = new PassFailJFrame("Test Instructions", INSTRUCTIONS, 5L, 35, 50); PassFailJFrame.addTestWindow(frame); PassFailJFrame.positionTestWindow(frame, PassFailJFrame.Position.HORIZONTAL); From 202c0137b86cd7bcbe0c1eddf2657f45698ab667 Mon Sep 17 00:00:00 2001 From: Frederic Thevenet Date: Wed, 25 Oct 2023 12:58:01 +0000 Subject: [PATCH 17/19] 8318669: Target OS detection in 'test-prebuilt' makefile target is incorrect when running on MSYS2 Reviewed-by: ihse, erikj --- make/RunTestsPrebuilt.gmk | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/make/RunTestsPrebuilt.gmk b/make/RunTestsPrebuilt.gmk index ae14b1e533652..93febe5ed31d9 100644 --- a/make/RunTestsPrebuilt.gmk +++ b/make/RunTestsPrebuilt.gmk @@ -158,6 +158,10 @@ ifeq ($(UNAME_OS), CYGWIN) OPENJDK_TARGET_OS := windows OPENJDK_TARGET_OS_TYPE := windows OPENJDK_TARGET_OS_ENV := windows.cygwin +else ifeq ($(UNAME_OS), MINGW64) + OPENJDK_TARGET_OS := windows + OPENJDK_TARGET_OS_TYPE := windows + OPENJDK_TARGET_OS_ENV := windows.msys2 else OPENJDK_TARGET_OS_TYPE:=unix ifeq ($(UNAME_OS), Linux) @@ -170,6 +174,9 @@ else OPENJDK_TARGET_OS_ENV := $(OPENJDK_TARGET_OS) endif +# Sanity check env detection +$(info Detected target OS, type and env: [$(OPENJDK_TARGET_OS)] [$(OPENJDK_TARGET_OS_TYPE)] [$(OPENJDK_TARGET_OS_ENV)]) + # Assume little endian unless otherwise specified OPENJDK_TARGET_CPU_ENDIAN := little From 3abd772672a4dfd984459283235f3b1d8fb28a49 Mon Sep 17 00:00:00 2001 From: Alexey Ivanov Date: Wed, 25 Oct 2023 13:25:34 +0000 Subject: [PATCH 18/19] 8316017: Refactor timeout handler in PassFailJFrame Reviewed-by: prr --- .../awt/regtesthelpers/PassFailJFrame.java | 173 ++++++++++++------ 1 file changed, 116 insertions(+), 57 deletions(-) diff --git a/test/jdk/java/awt/regtesthelpers/PassFailJFrame.java b/test/jdk/java/awt/regtesthelpers/PassFailJFrame.java index d2e650b213427..e4fe393fd6e36 100644 --- a/test/jdk/java/awt/regtesthelpers/PassFailJFrame.java +++ b/test/jdk/java/awt/regtesthelpers/PassFailJFrame.java @@ -34,6 +34,8 @@ import java.awt.Robot; import java.awt.Toolkit; import java.awt.Window; +import java.awt.event.ActionEvent; +import java.awt.event.ActionListener; import java.awt.event.WindowAdapter; import java.awt.event.WindowEvent; import java.awt.event.WindowListener; @@ -155,14 +157,26 @@ public class PassFailJFrame { * Prefix for the user-provided failure reason. */ private static final String FAILURE_REASON = "Failure Reason:\n"; + /** + * The failure reason message when the user didn't provide one. + */ + private static final String EMPTY_REASON = "(no reason provided)"; private static final List windowList = new ArrayList<>(); - private static final Timer timer = new Timer(0, null); + private static final CountDownLatch latch = new CountDownLatch(1); - private static volatile boolean failed; - private static volatile boolean timeout; - private static volatile String testFailedReason; + private static TimeoutHandler timeoutHandler; + + /** + * The description of why the test fails. + *

    + * Note: do not use this field directly, + * use the {@link #setFailureReason(String) setFailureReason} and + * {@link #getFailureReason() getFailureReason} methods to modify and + * to read its value. + */ + private static String failureReason; private static final AtomicInteger imgCounter = new AtomicInteger(0); @@ -319,42 +333,27 @@ private static void createUI(String title, String instructions, frame = new JFrame(title); frame.setLayout(new BorderLayout()); + JLabel testTimeoutLabel = new JLabel("", JLabel.CENTER); + timeoutHandler = new TimeoutHandler(testTimeoutLabel, testTimeOut); + frame.add(testTimeoutLabel, BorderLayout.NORTH); + JTextComponent text = instructions.startsWith("") ? configureHTML(instructions, rows, columns) : configurePlainText(instructions, rows, columns); text.setEditable(false); - long tTimeout = TimeUnit.MINUTES.toMillis(testTimeOut); - - final JLabel testTimeoutLabel = new JLabel(String.format("Test " + - "timeout: %s", convertMillisToTimeStr(tTimeout)), JLabel.CENTER); - final long startTime = System.currentTimeMillis(); - timer.setDelay(1000); - timer.addActionListener((e) -> { - long leftTime = tTimeout - (System.currentTimeMillis() - startTime); - if ((leftTime < 0) || failed) { - timer.stop(); - testFailedReason = FAILURE_REASON - + "Timeout User did not perform testing."; - timeout = true; - latch.countDown(); - } - testTimeoutLabel.setText(String.format("Test timeout: %s", convertMillisToTimeStr(leftTime))); - }); - timer.start(); - frame.add(testTimeoutLabel, BorderLayout.NORTH); frame.add(new JScrollPane(text), BorderLayout.CENTER); JButton btnPass = new JButton("Pass"); btnPass.addActionListener((e) -> { latch.countDown(); - timer.stop(); + timeoutHandler.stop(); }); JButton btnFail = new JButton("Fail"); btnFail.addActionListener((e) -> { - getFailureReason(); - timer.stop(); + requestFailureReason(); + timeoutHandler.stop(); }); JPanel buttonsPanel = new JPanel(); @@ -480,12 +479,58 @@ public interface InstructionUI { } + private static final class TimeoutHandler implements ActionListener { + private final long endTime; + + private final Timer timer; + + private final JLabel label; + + public TimeoutHandler(final JLabel label, final long testTimeOut) { + endTime = System.currentTimeMillis() + TimeUnit.MINUTES.toMillis(testTimeOut); + + this.label = label; + + timer = new Timer(1000, this); + timer.start(); + updateTime(testTimeOut); + } + + @Override + public void actionPerformed(ActionEvent e) { + long leftTime = endTime - System.currentTimeMillis(); + if (leftTime < 0) { + timer.stop(); + setFailureReason(FAILURE_REASON + + "Timeout - User did not perform testing."); + latch.countDown(); + } + updateTime(leftTime); + } + + private void updateTime(final long leftTime) { + if (leftTime < 0) { + label.setText("Test timeout: 00:00:00"); + return; + } + long hours = leftTime / 3_600_000; + long minutes = (leftTime - hours * 3_600_000) / 60_000; + long seconds = (leftTime - hours * 3_600_000 - minutes * 60_000) / 1_000; + label.setText(String.format("Test timeout: %02d:%02d:%02d", + hours, minutes, seconds)); + } + + public void stop() { + timer.stop(); + } + } + + private static final class WindowClosingHandler extends WindowAdapter { @Override public void windowClosing(WindowEvent e) { - testFailedReason = FAILURE_REASON - + "User closed a window"; - failed = true; + setFailureReason(FAILURE_REASON + + "User closed a window"); latch.countDown(); } } @@ -579,14 +624,28 @@ private static void captureScreen(CaptureType type) { JOptionPane.INFORMATION_MESSAGE); } - private static String convertMillisToTimeStr(long millis) { - if (millis < 0) { - return "00:00:00"; + /** + * Sets the failure reason which describes why the test fails. + * This method ensures the {@code failureReason} field does not change + * after it's set to a non-{@code null} value. + * @param reason the description of why the test fails + * @throws IllegalArgumentException if the {@code reason} parameter + * is {@code null} + */ + private static synchronized void setFailureReason(final String reason) { + if (reason == null) { + throw new IllegalArgumentException("The failure reason must not be null"); } - long hours = millis / 3_600_000; - long minutes = (millis - hours * 3_600_000) / 60_000; - long seconds = (millis - hours * 3_600_000 - minutes * 60_000) / 1_000; - return String.format("%02d:%02d:%02d", hours, minutes, seconds); + if (failureReason == null) { + failureReason = reason; + } + } + + /** + * {@return the description of why the test fails} + */ + private static synchronized String getFailureReason() { + return failureReason; } /** @@ -607,30 +666,18 @@ public void awaitAndCheck() throws InterruptedException, InvocationTargetExcepti latch.await(); invokeAndWait(PassFailJFrame::disposeWindows); - if (timeout) { - throw new RuntimeException(testFailedReason); - } - - if (failed) { - throw new RuntimeException("Test failed! : " + testFailedReason); + String failure = getFailureReason(); + if (failure != null) { + throw new RuntimeException(failure); } System.out.println("Test passed!"); } /** - * Disposes of all the windows. It disposes of the test instruction frame - * and all other windows added via {@link #addTestWindow(Window)}. - */ - private static synchronized void disposeWindows() { - windowList.forEach(Window::dispose); - } - - /** - * Read the test failure reason and add the reason to the test result - * example in the jtreg .jtr file. + * Requests the description of the test failure reason from the tester. */ - private static void getFailureReason() { + private static void requestFailureReason() { final JDialog dialog = new JDialog(frame, "Test Failure ", true); dialog.setTitle("Failure reason"); JPanel jPanel = new JPanel(new BorderLayout()); @@ -638,7 +685,9 @@ private static void getFailureReason() { JButton okButton = new JButton("OK"); okButton.addActionListener((ae) -> { - testFailedReason = FAILURE_REASON + jTextArea.getText(); + String text = jTextArea.getText(); + setFailureReason(FAILURE_REASON + + (!text.isEmpty() ? text : EMPTY_REASON)); dialog.setVisible(false); }); @@ -653,11 +702,22 @@ private static void getFailureReason() { dialog.pack(); dialog.setVisible(true); - failed = true; + // Ensure the test fails even if the dialog is closed + // without clicking the OK button + setFailureReason(FAILURE_REASON + EMPTY_REASON); + dialog.dispose(); latch.countDown(); } + /** + * Disposes of all the windows. It disposes of the test instruction frame + * and all other windows added via {@link #addTestWindow(Window)}. + */ + private static synchronized void disposeWindows() { + windowList.forEach(Window::dispose); + } + private static void positionInstructionFrame(final Position position) { Dimension screenSize = Toolkit.getDefaultToolkit().getScreenSize(); @@ -827,8 +887,7 @@ public static void forceFail() { * @param reason the reason why the test is failed */ public static void forceFail(String reason) { - failed = true; - testFailedReason = FAILURE_REASON + reason; + setFailureReason(FAILURE_REASON + reason); latch.countDown(); } From b026d0b480dcd4c0a3346078dd10047653ed3751 Mon Sep 17 00:00:00 2001 From: Roland Westrelin Date: Wed, 25 Oct 2023 13:32:56 +0000 Subject: [PATCH 19/19] 8312980: C2: "malformed control flow" created during incremental inlining Co-authored-by: Emanuel Peter Reviewed-by: thartmann, epeter --- src/hotspot/share/opto/replacednodes.cpp | 206 +++++++++++++----- src/hotspot/share/opto/replacednodes.hpp | 4 + .../TestReplacedNodesAfterLateInline.java | 80 +++++++ ...ReplacedNodesAfterLateInlineManyPaths.java | 72 ++++++ 4 files changed, 306 insertions(+), 56 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/inlining/TestReplacedNodesAfterLateInline.java create mode 100644 test/hotspot/jtreg/compiler/inlining/TestReplacedNodesAfterLateInlineManyPaths.java diff --git a/src/hotspot/share/opto/replacednodes.cpp b/src/hotspot/share/opto/replacednodes.cpp index 78c1703799e5e..56c5ee1c0bfb7 100644 --- a/src/hotspot/share/opto/replacednodes.cpp +++ b/src/hotspot/share/opto/replacednodes.cpp @@ -106,88 +106,175 @@ void ReplacedNodes::apply(Node* n, uint idx) { } } -static void enqueue_use(Node* n, Node* use, Unique_Node_List& work) { - if (use->is_Phi()) { - Node* r = use->in(0); - assert(r->is_Region(), "Phi should have Region"); - for (uint i = 1; i < use->req(); i++) { - if (use->in(i) == n) { - work.push(r->in(i)); - } - } - } else { - work.push(use); - } -} - // Perform node replacement following late inlining. void ReplacedNodes::apply(Compile* C, Node* ctl) { // ctl is the control on exit of the method that was late inlined if (is_empty()) { return; } + ResourceMark rm; + Node_Stack stack(0); + Unique_Node_List to_fix; // nodes to clone + uses at the end of the chain that need to updated + VectorSet seen; + VectorSet valid_control; + for (int i = 0; i < _replaced_nodes->length(); i++) { ReplacedNode replaced = _replaced_nodes->at(i); Node* initial = replaced.initial(); Node* improved = replaced.improved(); assert (ctl != nullptr && !ctl->is_top(), "replaced node should have actual control"); - ResourceMark rm; - Unique_Node_List work; - // Go over all the uses of the node that is considered for replacement... - for (DUIterator j = initial->outs(); initial->has_out(j); j++) { - Node* use = initial->out(j); + if (initial->outcnt() == 0) { + continue; + } - if (use == improved || use->outcnt() == 0) { - continue; - } - work.clear(); - enqueue_use(initial, use, work); - bool replace = true; - // Check that this use is dominated by ctl. Go ahead with the replacement if it is. - while (work.size() != 0 && replace) { - Node* n = work.pop(); - if (use->outcnt() == 0) { - continue; + // Find uses of initial that are dominated by ctl so, initial can be replaced by improved. + // Proving domination here is not straightforward. To do so, we follow uses of initial, and uses of uses until we + // encounter a node which is a control node or is pinned at some control. Then, we try to prove this control is + // dominated by ctl. If that's the case, it's legal to replace initial by improved but for this chain of uses only. + // It may not be the case for some other chain of uses, so we clone that chain and perform the replacement only for + // these uses. + assert(stack.is_empty(), ""); + stack.push(initial, 1); + Node* use = initial->raw_out(0); + stack.push(use, 0); + + while (!stack.is_empty()) { + assert(stack.size() > 1, "at least initial + one use"); + Node* n = stack.node(); + + uint current_size = stack.size(); + + if (seen.test_set(n->_idx)) { + if (to_fix.member(n)) { + collect_nodes_to_clone(stack, to_fix); } - if (n->is_CFG() || (n->in(0) != nullptr && !n->in(0)->is_top())) { - // Skip projections, since some of the multi nodes aren't CFG (e.g., LoadStore and SCMemProj). - if (n->is_Proj()) { - n = n->in(0); + } else if (n->outcnt() != 0 && n != improved) { + if (n->is_Phi()) { + Node* region = n->in(0); + Node* prev = stack.node_at(stack.size() - 2); + for (uint j = 1; j < region->req(); ++j) { + if (n->in(j) == prev) { + Node* in = region->in(j); + if (in != nullptr && !in->is_top()) { + if (is_dominator(ctl, in)) { + valid_control.set(in->_idx); + collect_nodes_to_clone(stack, to_fix); + } + } + } } - if (!n->is_CFG()) { - n = n->in(0); + } else if (n->is_CFG()) { + if (is_dominator(ctl, n)) { + collect_nodes_to_clone(stack, to_fix); } - assert(n->is_CFG(), "should be CFG now"); - int depth = 0; - while(n != ctl) { - n = IfNode::up_one_dom(n); - depth++; - // limit search depth - if (depth >= 100 || n == nullptr) { - replace = false; - break; - } + } else if (n->in(0) != nullptr && n->in(0)->is_CFG()) { + Node* c = n->in(0); + if (is_dominator(ctl, c)) { + collect_nodes_to_clone(stack, to_fix); } } else { - for (DUIterator k = n->outs(); n->has_out(k); k++) { - enqueue_use(n, n->out(k), work); + uint idx = stack.index(); + if (idx < n->outcnt()) { + stack.set_index(idx + 1); + stack.push(n->raw_out(idx), 0); } } } - if (replace) { - bool is_in_table = C->initial_gvn()->hash_delete(use); - int replaced = use->replace_edge(initial, improved); - if (is_in_table) { - C->initial_gvn()->hash_find_insert(use); + if (stack.size() == current_size) { + for (;;) { + stack.pop(); + if (stack.is_empty()) { + break; + } + n = stack.node(); + uint idx = stack.index(); + if (idx < n->outcnt()) { + stack.set_index(idx + 1); + stack.push(n->raw_out(idx), 0); + break; + } } - C->record_for_igvn(use); + } + } + } + if (to_fix.size() > 0) { + uint hash_table_size = _replaced_nodes->length(); + for (uint i = 0; i < to_fix.size(); ++i) { + Node* n = to_fix.at(i); + if (n->is_CFG() || n->in(0) != nullptr) { // End of a chain is not cloned + continue; + } + hash_table_size++; + } + // Map from current node to cloned/replaced node + ResizeableResourceHashtable clones(hash_table_size, hash_table_size); + // Record mapping from initial to improved nodes + for (int i = 0; i < _replaced_nodes->length(); i++) { + ReplacedNode replaced = _replaced_nodes->at(i); + Node* initial = replaced.initial(); + Node* improved = replaced.improved(); + clones.put(initial, improved); + // If initial needs to be cloned but is also improved then there's no need to clone it. + if (to_fix.member(initial)) { + to_fix.remove(initial); + } + } - assert(replaced > 0, "inconsistent"); - --j; + // Clone nodes and record mapping from current to cloned nodes + for (uint i = 0; i < to_fix.size(); ++i) { + Node* n = to_fix.at(i); + if (n->is_CFG() || n->in(0) != nullptr) { // End of a chain + continue; } + Node* clone = n->clone(); + bool added = clones.put(n, clone); + assert(added, "clone node must be added to mapping"); + C->initial_gvn()->set_type_bottom(clone); + to_fix.map(i, clone); // Update list of nodes with cloned node + } + + // Fix edges in cloned nodes and use at the end of the chain + for (uint i = 0; i < to_fix.size(); ++i) { + Node* n = to_fix.at(i); + bool is_in_table = C->initial_gvn()->hash_delete(n); + uint updates = 0; + for (uint j = 0; j < n->req(); ++j) { + Node* in = n->in(j); + if (in == nullptr || (n->is_Phi() && n->in(0)->in(j) == nullptr)) { + continue; + } + if (n->is_Phi() && !valid_control.test(n->in(0)->in(j)->_idx)) { + continue; + } + Node** clone_ptr = clones.get(in); + if (clone_ptr != nullptr) { + Node* clone = *clone_ptr; + n->set_req(j, clone); + updates++; + } + } + assert(updates > 0, ""); + C->record_for_igvn(n); + if (is_in_table) { + C->initial_gvn()->hash_find_insert(n); + } + } + } +} + +bool ReplacedNodes::is_dominator(const Node* ctl, Node* n) const { + assert(n->is_CFG(), "should be CFG now"); + int depth = 0; + while (n != ctl) { + n = IfNode::up_one_dom(n); + depth++; + // limit search depth + if (depth >= 100 || n == nullptr) { + return false; } } + return true; } void ReplacedNodes::dump(outputStream *st) const { @@ -224,3 +311,10 @@ void ReplacedNodes::merge_with(const ReplacedNodes& other) { _replaced_nodes->trunc_to(len - shift); } } + +void ReplacedNodes::collect_nodes_to_clone(const Node_Stack& stack, Unique_Node_List& to_fix) { + for (uint i = stack.size() - 1; i >= 1; i--) { + Node* n = stack.node_at(i); + to_fix.push(n); + } +} diff --git a/src/hotspot/share/opto/replacednodes.hpp b/src/hotspot/share/opto/replacednodes.hpp index c569e55ce5ff2..9b66a2c5f49b8 100644 --- a/src/hotspot/share/opto/replacednodes.hpp +++ b/src/hotspot/share/opto/replacednodes.hpp @@ -76,6 +76,10 @@ class ReplacedNodes { bool is_empty() const; void dump(outputStream *st) const; void apply(Compile* C, Node* ctl); + + bool is_dominator(const Node* ctl, Node* n) const; + + void collect_nodes_to_clone(const Node_Stack& stack, Unique_Node_List& to_fix); }; #endif // SHARE_OPTO_REPLACEDNODES_HPP diff --git a/test/hotspot/jtreg/compiler/inlining/TestReplacedNodesAfterLateInline.java b/test/hotspot/jtreg/compiler/inlining/TestReplacedNodesAfterLateInline.java new file mode 100644 index 0000000000000..26e1c7740da9c --- /dev/null +++ b/test/hotspot/jtreg/compiler/inlining/TestReplacedNodesAfterLateInline.java @@ -0,0 +1,80 @@ +/* + * Copyright (c) 2023, Red Hat, Inc. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * bug 8312980 + * @summary C2: "malformed control flow" created during incremental inlining + * @requires vm.compiler2.enabled + * @run main/othervm -XX:-TieredCompilation -XX:-BackgroundCompilation -XX:+IgnoreUnrecognizedVMOptions -XX:+AlwaysIncrementalInline TestReplacedNodesAfterLateInline + */ + +public class TestReplacedNodesAfterLateInline { + private static B fieldB = new B(); + private static A fieldA = new A(); + private static volatile int volatileField; + + public static void main(String[] args) { + for (int i = 0; i < 20_000; i++) { + test(false, fieldA, true); + test(false, fieldA, false); + testHelper(fieldB); + testHelper2(fieldB, true, false, true); + testHelper2(fieldA, false, true, true); + continue; + } + } + + private static int test(boolean flag, Object o, boolean flag2) { + if (o == null) { + } + if (flag2) { + return testHelper2(o, true, true, flag); + } + return ((A) o).field; + } + + private static int testHelper2(Object o, boolean flag, boolean flag2, boolean flag3) { + if (flag3) { + if (flag) { + testHelper(o); + } + if (flag2) { + return ((A) o).field; + } + } + volatileField = 42; + return volatileField; + } + + private static void testHelper(Object o) { + B b = (B)o; + } + + private static class A { + public int field; + } + + private static class B { + } +} diff --git a/test/hotspot/jtreg/compiler/inlining/TestReplacedNodesAfterLateInlineManyPaths.java b/test/hotspot/jtreg/compiler/inlining/TestReplacedNodesAfterLateInlineManyPaths.java new file mode 100644 index 0000000000000..b30e5652420b8 --- /dev/null +++ b/test/hotspot/jtreg/compiler/inlining/TestReplacedNodesAfterLateInlineManyPaths.java @@ -0,0 +1,72 @@ +/* + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * bug 8312980 + * @summary C2: "malformed control flow" created during incremental inlining + * @requires vm.compiler2.enabled + * @run main/othervm -XX:CompileCommand=compileonly,TestReplacedNodesAfterLateInlineManyPaths::* -XX:-BackgroundCompilation + * -XX:+IgnoreUnrecognizedVMOptions -XX:+AlwaysIncrementalInline TestReplacedNodesAfterLateInlineManyPaths + */ + +public class TestReplacedNodesAfterLateInlineManyPaths { + public static void main(String[] args) { + for (int i = 0; i < 20_000; i++) { + test("" + i); + } + } + + public static int test(String s) { + int result = 0; + int len = s.length(); + int i = 0; + while (i < len) { + // charAt is inlined late, and i is constrained by CastII(i >= 0) + // The constraint comes from intrinsic checkIndex + s.charAt(i); + // Graph below intentionally branches out 4x, and merges again (4-fold diamonds). + // This creates an exponential explosion in number of paths. + int e = i; + e = (e & 7) + (e & 31) + (e & 1111) + (e & 1000_000); + e = (e & 7) + (e & 31) + (e & 1111) + (e & 1000_000); + e = (e & 7) + (e & 31) + (e & 1111) + (e & 1000_000); + e = (e & 7) + (e & 31) + (e & 1111) + (e & 1000_000); + e = (e & 7) + (e & 31) + (e & 1111) + (e & 1000_000); + // Comment out lines below to make it not assert + // assert(C->live_nodes() <= C->max_node_limit()) failed: Live Node limit exceeded limit + e = (e & 7) + (e & 31) + (e & 1111) + (e & 1000_000); + e = (e & 7) + (e & 31) + (e & 1111) + (e & 1000_000); + e = (e & 7) + (e & 31) + (e & 1111) + (e & 1000_000); + e = (e & 7) + (e & 31) + (e & 1111) + (e & 1000_000); + e = (e & 7) + (e & 31) + (e & 1111) + (e & 1000_000); + e = (e & 7) + (e & 31) + (e & 1111) + (e & 1000_000); + e = (e & 7) + (e & 31) + (e & 1111) + (e & 1000_000); + e = (e & 7) + (e & 31) + (e & 1111) + (e & 1000_000); + e = (e & 7) + (e & 31) + (e & 1111) + (e & 1000_000); + result += e; + i++; + } + return result; + } +}