From f0b7eb519ae1cf6f4cb2baea55b5e02a46a8192e Mon Sep 17 00:00:00 2001 From: Eirik Bjorsnos Date: Thu, 16 Nov 2023 06:30:29 +0000 Subject: [PATCH 01/76] 8304020: Speed up test/jdk/java/util/zip/ZipFile/TestTooManyEntries.java and clarify its purpose Reviewed-by: lancea, martin, jpai --- test/jdk/TEST.groups | 1 - .../util/zip/ZipFile/CenSizeTooLarge.java | 226 ++++++++++++++++++ .../util/zip/ZipFile/TestTooManyEntries.java | 89 ------- 3 files changed, 226 insertions(+), 90 deletions(-) create mode 100644 test/jdk/java/util/zip/ZipFile/CenSizeTooLarge.java delete mode 100644 test/jdk/java/util/zip/ZipFile/TestTooManyEntries.java diff --git a/test/jdk/TEST.groups b/test/jdk/TEST.groups index a39dfe8b09d..2f39f3fdcef 100644 --- a/test/jdk/TEST.groups +++ b/test/jdk/TEST.groups @@ -586,7 +586,6 @@ jdk_core_manual_no_input = \ java/net/httpclient/BodyProcessorInputStreamTest.java \ java/net/httpclient/HttpInputStreamTest.java \ java/util/zip/ZipFile/TestZipFile.java \ - java/util/zip/ZipFile/TestTooManyEntries.java \ javax/net/ssl/compatibility/AlpnTest.java \ javax/net/ssl/compatibility/BasicConnectTest.java \ javax/net/ssl/compatibility/HrrTest.java \ diff --git a/test/jdk/java/util/zip/ZipFile/CenSizeTooLarge.java b/test/jdk/java/util/zip/ZipFile/CenSizeTooLarge.java new file mode 100644 index 00000000000..4336377e0c8 --- /dev/null +++ b/test/jdk/java/util/zip/ZipFile/CenSizeTooLarge.java @@ -0,0 +1,226 @@ +/* + * 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 + * 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 8272746 + * @summary Verify that ZipFile rejects a ZIP with a CEN size which does not fit in a Java byte array + * @run junit CenSizeTooLarge + */ + +import org.junit.Before; +import org.junit.Test; + +import java.io.*; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +import java.nio.channels.FileChannel; +import java.nio.charset.StandardCharsets; +import java.time.LocalDateTime; +import java.util.Arrays; +import java.util.zip.ZipEntry; +import java.util.zip.ZipException; +import java.util.zip.ZipFile; +import java.util.zip.ZipOutputStream; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class CenSizeTooLarge { + // Maximum allowed CEN size allowed by the ZipFile implementation + static final int MAX_CEN_SIZE = Integer.MAX_VALUE - ZipFile.ENDHDR - 1; + + /** + * From the APPNOTE.txt specification: + * 4.4.10 file name length: (2 bytes) + * 4.4.11 extra field length: (2 bytes) + * 4.4.12 file comment length: (2 bytes) + * + * The length of the file name, extra field, and comment + * fields respectively. The combined length of any + * directory record and these three fields SHOULD NOT + * generally exceed 65,535 bytes. + * + * Since ZipOutputStream does not enforce the 'combined length' clause, + * we simply use 65,535 (0xFFFF) for the purpose of this test. + */ + static final int MAX_EXTRA_FIELD_SIZE = 65_535; + + // Data size (unsigned short) + // Field size minus the leading header 'tag' and 'data size' fields (2 bytes each) + static final short MAX_DATA_SIZE = (short) (MAX_EXTRA_FIELD_SIZE - 2 * Short.BYTES); + + // Tag for the 'unknown' field type, specified in APPNOTE.txt 'Third party mappings' + static final short UNKNOWN_ZIP_TAG = (short) 0x9902; + + // Entry names produced in this test are fixed-length + public static final int NAME_LENGTH = 10; + + // Use a shared LocalDateTime on all entries to save processing time + static final LocalDateTime TIME_LOCAL = LocalDateTime.now(); + + // The size of one CEN header, including the name and the extra field + static final int CEN_HEADER_SIZE = ZipFile.CENHDR + NAME_LENGTH + MAX_EXTRA_FIELD_SIZE; + + // The number of entries needed to exceed the MAX_CEN_SIZE + static final int NUM_ENTRIES = (MAX_CEN_SIZE / CEN_HEADER_SIZE) + 1; + + // Helps SparseOutputStream detect write of the last CEN entry + private static final String LAST_CEN_COMMENT = "LastCEN"; + private static final byte[] LAST_CEN_COMMENT_BYTES = LAST_CEN_COMMENT.getBytes(StandardCharsets.UTF_8); + + // Expected ZipException message when the CEN does not fit in a Java byte array + private static final String CEN_TOO_LARGE_MESSAGE = "invalid END header (central directory size too large)"; + + // Zip file to create for testing + private File hugeZipFile; + + /** + * Create a zip file with a CEN size which does not fit within a Java byte array + */ + @Before + public void setup() throws IOException { + hugeZipFile = new File("cen-too-large.zip"); + hugeZipFile.deleteOnExit(); + + try (OutputStream out = new SparseOutputStream(new FileOutputStream(hugeZipFile)); + ZipOutputStream zip = new ZipOutputStream(out)) { + + // Keep track of entries so we can update extra data before the CEN is written + ZipEntry[] entries = new ZipEntry[NUM_ENTRIES]; + + // Add entries until MAX_CEN_SIZE is reached + for (int i = 0; i < NUM_ENTRIES; i++) { + // Create a fixed-length name for the entry + String name = Integer.toString(i); + name = "0".repeat(NAME_LENGTH - name.length()) + name; + + // Create and track the entry + ZipEntry entry = entries[i] = new ZipEntry(name); + + // Use STORED for faster processing + entry.setMethod(ZipEntry.STORED); + entry.setSize(0); + entry.setCrc(0); + + // Set the time/date field for faster processing + entry.setTimeLocal(TIME_LOCAL); + + if (i == NUM_ENTRIES -1) { + // Help SparseOutputStream detect the last CEN entry write + entry.setComment(LAST_CEN_COMMENT); + } + // Add the entry + zip.putNextEntry(entry); + + + } + // Finish writing the last entry + zip.closeEntry(); + + // Before the CEN headers are written, set the extra data on each entry + byte[] extra = makeLargeExtraField(); + for (ZipEntry entry : entries) { + entry.setExtra(extra); + } + } + } + + /** + * Validates that a ZipException is thrown with the expected message when + * the ZipFile is initialized with a ZIP whose CEN exeeds {@link #MAX_CEN_SIZE} + */ + @Test + public void centralDirectoryTooLargeToFitInByteArray() { + ZipException ex = assertThrows(ZipException.class, () -> new ZipFile(hugeZipFile)); + assertEquals(CEN_TOO_LARGE_MESSAGE, ex.getMessage()); + } + + /** + * We can reduce the number of written CEN headers by making each CEN header maximally large. + * We do this by adding the extra field produced by this method to each CEN header. + *

+ * The structure of an extra field is as follows: + *

+ * Header ID (Two bytes, describes the type of the field, also called 'tag') + * Data Size (Two byte short) + * Data Block (Contents depend on field type) + */ + private byte[] makeLargeExtraField() { + // Make a maximally sized extra field + byte[] extra = new byte[MAX_EXTRA_FIELD_SIZE]; + // Little-endian ByteBuffer for updating the header fields + ByteBuffer buffer = ByteBuffer.wrap(extra).order(ByteOrder.LITTLE_ENDIAN); + + // We use the 'unknown' tag, specified in APPNOTE.TXT, 4.6.1 Third party mappings' + buffer.putShort(UNKNOWN_ZIP_TAG); + + // Size of the actual (empty) data + buffer.putShort(MAX_DATA_SIZE); + return extra; + } + + /** + * By writing sparse 'holes' until the last CEN is detected, we can save disk space + * used by this test from ~2GB to ~4K. Instances of this class should be passed + * directly to the ZipOutputStream constructor, without any buffering. Otherwise, + * writes from ZipOutputStream may not be detected correctly. + */ + private static class SparseOutputStream extends FilterOutputStream { + private final FileChannel channel; + private boolean sparse = true; // True until the last CEN is written + private long position = 0; + + public SparseOutputStream(FileOutputStream fos) { + super(fos); + this.channel = fos.getChannel(); + } + + @Override + public void write(byte[] b, int off, int len) throws IOException { + position += len; + if (sparse) { + // Until finding the last CEN, we don't actually write anything, + // but instead simply advance the position, creating a sparse file + channel.position(position); + // Check for last CEN record + if (Arrays.equals(LAST_CEN_COMMENT_BYTES, 0, LAST_CEN_COMMENT_BYTES.length, b, off, len)) { + // From here on, write actual bytes + sparse = false; + } + } else { + // Regular write + out.write(b, off, len); + } + } + + @Override + public void write(int b) throws IOException { + position++; + if (sparse) { + channel.position(position); + } else { + out.write(b); + } + } + } +} diff --git a/test/jdk/java/util/zip/ZipFile/TestTooManyEntries.java b/test/jdk/java/util/zip/ZipFile/TestTooManyEntries.java deleted file mode 100644 index 07ed760c1b7..00000000000 --- a/test/jdk/java/util/zip/ZipFile/TestTooManyEntries.java +++ /dev/null @@ -1,89 +0,0 @@ -/* - * Copyright (c) 2022, 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 8272746 - * @summary ZipFile can't open big file (NegativeArraySizeException) - * @requires (sun.arch.data.model == "64" & os.maxMemory > 8g) - * @run testng/manual/othervm -Xmx8g TestTooManyEntries - */ - -import org.testng.annotations.BeforeTest; -import org.testng.annotations.Test; - -import java.io.File; -import java.io.BufferedOutputStream; -import java.io.FileOutputStream; -import java.io.IOException; -import java.util.zip.ZipEntry; -import java.util.zip.ZipException; -import java.util.zip.ZipFile; -import java.util.zip.ZipOutputStream; -import java.util.UUID; - -import static org.testng.Assert.assertThrows; - -public class TestTooManyEntries { - // Number of directories in the zip file - private static final int DIR_COUNT = 25000; - // Number of entries per directory - private static final int ENTRIES_IN_DIR = 1000; - - // Zip file to create for testing - private File hugeZipFile; - - /** - * Create a zip file and add entries that exceed the CEN limit. - * @throws IOException if an error occurs creating the ZIP File - */ - @BeforeTest - public void setup() throws IOException { - hugeZipFile = File.createTempFile("hugeZip", ".zip", new File(".")); - hugeZipFile.deleteOnExit(); - long startTime = System.currentTimeMillis(); - try (ZipOutputStream zip = new ZipOutputStream(new BufferedOutputStream(new FileOutputStream(hugeZipFile)))) { - for (int dirN = 0; dirN < DIR_COUNT; dirN++) { - String dirName = UUID.randomUUID() + "/"; - for (int fileN = 0; fileN < ENTRIES_IN_DIR; fileN++) { - ZipEntry entry = new ZipEntry(dirName + UUID.randomUUID()); - zip.putNextEntry(entry); - zip.closeEntry(); // all files are empty - } - if ((dirN + 1) % 1000 == 0) { - System.out.printf("%s / %s of entries written, file size is %sMb (%ss)%n", - (dirN + 1) * ENTRIES_IN_DIR, DIR_COUNT * ENTRIES_IN_DIR, hugeZipFile.length() / 1024 / 1024, - (System.currentTimeMillis() - startTime) / 1000); - } - } - } - } - - /** - * Validates that the ZipException is thrown when the ZipFile class - * is initialized with a zip file whose entries exceed the CEN limit. - */ - @Test - public void test() { - assertThrows(ZipException.class, () -> new ZipFile(hugeZipFile)); - } -} From 3452210b3652bf936e3c34675f2648852eb7cdf1 Mon Sep 17 00:00:00 2001 From: Cesar Soares Lucas Date: Thu, 16 Nov 2023 06:51:26 +0000 Subject: [PATCH 02/76] 8283140: Remove unused encoding classes/operands from x86_64.ad Reviewed-by: thartmann, dlong --- src/hotspot/cpu/x86/x86_64.ad | 1025 ++------------------------------- 1 file changed, 35 insertions(+), 990 deletions(-) diff --git a/src/hotspot/cpu/x86/x86_64.ad b/src/hotspot/cpu/x86/x86_64.ad index 5b1e206ae9a..4cdd3bb7bbb 100644 --- a/src/hotspot/cpu/x86/x86_64.ad +++ b/src/hotspot/cpu/x86/x86_64.ad @@ -518,187 +518,6 @@ int CallDynamicJavaDirectNode::compute_padding(int current_offset) const return align_up(current_offset, alignment_required()) - current_offset; } -// EMIT_RM() -void emit_rm(CodeBuffer &cbuf, int f1, int f2, int f3) { - unsigned char c = (unsigned char) ((f1 << 6) | (f2 << 3) | f3); - cbuf.insts()->emit_int8(c); -} - -// EMIT_CC() -void emit_cc(CodeBuffer &cbuf, int f1, int f2) { - unsigned char c = (unsigned char) (f1 | f2); - cbuf.insts()->emit_int8(c); -} - -// EMIT_OPCODE() -void emit_opcode(CodeBuffer &cbuf, int code) { - cbuf.insts()->emit_int8((unsigned char) code); -} - -// EMIT_OPCODE() w/ relocation information -void emit_opcode(CodeBuffer &cbuf, - int code, relocInfo::relocType reloc, int offset, int format) -{ - cbuf.relocate(cbuf.insts_mark() + offset, reloc, format); - emit_opcode(cbuf, code); -} - -// EMIT_D8() -void emit_d8(CodeBuffer &cbuf, int d8) { - cbuf.insts()->emit_int8((unsigned char) d8); -} - -// EMIT_D16() -void emit_d16(CodeBuffer &cbuf, int d16) { - cbuf.insts()->emit_int16(d16); -} - -// EMIT_D32() -void emit_d32(CodeBuffer &cbuf, int d32) { - cbuf.insts()->emit_int32(d32); -} - -// EMIT_D64() -void emit_d64(CodeBuffer &cbuf, int64_t d64) { - cbuf.insts()->emit_int64(d64); -} - -// emit 32 bit value and construct relocation entry from relocInfo::relocType -void emit_d32_reloc(CodeBuffer& cbuf, - int d32, - relocInfo::relocType reloc, - int format) -{ - assert(reloc != relocInfo::external_word_type, "use 2-arg emit_d32_reloc"); - cbuf.relocate(cbuf.insts_mark(), reloc, format); - cbuf.insts()->emit_int32(d32); -} - -// emit 32 bit value and construct relocation entry from RelocationHolder -void emit_d32_reloc(CodeBuffer& cbuf, int d32, RelocationHolder const& rspec, int format) { -#ifdef ASSERT - if (rspec.reloc()->type() == relocInfo::oop_type && - d32 != 0 && d32 != (intptr_t) Universe::non_oop_word()) { - assert(Universe::heap()->is_in((address)(intptr_t)d32), "should be real oop"); - assert(oopDesc::is_oop(cast_to_oop((intptr_t)d32)), "cannot embed broken oops in code"); - } -#endif - cbuf.relocate(cbuf.insts_mark(), rspec, format); - cbuf.insts()->emit_int32(d32); -} - -void emit_d32_reloc(CodeBuffer& cbuf, address addr) { - address next_ip = cbuf.insts_end() + 4; - emit_d32_reloc(cbuf, (int) (addr - next_ip), - external_word_Relocation::spec(addr), - RELOC_DISP32); -} - - -// emit 64 bit value and construct relocation entry from relocInfo::relocType -void emit_d64_reloc(CodeBuffer& cbuf, int64_t d64, relocInfo::relocType reloc, int format) { - cbuf.relocate(cbuf.insts_mark(), reloc, format); - cbuf.insts()->emit_int64(d64); -} - -// emit 64 bit value and construct relocation entry from RelocationHolder -void emit_d64_reloc(CodeBuffer& cbuf, int64_t d64, RelocationHolder const& rspec, int format) { -#ifdef ASSERT - if (rspec.reloc()->type() == relocInfo::oop_type && - d64 != 0 && d64 != (int64_t) Universe::non_oop_word()) { - assert(Universe::heap()->is_in((address)d64), "should be real oop"); - assert(oopDesc::is_oop(cast_to_oop(d64)), "cannot embed broken oops in code"); - } -#endif - cbuf.relocate(cbuf.insts_mark(), rspec, format); - cbuf.insts()->emit_int64(d64); -} - -// Access stack slot for load or store -void store_to_stackslot(CodeBuffer &cbuf, int opcode, int rm_field, int disp) -{ - emit_opcode(cbuf, opcode); // (e.g., FILD [RSP+src]) - if (-0x80 <= disp && disp < 0x80) { - emit_rm(cbuf, 0x01, rm_field, RSP_enc); // R/M byte - emit_rm(cbuf, 0x00, RSP_enc, RSP_enc); // SIB byte - emit_d8(cbuf, disp); // Displacement // R/M byte - } else { - emit_rm(cbuf, 0x02, rm_field, RSP_enc); // R/M byte - emit_rm(cbuf, 0x00, RSP_enc, RSP_enc); // SIB byte - emit_d32(cbuf, disp); // Displacement // R/M byte - } -} - - // rRegI ereg, memory mem) %{ // emit_reg_mem -void encode_RegMem(CodeBuffer &cbuf, - int reg, - int base, int index, int scale, int disp, relocInfo::relocType disp_reloc) -{ - assert(disp_reloc == relocInfo::none, "cannot have disp"); - int regenc = reg & 7; - int baseenc = base & 7; - int indexenc = index & 7; - - // There is no index & no scale, use form without SIB byte - if (index == 0x4 && scale == 0 && base != RSP_enc && base != R12_enc) { - // If no displacement, mode is 0x0; unless base is [RBP] or [R13] - if (disp == 0 && base != RBP_enc && base != R13_enc) { - emit_rm(cbuf, 0x0, regenc, baseenc); // * - } else if (-0x80 <= disp && disp < 0x80 && disp_reloc == relocInfo::none) { - // If 8-bit displacement, mode 0x1 - emit_rm(cbuf, 0x1, regenc, baseenc); // * - emit_d8(cbuf, disp); - } else { - // If 32-bit displacement - if (base == -1) { // Special flag for absolute address - emit_rm(cbuf, 0x0, regenc, 0x5); // * - if (disp_reloc != relocInfo::none) { - emit_d32_reloc(cbuf, disp, relocInfo::oop_type, RELOC_DISP32); - } else { - emit_d32(cbuf, disp); - } - } else { - // Normal base + offset - emit_rm(cbuf, 0x2, regenc, baseenc); // * - if (disp_reloc != relocInfo::none) { - emit_d32_reloc(cbuf, disp, relocInfo::oop_type, RELOC_DISP32); - } else { - emit_d32(cbuf, disp); - } - } - } - } else { - // Else, encode with the SIB byte - // If no displacement, mode is 0x0; unless base is [RBP] or [R13] - if (disp == 0 && base != RBP_enc && base != R13_enc) { - // If no displacement - emit_rm(cbuf, 0x0, regenc, 0x4); // * - emit_rm(cbuf, scale, indexenc, baseenc); - } else { - if (-0x80 <= disp && disp < 0x80 && disp_reloc == relocInfo::none) { - // If 8-bit displacement, mode 0x1 - emit_rm(cbuf, 0x1, regenc, 0x4); // * - emit_rm(cbuf, scale, indexenc, baseenc); - emit_d8(cbuf, disp); - } else { - // If 32-bit displacement - if (base == 0x04 ) { - emit_rm(cbuf, 0x2, regenc, 0x4); - emit_rm(cbuf, scale, indexenc, 0x04); // XXX is this valid??? - } else { - emit_rm(cbuf, 0x2, regenc, 0x4); - emit_rm(cbuf, scale, indexenc, baseenc); // * - } - if (disp_reloc != relocInfo::none) { - emit_d32_reloc(cbuf, disp, relocInfo::oop_type, RELOC_DISP32); - } else { - emit_d32(cbuf, disp); - } - } - } - } -} - // This could be in MacroAssembler but it's fairly C2 specific void emit_cmpfp_fixup(MacroAssembler& _masm) { Label exit; @@ -996,20 +815,10 @@ void MachEpilogNode::emit(CodeBuffer& cbuf, PhaseRegAlloc* ra_) const // Note that VerifyStackAtCalls' Majik cookie does not change the frame size popped here if (framesize) { - emit_opcode(cbuf, Assembler::REX_W); - if (framesize < 0x80) { - emit_opcode(cbuf, 0x83); // addq rsp, #framesize - emit_rm(cbuf, 0x3, 0x00, RSP_enc); - emit_d8(cbuf, framesize); - } else { - emit_opcode(cbuf, 0x81); // addq rsp, #framesize - emit_rm(cbuf, 0x3, 0x00, RSP_enc); - emit_d32(cbuf, framesize); - } + __ addq(rsp, framesize); } - // popq rbp - emit_opcode(cbuf, 0x58 | RBP_enc); + __ popq(rbp); if (StackReservedPages > 0 && C->has_reserved_stack_access()) { __ reserved_stack_check(); @@ -1646,19 +1455,9 @@ void BoxLockNode::emit(CodeBuffer& cbuf, PhaseRegAlloc* ra_) const { int offset = ra_->reg2offset(in_RegMask(0).find_first_elem()); int reg = ra_->get_encode(this); - if (offset >= 0x80) { - emit_opcode(cbuf, reg < 8 ? Assembler::REX_W : Assembler::REX_WR); - emit_opcode(cbuf, 0x8D); // LEA reg,[SP+offset] - emit_rm(cbuf, 0x2, reg & 7, 0x04); - emit_rm(cbuf, 0x0, 0x04, RSP_enc); - emit_d32(cbuf, offset); - } else { - emit_opcode(cbuf, reg < 8 ? Assembler::REX_W : Assembler::REX_WR); - emit_opcode(cbuf, 0x8D); // LEA reg,[SP+offset] - emit_rm(cbuf, 0x1, reg & 7, 0x04); - emit_rm(cbuf, 0x0, 0x04, RSP_enc); - emit_d8(cbuf, offset); - } + + MacroAssembler masm(&cbuf); + masm.lea(as_Register(reg), Address(rsp, offset)); } uint BoxLockNode::size(PhaseRegAlloc *ra_) const @@ -1857,60 +1656,6 @@ const RegMask Matcher::method_handle_invoke_SP_save_mask() { // tertiary opcode. Only the opcode sections which a particular // instruction needs for encoding need to be specified. encode %{ - // Build emit functions for each basic byte or larger field in the - // intel encoding scheme (opcode, rm, sib, immediate), and call them - // from C++ code in the enc_class source block. Emit functions will - // live in the main source block for now. In future, we can - // generalize this by adding a syntax that specifies the sizes of - // fields in an order, so that the adlc can build the emit functions - // automagically - - // Emit primary opcode - enc_class OpcP - %{ - emit_opcode(cbuf, $primary); - %} - - // Emit secondary opcode - enc_class OpcS - %{ - emit_opcode(cbuf, $secondary); - %} - - // Emit tertiary opcode - enc_class OpcT - %{ - emit_opcode(cbuf, $tertiary); - %} - - // Emit opcode directly - enc_class Opcode(immI d8) - %{ - emit_opcode(cbuf, $d8$$constant); - %} - - // Emit size prefix - enc_class SizePrefix - %{ - emit_opcode(cbuf, 0x66); - %} - - enc_class reg(rRegI reg) - %{ - emit_rm(cbuf, 0x3, 0, $reg$$reg & 7); - %} - - enc_class reg_reg(rRegI dst, rRegI src) - %{ - emit_rm(cbuf, 0x3, $dst$$reg & 7, $src$$reg & 7); - %} - - enc_class opc_reg_reg(immI opcode, rRegI dst, rRegI src) - %{ - emit_opcode(cbuf, $opcode$$constant); - emit_rm(cbuf, 0x3, $dst$$reg & 7, $src$$reg & 7); - %} - enc_class cdql_enc(no_rax_rdx_RegI div) %{ // Full implementation of Java idiv and irem; checks for @@ -2028,97 +1773,6 @@ encode %{ __ bind(done); %} - // Opcde enc_class for 8/32 bit immediate instructions with sign-extension - enc_class OpcSE(immI imm) - %{ - // Emit primary opcode and set sign-extend bit - // Check for 8-bit immediate, and set sign extend bit in opcode - if (-0x80 <= $imm$$constant && $imm$$constant < 0x80) { - emit_opcode(cbuf, $primary | 0x02); - } else { - // 32-bit immediate - emit_opcode(cbuf, $primary); - } - %} - - enc_class OpcSErm(rRegI dst, immI imm) - %{ - // OpcSEr/m - int dstenc = $dst$$reg; - if (dstenc >= 8) { - emit_opcode(cbuf, Assembler::REX_B); - dstenc -= 8; - } - // Emit primary opcode and set sign-extend bit - // Check for 8-bit immediate, and set sign extend bit in opcode - if (-0x80 <= $imm$$constant && $imm$$constant < 0x80) { - emit_opcode(cbuf, $primary | 0x02); - } else { - // 32-bit immediate - emit_opcode(cbuf, $primary); - } - // Emit r/m byte with secondary opcode, after primary opcode. - emit_rm(cbuf, 0x3, $secondary, dstenc); - %} - - enc_class OpcSErm_wide(rRegL dst, immI imm) - %{ - // OpcSEr/m - int dstenc = $dst$$reg; - if (dstenc < 8) { - emit_opcode(cbuf, Assembler::REX_W); - } else { - emit_opcode(cbuf, Assembler::REX_WB); - dstenc -= 8; - } - // Emit primary opcode and set sign-extend bit - // Check for 8-bit immediate, and set sign extend bit in opcode - if (-0x80 <= $imm$$constant && $imm$$constant < 0x80) { - emit_opcode(cbuf, $primary | 0x02); - } else { - // 32-bit immediate - emit_opcode(cbuf, $primary); - } - // Emit r/m byte with secondary opcode, after primary opcode. - emit_rm(cbuf, 0x3, $secondary, dstenc); - %} - - enc_class Con8or32(immI imm) - %{ - // Check for 8-bit immediate, and set sign extend bit in opcode - if (-0x80 <= $imm$$constant && $imm$$constant < 0x80) { - $$$emit8$imm$$constant; - } else { - // 32-bit immediate - $$$emit32$imm$$constant; - } - %} - - enc_class opc2_reg(rRegI dst) - %{ - // BSWAP - emit_cc(cbuf, $secondary, $dst$$reg); - %} - - enc_class opc3_reg(rRegI dst) - %{ - // BSWAP - emit_cc(cbuf, $tertiary, $dst$$reg); - %} - - enc_class reg_opc(rRegI div) - %{ - // INC, DEC, IDIV, IMOD, JMP indirect, ... - emit_rm(cbuf, 0x3, $secondary, $div$$reg & 7); - %} - - enc_class enc_cmov(cmpOp cop) - %{ - // CMOV - $$$emit8$primary; - emit_cc(cbuf, $secondary, $cop$$cmpcode); - %} - enc_class enc_PartialSubtypeCheck() %{ Register Rrdi = as_Register(RDI_enc); // result register @@ -2165,30 +1819,25 @@ encode %{ // CALL to fixup routine. Fixup routine uses ScopeDesc info to // determine who we intended to call. MacroAssembler _masm(&cbuf); - cbuf.set_insts_mark(); if (!_method) { - $$$emit8$primary; - emit_d32_reloc(cbuf, (int) ($meth$$method - ((intptr_t) cbuf.insts_end()) - 4), - runtime_call_Relocation::spec(), - RELOC_DISP32); + __ call(RuntimeAddress(CAST_FROM_FN_PTR(address, $meth$$method))); } else if (_method->intrinsic_id() == vmIntrinsicID::_ensureMaterializedForStackWalk) { // The NOP here is purely to ensure that eliding a call to // JVM_EnsureMaterializedForStackWalk doesn't change the code size. __ addr_nop_5(); __ block_comment("call JVM_EnsureMaterializedForStackWalk (elided)"); } else { - $$$emit8$primary; int method_index = resolved_method_index(cbuf); RelocationHolder rspec = _optimized_virtual ? opt_virtual_call_Relocation::spec(method_index) : static_call_Relocation::spec(method_index); - emit_d32_reloc(cbuf, (int) ($meth$$method - ((intptr_t) cbuf.insts_end()) - 4), - rspec, RELOC_DISP32); - address mark = cbuf.insts_mark(); + address mark = __ pc(); + int call_offset = __ offset(); + __ call(AddressLiteral(CAST_FROM_FN_PTR(address, $meth$$method), rspec)); if (CodeBuffer::supports_shared_stubs() && _method->can_be_statically_bound()) { // Calls of the same statically bound method can share // a stub to the interpreter. - cbuf.shared_stub_to_interp_for(_method, cbuf.insts()->mark_off()); + cbuf.shared_stub_to_interp_for(_method, call_offset); } else { // Emit stubs for static call. address stub = CompiledStaticCall::emit_to_interp_stub(cbuf, mark); @@ -2198,7 +1847,6 @@ encode %{ } } } - _masm.clear_inst_mark(); __ post_call_nop(); %} @@ -2208,529 +1856,6 @@ encode %{ __ post_call_nop(); %} - enc_class reg_opc_imm(rRegI dst, immI8 shift) - %{ - // SAL, SAR, SHR - int dstenc = $dst$$reg; - if (dstenc >= 8) { - emit_opcode(cbuf, Assembler::REX_B); - dstenc -= 8; - } - $$$emit8$primary; - emit_rm(cbuf, 0x3, $secondary, dstenc); - $$$emit8$shift$$constant; - %} - - enc_class reg_opc_imm_wide(rRegL dst, immI8 shift) - %{ - // SAL, SAR, SHR - int dstenc = $dst$$reg; - if (dstenc < 8) { - emit_opcode(cbuf, Assembler::REX_W); - } else { - emit_opcode(cbuf, Assembler::REX_WB); - dstenc -= 8; - } - $$$emit8$primary; - emit_rm(cbuf, 0x3, $secondary, dstenc); - $$$emit8$shift$$constant; - %} - - enc_class load_immI(rRegI dst, immI src) - %{ - int dstenc = $dst$$reg; - if (dstenc >= 8) { - emit_opcode(cbuf, Assembler::REX_B); - dstenc -= 8; - } - emit_opcode(cbuf, 0xB8 | dstenc); - $$$emit32$src$$constant; - %} - - enc_class load_immL(rRegL dst, immL src) - %{ - int dstenc = $dst$$reg; - if (dstenc < 8) { - emit_opcode(cbuf, Assembler::REX_W); - } else { - emit_opcode(cbuf, Assembler::REX_WB); - dstenc -= 8; - } - emit_opcode(cbuf, 0xB8 | dstenc); - emit_d64(cbuf, $src$$constant); - %} - - enc_class load_immUL32(rRegL dst, immUL32 src) - %{ - // same as load_immI, but this time we care about zeroes in the high word - int dstenc = $dst$$reg; - if (dstenc >= 8) { - emit_opcode(cbuf, Assembler::REX_B); - dstenc -= 8; - } - emit_opcode(cbuf, 0xB8 | dstenc); - $$$emit32$src$$constant; - %} - - enc_class load_immL32(rRegL dst, immL32 src) - %{ - int dstenc = $dst$$reg; - if (dstenc < 8) { - emit_opcode(cbuf, Assembler::REX_W); - } else { - emit_opcode(cbuf, Assembler::REX_WB); - dstenc -= 8; - } - emit_opcode(cbuf, 0xC7); - emit_rm(cbuf, 0x03, 0x00, dstenc); - $$$emit32$src$$constant; - %} - - enc_class load_immP31(rRegP dst, immP32 src) - %{ - // same as load_immI, but this time we care about zeroes in the high word - int dstenc = $dst$$reg; - if (dstenc >= 8) { - emit_opcode(cbuf, Assembler::REX_B); - dstenc -= 8; - } - emit_opcode(cbuf, 0xB8 | dstenc); - $$$emit32$src$$constant; - %} - - enc_class load_immP(rRegP dst, immP src) - %{ - int dstenc = $dst$$reg; - if (dstenc < 8) { - emit_opcode(cbuf, Assembler::REX_W); - } else { - emit_opcode(cbuf, Assembler::REX_WB); - dstenc -= 8; - } - emit_opcode(cbuf, 0xB8 | dstenc); - // This next line should be generated from ADLC - if ($src->constant_reloc() != relocInfo::none) { - emit_d64_reloc(cbuf, $src$$constant, $src->constant_reloc(), RELOC_IMM64); - } else { - emit_d64(cbuf, $src$$constant); - } - %} - - enc_class Con32(immI src) - %{ - // Output immediate - $$$emit32$src$$constant; - %} - - enc_class Con32F_as_bits(immF src) - %{ - // Output Float immediate bits - jfloat jf = $src$$constant; - jint jf_as_bits = jint_cast(jf); - emit_d32(cbuf, jf_as_bits); - %} - - enc_class Con16(immI src) - %{ - // Output immediate - $$$emit16$src$$constant; - %} - - // How is this different from Con32??? XXX - enc_class Con_d32(immI src) - %{ - emit_d32(cbuf,$src$$constant); - %} - - enc_class conmemref (rRegP t1) %{ // Con32(storeImmI) - // Output immediate memory reference - emit_rm(cbuf, 0x00, $t1$$reg, 0x05 ); - emit_d32(cbuf, 0x00); - %} - - enc_class lock_prefix() - %{ - emit_opcode(cbuf, 0xF0); // lock - %} - - enc_class REX_mem(memory mem) - %{ - if ($mem$$base >= 8) { - if ($mem$$index < 8) { - emit_opcode(cbuf, Assembler::REX_B); - } else { - emit_opcode(cbuf, Assembler::REX_XB); - } - } else { - if ($mem$$index >= 8) { - emit_opcode(cbuf, Assembler::REX_X); - } - } - %} - - enc_class REX_mem_wide(memory mem) - %{ - if ($mem$$base >= 8) { - if ($mem$$index < 8) { - emit_opcode(cbuf, Assembler::REX_WB); - } else { - emit_opcode(cbuf, Assembler::REX_WXB); - } - } else { - if ($mem$$index < 8) { - emit_opcode(cbuf, Assembler::REX_W); - } else { - emit_opcode(cbuf, Assembler::REX_WX); - } - } - %} - - // for byte regs - enc_class REX_breg(rRegI reg) - %{ - if ($reg$$reg >= 4) { - emit_opcode(cbuf, $reg$$reg < 8 ? Assembler::REX : Assembler::REX_B); - } - %} - - // for byte regs - enc_class REX_reg_breg(rRegI dst, rRegI src) - %{ - if ($dst$$reg < 8) { - if ($src$$reg >= 4) { - emit_opcode(cbuf, $src$$reg < 8 ? Assembler::REX : Assembler::REX_B); - } - } else { - if ($src$$reg < 8) { - emit_opcode(cbuf, Assembler::REX_R); - } else { - emit_opcode(cbuf, Assembler::REX_RB); - } - } - %} - - // for byte regs - enc_class REX_breg_mem(rRegI reg, memory mem) - %{ - if ($reg$$reg < 8) { - if ($mem$$base < 8) { - if ($mem$$index >= 8) { - emit_opcode(cbuf, Assembler::REX_X); - } else if ($reg$$reg >= 4) { - emit_opcode(cbuf, Assembler::REX); - } - } else { - if ($mem$$index < 8) { - emit_opcode(cbuf, Assembler::REX_B); - } else { - emit_opcode(cbuf, Assembler::REX_XB); - } - } - } else { - if ($mem$$base < 8) { - if ($mem$$index < 8) { - emit_opcode(cbuf, Assembler::REX_R); - } else { - emit_opcode(cbuf, Assembler::REX_RX); - } - } else { - if ($mem$$index < 8) { - emit_opcode(cbuf, Assembler::REX_RB); - } else { - emit_opcode(cbuf, Assembler::REX_RXB); - } - } - } - %} - - enc_class REX_reg(rRegI reg) - %{ - if ($reg$$reg >= 8) { - emit_opcode(cbuf, Assembler::REX_B); - } - %} - - enc_class REX_reg_wide(rRegI reg) - %{ - if ($reg$$reg < 8) { - emit_opcode(cbuf, Assembler::REX_W); - } else { - emit_opcode(cbuf, Assembler::REX_WB); - } - %} - - enc_class REX_reg_reg(rRegI dst, rRegI src) - %{ - if ($dst$$reg < 8) { - if ($src$$reg >= 8) { - emit_opcode(cbuf, Assembler::REX_B); - } - } else { - if ($src$$reg < 8) { - emit_opcode(cbuf, Assembler::REX_R); - } else { - emit_opcode(cbuf, Assembler::REX_RB); - } - } - %} - - enc_class REX_reg_reg_wide(rRegI dst, rRegI src) - %{ - if ($dst$$reg < 8) { - if ($src$$reg < 8) { - emit_opcode(cbuf, Assembler::REX_W); - } else { - emit_opcode(cbuf, Assembler::REX_WB); - } - } else { - if ($src$$reg < 8) { - emit_opcode(cbuf, Assembler::REX_WR); - } else { - emit_opcode(cbuf, Assembler::REX_WRB); - } - } - %} - - enc_class REX_reg_mem(rRegI reg, memory mem) - %{ - if ($reg$$reg < 8) { - if ($mem$$base < 8) { - if ($mem$$index >= 8) { - emit_opcode(cbuf, Assembler::REX_X); - } - } else { - if ($mem$$index < 8) { - emit_opcode(cbuf, Assembler::REX_B); - } else { - emit_opcode(cbuf, Assembler::REX_XB); - } - } - } else { - if ($mem$$base < 8) { - if ($mem$$index < 8) { - emit_opcode(cbuf, Assembler::REX_R); - } else { - emit_opcode(cbuf, Assembler::REX_RX); - } - } else { - if ($mem$$index < 8) { - emit_opcode(cbuf, Assembler::REX_RB); - } else { - emit_opcode(cbuf, Assembler::REX_RXB); - } - } - } - %} - - enc_class REX_reg_mem_wide(rRegL reg, memory mem) - %{ - if ($reg$$reg < 8) { - if ($mem$$base < 8) { - if ($mem$$index < 8) { - emit_opcode(cbuf, Assembler::REX_W); - } else { - emit_opcode(cbuf, Assembler::REX_WX); - } - } else { - if ($mem$$index < 8) { - emit_opcode(cbuf, Assembler::REX_WB); - } else { - emit_opcode(cbuf, Assembler::REX_WXB); - } - } - } else { - if ($mem$$base < 8) { - if ($mem$$index < 8) { - emit_opcode(cbuf, Assembler::REX_WR); - } else { - emit_opcode(cbuf, Assembler::REX_WRX); - } - } else { - if ($mem$$index < 8) { - emit_opcode(cbuf, Assembler::REX_WRB); - } else { - emit_opcode(cbuf, Assembler::REX_WRXB); - } - } - } - %} - - enc_class reg_mem(rRegI ereg, memory mem) - %{ - // High registers handle in encode_RegMem - int reg = $ereg$$reg; - int base = $mem$$base; - int index = $mem$$index; - int scale = $mem$$scale; - int disp = $mem$$disp; - relocInfo::relocType disp_reloc = $mem->disp_reloc(); - - encode_RegMem(cbuf, reg, base, index, scale, disp, disp_reloc); - %} - - enc_class RM_opc_mem(immI rm_opcode, memory mem) - %{ - int rm_byte_opcode = $rm_opcode$$constant; - - // High registers handle in encode_RegMem - int base = $mem$$base; - int index = $mem$$index; - int scale = $mem$$scale; - int displace = $mem$$disp; - - relocInfo::relocType disp_reloc = $mem->disp_reloc(); // disp-as-oop when - // working with static - // globals - encode_RegMem(cbuf, rm_byte_opcode, base, index, scale, displace, - disp_reloc); - %} - - enc_class reg_lea(rRegI dst, rRegI src0, immI src1) - %{ - int reg_encoding = $dst$$reg; - int base = $src0$$reg; // 0xFFFFFFFF indicates no base - int index = 0x04; // 0x04 indicates no index - int scale = 0x00; // 0x00 indicates no scale - int displace = $src1$$constant; // 0x00 indicates no displacement - relocInfo::relocType disp_reloc = relocInfo::none; - encode_RegMem(cbuf, reg_encoding, base, index, scale, displace, - disp_reloc); - %} - - enc_class neg_reg(rRegI dst) - %{ - int dstenc = $dst$$reg; - if (dstenc >= 8) { - emit_opcode(cbuf, Assembler::REX_B); - dstenc -= 8; - } - // NEG $dst - emit_opcode(cbuf, 0xF7); - emit_rm(cbuf, 0x3, 0x03, dstenc); - %} - - enc_class neg_reg_wide(rRegI dst) - %{ - int dstenc = $dst$$reg; - if (dstenc < 8) { - emit_opcode(cbuf, Assembler::REX_W); - } else { - emit_opcode(cbuf, Assembler::REX_WB); - dstenc -= 8; - } - // NEG $dst - emit_opcode(cbuf, 0xF7); - emit_rm(cbuf, 0x3, 0x03, dstenc); - %} - - enc_class setLT_reg(rRegI dst) - %{ - int dstenc = $dst$$reg; - if (dstenc >= 8) { - emit_opcode(cbuf, Assembler::REX_B); - dstenc -= 8; - } else if (dstenc >= 4) { - emit_opcode(cbuf, Assembler::REX); - } - // SETLT $dst - emit_opcode(cbuf, 0x0F); - emit_opcode(cbuf, 0x9C); - emit_rm(cbuf, 0x3, 0x0, dstenc); - %} - - enc_class setNZ_reg(rRegI dst) - %{ - int dstenc = $dst$$reg; - if (dstenc >= 8) { - emit_opcode(cbuf, Assembler::REX_B); - dstenc -= 8; - } else if (dstenc >= 4) { - emit_opcode(cbuf, Assembler::REX); - } - // SETNZ $dst - emit_opcode(cbuf, 0x0F); - emit_opcode(cbuf, 0x95); - emit_rm(cbuf, 0x3, 0x0, dstenc); - %} - - - // Compare the lonogs and set -1, 0, or 1 into dst - enc_class cmpl3_flag(rRegL src1, rRegL src2, rRegI dst) - %{ - int src1enc = $src1$$reg; - int src2enc = $src2$$reg; - int dstenc = $dst$$reg; - - // cmpq $src1, $src2 - if (src1enc < 8) { - if (src2enc < 8) { - emit_opcode(cbuf, Assembler::REX_W); - } else { - emit_opcode(cbuf, Assembler::REX_WB); - } - } else { - if (src2enc < 8) { - emit_opcode(cbuf, Assembler::REX_WR); - } else { - emit_opcode(cbuf, Assembler::REX_WRB); - } - } - emit_opcode(cbuf, 0x3B); - emit_rm(cbuf, 0x3, src1enc & 7, src2enc & 7); - - // movl $dst, -1 - if (dstenc >= 8) { - emit_opcode(cbuf, Assembler::REX_B); - } - emit_opcode(cbuf, 0xB8 | (dstenc & 7)); - emit_d32(cbuf, -1); - - // jl,s done - emit_opcode(cbuf, 0x7C); - emit_d8(cbuf, dstenc < 4 ? 0x06 : 0x08); - - // setne $dst - if (dstenc >= 4) { - emit_opcode(cbuf, dstenc < 8 ? Assembler::REX : Assembler::REX_B); - } - emit_opcode(cbuf, 0x0F); - emit_opcode(cbuf, 0x95); - emit_opcode(cbuf, 0xC0 | (dstenc & 7)); - - // movzbl $dst, $dst - if (dstenc >= 4) { - emit_opcode(cbuf, dstenc < 8 ? Assembler::REX : Assembler::REX_RB); - } - emit_opcode(cbuf, 0x0F); - emit_opcode(cbuf, 0xB6); - emit_rm(cbuf, 0x3, dstenc & 7, dstenc & 7); - %} - - enc_class Push_ResultXD(regD dst) %{ - MacroAssembler _masm(&cbuf); - __ fstp_d(Address(rsp, 0)); - __ movdbl($dst$$XMMRegister, Address(rsp, 0)); - __ addptr(rsp, 8); - %} - - enc_class Push_SrcXD(regD src) %{ - MacroAssembler _masm(&cbuf); - __ subptr(rsp, 8); - __ movdbl(Address(rsp, 0), $src$$XMMRegister); - __ fld_d(Address(rsp, 0)); - %} - - - enc_class enc_rethrow() - %{ - cbuf.set_insts_mark(); - emit_opcode(cbuf, 0xE9); // jmp entry - emit_d32_reloc(cbuf, - (int) (OptoRuntime::rethrow_stub() - cbuf.insts_end() - 4), - runtime_call_Relocation::spec(), - RELOC_DISP32); - %} - %} @@ -4588,17 +3713,6 @@ pipe_class pipe_cmov_reg_long( rFlagsReg cr, rRegL dst, rRegL src) DECODE : S0(2); // any 2 decoders %} -// XXX -// // Conditional move double reg-reg -// pipe_class pipe_cmovD_reg( rFlagsReg cr, regDPR1 dst, regD src) -// %{ -// single_instruction; -// dst : S4(write); -// src : S3(read); -// cr : S3(read); -// DECODE : S0; // any decoder -// %} - // Float reg-reg operation pipe_class fpu_reg(regD dst) %{ @@ -5909,7 +5023,7 @@ instruct loadConD0(regD dst, immD0 src) format %{ "xorpd $dst, $dst\t# double 0.0" %} ins_encode %{ - __ xorpd ($dst$$XMMRegister, $dst$$XMMRegister); + __ xorpd($dst$$XMMRegister, $dst$$XMMRegister); %} ins_pipe(pipe_slow); %} @@ -5920,8 +5034,9 @@ instruct loadSSI(rRegI dst, stackSlotI src) ins_cost(125); format %{ "movl $dst, $src\t# int stk" %} - opcode(0x8B); - ins_encode(REX_reg_mem(dst, src), OpcP, reg_mem(dst, src)); + ins_encode %{ + __ movl($dst$$Register, $src$$Address); + %} ins_pipe(ialu_reg_mem); %} @@ -5931,8 +5046,9 @@ instruct loadSSL(rRegL dst, stackSlotL src) ins_cost(125); format %{ "movq $dst, $src\t# long stk" %} - opcode(0x8B); - ins_encode(REX_reg_mem_wide(dst, src), OpcP, reg_mem(dst, src)); + ins_encode %{ + __ movq($dst$$Register, $src$$Address); + %} ins_pipe(ialu_reg_mem); %} @@ -5942,8 +5058,9 @@ instruct loadSSP(rRegP dst, stackSlotP src) ins_cost(125); format %{ "movq $dst, $src\t# ptr stk" %} - opcode(0x8B); - ins_encode(REX_reg_mem_wide(dst, src), OpcP, reg_mem(dst, src)); + ins_encode %{ + __ movq($dst$$Register, $src$$Address); + %} ins_pipe(ialu_reg_mem); %} @@ -6401,8 +5518,9 @@ instruct storeSSI(stackSlotI dst, rRegI src) ins_cost(100); format %{ "movl $dst, $src\t# int stk" %} - opcode(0x89); - ins_encode(REX_reg_mem(src, dst), OpcP, reg_mem(src, dst)); + ins_encode %{ + __ movl($dst$$Address, $src$$Register); + %} ins_pipe( ialu_mem_reg ); %} @@ -6412,8 +5530,9 @@ instruct storeSSL(stackSlotL dst, rRegL src) ins_cost(100); format %{ "movq $dst, $src\t# long stk" %} - opcode(0x89); - ins_encode(REX_reg_mem_wide(src, dst), OpcP, reg_mem(src, dst)); + ins_encode %{ + __ movq($dst$$Address, $src$$Register); + %} ins_pipe(ialu_mem_reg); %} @@ -6423,8 +5542,9 @@ instruct storeSSP(stackSlotP dst, rRegP src) ins_cost(100); format %{ "movq $dst, $src\t# ptr stk" %} - opcode(0x89); - ins_encode(REX_reg_mem_wide(src, dst), OpcP, reg_mem(src, dst)); + ins_encode %{ + __ movq($dst$$Address, $src$$Register); + %} ins_pipe(ialu_mem_reg); %} @@ -7379,33 +6499,6 @@ instruct cmovP_regUCF2_eq(cmpOpUCF2 cop, rFlagsRegUCF cr, rRegP dst, rRegP src) ins_pipe(pipe_cmov_reg); %} -// DISABLED: Requires the ADLC to emit a bottom_type call that -// correctly meets the two pointer arguments; one is an incoming -// register but the other is a memory operand. ALSO appears to -// be buggy with implicit null checks. -// -//// Conditional move -//instruct cmovP_mem(cmpOp cop, rFlagsReg cr, rRegP dst, memory src) -//%{ -// match(Set dst (CMoveP (Binary cop cr) (Binary dst (LoadP src)))); -// ins_cost(250); -// format %{ "CMOV$cop $dst,$src\t# ptr" %} -// opcode(0x0F,0x40); -// ins_encode( enc_cmov(cop), reg_mem( dst, src ) ); -// ins_pipe( pipe_cmov_mem ); -//%} -// -//// Conditional move -//instruct cmovP_memU(cmpOpU cop, rFlagsRegU cr, rRegP dst, memory src) -//%{ -// match(Set dst (CMoveP (Binary cop cr) (Binary dst (LoadP src)))); -// ins_cost(250); -// format %{ "CMOV$cop $dst,$src\t# ptr" %} -// opcode(0x0F,0x40); -// ins_encode( enc_cmov(cop), reg_mem( dst, src ) ); -// ins_pipe( pipe_cmov_mem ); -//%} - instruct cmovL_imm_01(rRegL dst, immI_1 src, rFlagsReg cr, cmpOp cop) %{ predicate(n->in(2)->in(2)->is_Con() && n->in(2)->in(2)->get_long() == 0); @@ -7560,18 +6653,6 @@ instruct cmovF_reg(cmpOp cop, rFlagsReg cr, regF dst, regF src) ins_pipe(pipe_slow); %} -// instruct cmovF_mem(cmpOp cop, rFlagsReg cr, regF dst, memory src) -// %{ -// match(Set dst (CMoveF (Binary cop cr) (Binary dst (LoadL src)))); - -// ins_cost(200); // XXX -// format %{ "jn$cop skip\t# signed cmove float\n\t" -// "movss $dst, $src\n" -// "skip:" %} -// ins_encode(enc_cmovf_mem_branch(cop, dst, src)); -// ins_pipe(pipe_slow); -// %} - instruct cmovF_regU(cmpOpU cop, rFlagsRegU cr, regF dst, regF src) %{ match(Set dst (CMoveF (Binary cop cr) (Binary dst src))); @@ -8652,8 +7733,9 @@ instruct subP_rReg(rRegP dst, rRegI src, immI_0 zero, rFlagsReg cr) effect(KILL cr); format %{ "subq $dst, $src\t# ptr - int" %} - opcode(0x2B); - ins_encode(REX_reg_reg_wide(dst, src), OpcP, reg_reg(dst, src)); + ins_encode %{ + __ subq($dst$$Register, $src$$Register); + %} ins_pipe(ialu_reg_reg); %} @@ -11305,23 +10387,6 @@ instruct convI2L_reg_reg(rRegL dst, rRegI src) ins_pipe(ialu_reg_reg); %} -// instruct convI2L_reg_reg_foo(rRegL dst, rRegI src) -// %{ -// match(Set dst (ConvI2L src)); -// // predicate(_kids[0]->_leaf->as_Type()->type()->is_int()->_lo >= 0 && -// // _kids[0]->_leaf->as_Type()->type()->is_int()->_hi >= 0); -// predicate(((const TypeNode*) n)->type()->is_long()->_hi == -// (unsigned int) ((const TypeNode*) n)->type()->is_long()->_hi && -// ((const TypeNode*) n)->type()->is_long()->_lo == -// (unsigned int) ((const TypeNode*) n)->type()->is_long()->_lo); - -// format %{ "movl $dst, $src\t# unsigned i2l" %} -// ins_encode(enc_copy(dst, src)); -// // opcode(0x63); // needs REX.W -// // ins_encode(REX_reg_reg_wide(dst, src), OpcP, reg_reg(dst,src)); -// ins_pipe(ialu_reg_reg); -// %} - // Zero-extend convert int to long instruct convI2L_reg_reg_zex(rRegL dst, rRegI src, immL_32bits mask) %{ @@ -12600,17 +11665,6 @@ instruct compU_rReg_mem(rFlagsRegU cr, rRegI op1, memory op2) ins_pipe(ialu_cr_reg_mem); %} -// // // Cisc-spilled version of cmpU_rReg -// //instruct compU_mem_rReg(rFlagsRegU cr, memory op1, rRegI op2) -// //%{ -// // match(Set cr (CmpU (LoadI op1) op2)); -// // -// // format %{ "CMPu $op1,$op2" %} -// // ins_cost(500); -// // opcode(0x39); /* Opcode 39 /r */ -// // ins_encode( OpcP, reg_mem( op1, op2) ); -// //%} - instruct testU_reg(rFlagsRegU cr, rRegI src, immI_0 zero) %{ match(Set cr (CmpU src zero)); @@ -12646,17 +11700,6 @@ instruct compP_rReg_mem(rFlagsRegU cr, rRegP op1, memory op2) ins_pipe(ialu_cr_reg_mem); %} -// // // Cisc-spilled version of cmpP_rReg -// //instruct compP_mem_rReg(rFlagsRegU cr, memory op1, rRegP op2) -// //%{ -// // match(Set cr (CmpP (LoadP op1) op2)); -// // -// // format %{ "CMPu $op1,$op2" %} -// // ins_cost(500); -// // opcode(0x39); /* Opcode 39 /r */ -// // ins_encode( OpcP, reg_mem( op1, op2) ); -// //%} - // XXX this is generalized by compP_rReg_mem??? // Compare raw pointer (used in out-of-heap check). // Only works because non-oop pointers must be raw pointers @@ -13633,7 +12676,9 @@ instruct RethrowException() // use the following format syntax format %{ "jmp rethrow_stub" %} - ins_encode(enc_rethrow); + ins_encode %{ + __ jump(RuntimeAddress(OptoRuntime::rethrow_stub()), noreg); + %} ins_pipe(pipe_jmp); %} From eaa4417f5cdc14cb08c4f694ce9705cb3e0ef167 Mon Sep 17 00:00:00 2001 From: Thomas Stuefe Date: Thu, 16 Nov 2023 07:29:37 +0000 Subject: [PATCH 03/76] 8319301: Static analysis warnings after JDK-8318016 Reviewed-by: thartmann, kvn --- src/hotspot/share/compiler/compilerOracle.cpp | 12 +-- .../commands/MemLimitTest.java | 84 +++++++++++++++++++ 2 files changed, 91 insertions(+), 5 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/compilercontrol/commands/MemLimitTest.java diff --git a/src/hotspot/share/compiler/compilerOracle.cpp b/src/hotspot/share/compiler/compilerOracle.cpp index 4ce0cd772e7..695d5eee0cc 100644 --- a/src/hotspot/share/compiler/compilerOracle.cpp +++ b/src/hotspot/share/compiler/compilerOracle.cpp @@ -653,6 +653,7 @@ static bool parseMemLimit(const char* line, intx& value, int& bytes_read, char* char* end; if (!parse_integer(line, &end, &s)) { jio_snprintf(errorbuf, buf_size, "MemLimit: invalid value"); + return false; } bytes_read = (int)(end - line); @@ -666,7 +667,7 @@ static bool parseMemLimit(const char* line, intx& value, int& bytes_read, char* bytes_read += 5; } else { jio_snprintf(errorbuf, buf_size, "MemLimit: invalid option"); - return true; + return false; } } value = v; @@ -705,9 +706,11 @@ static void scan_value(enum OptionType type, char* line, int& total_bytes_read, total_bytes_read += skipped; if (type == OptionType::Intx) { intx value; - // Special handling for memlimit - bool success = (option == CompileCommand::MemLimit) && parseMemLimit(line, value, bytes_read, errorbuf, buf_size); - if (!success) { + bool success = false; + if (option == CompileCommand::MemLimit) { + // Special parsing for MemLimit + success = parseMemLimit(line, value, bytes_read, errorbuf, buf_size); + } else { // Is it a raw number? success = sscanf(line, "" INTX_FORMAT "%n", &value, &bytes_read) == 1; } @@ -733,7 +736,6 @@ static void scan_value(enum OptionType type, char* line, int& total_bytes_read, total_bytes_read += bytes_read; line += bytes_read; register_command(matcher, option, value); - return; } else { jio_snprintf(errorbuf, buf_size, "Value cannot be read for option '%s' of type '%s'", ccname, type_str); } diff --git a/test/hotspot/jtreg/compiler/compilercontrol/commands/MemLimitTest.java b/test/hotspot/jtreg/compiler/compilercontrol/commands/MemLimitTest.java new file mode 100644 index 00000000000..5cf86794524 --- /dev/null +++ b/test/hotspot/jtreg/compiler/compilercontrol/commands/MemLimitTest.java @@ -0,0 +1,84 @@ +/* + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright 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 8319301 + * @summary Tests various ways to call memlimit + * @library /test/lib / + * + * @run driver compiler.compilercontrol.commands.MemLimitTest + */ + +package compiler.compilercontrol.commands; + +import jdk.test.lib.process.OutputAnalyzer; +import jdk.test.lib.process.ProcessTools; + +public class MemLimitTest { + + static void do_test(String option, boolean expectSuccess, int expectedValue) throws Exception { + OutputAnalyzer output = ProcessTools.executeTestJvm("-Xmx64m", "-XX:CompileCommand=" + option, "-version"); + if (expectSuccess) { + output.shouldHaveExitValue(0); + output.shouldNotContain("error occurred"); + // On success, we expect the command to be registered with the expected value + output.shouldContain("CompileCommand: MemLimit *.* intx MemLimit = " + expectedValue); + } else { + // On error, we don't expec a command registered. + output.shouldNotHaveExitValue(0); + output.shouldNotMatch("# A fatal error.*"); + output.shouldContain("CompileCommand: An error occurred during parsing"); + output.shouldNotContain("CompileCommand: MemStat"); // on error, no command should be registered! + } + } + + public static void main(String[] args) throws Exception { + // Check parsing of the MemLimit option. For functional tests, please see + // test/hotspot/jtreg/compiler/print/CompileCommandMemLimit.java + + // Negative tests + + // Missing value + do_test("MemLimit,*.*", false, 0); + + // Not a parseable number. Should not crash + do_test("MemLimit,*.*,hallo", false, 0); + + // Invalid option. + do_test("MemLimit,*.*,444m~hallo", false, 0); + + // Positive tests + + // "stop" mode is encoded as positive size + do_test("MemLimit,*.*,444m~stop", true, 465567744); + + // "crash" mode is encoded as negative size + do_test("MemLimit,*.*,444m~crash", true, -465567744); + + // if omitted, stop is default + do_test("MemLimit,*.*,444m", true, 465567744); + + } +} From 2db9ea9bbf6d4b7875b0c62721f76f016fd7257e Mon Sep 17 00:00:00 2001 From: Christian Hagedorn Date: Thu, 16 Nov 2023 07:37:45 +0000 Subject: [PATCH 04/76] 8317723: C2: CountedLoopEndNodes and Zero Trip Guards are wrongly treated as Runtime Predicate Reviewed-by: thartmann, epeter --- src/hotspot/share/opto/cfgnode.hpp | 1 + src/hotspot/share/opto/ifnode.cpp | 7 ++ src/hotspot/share/opto/predicates.cpp | 16 ++- src/hotspot/share/opto/predicates.hpp | 1 + .../TestWrongRuntimePredicateDetection.java | 103 ++++++++++++++++++ 5 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 test/hotspot/jtreg/compiler/predicates/TestWrongRuntimePredicateDetection.java diff --git a/src/hotspot/share/opto/cfgnode.hpp b/src/hotspot/share/opto/cfgnode.hpp index 81ac180dcfa..bc6f181aa07 100644 --- a/src/hotspot/share/opto/cfgnode.hpp +++ b/src/hotspot/share/opto/cfgnode.hpp @@ -431,6 +431,7 @@ class IfNode : public MultiBranchNode { Node* fold_compares(PhaseIterGVN* phase); static Node* up_one_dom(Node* curr, bool linear_only = false); Node* dominated_by(Node* prev_dom, PhaseIterGVN* igvn); + bool is_zero_trip_guard() const; // Takes the type of val and filters it through the test represented // by if_proj and returns a more refined type if one is produced. diff --git a/src/hotspot/share/opto/ifnode.cpp b/src/hotspot/share/opto/ifnode.cpp index beb11b65321..2751e891262 100644 --- a/src/hotspot/share/opto/ifnode.cpp +++ b/src/hotspot/share/opto/ifnode.cpp @@ -1774,6 +1774,13 @@ Node* IfProjNode::Identity(PhaseGVN* phase) { return this; } +bool IfNode::is_zero_trip_guard() const { + if (in(1)->is_Bool() && in(1)->in(1)->is_Cmp()) { + return in(1)->in(1)->in(1)->Opcode() == Op_OpaqueZeroTripGuard; + } + return false; +} + #ifndef PRODUCT //------------------------------dump_spec-------------------------------------- void IfNode::dump_spec(outputStream *st) const { diff --git a/src/hotspot/share/opto/predicates.cpp b/src/hotspot/share/opto/predicates.cpp index b8bdb221a75..c38ff4bcf02 100644 --- a/src/hotspot/share/opto/predicates.cpp +++ b/src/hotspot/share/opto/predicates.cpp @@ -77,13 +77,26 @@ Deoptimization::DeoptReason RuntimePredicate::uncommon_trap_reason(IfProjNode* i } bool RuntimePredicate::is_success_proj(Node* node, Deoptimization::DeoptReason deopt_reason) { - if (node->is_IfProj() && !node->in(0)->is_ParsePredicate()) { + if (may_be_runtime_predicate_if(node)) { return deopt_reason == uncommon_trap_reason(node->as_IfProj()); } else { return false; } } +// A Runtime Predicate must have an If or a RangeCheck node, while the If should not be a zero trip guard check. +bool RuntimePredicate::may_be_runtime_predicate_if(Node* node) { + if (node->is_IfProj()) { + const IfNode* if_node = node->in(0)->as_If(); + const int opcode_if = if_node->Opcode(); + if ((opcode_if == Op_If && !if_node->is_zero_trip_guard()) + || opcode_if == Op_RangeCheck) { + return true; + } + } + return false; +} + ParsePredicateIterator::ParsePredicateIterator(const Predicates& predicates) : _current_index(0) { const PredicateBlock* loop_limit_check_predicate_block = predicates.loop_limit_check_predicate_block(); if (loop_limit_check_predicate_block->has_parse_predicate()) { @@ -118,6 +131,7 @@ void PredicateBlock::verify_block() { const int opcode = next->Opcode(); assert(next->is_IfProj() || opcode == Op_If || opcode == Op_RangeCheck, "Regular Predicates consist of an IfProj and an If or RangeCheck node"); + assert(opcode != Op_If || !next->as_If()->is_zero_trip_guard(), "should not be zero trip guard"); next = next->in(0); } } diff --git a/src/hotspot/share/opto/predicates.hpp b/src/hotspot/share/opto/predicates.hpp index d273ab09dc4..89d8bbad030 100644 --- a/src/hotspot/share/opto/predicates.hpp +++ b/src/hotspot/share/opto/predicates.hpp @@ -257,6 +257,7 @@ class ParsePredicate : public StackObj { // Utility class for queries on Runtime Predicates. class RuntimePredicate : public StackObj { static Deoptimization::DeoptReason uncommon_trap_reason(IfProjNode* if_proj); + static bool may_be_runtime_predicate_if(Node* node); public: static bool is_success_proj(Node* node, Deoptimization::DeoptReason deopt_reason); diff --git a/test/hotspot/jtreg/compiler/predicates/TestWrongRuntimePredicateDetection.java b/test/hotspot/jtreg/compiler/predicates/TestWrongRuntimePredicateDetection.java new file mode 100644 index 00000000000..55ad781aa4b --- /dev/null +++ b/test/hotspot/jtreg/compiler/predicates/TestWrongRuntimePredicateDetection.java @@ -0,0 +1,103 @@ +/* + * 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 8317723 + * @library /test/lib + * @summary Test that CountedLoopEndNodes and zero trip guard check If nodes are not treated as Runtime Predicates. + * @run main/othervm -XX:-TieredCompilation -Xbatch + * -XX:CompileCommand=compileonly,compiler.predicates.TestWrongRuntimePredicateDetection::test* + * compiler.predicates.TestWrongRuntimePredicateDetection + */ + +package compiler.predicates; + +public class TestWrongRuntimePredicateDetection { + static int[] iArr = new int[50]; + static long instanceCount; + static boolean bFld = true; + static volatile byte byFld; + static long[][] lArrFld; + + + public static void main(String[] x) { + for (int i = 0; i < 1000; i++) { + testCountedLoopEndAsRuntimePredicate(); + } + for (int i = 0; i < 10; i++) { + testZeroTripGuardAsRuntimePredicate(); + } + } + + static void testCountedLoopEndAsRuntimePredicate() { + int i22 = 7, i26, i28, i29 = 8, i31 = 1; + float f4; + do { + for (int i = 0; i < 10000; i++) { + if (bFld) { + break; + } + instanceCount = byFld; + } + for (i26 = 4; 80 > i26; i26 += 2) ; + } while (++i22 < 315); + i28 = 6; + while ((i28 -= 3) > 0) { + for (f4 = i28; f4 < 53; f4++) { + bFld = false; + } + instanceCount = i26; + do { + switch ((i26 >>> 1) % 2 * 5 + 6) { + case 12: + case 10: + lArrFld[i31][1] = i29; + } + } while (++i31 < 53); + } + } + + static void testZeroTripGuardAsRuntimePredicate() { + int m; + int a[] = new int[50]; + for (int j = 0; j < a.length; j++) { + a[j] = j; + } + + for (int j = 4; j < 42; j++) { + for (int k = 1; k < 5; k++) { + iArr[1] = 34; + switch (j % 4) { + case 0: + iArr = iArr; + case 1: + case 3: + m = 3; + } + } + } + } + +} From c36ec2ca70248c2e4676fd725fbb132c3b929908 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Lund=C3=A9n?= Date: Thu, 16 Nov 2023 07:41:13 +0000 Subject: [PATCH 05/76] 8316653: Large NMethodSizeLimit triggers assert during C1 code buffer allocation Reviewed-by: kvn, rcastanedalo, thartmann --- src/hotspot/share/c1/c1_Compilation.hpp | 6 +++--- src/hotspot/share/c1/c1_Compiler.cpp | 2 +- src/hotspot/share/c1/c1_Compiler.hpp | 2 +- src/hotspot/share/code/codeBlob.cpp | 2 +- src/hotspot/share/code/codeBlob.hpp | 2 +- src/hotspot/share/code/codeCache.cpp | 14 +++++++++----- src/hotspot/share/code/codeCache.hpp | 4 ++-- .../jtreg/compiler/arguments/TestC1Globals.java | 10 ++++++++++ 8 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/hotspot/share/c1/c1_Compilation.hpp b/src/hotspot/share/c1/c1_Compilation.hpp index e9de43e8146..b0753a94d1b 100644 --- a/src/hotspot/share/c1/c1_Compilation.hpp +++ b/src/hotspot/share/c1/c1_Compilation.hpp @@ -213,10 +213,10 @@ class Compilation: public StackObj { bool bailed_out() const { return _bailout_msg != nullptr; } const char* bailout_msg() const { return _bailout_msg; } - static int desired_max_code_buffer_size() { - return (int)NMethodSizeLimit; // default 64K + static uint desired_max_code_buffer_size() { + return (uint)NMethodSizeLimit; // default 64K } - static int desired_max_constant_size() { + static uint desired_max_constant_size() { return desired_max_code_buffer_size() / 10; } diff --git a/src/hotspot/share/c1/c1_Compiler.cpp b/src/hotspot/share/c1/c1_Compiler.cpp index 567cd5fa8b5..def4b63fdf6 100644 --- a/src/hotspot/share/c1/c1_Compiler.cpp +++ b/src/hotspot/share/c1/c1_Compiler.cpp @@ -77,7 +77,7 @@ void Compiler::initialize() { } } -int Compiler::code_buffer_size() { +uint Compiler::code_buffer_size() { return Compilation::desired_max_code_buffer_size() + Compilation::desired_max_constant_size(); } diff --git a/src/hotspot/share/c1/c1_Compiler.hpp b/src/hotspot/share/c1/c1_Compiler.hpp index 9095a13297e..0160ee63574 100644 --- a/src/hotspot/share/c1/c1_Compiler.hpp +++ b/src/hotspot/share/c1/c1_Compiler.hpp @@ -60,7 +60,7 @@ class Compiler: public AbstractCompiler { static bool is_intrinsic_supported(vmIntrinsics::ID id); // Size of the code buffer - static int code_buffer_size(); + static uint code_buffer_size(); }; #endif // SHARE_C1_C1_COMPILER_HPP diff --git a/src/hotspot/share/code/codeBlob.cpp b/src/hotspot/share/code/codeBlob.cpp index c49fca717c9..75cf3fbf2d4 100644 --- a/src/hotspot/share/code/codeBlob.cpp +++ b/src/hotspot/share/code/codeBlob.cpp @@ -249,7 +249,7 @@ BufferBlob::BufferBlob(const char* name, int size) : RuntimeBlob(name, sizeof(BufferBlob), size, CodeOffsets::frame_never_safe, /*locs_size:*/ 0) {} -BufferBlob* BufferBlob::create(const char* name, int buffer_size) { +BufferBlob* BufferBlob::create(const char* name, uint buffer_size) { ThreadInVMfromUnknown __tiv; // get to VM state in case we block on CodeCache_lock BufferBlob* blob = nullptr; diff --git a/src/hotspot/share/code/codeBlob.hpp b/src/hotspot/share/code/codeBlob.hpp index 4e7093bed24..c6c1e16b00d 100644 --- a/src/hotspot/share/code/codeBlob.hpp +++ b/src/hotspot/share/code/codeBlob.hpp @@ -410,7 +410,7 @@ class BufferBlob: public RuntimeBlob { public: // Creation - static BufferBlob* create(const char* name, int buffer_size); + static BufferBlob* create(const char* name, uint buffer_size); static BufferBlob* create(const char* name, CodeBuffer* cb); static void free(BufferBlob* buf); diff --git a/src/hotspot/share/code/codeCache.cpp b/src/hotspot/share/code/codeCache.cpp index 45095829ca5..e99c6230a8e 100644 --- a/src/hotspot/share/code/codeCache.cpp +++ b/src/hotspot/share/code/codeCache.cpp @@ -520,10 +520,10 @@ CodeBlob* CodeCache::next_blob(CodeHeap* heap, CodeBlob* cb) { * run the constructor for the CodeBlob subclass he is busy * instantiating. */ -CodeBlob* CodeCache::allocate(int size, CodeBlobType code_blob_type, bool handle_alloc_failure, CodeBlobType orig_code_blob_type) { +CodeBlob* CodeCache::allocate(uint size, CodeBlobType code_blob_type, bool handle_alloc_failure, CodeBlobType orig_code_blob_type) { assert_locked_or_safepoint(CodeCache_lock); - assert(size > 0, "Code cache allocation request must be > 0 but is %d", size); - if (size <= 0) { + assert(size > 0, "Code cache allocation request must be > 0"); + if (size == 0) { return nullptr; } CodeBlob* cb = nullptr; @@ -1575,10 +1575,14 @@ void CodeCache::print_memory_overhead() { #ifndef PRODUCT -void CodeCache::print_trace(const char* event, CodeBlob* cb, int size) { +void CodeCache::print_trace(const char* event, CodeBlob* cb, uint size) { if (PrintCodeCache2) { // Need to add a new flag ResourceMark rm; - if (size == 0) size = cb->size(); + if (size == 0) { + int s = cb->size(); + assert(s >= 0, "CodeBlob size is negative: %d", s); + size = (uint) s; + } tty->print_cr("CodeCache %s: addr: " INTPTR_FORMAT ", size: 0x%x", event, p2i(cb), size); } } diff --git a/src/hotspot/share/code/codeCache.hpp b/src/hotspot/share/code/codeCache.hpp index 1b65fb3596e..51d697735ec 100644 --- a/src/hotspot/share/code/codeCache.hpp +++ b/src/hotspot/share/code/codeCache.hpp @@ -149,7 +149,7 @@ class CodeCache : AllStatic { static const GrowableArray* nmethod_heaps() { return _nmethod_heaps; } // Allocation/administration - static CodeBlob* allocate(int size, CodeBlobType code_blob_type, bool handle_alloc_failure = true, CodeBlobType orig_code_blob_type = CodeBlobType::All); // allocates a new CodeBlob + static CodeBlob* allocate(uint size, CodeBlobType code_blob_type, bool handle_alloc_failure = true, CodeBlobType orig_code_blob_type = CodeBlobType::All); // allocates a new CodeBlob static void commit(CodeBlob* cb); // called when the allocated CodeBlob has been filled static void free(CodeBlob* cb); // frees a CodeBlob static void free_unused_tail(CodeBlob* cb, size_t used); // frees the unused tail of a CodeBlob (only used by TemplateInterpreter::initialize()) @@ -226,7 +226,7 @@ class CodeCache : AllStatic { static void print_internals(); static void print_memory_overhead(); static void verify(); // verifies the code cache - static void print_trace(const char* event, CodeBlob* cb, int size = 0) PRODUCT_RETURN; + static void print_trace(const char* event, CodeBlob* cb, uint size = 0) PRODUCT_RETURN; static void print_summary(outputStream* st, bool detailed = true); // Prints a summary of the code cache usage static void log_state(outputStream* st); LINUX_ONLY(static void write_perf_map();) diff --git a/test/hotspot/jtreg/compiler/arguments/TestC1Globals.java b/test/hotspot/jtreg/compiler/arguments/TestC1Globals.java index faccf1eb545..b8ed2c36918 100644 --- a/test/hotspot/jtreg/compiler/arguments/TestC1Globals.java +++ b/test/hotspot/jtreg/compiler/arguments/TestC1Globals.java @@ -41,6 +41,16 @@ * compiler.arguments.TestC1Globals */ +/** + * @test + * @bug 8316653 + * @requires vm.debug + * @summary Test flag with max value + * + * @run main/othervm -XX:NMethodSizeLimit=2147483647 + * compiler.arguments.TestC1Globals + */ + /** * @test * @bug 8318817 From b4c2d1c1af76da4b326e7acea2ccb740728a8c7c Mon Sep 17 00:00:00 2001 From: Thomas Obermeier Date: Thu, 16 Nov 2023 08:38:15 +0000 Subject: [PATCH 06/76] 8319542: Fix boundaries of region to be tested with os::is_readable_range Reviewed-by: dlong, clanger --- src/hotspot/share/nmt/mallocTracker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/nmt/mallocTracker.cpp b/src/hotspot/share/nmt/mallocTracker.cpp index 3e7160d660c..ea14ddc1e20 100644 --- a/src/hotspot/share/nmt/mallocTracker.cpp +++ b/src/hotspot/share/nmt/mallocTracker.cpp @@ -212,7 +212,7 @@ bool MallocTracker::print_pointer_information(const void* p, outputStream* st) { const uint8_t* const end = here - (0x1000 + sizeof(MallocHeader)); // stop searching after 4k for (; here >= end; here -= smallest_possible_alignment) { // JDK-8306561: cast to a MallocHeader needs to guarantee it can reside in readable memory - if (!os::is_readable_range(here, here + sizeof(MallocHeader) - 1)) { + if (!os::is_readable_range(here, here + sizeof(MallocHeader))) { // Probably OOB, give up break; } From faeea07fe5d27e0c18c26f99705cc552e5ab9bdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Jeli=C5=84ski?= Date: Thu, 16 Nov 2023 08:55:18 +0000 Subject: [PATCH 07/76] 8319747: galoisCounterMode_AESCrypt stack walking broken Reviewed-by: kvn, sviswanathan --- src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp b/src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp index 322cf296949..1a256d0913d 100644 --- a/src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp +++ b/src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp @@ -260,9 +260,6 @@ address StubGenerator::generate_galoisCounterMode_AESCrypt() { #endif __ movptr(subkeyHtbl, subkeyH_mem); __ movptr(counter, counter_mem); -// Save rbp and rsp - __ push(rbp); - __ movq(rbp, rsp); // Align stack __ andq(rsp, -64); __ subptr(rsp, 96 * longSize); // Create space on the stack for htbl entries @@ -272,12 +269,12 @@ address StubGenerator::generate_galoisCounterMode_AESCrypt() { __ vzeroupper(); - __ movq(rsp, rbp); - __ pop(rbp); - // Restore state before leaving routine #ifdef _WIN64 + __ lea(rsp, Address(rbp, -6 * wordSize)); __ pop(rsi); +#else + __ lea(rsp, Address(rbp, -5 * wordSize)); #endif __ pop(rbx); __ pop(r15); From 73e19f60cd383cfa5ecbea5d9c57fb59f69c1608 Mon Sep 17 00:00:00 2001 From: Darragh Clarke Date: Thu, 16 Nov 2023 10:50:38 +0000 Subject: [PATCH 08/76] 8319825: jdk.net/jdk.net.ExtendedSocketOptions::IP_DONTFRAGMENT is missing @since 19 Reviewed-by: dfuchs, jpai --- src/jdk.net/share/classes/jdk/net/ExtendedSocketOptions.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/jdk.net/share/classes/jdk/net/ExtendedSocketOptions.java b/src/jdk.net/share/classes/jdk/net/ExtendedSocketOptions.java index 793d3b28162..51221df64ea 100644 --- a/src/jdk.net/share/classes/jdk/net/ExtendedSocketOptions.java +++ b/src/jdk.net/share/classes/jdk/net/ExtendedSocketOptions.java @@ -220,6 +220,8 @@ private ExtendedSocketOptions() { } * by the sending and receiving nodes exclusively. Setting this option for * an IPv6 socket ensures that packets to be sent are never fragmented, in * which case, the local network MTU must be observed. + * + * @since 19 */ public static final SocketOption IP_DONTFRAGMENT = new ExtSocketOption("IP_DONTFRAGMENT", Boolean.class); From 1d9688667e667dc710d64e52f1e918e047beaca3 Mon Sep 17 00:00:00 2001 From: Darragh Clarke Date: Thu, 16 Nov 2023 10:54:47 +0000 Subject: [PATCH 09/76] 8319531: FileServerHandler::discardRequestBody could be improved Reviewed-by: dfuchs, jpai, michaelm --- .../net/httpserver/LeftOverInputStream.java | 46 +++++++++++++------ .../simpleserver/FileServerHandler.java | 4 +- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/jdk.httpserver/share/classes/sun/net/httpserver/LeftOverInputStream.java b/src/jdk.httpserver/share/classes/sun/net/httpserver/LeftOverInputStream.java index d3a6e1b08c3..772fa621076 100644 --- a/src/jdk.httpserver/share/classes/sun/net/httpserver/LeftOverInputStream.java +++ b/src/jdk.httpserver/share/classes/sun/net/httpserver/LeftOverInputStream.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2007, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 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,6 +46,7 @@ abstract class LeftOverInputStream extends FilterInputStream { protected boolean closed = false; protected boolean eof = false; byte[] one = new byte [1]; + private static final int MAX_SKIP_BUFFER_SIZE = 2048; public LeftOverInputStream (ExchangeImpl t, InputStream src) { super (src); @@ -99,6 +100,32 @@ public synchronized int read (byte[]b, int off, int len) throws IOException { return readImpl (b, off, len); } + @Override + public synchronized long skip(long n) throws IOException { + long remaining = n; + int nr; + + if (n <= 0) { + return 0; + } + + int size = (int)Math.min(MAX_SKIP_BUFFER_SIZE, remaining); + byte[] skipBuffer = new byte[size]; + while (remaining > 0) { + if (server.isFinishing()) { + break; + } + nr = readImpl(skipBuffer, 0, (int)Math.min(size, remaining)); + if (nr < 0) { + eof = true; + break; + } + remaining -= nr; + } + + return n - remaining; + } + /** * read and discard up to l bytes or "eof" occurs, * (whichever is first). Then return true if the stream @@ -106,20 +133,11 @@ public synchronized int read (byte[]b, int off, int len) throws IOException { * (still bytes to be read) */ public boolean drain (long l) throws IOException { - int bufSize = 2048; - byte[] db = new byte [bufSize]; while (l > 0) { - if (server.isFinishing()) { - break; - } - long len = readImpl (db, 0, bufSize); - if (len == -1) { - eof = true; - return true; - } else { - l = l - len; - } + long skip = skip(l); + if (skip <= 0) break; // might return 0 if isFinishing or EOF + l -= skip; } - return false; + return eof; } } diff --git a/src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/FileServerHandler.java b/src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/FileServerHandler.java index b3dc1ff5f4a..6d217174fdd 100644 --- a/src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/FileServerHandler.java +++ b/src/jdk.httpserver/share/classes/sun/net/httpserver/simpleserver/FileServerHandler.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 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 @@ -159,7 +159,7 @@ private void handleNotFound(HttpExchange exchange) throws IOException { private static void discardRequestBody(HttpExchange exchange) throws IOException { try (InputStream is = exchange.getRequestBody()) { - is.readAllBytes(); + is.skip(Integer.MAX_VALUE); } } From 6868b371c68cddbfaef4f5c6800d2c5ed64fb70f Mon Sep 17 00:00:00 2001 From: Roland Westrelin Date: Thu, 16 Nov 2023 12:41:16 +0000 Subject: [PATCH 10/76] 8318826: C2: "Bad graph detected in build_loop_late" with incremental inlining Reviewed-by: thartmann, chagedorn, kvn --- src/hotspot/share/opto/compile.cpp | 1 + src/hotspot/share/opto/phaseX.cpp | 369 +++++++++--------- src/hotspot/share/opto/phaseX.hpp | 5 +- src/hotspot/share/opto/replacednodes.cpp | 4 + .../TestNullAtCallAfterLateInline.java | 136 +++++++ 5 files changed, 330 insertions(+), 185 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/inlining/TestNullAtCallAfterLateInline.java diff --git a/src/hotspot/share/opto/compile.cpp b/src/hotspot/share/opto/compile.cpp index 3dfcd14b382..bf0b809b0de 100644 --- a/src/hotspot/share/opto/compile.cpp +++ b/src/hotspot/share/opto/compile.cpp @@ -286,6 +286,7 @@ void Compile::gvn_replace_by(Node* n, Node* nn) { initial_gvn()->hash_find_insert(use); } record_for_igvn(use); + PhaseIterGVN::add_users_of_use_to_worklist(nn, use, *_igvn_worklist); i -= uses_found; // we deleted 1 or more copies of this edge } } diff --git a/src/hotspot/share/opto/phaseX.cpp b/src/hotspot/share/opto/phaseX.cpp index af293bee9e6..88e4a8d6695 100644 --- a/src/hotspot/share/opto/phaseX.cpp +++ b/src/hotspot/share/opto/phaseX.cpp @@ -1449,9 +1449,9 @@ void PhaseIterGVN::subsume_node( Node *old, Node *nn ) { } //------------------------------add_users_to_worklist-------------------------- -void PhaseIterGVN::add_users_to_worklist0( Node *n ) { +void PhaseIterGVN::add_users_to_worklist0(Node* n, Unique_Node_List& worklist) { for (DUIterator_Fast imax, i = n->fast_outs(imax); i < imax; i++) { - _worklist.push(n->fast_out(i)); // Push on worklist + worklist.push(n->fast_out(i)); // Push on worklist } } @@ -1476,117 +1476,121 @@ static PhiNode* countedloop_phi_from_cmp(CmpNode* cmp, Node* n) { return nullptr; } -void PhaseIterGVN::add_users_to_worklist( Node *n ) { - add_users_to_worklist0(n); +void PhaseIterGVN::add_users_to_worklist(Node *n) { + add_users_to_worklist0(n, _worklist); + Unique_Node_List& worklist = _worklist; // Move users of node to worklist for (DUIterator_Fast imax, i = n->fast_outs(imax); i < imax; i++) { Node* use = n->fast_out(i); // Get use + add_users_of_use_to_worklist(n, use, worklist); + } +} - if( use->is_Multi() || // Multi-definer? Push projs on worklist - use->is_Store() ) // Enable store/load same address - add_users_to_worklist0(use); - - // If we changed the receiver type to a call, we need to revisit - // the Catch following the call. It's looking for a non-null - // receiver to know when to enable the regular fall-through path - // in addition to the NullPtrException path. - if (use->is_CallDynamicJava() && n == use->in(TypeFunc::Parms)) { - Node* p = use->as_CallDynamicJava()->proj_out_or_null(TypeFunc::Control); - if (p != nullptr) { - add_users_to_worklist0(p); - } +void PhaseIterGVN::add_users_of_use_to_worklist(Node* n, Node* use, Unique_Node_List& worklist) { + if(use->is_Multi() || // Multi-definer? Push projs on worklist + use->is_Store() ) // Enable store/load same address + add_users_to_worklist0(use, worklist); + + // If we changed the receiver type to a call, we need to revisit + // the Catch following the call. It's looking for a non-null + // receiver to know when to enable the regular fall-through path + // in addition to the NullPtrException path. + if (use->is_CallDynamicJava() && n == use->in(TypeFunc::Parms)) { + Node* p = use->as_CallDynamicJava()->proj_out_or_null(TypeFunc::Control); + if (p != nullptr) { + add_users_to_worklist0(p, worklist); } + } - uint use_op = use->Opcode(); - if(use->is_Cmp()) { // Enable CMP/BOOL optimization - add_users_to_worklist(use); // Put Bool on worklist - if (use->outcnt() > 0) { - Node* bol = use->raw_out(0); - if (bol->outcnt() > 0) { - Node* iff = bol->raw_out(0); - if (iff->outcnt() == 2) { - // Look for the 'is_x2logic' pattern: "x ? : 0 : 1" and put the - // phi merging either 0 or 1 onto the worklist - Node* ifproj0 = iff->raw_out(0); - Node* ifproj1 = iff->raw_out(1); - if (ifproj0->outcnt() > 0 && ifproj1->outcnt() > 0) { - Node* region0 = ifproj0->raw_out(0); - Node* region1 = ifproj1->raw_out(0); - if( region0 == region1 ) - add_users_to_worklist0(region0); - } + uint use_op = use->Opcode(); + if(use->is_Cmp()) { // Enable CMP/BOOL optimization + add_users_to_worklist0(use, worklist); // Put Bool on worklist + if (use->outcnt() > 0) { + Node* bol = use->raw_out(0); + if (bol->outcnt() > 0) { + Node* iff = bol->raw_out(0); + if (iff->outcnt() == 2) { + // Look for the 'is_x2logic' pattern: "x ? : 0 : 1" and put the + // phi merging either 0 or 1 onto the worklist + Node* ifproj0 = iff->raw_out(0); + Node* ifproj1 = iff->raw_out(1); + if (ifproj0->outcnt() > 0 && ifproj1->outcnt() > 0) { + Node* region0 = ifproj0->raw_out(0); + Node* region1 = ifproj1->raw_out(0); + if( region0 == region1 ) + add_users_to_worklist0(region0, worklist); } } } - if (use_op == Op_CmpI || use_op == Op_CmpL) { - Node* phi = countedloop_phi_from_cmp(use->as_Cmp(), n); - if (phi != nullptr) { - // Input to the cmp of a loop exit check has changed, thus - // the loop limit may have changed, which can then change the - // range values of the trip-count Phi. - _worklist.push(phi); - } + } + if (use_op == Op_CmpI || use_op == Op_CmpL) { + Node* phi = countedloop_phi_from_cmp(use->as_Cmp(), n); + if (phi != nullptr) { + // Input to the cmp of a loop exit check has changed, thus + // the loop limit may have changed, which can then change the + // range values of the trip-count Phi. + worklist.push(phi); } - if (use_op == Op_CmpI) { - Node* cmp = use; - Node* in1 = cmp->in(1); - Node* in2 = cmp->in(2); - // Notify CmpI / If pattern from CastIINode::Value (left pattern). - // Must also notify if in1 is modified and possibly turns into X (right pattern). - // - // in1 in2 in1 in2 - // | | | | - // +--- | --+ | | - // | | | | | - // CmpINode | CmpINode - // | | | - // BoolNode | BoolNode - // | | OR | - // IfNode | IfNode - // | | | - // IfProj | IfProj X - // | | | | - // CastIINode CastIINode - // - if (in1 != in2) { // if they are equal, the CmpI can fold them away - if (in1 == n) { - // in1 modified -> could turn into X -> do traversal based on right pattern. - for (DUIterator_Fast i2max, i2 = cmp->fast_outs(i2max); i2 < i2max; i2++) { - Node* bol = cmp->fast_out(i2); // For each Bool - if (bol->is_Bool()) { - for (DUIterator_Fast i3max, i3 = bol->fast_outs(i3max); i3 < i3max; i3++) { - Node* iff = bol->fast_out(i3); // For each If - if (iff->is_If()) { - for (DUIterator_Fast i4max, i4 = iff->fast_outs(i4max); i4 < i4max; i4++) { - Node* if_proj = iff->fast_out(i4); // For each IfProj - assert(if_proj->is_IfProj(), "If only has IfTrue and IfFalse as outputs"); - for (DUIterator_Fast i5max, i5 = if_proj->fast_outs(i5max); i5 < i5max; i5++) { - Node* castii = if_proj->fast_out(i5); // For each CastII - if (castii->is_CastII() && - castii->as_CastII()->carry_dependency()) { - _worklist.push(castii); - } + } + if (use_op == Op_CmpI) { + Node* cmp = use; + Node* in1 = cmp->in(1); + Node* in2 = cmp->in(2); + // Notify CmpI / If pattern from CastIINode::Value (left pattern). + // Must also notify if in1 is modified and possibly turns into X (right pattern). + // + // in1 in2 in1 in2 + // | | | | + // +--- | --+ | | + // | | | | | + // CmpINode | CmpINode + // | | | + // BoolNode | BoolNode + // | | OR | + // IfNode | IfNode + // | | | + // IfProj | IfProj X + // | | | | + // CastIINode CastIINode + // + if (in1 != in2) { // if they are equal, the CmpI can fold them away + if (in1 == n) { + // in1 modified -> could turn into X -> do traversal based on right pattern. + for (DUIterator_Fast i2max, i2 = cmp->fast_outs(i2max); i2 < i2max; i2++) { + Node* bol = cmp->fast_out(i2); // For each Bool + if (bol->is_Bool()) { + for (DUIterator_Fast i3max, i3 = bol->fast_outs(i3max); i3 < i3max; i3++) { + Node* iff = bol->fast_out(i3); // For each If + if (iff->is_If()) { + for (DUIterator_Fast i4max, i4 = iff->fast_outs(i4max); i4 < i4max; i4++) { + Node* if_proj = iff->fast_out(i4); // For each IfProj + assert(if_proj->is_IfProj(), "If only has IfTrue and IfFalse as outputs"); + for (DUIterator_Fast i5max, i5 = if_proj->fast_outs(i5max); i5 < i5max; i5++) { + Node* castii = if_proj->fast_out(i5); // For each CastII + if (castii->is_CastII() && + castii->as_CastII()->carry_dependency()) { + worklist.push(castii); } } } } } } - } else { - // Only in2 modified -> can assume X == in2 (left pattern). - assert(n == in2, "only in2 modified"); - // Find all CastII with input in1. - for (DUIterator_Fast jmax, j = in1->fast_outs(jmax); j < jmax; j++) { - Node* castii = in1->fast_out(j); - if (castii->is_CastII() && castii->as_CastII()->carry_dependency()) { - // Find If. - if (castii->in(0) != nullptr && castii->in(0)->in(0) != nullptr && castii->in(0)->in(0)->is_If()) { - Node* ifnode = castii->in(0)->in(0); - // Check that if connects to the cmp - if (ifnode->in(1) != nullptr && ifnode->in(1)->is_Bool() && ifnode->in(1)->in(1) == cmp) { - _worklist.push(castii); - } + } + } else { + // Only in2 modified -> can assume X == in2 (left pattern). + assert(n == in2, "only in2 modified"); + // Find all CastII with input in1. + for (DUIterator_Fast jmax, j = in1->fast_outs(jmax); j < jmax; j++) { + Node* castii = in1->fast_out(j); + if (castii->is_CastII() && castii->as_CastII()->carry_dependency()) { + // Find If. + if (castii->in(0) != nullptr && castii->in(0)->in(0) != nullptr && castii->in(0)->in(0)->is_If()) { + Node* ifnode = castii->in(0)->in(0); + // Check that if connects to the cmp + if (ifnode->in(1) != nullptr && ifnode->in(1)->is_Bool() && ifnode->in(1)->in(1) == cmp) { + worklist.push(castii); } } } @@ -1594,108 +1598,107 @@ void PhaseIterGVN::add_users_to_worklist( Node *n ) { } } } + } - // If changed Cast input, notify down for Phi, Sub, and Xor - all do "uncast" - // Patterns: - // ConstraintCast+ -> Sub - // ConstraintCast+ -> Phi - // ConstraintCast+ -> Xor - if (use->is_ConstraintCast()) { - auto push_the_uses_to_worklist = [&](Node* n){ - if (n->is_Phi() || n->is_Sub() || n->Opcode() == Op_XorI || n->Opcode() == Op_XorL) { - _worklist.push(n); - } - }; - auto is_boundary = [](Node* n){ return !n->is_ConstraintCast(); }; - use->visit_uses(push_the_uses_to_worklist, is_boundary); - - } - // If changed LShift inputs, check RShift users for useless sign-ext - if( use_op == Op_LShiftI ) { - for (DUIterator_Fast i2max, i2 = use->fast_outs(i2max); i2 < i2max; i2++) { - Node* u = use->fast_out(i2); - if (u->Opcode() == Op_RShiftI) - _worklist.push(u); + // If changed Cast input, notify down for Phi, Sub, and Xor - all do "uncast" + // Patterns: + // ConstraintCast+ -> Sub + // ConstraintCast+ -> Phi + // ConstraintCast+ -> Xor + if (use->is_ConstraintCast()) { + auto push_the_uses_to_worklist = [&](Node* n){ + if (n->is_Phi() || n->is_Sub() || n->Opcode() == Op_XorI || n->Opcode() == Op_XorL) { + worklist.push(n); } - } - // If changed LShift inputs, check And users for shift and mask (And) operation - if (use_op == Op_LShiftI || use_op == Op_LShiftL) { - for (DUIterator_Fast i2max, i2 = use->fast_outs(i2max); i2 < i2max; i2++) { - Node* u = use->fast_out(i2); - if (u->Opcode() == Op_AndI || u->Opcode() == Op_AndL) { - _worklist.push(u); - } + }; + auto is_boundary = [](Node* n){ return !n->is_ConstraintCast(); }; + use->visit_uses(push_the_uses_to_worklist, is_boundary); + } + // If changed LShift inputs, check RShift users for useless sign-ext + if( use_op == Op_LShiftI ) { + for (DUIterator_Fast i2max, i2 = use->fast_outs(i2max); i2 < i2max; i2++) { + Node* u = use->fast_out(i2); + if (u->Opcode() == Op_RShiftI) + worklist.push(u); + } + } + // If changed LShift inputs, check And users for shift and mask (And) operation + if (use_op == Op_LShiftI || use_op == Op_LShiftL) { + for (DUIterator_Fast i2max, i2 = use->fast_outs(i2max); i2 < i2max; i2++) { + Node* u = use->fast_out(i2); + if (u->Opcode() == Op_AndI || u->Opcode() == Op_AndL) { + worklist.push(u); } } - // If changed AddI/SubI inputs, check CmpU for range check optimization. - if (use_op == Op_AddI || use_op == Op_SubI) { - for (DUIterator_Fast i2max, i2 = use->fast_outs(i2max); i2 < i2max; i2++) { - Node* u = use->fast_out(i2); - if (u->is_Cmp() && (u->Opcode() == Op_CmpU)) { - _worklist.push(u); - } + } + // If changed AddI/SubI inputs, check CmpU for range check optimization. + if (use_op == Op_AddI || use_op == Op_SubI) { + for (DUIterator_Fast i2max, i2 = use->fast_outs(i2max); i2 < i2max; i2++) { + Node* u = use->fast_out(i2); + if (u->is_Cmp() && (u->Opcode() == Op_CmpU)) { + worklist.push(u); } } - // If changed AddP inputs, check Stores for loop invariant - if( use_op == Op_AddP ) { - for (DUIterator_Fast i2max, i2 = use->fast_outs(i2max); i2 < i2max; i2++) { - Node* u = use->fast_out(i2); - if (u->is_Mem()) - _worklist.push(u); - } + } + // If changed AddP inputs, check Stores for loop invariant + if( use_op == Op_AddP ) { + for (DUIterator_Fast i2max, i2 = use->fast_outs(i2max); i2 < i2max; i2++) { + Node* u = use->fast_out(i2); + if (u->is_Mem()) + worklist.push(u); } - // If changed initialization activity, check dependent Stores - if (use_op == Op_Allocate || use_op == Op_AllocateArray) { - InitializeNode* init = use->as_Allocate()->initialization(); - if (init != nullptr) { - Node* imem = init->proj_out_or_null(TypeFunc::Memory); - if (imem != nullptr) add_users_to_worklist0(imem); - } + } + // If changed initialization activity, check dependent Stores + if (use_op == Op_Allocate || use_op == Op_AllocateArray) { + InitializeNode* init = use->as_Allocate()->initialization(); + if (init != nullptr) { + Node* imem = init->proj_out_or_null(TypeFunc::Memory); + if (imem != nullptr) add_users_to_worklist0(imem, worklist); } - // If the ValidLengthTest input changes then the fallthrough path out of the AllocateArray may have become dead. - // CatchNode::Value() is responsible for killing that path. The CatchNode has to be explicitly enqueued for igvn - // to guarantee the change is not missed. - if (use_op == Op_AllocateArray && n == use->in(AllocateNode::ValidLengthTest)) { - Node* p = use->as_AllocateArray()->proj_out_or_null(TypeFunc::Control); - if (p != nullptr) { - add_users_to_worklist0(p); - } + } + // If the ValidLengthTest input changes then the fallthrough path out of the AllocateArray may have become dead. + // CatchNode::Value() is responsible for killing that path. The CatchNode has to be explicitly enqueued for igvn + // to guarantee the change is not missed. + if (use_op == Op_AllocateArray && n == use->in(AllocateNode::ValidLengthTest)) { + Node* p = use->as_AllocateArray()->proj_out_or_null(TypeFunc::Control); + if (p != nullptr) { + add_users_to_worklist0(p, worklist); } + } - if (use_op == Op_Initialize) { - Node* imem = use->as_Initialize()->proj_out_or_null(TypeFunc::Memory); - if (imem != nullptr) add_users_to_worklist0(imem); - } - // Loading the java mirror from a Klass requires two loads and the type - // of the mirror load depends on the type of 'n'. See LoadNode::Value(). - // LoadBarrier?(LoadP(LoadP(AddP(foo:Klass, #java_mirror)))) - BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2(); - bool has_load_barrier_nodes = bs->has_load_barrier_nodes(); - - if (use_op == Op_LoadP && use->bottom_type()->isa_rawptr()) { - for (DUIterator_Fast i2max, i2 = use->fast_outs(i2max); i2 < i2max; i2++) { - Node* u = use->fast_out(i2); - const Type* ut = u->bottom_type(); - if (u->Opcode() == Op_LoadP && ut->isa_instptr()) { - if (has_load_barrier_nodes) { - // Search for load barriers behind the load - for (DUIterator_Fast i3max, i3 = u->fast_outs(i3max); i3 < i3max; i3++) { - Node* b = u->fast_out(i3); - if (bs->is_gc_barrier_node(b)) { - _worklist.push(b); - } + if (use_op == Op_Initialize) { + Node* imem = use->as_Initialize()->proj_out_or_null(TypeFunc::Memory); + if (imem != nullptr) add_users_to_worklist0(imem, worklist); + } + // Loading the java mirror from a Klass requires two loads and the type + // of the mirror load depends on the type of 'n'. See LoadNode::Value(). + // LoadBarrier?(LoadP(LoadP(AddP(foo:Klass, #java_mirror)))) + BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2(); + bool has_load_barrier_nodes = bs->has_load_barrier_nodes(); + + if (use_op == Op_LoadP && use->bottom_type()->isa_rawptr()) { + for (DUIterator_Fast i2max, i2 = use->fast_outs(i2max); i2 < i2max; i2++) { + Node* u = use->fast_out(i2); + const Type* ut = u->bottom_type(); + if (u->Opcode() == Op_LoadP && ut->isa_instptr()) { + if (has_load_barrier_nodes) { + // Search for load barriers behind the load + for (DUIterator_Fast i3max, i3 = u->fast_outs(i3max); i3 < i3max; i3++) { + Node* b = u->fast_out(i3); + if (bs->is_gc_barrier_node(b)) { + worklist.push(b); } } - _worklist.push(u); } + worklist.push(u); } } - if (use->Opcode() == Op_OpaqueZeroTripGuard) { - assert(use->outcnt() <= 1, "OpaqueZeroTripGuard can't be shared"); - if (use->outcnt() == 1) { - Node* cmp = use->unique_out(); - _worklist.push(cmp); - } + } + if (use->Opcode() == Op_OpaqueZeroTripGuard) { + assert(use->outcnt() <= 1, "OpaqueZeroTripGuard can't be shared"); + if (use->outcnt() == 1) { + Node* cmp = use->unique_out(); + worklist.push(cmp); } } } diff --git a/src/hotspot/share/opto/phaseX.hpp b/src/hotspot/share/opto/phaseX.hpp index 057caeb553f..da3d18b6084 100644 --- a/src/hotspot/share/opto/phaseX.hpp +++ b/src/hotspot/share/opto/phaseX.hpp @@ -528,8 +528,9 @@ class PhaseIterGVN : public PhaseGVN { } // Add users of 'n' to worklist - void add_users_to_worklist0( Node *n ); - void add_users_to_worklist ( Node *n ); + static void add_users_to_worklist0(Node* n, Unique_Node_List& worklist); + static void add_users_of_use_to_worklist(Node* n, Node* use, Unique_Node_List& worklist); + void add_users_to_worklist(Node* n); // Replace old node with new one. void replace_node( Node *old, Node *nn ) { diff --git a/src/hotspot/share/opto/replacednodes.cpp b/src/hotspot/share/opto/replacednodes.cpp index 56c5ee1c0bf..0b4d4efdf78 100644 --- a/src/hotspot/share/opto/replacednodes.cpp +++ b/src/hotspot/share/opto/replacednodes.cpp @@ -222,6 +222,7 @@ void ReplacedNodes::apply(Compile* C, Node* ctl) { } // Clone nodes and record mapping from current to cloned nodes + uint index_before_clone = C->unique(); 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 @@ -251,6 +252,9 @@ void ReplacedNodes::apply(Compile* C, Node* ctl) { if (clone_ptr != nullptr) { Node* clone = *clone_ptr; n->set_req(j, clone); + if (n->_idx < index_before_clone) { + PhaseIterGVN::add_users_of_use_to_worklist(clone, n, *C->igvn_worklist()); + } updates++; } } diff --git a/test/hotspot/jtreg/compiler/inlining/TestNullAtCallAfterLateInline.java b/test/hotspot/jtreg/compiler/inlining/TestNullAtCallAfterLateInline.java new file mode 100644 index 00000000000..9c5e2b9bc1f --- /dev/null +++ b/test/hotspot/jtreg/compiler/inlining/TestNullAtCallAfterLateInline.java @@ -0,0 +1,136 @@ +/* + * 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 8318826 + * @summary C2: "Bad graph detected in build_loop_late" with incremental inlining + * @requires vm.compiler2.enabled + * @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:+AlwaysIncrementalInline -XX:-BackgroundCompilation TestNullAtCallAfterLateInline + */ + + +public class TestNullAtCallAfterLateInline { + private static final C c = new C(); + private static final A a = new A(); + private static volatile int volatileField; + private static B b= new B(); + + public static void main(String[] args) { + for (int i = 0; i < 20_000; i++) { + testHelper1(0, true); + testHelper1(1, true); + testHelper1(2, true); + test1(false); + testHelper2(0, true, b); + testHelper2(1, true, c); + testHelper2(2, true, a); + inlined2(null, 3); + test2(false, null); + } + } + + private static int test1(boolean flag) { + for (int i = 0; i < 10; i++) { + } + return testHelper1(3, flag); + } + + private static int testHelper1(int i, boolean flag) { + int v; + if (flag) { + A a = inlined1(i); + a.notInlined(); + v = a.field; + } else { + volatileField = 42; + v = volatileField; + } + return v; + } + + private static A inlined1(int i) { + if (i == 0) { + return b; + } else if (i == 1) { + return c; + } else if (i == 2) { + return a; + } + return null; + } + + private static int test2(boolean flag, A a) { + for (int i = 0; i < 10; i++) { + } + return testHelper2(3, flag, a); + } + + private static int testHelper2(int i, boolean flag, A a) { + int v; + if (flag) { + inlined2(a, i); + a.notInlined(); + v = a.field; + } else { + volatileField = 42; + v = volatileField; + } + return v; + } + + private static void inlined2(Object a, int i) { + if (i == 0) { + if (!(a instanceof B)) { + } + } else if (i == 1) { + if (!(a instanceof C)) { + + } + } else if (i == 2) { + if (!(a instanceof A)) { + + } + } else { + if (!(a instanceof D)) { + } + } + } + + private static class A { + public int field; + + void notInlined() {} + } + + private static class B extends A { + void notInlined() {} + } + + private static class C extends A { + void notInlined() {} + } + + private static class D { + } +} From f33c874b6e624ad81572a2f806e198dd692a31a6 Mon Sep 17 00:00:00 2001 From: Roland Westrelin Date: Thu, 16 Nov 2023 12:43:50 +0000 Subject: [PATCH 11/76] 8319764: C2 compilation asserts during incremental inlining because Phi input is out of bounds Reviewed-by: thartmann, chagedorn --- src/hotspot/share/opto/replacednodes.cpp | 12 +- ...tLateInlineReplacedNodesExceptionPath.java | 135 ++++++++++++++++++ 2 files changed, 141 insertions(+), 6 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/inlining/TestLateInlineReplacedNodesExceptionPath.java diff --git a/src/hotspot/share/opto/replacednodes.cpp b/src/hotspot/share/opto/replacednodes.cpp index 0b4d4efdf78..992b7621588 100644 --- a/src/hotspot/share/opto/replacednodes.cpp +++ b/src/hotspot/share/opto/replacednodes.cpp @@ -152,12 +152,12 @@ void ReplacedNodes::apply(Compile* C, Node* ctl) { } 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)) { + if (n->req() == region->req()) { // ignore dead phis + 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() && is_dominator(ctl, in)) { valid_control.set(in->_idx); collect_nodes_to_clone(stack, to_fix); } diff --git a/test/hotspot/jtreg/compiler/inlining/TestLateInlineReplacedNodesExceptionPath.java b/test/hotspot/jtreg/compiler/inlining/TestLateInlineReplacedNodesExceptionPath.java new file mode 100644 index 00000000000..e6d6a4648e7 --- /dev/null +++ b/test/hotspot/jtreg/compiler/inlining/TestLateInlineReplacedNodesExceptionPath.java @@ -0,0 +1,135 @@ +/* + * 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 8319764 + * @summary C2 compilation asserts during incremental inlining because Phi input is out of bounds + * @requires vm.compiler2.enabled + * @run main/othervm -XX:-TieredCompilation -XX:-BackgroundCompilation -XX:CompileCommand=dontinline,TestLateInlineReplacedNodesExceptionPath::notInlined + * -XX:+UnlockDiagnosticVMOptions -XX:+StressIGVN -XX:StressSeed=1246687813 TestLateInlineReplacedNodesExceptionPath + * @run main/othervm -XX:-TieredCompilation -XX:-BackgroundCompilation -XX:CompileCommand=dontinline,TestLateInlineReplacedNodesExceptionPath::notInlined + * -XX:+UnlockDiagnosticVMOptions -XX:+StressIGVN TestLateInlineReplacedNodesExceptionPath + */ + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; + +public class TestLateInlineReplacedNodesExceptionPath { + private static A fieldA; + private static C fieldC = new C(); + + public static void main(String[] args) throws Throwable { + A a = new A(); + B b = new B(); + for (int i = 0; i < 20_000; i++) { + fieldA = a; + test1(true); + fieldA = b; + test1(true); + inlined1(true); + inlined1(false); + inlined2(true); + inlined2(false); + } + } + + static final MethodHandle mh1; + static MethodHandle mh2; + static final MethodHandle mh3; + static MethodHandle mh4; + + static { + try { + MethodHandles.Lookup lookup = MethodHandles.lookup(); + mh1 = lookup.findStatic(TestLateInlineReplacedNodesExceptionPath.class, "lateInlined1", MethodType.methodType(void.class, C.class)); + mh2 = mh1; + mh3 = lookup.findStatic(TestLateInlineReplacedNodesExceptionPath.class, "lateInlined2", MethodType.methodType(void.class, C.class)); + mh4 = mh3; + } catch (NoSuchMethodException | IllegalAccessException e) { + e.printStackTrace(); + throw new RuntimeException("Method handle lookup failed"); + } + } + + private static void lateInlined1(C c) { + fieldA.m(c); + c.field++; + fieldA.m(c); + } + + private static void lateInlined2(C c) { + c.field++; + } + + private static void test1(boolean flag) throws Throwable { + final C c = fieldC; + MethodHandle mh = null; + if (flag) { + mh = inlined1(flag); + } + mh.invokeExact(c); + mh = null; + if (flag) { + mh = inlined2(flag); + } + mh.invokeExact(c); + } + + private static MethodHandle inlined1(boolean flag) { + if (flag) { + return mh1; + } + return mh2; + } + + private static MethodHandle inlined2(boolean flag) { + if (flag) { + return mh3; + } + return mh4; + } + + private static void notInlined() { + } + + private static class A { + public void m(C c) { + c.field++; + notInlined(); + } + + } + + private static class B extends A { + + public void m(C c) { + notInlined(); + } + } + + private static class C { + public int field; + } +} From 9faead1469481e268b451f2853c8fec8613426b9 Mon Sep 17 00:00:00 2001 From: Matthias Baesken Date: Thu, 16 Nov 2023 12:55:06 +0000 Subject: [PATCH 12/76] 8319927: Log that IEEE rounding mode was corrupted by loading a library Reviewed-by: goetz, lucy --- src/hotspot/os/bsd/os_bsd.cpp | 18 +++++++++++++++- src/hotspot/os/linux/os_linux.cpp | 21 ++++++++++++++++--- .../floatingpoint/TestSubnormalDouble.java | 2 +- .../floatingpoint/TestSubnormalFloat.java | 2 +- 4 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/hotspot/os/bsd/os_bsd.cpp b/src/hotspot/os/bsd/os_bsd.cpp index ddfd3a5ec78..f5968d20420 100644 --- a/src/hotspot/os/bsd/os_bsd.cpp +++ b/src/hotspot/os/bsd/os_bsd.cpp @@ -978,6 +978,12 @@ bool os::dll_address_to_library_name(address addr, char* buf, void *os::Bsd::dlopen_helper(const char *filename, int mode) { #ifndef IA32 + bool ieee_handling = IEEE_subnormal_handling_OK(); + if (!ieee_handling) { + Events::log_dll_message(nullptr, "IEEE subnormal handling check failed before loading %s", filename); + log_info(os)("IEEE subnormal handling check failed before loading %s", filename); + } + // Save and restore the floating-point environment around dlopen(). // There are known cases where global library initialization sets // FPU flags that affect computation accuracy, for example, enabling @@ -1004,7 +1010,17 @@ void *os::Bsd::dlopen_helper(const char *filename, int mode) { // flags. Silently fix things now. int rtn = fesetenv(&default_fenv); assert(rtn == 0, "fesetenv must succeed"); - assert(IEEE_subnormal_handling_OK(), "fsetenv didn't work"); + bool ieee_handling_after_issue = IEEE_subnormal_handling_OK(); + + if (ieee_handling_after_issue) { + Events::log_dll_message(nullptr, "IEEE subnormal handling had to be corrected after loading %s", filename); + log_info(os)("IEEE subnormal handling had to be corrected after loading %s", filename); + } else { + Events::log_dll_message(nullptr, "IEEE subnormal handling could not be corrected after loading %s", filename); + log_info(os)("IEEE subnormal handling could not be corrected after loading %s", filename); + } + + assert(ieee_handling_after_issue, "fesetenv didn't work"); } #endif // IA32 diff --git a/src/hotspot/os/linux/os_linux.cpp b/src/hotspot/os/linux/os_linux.cpp index 90c7c21d619..5bd817b2872 100644 --- a/src/hotspot/os/linux/os_linux.cpp +++ b/src/hotspot/os/linux/os_linux.cpp @@ -1804,6 +1804,12 @@ void * os::dll_load(const char *filename, char *ebuf, int ebuflen) { void * os::Linux::dlopen_helper(const char *filename, char *ebuf, int ebuflen) { #ifndef IA32 + bool ieee_handling = IEEE_subnormal_handling_OK(); + if (!ieee_handling) { + Events::log_dll_message(nullptr, "IEEE subnormal handling check failed before loading %s", filename); + log_info(os)("IEEE subnormal handling check failed before loading %s", filename); + } + // Save and restore the floating-point environment around dlopen(). // There are known cases where global library initialization sets // FPU flags that affect computation accuracy, for example, enabling @@ -1843,11 +1849,20 @@ void * os::Linux::dlopen_helper(const char *filename, char *ebuf, int ebuflen) { #ifndef IA32 // Quickly test to make sure subnormals are correctly handled. if (! IEEE_subnormal_handling_OK()) { - // We just dlopen()ed a library that mangled the floating-point - // flags. Silently fix things now. + // We just dlopen()ed a library that mangled the floating-point flags. + // Attempt to fix things now. int rtn = fesetenv(&default_fenv); assert(rtn == 0, "fesetenv must succeed"); - assert(IEEE_subnormal_handling_OK(), "fsetenv didn't work"); + bool ieee_handling_after_issue = IEEE_subnormal_handling_OK(); + + if (ieee_handling_after_issue) { + Events::log_dll_message(nullptr, "IEEE subnormal handling had to be corrected after loading %s", filename); + log_info(os)("IEEE subnormal handling had to be corrected after loading %s", filename); + } else { + Events::log_dll_message(nullptr, "IEEE subnormal handling could not be corrected after loading %s", filename); + log_info(os)("IEEE subnormal handling could not be corrected after loading %s", filename); + } + assert(ieee_handling_after_issue, "fesetenv didn't work"); } #endif // IA32 } diff --git a/test/hotspot/jtreg/compiler/floatingpoint/TestSubnormalDouble.java b/test/hotspot/jtreg/compiler/floatingpoint/TestSubnormalDouble.java index 7c556f1db44..b755606124a 100644 --- a/test/hotspot/jtreg/compiler/floatingpoint/TestSubnormalDouble.java +++ b/test/hotspot/jtreg/compiler/floatingpoint/TestSubnormalDouble.java @@ -25,7 +25,7 @@ * @test * @bug 8295159 * @summary DSO created with -ffast-math breaks Java floating-point arithmetic - * @run main/othervm/native compiler.floatingpoint.TestSubnormalDouble + * @run main/othervm/native -Xlog:os=info compiler.floatingpoint.TestSubnormalDouble */ package compiler.floatingpoint; diff --git a/test/hotspot/jtreg/compiler/floatingpoint/TestSubnormalFloat.java b/test/hotspot/jtreg/compiler/floatingpoint/TestSubnormalFloat.java index f09ee1cf004..6a19750e4d0 100644 --- a/test/hotspot/jtreg/compiler/floatingpoint/TestSubnormalFloat.java +++ b/test/hotspot/jtreg/compiler/floatingpoint/TestSubnormalFloat.java @@ -25,7 +25,7 @@ * @test * @bug 8295159 * @summary DSO created with -ffast-math breaks Java floating-point arithmetic - * @run main/othervm/native compiler.floatingpoint.TestSubnormalFloat + * @run main/othervm/native -Xlog:os=info compiler.floatingpoint.TestSubnormalFloat */ package compiler.floatingpoint; From 87be6b69fe985eee01fc3344f9153d774db792c1 Mon Sep 17 00:00:00 2001 From: Stefan Karlsson Date: Thu, 16 Nov 2023 14:33:50 +0000 Subject: [PATCH 13/76] 8318757: VM_ThreadDump asserts in interleaved ObjectMonitor::deflate_monitor calls Reviewed-by: shade, aboldtch, pchilanomate, dcubed --- src/hotspot/share/prims/jvmtiEnvBase.cpp | 4 +- src/hotspot/share/prims/whitebox.cpp | 2 +- .../share/runtime/monitorDeflationThread.cpp | 2 +- src/hotspot/share/runtime/synchronizer.cpp | 161 +++++------------- src/hotspot/share/runtime/synchronizer.hpp | 86 +++------- src/hotspot/share/runtime/vmOperations.cpp | 131 ++++++++++++-- src/hotspot/share/runtime/vmOperations.hpp | 4 +- src/hotspot/share/services/threadService.cpp | 19 +-- src/hotspot/share/services/threadService.hpp | 6 +- test/hotspot/jtreg/TEST.groups | 1 + .../runtime/Monitor/ConcurrentDeflation.java | 81 +++++++++ 11 files changed, 279 insertions(+), 218 deletions(-) create mode 100644 test/hotspot/jtreg/runtime/Monitor/ConcurrentDeflation.java diff --git a/src/hotspot/share/prims/jvmtiEnvBase.cpp b/src/hotspot/share/prims/jvmtiEnvBase.cpp index ec4beb3594d..c402db0cd75 100644 --- a/src/hotspot/share/prims/jvmtiEnvBase.cpp +++ b/src/hotspot/share/prims/jvmtiEnvBase.cpp @@ -990,7 +990,7 @@ JvmtiEnvBase::get_owned_monitors(JavaThread *calling_thread, JavaThread* java_th // Get off stack monitors. (e.g. acquired via jni MonitorEnter). JvmtiMonitorClosure jmc(calling_thread, owned_monitors_list, this); - ObjectSynchronizer::monitors_iterate(&jmc, java_thread); + ObjectSynchronizer::owned_monitors_iterate(&jmc, java_thread); err = jmc.error(); return err; @@ -1017,7 +1017,7 @@ JvmtiEnvBase::get_owned_monitors(JavaThread* calling_thread, JavaThread* java_th // Get off stack monitors. (e.g. acquired via jni MonitorEnter). JvmtiMonitorClosure jmc(calling_thread, owned_monitors_list, this); - ObjectSynchronizer::monitors_iterate(&jmc, java_thread); + ObjectSynchronizer::owned_monitors_iterate(&jmc, java_thread); err = jmc.error(); return err; diff --git a/src/hotspot/share/prims/whitebox.cpp b/src/hotspot/share/prims/whitebox.cpp index 0a03198bdf7..c15686dc87c 100644 --- a/src/hotspot/share/prims/whitebox.cpp +++ b/src/hotspot/share/prims/whitebox.cpp @@ -1849,7 +1849,7 @@ WB_END WB_ENTRY(jboolean, WB_DeflateIdleMonitors(JNIEnv* env, jobject wb)) log_info(monitorinflation)("WhiteBox initiated DeflateIdleMonitors"); - return ObjectSynchronizer::request_deflate_idle_monitors(); + return ObjectSynchronizer::request_deflate_idle_monitors_from_wb(); WB_END WB_ENTRY(void, WB_ForceSafepoint(JNIEnv* env, jobject wb)) diff --git a/src/hotspot/share/runtime/monitorDeflationThread.cpp b/src/hotspot/share/runtime/monitorDeflationThread.cpp index 01b33182973..3f2dcc6a673 100644 --- a/src/hotspot/share/runtime/monitorDeflationThread.cpp +++ b/src/hotspot/share/runtime/monitorDeflationThread.cpp @@ -94,6 +94,6 @@ void MonitorDeflationThread::monitor_deflation_thread_entry(JavaThread* jt, TRAP } } - (void)ObjectSynchronizer::deflate_idle_monitors(/* ObjectMonitorsHashtable is not needed here */ nullptr); + (void)ObjectSynchronizer::deflate_idle_monitors(); } } diff --git a/src/hotspot/share/runtime/synchronizer.cpp b/src/hotspot/share/runtime/synchronizer.cpp index c83df922e0d..e23eb267668 100644 --- a/src/hotspot/share/runtime/synchronizer.cpp +++ b/src/hotspot/share/runtime/synchronizer.cpp @@ -63,45 +63,6 @@ #include "utilities/linkedlist.hpp" #include "utilities/preserveException.hpp" -class ObjectMonitorsHashtable::PtrList : - public LinkedListImpl {}; - -class CleanupObjectMonitorsHashtable: StackObj { - public: - bool do_entry(void*& key, ObjectMonitorsHashtable::PtrList*& list) { - list->clear(); // clear the LinkListNodes - delete list; // then delete the LinkedList - return true; - } -}; - -ObjectMonitorsHashtable::~ObjectMonitorsHashtable() { - CleanupObjectMonitorsHashtable cleanup; - _ptrs->unlink(&cleanup); // cleanup the LinkedLists - delete _ptrs; // then delete the hash table -} - -void ObjectMonitorsHashtable::add_entry(void* key, ObjectMonitor* om) { - ObjectMonitorsHashtable::PtrList* list = get_entry(key); - if (list == nullptr) { - // Create new list and add it to the hash table: - list = new (mtThread) ObjectMonitorsHashtable::PtrList; - add_entry(key, list); - } - list->add(om); // Add the ObjectMonitor to the list. - _om_count++; -} - -bool ObjectMonitorsHashtable::has_entry(void* key, ObjectMonitor* om) { - ObjectMonitorsHashtable::PtrList* list = get_entry(key); - if (list == nullptr || list->find(om) == nullptr) { - return false; - } - return true; -} - void MonitorList::add(ObjectMonitor* m) { ObjectMonitor* head; do { @@ -1099,57 +1060,40 @@ JavaThread* ObjectSynchronizer::get_lock_owner(ThreadsList * t_list, Handle h_ob // Visitors ... -// Iterate ObjectMonitors where the owner == thread; this does NOT include -// ObjectMonitors where owner is set to a stack-lock address in thread. -// -// This version of monitors_iterate() works with the in-use monitor list. -// -void ObjectSynchronizer::monitors_iterate(MonitorClosure* closure, JavaThread* thread) { +// Iterate ObjectMonitors owned by any thread and where the owner `filter` +// returns true. +template +void ObjectSynchronizer::owned_monitors_iterate_filtered(MonitorClosure* closure, OwnerFilter filter) { MonitorList::Iterator iter = _in_use_list.iterator(); while (iter.has_next()) { ObjectMonitor* mid = iter.next(); - if (mid->owner() != thread) { - // Not owned by the target thread and intentionally skips when owner - // is set to a stack-lock address in the target thread. - continue; - } - if (!mid->is_being_async_deflated() && mid->object_peek() != nullptr) { - // Only process with closure if the object is set. - - // monitors_iterate() is only called at a safepoint or when the - // target thread is suspended or when the target thread is - // operating on itself. The current closures in use today are - // only interested in an owned ObjectMonitor and ownership - // cannot be dropped under the calling contexts so the - // ObjectMonitor cannot be async deflated. + + // This function is only called at a safepoint or when the + // target thread is suspended or when the target thread is + // operating on itself. The current closures in use today are + // only interested in an owned ObjectMonitor and ownership + // cannot be dropped under the calling contexts so the + // ObjectMonitor cannot be async deflated. + if (mid->has_owner() && filter(mid->owner_raw())) { + assert(!mid->is_being_async_deflated(), "Owned monitors should not be deflating"); + assert(mid->object_peek() != nullptr, "Owned monitors should not have a dead object"); + closure->do_monitor(mid); } } } -// This version of monitors_iterate() works with the specified linked list. -// -void ObjectSynchronizer::monitors_iterate(MonitorClosure* closure, - ObjectMonitorsHashtable::PtrList* list, - JavaThread* thread) { - typedef LinkedListIterator ObjectMonitorIterator; - ObjectMonitorIterator iter(list->head()); - while (!iter.is_empty()) { - ObjectMonitor* mid = *iter.next(); - // Owner set to a stack-lock address in thread should never be seen here: - assert(mid->owner() == thread, "must be"); - if (!mid->is_being_async_deflated() && mid->object_peek() != nullptr) { - // Only process with closure if the object is set. - - // monitors_iterate() is only called at a safepoint or when the - // target thread is suspended or when the target thread is - // operating on itself. The current closures in use today are - // only interested in an owned ObjectMonitor and ownership - // cannot be dropped under the calling contexts so the - // ObjectMonitor cannot be async deflated. - closure->do_monitor(mid); - } - } +// Iterate ObjectMonitors where the owner == thread; this does NOT include +// ObjectMonitors where owner is set to a stack-lock address in thread. +void ObjectSynchronizer::owned_monitors_iterate(MonitorClosure* closure, JavaThread* thread) { + auto thread_filter = [&](void* owner) { return owner == thread; }; + return owned_monitors_iterate_filtered(closure, thread_filter); +} + +// Iterate ObjectMonitors owned by any thread. +void ObjectSynchronizer::owned_monitors_iterate(MonitorClosure* closure) { + auto all_filter = [&](void* owner) { return true; }; + return owned_monitors_iterate_filtered(closure, all_filter); } static bool monitors_used_above_threshold(MonitorList* list) { @@ -1256,16 +1200,20 @@ bool ObjectSynchronizer::is_async_deflation_needed() { return false; } -bool ObjectSynchronizer::request_deflate_idle_monitors() { +void ObjectSynchronizer::request_deflate_idle_monitors() { + MonitorLocker ml(MonitorDeflation_lock, Mutex::_no_safepoint_check_flag); + set_is_async_deflation_requested(true); + ml.notify_all(); +} + +bool ObjectSynchronizer::request_deflate_idle_monitors_from_wb() { JavaThread* current = JavaThread::current(); bool ret_code = false; jlong last_time = last_async_deflation_time_ns(); - set_is_async_deflation_requested(true); - { - MonitorLocker ml(MonitorDeflation_lock, Mutex::_no_safepoint_check_flag); - ml.notify_all(); - } + + request_deflate_idle_monitors(); + const int N_CHECKS = 5; for (int i = 0; i < N_CHECKS; i++) { // sleep for at most 5 seconds if (last_async_deflation_time_ns() > last_time) { @@ -1582,16 +1530,8 @@ void ObjectSynchronizer::chk_for_block_req(JavaThread* current, const char* op_n // Walk the in-use list and deflate (at most MonitorDeflationMax) idle // ObjectMonitors. Returns the number of deflated ObjectMonitors. // -// If table != nullptr, we gather owned ObjectMonitors indexed by the -// owner in the table. Please note that ObjectMonitors where the owner -// is set to a stack-lock address are NOT associated with the JavaThread -// that holds that stack-lock. All of the current consumers of -// ObjectMonitorsHashtable info only care about JNI locked monitors and -// those do not have the owner set to a stack-lock address. -// size_t ObjectSynchronizer::deflate_monitor_list(Thread* current, LogStream* ls, - elapsedTimer* timer_p, - ObjectMonitorsHashtable* table) { + elapsedTimer* timer_p) { MonitorList::Iterator iter = _in_use_list.iterator(); size_t deflated_count = 0; @@ -1602,18 +1542,6 @@ size_t ObjectSynchronizer::deflate_monitor_list(Thread* current, LogStream* ls, ObjectMonitor* mid = iter.next(); if (mid->deflate_monitor()) { deflated_count++; - } else if (table != nullptr) { - // The caller is interested in the owned ObjectMonitors. This does - // not include when owner is set to a stack-lock address in thread. - // This also does not capture unowned ObjectMonitors that cannot be - // deflated because of a waiter. - void* key = mid->owner(); - // Since deflate_idle_monitors() and deflate_monitor_list() can be - // called more than once, we have to make sure the entry has not - // already been added. - if (key != nullptr && !table->has_entry(key, mid)) { - table->add_entry(key, mid); - } } if (current->is_Java_thread()) { @@ -1657,9 +1585,8 @@ static size_t delete_monitors(GrowableArray* delete_list) { } // This function is called by the MonitorDeflationThread to deflate -// ObjectMonitors. It is also called via do_final_audit_and_print_stats() -// and VM_ThreadDump::doit() by the VMThread. -size_t ObjectSynchronizer::deflate_idle_monitors(ObjectMonitorsHashtable* table) { +// ObjectMonitors. It is also called via do_final_audit_and_print_stats(). +size_t ObjectSynchronizer::deflate_idle_monitors() { Thread* current = Thread::current(); if (current->is_Java_thread()) { // The async deflation request has been processed. @@ -1684,7 +1611,7 @@ size_t ObjectSynchronizer::deflate_idle_monitors(ObjectMonitorsHashtable* table) } // Deflate some idle ObjectMonitors. - size_t deflated_count = deflate_monitor_list(current, ls, &timer, table); + size_t deflated_count = deflate_monitor_list(current, ls, &timer); size_t unlinked_count = 0; size_t deleted_count = 0; if (deflated_count > 0 || is_final_audit()) { @@ -1766,10 +1693,6 @@ size_t ObjectSynchronizer::deflate_idle_monitors(ObjectMonitorsHashtable* table) } ls->print_cr("end deflating: in_use_list stats: ceiling=" SIZE_FORMAT ", count=" SIZE_FORMAT ", max=" SIZE_FORMAT, in_use_list_ceiling(), _in_use_list.count(), _in_use_list.max()); - if (table != nullptr) { - ls->print_cr("ObjectMonitorsHashtable: key_count=" SIZE_FORMAT ", om_count=" SIZE_FORMAT, - table->key_count(), table->om_count()); - } } OM_PERFDATA_OP(MonExtant, set_value(_in_use_list.count())); @@ -1822,7 +1745,7 @@ void ObjectSynchronizer::release_monitors_owned_by_thread(JavaThread* current) { assert(current == JavaThread::current(), "must be current Java thread"); NoSafepointVerifier nsv; ReleaseJavaMonitorsClosure rjmc(current); - ObjectSynchronizer::monitors_iterate(&rjmc, current); + ObjectSynchronizer::owned_monitors_iterate(&rjmc, current); assert(!current->has_pending_exception(), "Should not be possible"); current->clear_pending_exception(); assert(current->held_monitor_count() == 0, "Should not be possible"); @@ -1879,7 +1802,7 @@ void ObjectSynchronizer::do_final_audit_and_print_stats() { // Do deflations in order to reduce the in-use monitor population // that is reported by ObjectSynchronizer::log_in_use_monitor_details() // which is called by ObjectSynchronizer::audit_and_print_stats(). - while (deflate_idle_monitors(/* ObjectMonitorsHashtable is not needed here */ nullptr) > 0) { + while (deflate_idle_monitors() > 0) { ; // empty } // The other audit_and_print_stats() call is done at the Debug diff --git a/src/hotspot/share/runtime/synchronizer.hpp b/src/hotspot/share/runtime/synchronizer.hpp index 66d08a9d998..68c177d3b05 100644 --- a/src/hotspot/share/runtime/synchronizer.hpp +++ b/src/hotspot/share/runtime/synchronizer.hpp @@ -36,55 +36,6 @@ class LogStream; class ObjectMonitor; class ThreadsList; -// Hash table of void* to a list of ObjectMonitor* owned by the JavaThread. -// The JavaThread's owner key is either a JavaThread* or a stack lock -// address in the JavaThread so we use "void*". -// -class ObjectMonitorsHashtable { - private: - static unsigned int ptr_hash(void* const& s1) { - // 2654435761 = 2^32 * Phi (golden ratio) - return (unsigned int)(((uint32_t)(uintptr_t)s1) * 2654435761u); - } - - public: - class PtrList; - - private: - // ResourceHashtable SIZE is specified at compile time so we - // use 1031 which is the first prime after 1024. - typedef ResourceHashtable PtrTable; - PtrTable* _ptrs; - size_t _key_count; - size_t _om_count; - - public: - // ResourceHashtable is passed to various functions and populated in - // different places so we allocate it using C_HEAP to make it immune - // from any ResourceMarks that happen to be in the code paths. - ObjectMonitorsHashtable() : _ptrs(new (mtThread) PtrTable), _key_count(0), _om_count(0) {} - - ~ObjectMonitorsHashtable(); - - void add_entry(void* key, ObjectMonitor* om); - - void add_entry(void* key, PtrList* list) { - _ptrs->put(key, list); - _key_count++; - } - - PtrList* get_entry(void* key) { - PtrList** listpp = _ptrs->get(key); - return (listpp == nullptr) ? nullptr : *listpp; - } - - bool has_entry(void* key, ObjectMonitor* om); - - size_t key_count() { return _key_count; } - size_t om_count() { return _om_count; } -}; - class MonitorList { friend class VMStructs; @@ -172,29 +123,30 @@ class ObjectSynchronizer : AllStatic { // JNI detach support static void release_monitors_owned_by_thread(JavaThread* current); + // Iterate ObjectMonitors owned by any thread and where the owner `filter` + // returns true. + template + static void owned_monitors_iterate_filtered(MonitorClosure* closure, OwnerFilter filter); + // Iterate ObjectMonitors where the owner == thread; this does NOT include - // ObjectMonitors where owner is set to a stack lock address in thread: - // - // This version of monitors_iterate() works with the in-use monitor list. - static void monitors_iterate(MonitorClosure* m, JavaThread* thread); - // This version of monitors_iterate() works with the specified linked list. - static void monitors_iterate(MonitorClosure* closure, - ObjectMonitorsHashtable::PtrList* list, - JavaThread* thread); + // ObjectMonitors where owner is set to a stack lock address in thread. + static void owned_monitors_iterate(MonitorClosure* m, JavaThread* thread); + + // Iterate ObjectMonitors owned by any thread. + static void owned_monitors_iterate(MonitorClosure* closure); // Initialize the gInflationLocks static void initialize(); - // GC: we currently use aggressive monitor deflation policy - // Basically we try to deflate all monitors that are not busy. - static size_t deflate_idle_monitors(ObjectMonitorsHashtable* table); + // We currently use aggressive monitor deflation policy; + // basically we try to deflate all monitors that are not busy. + static size_t deflate_idle_monitors(); // Deflate idle monitors: static void chk_for_block_req(JavaThread* current, const char* op_name, const char* cnt_name, size_t cnt, LogStream* ls, elapsedTimer* timer_p); - static size_t deflate_monitor_list(Thread* current, LogStream* ls, elapsedTimer* timer_p, - ObjectMonitorsHashtable* table); + static size_t deflate_monitor_list(Thread* current, LogStream* ls, elapsedTimer* timer_p); static size_t in_use_list_ceiling(); static void dec_in_use_list_ceiling(); static void inc_in_use_list_ceiling(); @@ -204,7 +156,8 @@ class ObjectSynchronizer : AllStatic { static bool is_final_audit() { return _is_final_audit; } static void set_is_final_audit() { _is_final_audit = true; } static jlong last_async_deflation_time_ns() { return _last_async_deflation_time_ns; } - static bool request_deflate_idle_monitors(); // for whitebox test support + static void request_deflate_idle_monitors(); + static bool request_deflate_idle_monitors_from_wb(); // for whitebox test support static void set_is_async_deflation_requested(bool new_value) { _is_async_deflation_requested = new_value; } static jlong time_since_last_async_deflation_ms(); @@ -252,4 +205,11 @@ class ObjectLocker : public StackObj { void notify_all(TRAPS) { ObjectSynchronizer::notifyall(_obj, CHECK); } }; +// Interface to visit monitors +class ObjectMonitorsView { +public: + // Visit monitors that belong to the given thread + virtual void visit(MonitorClosure* closure, JavaThread* thread) = 0; +}; + #endif // SHARE_RUNTIME_SYNCHRONIZER_HPP diff --git a/src/hotspot/share/runtime/vmOperations.cpp b/src/hotspot/share/runtime/vmOperations.cpp index fefb96acc7a..fadbcb799f7 100644 --- a/src/hotspot/share/runtime/vmOperations.cpp +++ b/src/hotspot/share/runtime/vmOperations.cpp @@ -43,12 +43,14 @@ #include "runtime/interfaceSupport.inline.hpp" #include "runtime/javaThread.inline.hpp" #include "runtime/jniHandles.hpp" +#include "runtime/objectMonitor.inline.hpp" #include "runtime/stackFrameStream.inline.hpp" #include "runtime/synchronizer.hpp" #include "runtime/threads.hpp" #include "runtime/threadSMR.inline.hpp" #include "runtime/vmOperations.hpp" #include "services/threadService.hpp" +#include "utilities/ticks.hpp" #define VM_OP_NAME_INITIALIZE(name) #name, @@ -265,6 +267,105 @@ void VM_ThreadDump::doit_epilogue() { } } +// Hash table of void* to a list of ObjectMonitor* owned by the JavaThread. +// The JavaThread's owner key is either a JavaThread* or a stack lock +// address in the JavaThread so we use "void*". +// +class ObjectMonitorsDump : public MonitorClosure, public ObjectMonitorsView { + private: + static unsigned int ptr_hash(void* const& s1) { + // 2654435761 = 2^32 * Phi (golden ratio) + return (unsigned int)(((uint32_t)(uintptr_t)s1) * 2654435761u); + } + + private: + class ObjectMonitorLinkedList : + public LinkedListImpl {}; + + // ResourceHashtable SIZE is specified at compile time so we + // use 1031 which is the first prime after 1024. + typedef ResourceHashtable PtrTable; + PtrTable* _ptrs; + size_t _key_count; + size_t _om_count; + + void add_list(void* key, ObjectMonitorLinkedList* list) { + _ptrs->put(key, list); + _key_count++; + } + + ObjectMonitorLinkedList* get_list(void* key) { + ObjectMonitorLinkedList** listpp = _ptrs->get(key); + return (listpp == nullptr) ? nullptr : *listpp; + } + + void add(ObjectMonitor* monitor) { + void* key = monitor->owner(); + + ObjectMonitorLinkedList* list = get_list(key); + if (list == nullptr) { + // Create new list and add it to the hash table: + list = new (mtThread) ObjectMonitorLinkedList; + _ptrs->put(key, list); + _key_count++; + } + + assert(list->find(monitor) == nullptr, "Should not contain duplicates"); + list->add(monitor); // Add the ObjectMonitor to the list. + _om_count++; + } + + public: + // ResourceHashtable is passed to various functions and populated in + // different places so we allocate it using C_HEAP to make it immune + // from any ResourceMarks that happen to be in the code paths. + ObjectMonitorsDump() : _ptrs(new (mtThread) PtrTable), _key_count(0), _om_count(0) {} + + ~ObjectMonitorsDump() { + class CleanupObjectMonitorsDump: StackObj { + public: + bool do_entry(void*& key, ObjectMonitorLinkedList*& list) { + list->clear(); // clear the LinkListNodes + delete list; // then delete the LinkedList + return true; + } + } cleanup; + + _ptrs->unlink(&cleanup); // cleanup the LinkedLists + delete _ptrs; // then delete the hash table + } + + // Implements MonitorClosure used to collect all owned monitors in the system + void do_monitor(ObjectMonitor* monitor) override { + assert(monitor->has_owner(), "Expects only owned monitors"); + + if (monitor->is_owner_anonymous()) { + // There's no need to collect anonymous owned monitors + // because the callers of this code is only interested + // in JNI owned monitors. + return; + } + + add(monitor); + } + + // Implements the ObjectMonitorsView interface + void visit(MonitorClosure* closure, JavaThread* thread) override { + ObjectMonitorLinkedList* list = get_list(thread); + LinkedListIterator iter(list != nullptr ? list->head() : nullptr); + while (!iter.is_empty()) { + ObjectMonitor* monitor = *iter.next(); + closure->do_monitor(monitor); + } + } + + size_t key_count() { return _key_count; } + size_t om_count() { return _om_count; } +}; + void VM_ThreadDump::doit() { ResourceMark rm; @@ -279,16 +380,20 @@ void VM_ThreadDump::doit() { concurrent_locks.dump_at_safepoint(); } - ObjectMonitorsHashtable table; - ObjectMonitorsHashtable* tablep = nullptr; + ObjectMonitorsDump object_monitors; if (_with_locked_monitors) { - // The caller wants locked monitor information and that's expensive to gather - // when there are a lot of inflated monitors. So we deflate idle monitors and - // gather information about owned monitors at the same time. - tablep = &table; - while (ObjectSynchronizer::deflate_idle_monitors(tablep) > 0) { - ; /* empty */ - } + // Gather information about owned monitors. + ObjectSynchronizer::owned_monitors_iterate(&object_monitors); + + // If there are many object monitors in the system then the above iteration + // can start to take time. Be friendly to following thread dumps by telling + // the MonitorDeflationThread to deflate monitors. + // + // This is trying to be somewhat backwards compatible with the previous + // implementation, which performed monitor deflation right here. We might + // want to reconsider the need to trigger monitor deflation from the thread + // dumping and instead maybe tweak the deflation heuristics. + ObjectSynchronizer::request_deflate_idle_monitors(); } if (_num_threads == 0) { @@ -305,7 +410,7 @@ void VM_ThreadDump::doit() { if (_with_locked_synchronizers) { tcl = concurrent_locks.thread_concurrent_locks(jt); } - snapshot_thread(jt, tcl, tablep); + snapshot_thread(jt, tcl, &object_monitors); } } else { // Snapshot threads in the given _threads array @@ -340,15 +445,15 @@ void VM_ThreadDump::doit() { if (_with_locked_synchronizers) { tcl = concurrent_locks.thread_concurrent_locks(jt); } - snapshot_thread(jt, tcl, tablep); + snapshot_thread(jt, tcl, &object_monitors); } } } void VM_ThreadDump::snapshot_thread(JavaThread* java_thread, ThreadConcurrentLocks* tcl, - ObjectMonitorsHashtable* table) { + ObjectMonitorsView* monitors) { ThreadSnapshot* snapshot = _result->add_thread_snapshot(java_thread); - snapshot->dump_stack_at_safepoint(_max_depth, _with_locked_monitors, table, false); + snapshot->dump_stack_at_safepoint(_max_depth, _with_locked_monitors, monitors, false); snapshot->set_concurrent_locks(tcl); } diff --git a/src/hotspot/share/runtime/vmOperations.hpp b/src/hotspot/share/runtime/vmOperations.hpp index 152d72a0400..8778fe29ade 100644 --- a/src/hotspot/share/runtime/vmOperations.hpp +++ b/src/hotspot/share/runtime/vmOperations.hpp @@ -30,7 +30,7 @@ #include "runtime/vmOperation.hpp" #include "runtime/threadSMR.hpp" -class ObjectMonitorsHashtable; +class ObjectMonitorsView; // A hodge podge of commonly used VM Operations @@ -204,7 +204,7 @@ class VM_ThreadDump : public VM_Operation { bool _with_locked_synchronizers; void snapshot_thread(JavaThread* java_thread, ThreadConcurrentLocks* tcl, - ObjectMonitorsHashtable* table); + ObjectMonitorsView* monitors); public: VM_ThreadDump(ThreadDumpResult* result, diff --git a/src/hotspot/share/services/threadService.cpp b/src/hotspot/share/services/threadService.cpp index bae98f0aadf..e95c832894f 100644 --- a/src/hotspot/share/services/threadService.cpp +++ b/src/hotspot/share/services/threadService.cpp @@ -45,6 +45,7 @@ #include "runtime/init.hpp" #include "runtime/javaThread.inline.hpp" #include "runtime/objectMonitor.inline.hpp" +#include "runtime/synchronizer.hpp" #include "runtime/thread.inline.hpp" #include "runtime/threads.hpp" #include "runtime/threadSMR.inline.hpp" @@ -687,7 +688,7 @@ ThreadStackTrace::~ThreadStackTrace() { } } -void ThreadStackTrace::dump_stack_at_safepoint(int maxDepth, ObjectMonitorsHashtable* table, bool full) { +void ThreadStackTrace::dump_stack_at_safepoint(int maxDepth, ObjectMonitorsView* monitors, bool full) { assert(SafepointSynchronize::is_at_safepoint(), "all threads are stopped"); if (_thread->has_last_Java_frame()) { @@ -723,17 +724,7 @@ void ThreadStackTrace::dump_stack_at_safepoint(int maxDepth, ObjectMonitorsHasht // Iterate inflated monitors and find monitors locked by this thread // that are not found in the stack, e.g. JNI locked monitors: InflatedMonitorsClosure imc(this); - if (table != nullptr) { - // Get the ObjectMonitors locked by the target thread, if any, - // and does not include any where owner is set to a stack lock - // address in the target thread: - ObjectMonitorsHashtable::PtrList* list = table->get_entry(_thread); - if (list != nullptr) { - ObjectSynchronizer::monitors_iterate(&imc, list, _thread); - } - } else { - ObjectSynchronizer::monitors_iterate(&imc, _thread); - } + monitors->visit(&imc, _thread); } } @@ -988,9 +979,9 @@ ThreadSnapshot::~ThreadSnapshot() { } void ThreadSnapshot::dump_stack_at_safepoint(int max_depth, bool with_locked_monitors, - ObjectMonitorsHashtable* table, bool full) { + ObjectMonitorsView* monitors, bool full) { _stack_trace = new ThreadStackTrace(_thread, with_locked_monitors); - _stack_trace->dump_stack_at_safepoint(max_depth, table, full); + _stack_trace->dump_stack_at_safepoint(max_depth, monitors, full); } diff --git a/src/hotspot/share/services/threadService.hpp b/src/hotspot/share/services/threadService.hpp index 484741ad935..63ae767a9a4 100644 --- a/src/hotspot/share/services/threadService.hpp +++ b/src/hotspot/share/services/threadService.hpp @@ -38,7 +38,7 @@ #include "services/management.hpp" class DeadlockCycle; -class ObjectMonitorsHashtable; +class ObjectMonitorsView; class OopClosure; class StackFrameInfo; class ThreadConcurrentLocks; @@ -264,7 +264,7 @@ class ThreadSnapshot : public CHeapObj { ThreadConcurrentLocks* get_concurrent_locks() { return _concurrent_locks; } void dump_stack_at_safepoint(int max_depth, bool with_locked_monitors, - ObjectMonitorsHashtable* table, bool full); + ObjectMonitorsView* monitors, bool full); void set_concurrent_locks(ThreadConcurrentLocks* l) { _concurrent_locks = l; } void metadata_do(void f(Metadata*)); }; @@ -287,7 +287,7 @@ class ThreadStackTrace : public CHeapObj { int get_stack_depth() { return _depth; } void add_stack_frame(javaVFrame* jvf); - void dump_stack_at_safepoint(int max_depth, ObjectMonitorsHashtable* table, bool full); + void dump_stack_at_safepoint(int max_depth, ObjectMonitorsView* monitors, bool full); Handle allocate_fill_stack_trace_element_array(TRAPS); void metadata_do(void f(Metadata*)); GrowableArray* jni_locked_monitors() { return _jni_locked_monitors; } diff --git a/test/hotspot/jtreg/TEST.groups b/test/hotspot/jtreg/TEST.groups index 6ee8e99a188..4573c0ebb9b 100644 --- a/test/hotspot/jtreg/TEST.groups +++ b/test/hotspot/jtreg/TEST.groups @@ -388,6 +388,7 @@ tier1_runtime = \ -runtime/modules/LoadUnloadModuleStress.java \ -runtime/modules/ModuleStress/ExportModuleStressTest.java \ -runtime/modules/ModuleStress/ModuleStressGC.java \ + -runtime/Monitor/ConcurrentDeflation.java \ -runtime/ReservedStack \ -runtime/SelectionResolution/AbstractMethodErrorTest.java \ -runtime/SelectionResolution/IllegalAccessErrorTest.java \ diff --git a/test/hotspot/jtreg/runtime/Monitor/ConcurrentDeflation.java b/test/hotspot/jtreg/runtime/Monitor/ConcurrentDeflation.java new file mode 100644 index 00000000000..9ceec3b9d8d --- /dev/null +++ b/test/hotspot/jtreg/runtime/Monitor/ConcurrentDeflation.java @@ -0,0 +1,81 @@ +/* + * 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 jdk.test.lib.process.OutputAnalyzer; +import jdk.test.lib.Platform; +import jdk.test.lib.process.ProcessTools; + +import java.lang.management.ManagementFactory; +import java.lang.management.ThreadInfo; +import java.lang.management.ThreadMXBean; + +/* + * @test + * @bug 8318757 + * @summary Test concurrent monitor deflation by MonitorDeflationThread and thread dumping + * @library /test/lib + * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:GuaranteedAsyncDeflationInterval=2000 -XX:LockingMode=0 ConcurrentDeflation + */ + +public class ConcurrentDeflation { + public static final long TOTAL_RUN_TIME_NS = 10_000_000_000L; + public static Object[] monitors = new Object[1000]; + public static int monitorCount; + + public static void main(String[] args) throws Exception { + Thread threadDumper = new Thread(() -> dumpThreads()); + threadDumper.start(); + Thread monitorCreator = new Thread(() -> createMonitors()); + monitorCreator.start(); + + threadDumper.join(); + monitorCreator.join(); + } + + static private void dumpThreads() { + ThreadMXBean threadBean = ManagementFactory.getThreadMXBean(); + int dumpCount = 0; + long startTime = System.nanoTime(); + while (System.nanoTime() - startTime < TOTAL_RUN_TIME_NS) { + threadBean.dumpAllThreads(true, false); + dumpCount++; + try { + Thread.sleep(10); + } catch (InterruptedException e) {} + } + System.out.println("Dumped all thread info " + dumpCount + " times"); + } + + static private void createMonitors() { + int index = 0; + long startTime = System.nanoTime(); + while (System.nanoTime() - startTime < TOTAL_RUN_TIME_NS) { + index = index++ % 1000; + monitors[index] = new Object(); + synchronized (monitors[index]) { + monitorCount++; + } + } + System.out.println("Created " + monitorCount + " monitors"); + } +} From 9e7a3ae27766034fd5e107dba6fa93b8bf3af951 Mon Sep 17 00:00:00 2001 From: Stefan Karlsson Date: Thu, 16 Nov 2023 14:36:34 +0000 Subject: [PATCH 14/76] 8319630: Monitor final audit log lacks separator Reviewed-by: dholmes, shade --- src/hotspot/share/runtime/objectMonitor.cpp | 27 ++++++++++----------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/hotspot/share/runtime/objectMonitor.cpp b/src/hotspot/share/runtime/objectMonitor.cpp index 12a0e953771..1266514c8ee 100644 --- a/src/hotspot/share/runtime/objectMonitor.cpp +++ b/src/hotspot/share/runtime/objectMonitor.cpp @@ -663,20 +663,19 @@ void ObjectMonitor::install_displaced_markword_in_object(const oop obj) { // Convert the fields used by is_busy() to a string that can be // used for diagnostic output. const char* ObjectMonitor::is_busy_to_string(stringStream* ss) { - ss->print("is_busy: waiters=%d, ", _waiters); - if (contentions() > 0) { - ss->print("contentions=%d, ", contentions()); - } else { - ss->print("contentions=0"); - } - if (!owner_is_DEFLATER_MARKER()) { - ss->print("owner=" INTPTR_FORMAT, p2i(owner_raw())); - } else { - // We report null instead of DEFLATER_MARKER here because is_busy() - // ignores DEFLATER_MARKER values. - ss->print("owner=" INTPTR_FORMAT, NULL_WORD); - } - ss->print(", cxq=" INTPTR_FORMAT ", EntryList=" INTPTR_FORMAT, p2i(_cxq), + ss->print("is_busy: waiters=%d" + ", contentions=%d" + ", owner=" PTR_FORMAT + ", cxq=" PTR_FORMAT + ", EntryList=" PTR_FORMAT, + _waiters, + (contentions() > 0 ? contentions() : 0), + owner_is_DEFLATER_MARKER() + // We report null instead of DEFLATER_MARKER here because is_busy() + // ignores DEFLATER_MARKER values. + ? p2i(nullptr) + : p2i(owner_raw()), + p2i(_cxq), p2i(_EntryList)); return ss->base(); } From f3ed27582e16c3a323f590863cbeec6d35e20b58 Mon Sep 17 00:00:00 2001 From: Alexander Zvegintsev Date: Thu, 16 Nov 2023 14:59:27 +0000 Subject: [PATCH 15/76] 8319103: Popups that request focus are not shown on Linux with Wayland Reviewed-by: serb, prr --- .../unix/classes/sun/awt/UNIXToolkit.java | 36 +++++-- .../JPopupMenu/FocusablePopupDismissTest.java | 94 +++++++++++++++++++ 2 files changed, 123 insertions(+), 7 deletions(-) create mode 100644 test/jdk/javax/swing/JPopupMenu/FocusablePopupDismissTest.java diff --git a/src/java.desktop/unix/classes/sun/awt/UNIXToolkit.java b/src/java.desktop/unix/classes/sun/awt/UNIXToolkit.java index 35d7cba47de..03693e6eb8d 100644 --- a/src/java.desktop/unix/classes/sun/awt/UNIXToolkit.java +++ b/src/java.desktop/unix/classes/sun/awt/UNIXToolkit.java @@ -502,6 +502,21 @@ public boolean isRunningOnWayland() { @Override public void windowLostFocus(WindowEvent e) { Window window = e.getWindow(); + Window oppositeWindow = e.getOppositeWindow(); + + // The focus can move between the window calling the popup, + // and the popup window itself. + // We only dismiss the popup in other cases. + if (oppositeWindow != null) { + if (window == oppositeWindow.getParent() ) { + addWaylandWindowFocusListenerToWindow(oppositeWindow); + return; + } + if (window.getParent() == oppositeWindow) { + return; + } + } + window.removeWindowFocusListener(this); // AWT @@ -516,18 +531,22 @@ public void windowLostFocus(WindowEvent e) { } } + private static void addWaylandWindowFocusListenerToWindow(Window window) { + if (!Arrays + .asList(window.getWindowFocusListeners()) + .contains(waylandWindowFocusListener) + ) { + window.addWindowFocusListener(waylandWindowFocusListener); + } + } + @Override public void dismissPopupOnFocusLostIfNeeded(Window invoker) { - if (!isOnWayland() - || invoker == null - || Arrays - .asList(invoker.getWindowFocusListeners()) - .contains(waylandWindowFocusListener) - ) { + if (!isOnWayland() || invoker == null) { return; } - invoker.addWindowFocusListener(waylandWindowFocusListener); + addWaylandWindowFocusListenerToWindow(invoker); } @Override @@ -537,5 +556,8 @@ public void dismissPopupOnFocusLostIfNeededCleanUp(Window invoker) { } invoker.removeWindowFocusListener(waylandWindowFocusListener); + for (Window ownedWindow : invoker.getOwnedWindows()) { + ownedWindow.removeWindowFocusListener(waylandWindowFocusListener); + } } } diff --git a/test/jdk/javax/swing/JPopupMenu/FocusablePopupDismissTest.java b/test/jdk/javax/swing/JPopupMenu/FocusablePopupDismissTest.java new file mode 100644 index 00000000000..2704c9789e3 --- /dev/null +++ b/test/jdk/javax/swing/JPopupMenu/FocusablePopupDismissTest.java @@ -0,0 +1,94 @@ +/* + * 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 + * @key headful + * @bug 8319103 + * @requires (os.family == "linux") + * @library /java/awt/regtesthelpers + * @build PassFailJFrame + * @summary Tests if the focusable popup can be dismissed when the parent + * window or the popup itself loses focus in Wayland. + * @run main/manual FocusablePopupDismissTest + */ + +import javax.swing.JButton; +import javax.swing.JFrame; +import javax.swing.JPopupMenu; +import javax.swing.JTextField; +import java.awt.Window; +import java.util.List; + +public class FocusablePopupDismissTest { + private static final String INSTRUCTIONS = """ + A frame with a "Click me" button should appear next to the window + with this instruction. + + Click on the "Click me" button. + + If the JTextField popup with "Some text" is not showing on the screen, + click Fail. + + The following steps require some focusable system window to be displayed + on the screen. This could be a system settings window, file manager, etc. + + Click on the "Click me" button if the popup is not displayed + on the screen. + + While the popup is displayed, click on some other window on the desktop. + If the popup has disappeared, click Pass, otherwise click Fail. + """; + + public static void main(String[] args) throws Exception { + if (System.getenv("WAYLAND_DISPLAY") == null) { + //test is valid only when running on Wayland. + return; + } + + PassFailJFrame.builder() + .title("FocusablePopupDismissTest") + .instructions(INSTRUCTIONS) + .rows(20) + .columns(45) + .testUI(FocusablePopupDismissTest::createTestUI) + .build() + .awaitAndCheck(); + } + + static List createTestUI() { + JFrame frame = new JFrame("FocusablePopupDismissTest"); + JButton button = new JButton("Click me"); + frame.add(button); + + button.addActionListener(e -> { + JPopupMenu popupMenu = new JPopupMenu(); + JTextField textField = new JTextField("Some text", 10); + popupMenu.add(textField); + popupMenu.show(button, 0, button.getHeight()); + }); + frame.pack(); + + return List.of(frame); + } +} From b05e69f789fa8c9a5320be5a841317abd3b3a235 Mon Sep 17 00:00:00 2001 From: Sandhya Viswanathan Date: Thu, 16 Nov 2023 16:32:08 +0000 Subject: [PATCH 16/76] 8320209: VectorMaskGen clobbers rflags on x86_64 Reviewed-by: kvn, qamai, jbhateja --- src/hotspot/cpu/x86/x86.ad | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hotspot/cpu/x86/x86.ad b/src/hotspot/cpu/x86/x86.ad index 7251cd6ae72..2567246b206 100644 --- a/src/hotspot/cpu/x86/x86.ad +++ b/src/hotspot/cpu/x86/x86.ad @@ -8990,9 +8990,9 @@ instruct vmask_cmp_node(rRegI dst, vec src1, vec src2, kReg mask, kReg ktmp1, kR %} -instruct vmask_gen(kReg dst, rRegL len, rRegL temp) %{ +instruct vmask_gen(kReg dst, rRegL len, rRegL temp, rFlagsReg cr) %{ match(Set dst (VectorMaskGen len)); - effect(TEMP temp); + effect(TEMP temp, KILL cr); format %{ "vector_mask_gen32 $dst, $len \t! vector mask generator" %} ins_encode %{ __ genmask($dst$$KRegister, $len$$Register, $temp$$Register); From 52e2878cffd9cb704ad773b841dbab0d17eba896 Mon Sep 17 00:00:00 2001 From: Vicente Romero Date: Thu, 16 Nov 2023 16:41:58 +0000 Subject: [PATCH 17/76] 8319987: compilation of sealed classes leads to infinite recursion Reviewed-by: jlahoda --- .../com/sun/tools/javac/code/Types.java | 59 +++++++++++-------- .../CyclicHierarchyTest.java | 12 ++++ .../CyclicHierarchyTest.out | 3 + 3 files changed, 49 insertions(+), 25 deletions(-) create mode 100644 test/langtools/tools/javac/sealed/erroneous_hierarchy/CyclicHierarchyTest.java create mode 100644 test/langtools/tools/javac/sealed/erroneous_hierarchy/CyclicHierarchyTest.out diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java index 980ec3d9003..7783c7fb4f4 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java @@ -1665,37 +1665,46 @@ public boolean isCastable(Type t, Type s, Warner warn) { && (t.tsym.isSealed() || s.tsym.isSealed())) { return (t.isCompound() || s.isCompound()) ? true : - !areDisjoint((ClassSymbol)t.tsym, (ClassSymbol)s.tsym); + !(new DisjointChecker().areDisjoint((ClassSymbol)t.tsym, (ClassSymbol)s.tsym)); } return result; } // where - private boolean areDisjoint(ClassSymbol ts, ClassSymbol ss) { - if (isSubtype(erasure(ts.type), erasure(ss.type))) { - return false; - } - // if both are classes or both are interfaces, shortcut - if (ts.isInterface() == ss.isInterface() && isSubtype(erasure(ss.type), erasure(ts.type))) { - return false; - } - if (ts.isInterface() && !ss.isInterface()) { - /* so ts is interface but ss is a class - * an interface is disjoint from a class if the class is disjoint form the interface + class DisjointChecker { + Set> pairsSeen = new HashSet<>(); + private boolean areDisjoint(ClassSymbol ts, ClassSymbol ss) { + Pair newPair = new Pair<>(ts, ss); + /* if we are seeing the same pair again then there is an issue with the sealed hierarchy + * bail out, a detailed error will be reported downstream */ - return areDisjoint(ss, ts); - } - // a final class that is not subtype of ss is disjoint - if (!ts.isInterface() && ts.isFinal()) { - return true; - } - // if at least one is sealed - if (ts.isSealed() || ss.isSealed()) { - // permitted subtypes have to be disjoint with the other symbol - ClassSymbol sealedOne = ts.isSealed() ? ts : ss; - ClassSymbol other = sealedOne == ts ? ss : ts; - return sealedOne.permitted.stream().allMatch(sym -> areDisjoint((ClassSymbol)sym, other)); + if (!pairsSeen.add(newPair)) + return false; + if (isSubtype(erasure(ts.type), erasure(ss.type))) { + return false; + } + // if both are classes or both are interfaces, shortcut + if (ts.isInterface() == ss.isInterface() && isSubtype(erasure(ss.type), erasure(ts.type))) { + return false; + } + if (ts.isInterface() && !ss.isInterface()) { + /* so ts is interface but ss is a class + * an interface is disjoint from a class if the class is disjoint form the interface + */ + return areDisjoint(ss, ts); + } + // a final class that is not subtype of ss is disjoint + if (!ts.isInterface() && ts.isFinal()) { + return true; + } + // if at least one is sealed + if (ts.isSealed() || ss.isSealed()) { + // permitted subtypes have to be disjoint with the other symbol + ClassSymbol sealedOne = ts.isSealed() ? ts : ss; + ClassSymbol other = sealedOne == ts ? ss : ts; + return sealedOne.permitted.stream().allMatch(sym -> areDisjoint((ClassSymbol)sym, other)); + } + return false; } - return false; } private TypeRelation isCastable = new TypeRelation() { diff --git a/test/langtools/tools/javac/sealed/erroneous_hierarchy/CyclicHierarchyTest.java b/test/langtools/tools/javac/sealed/erroneous_hierarchy/CyclicHierarchyTest.java new file mode 100644 index 00000000000..b3f30c4297a --- /dev/null +++ b/test/langtools/tools/javac/sealed/erroneous_hierarchy/CyclicHierarchyTest.java @@ -0,0 +1,12 @@ +/* + * @test /nodynamiccopyright/ + * @bug 8319987 + * @summary compilation of sealed classes leads to infinite recursion + * @compile/fail/ref=CyclicHierarchyTest.out -XDrawDiagnostics CyclicHierarchyTest.java + */ + +class CyclicHierarchyTest { + sealed interface Action permits Add {} + sealed interface MathOp permits Add {} + sealed static class Add implements MathOp permits Add {} +} diff --git a/test/langtools/tools/javac/sealed/erroneous_hierarchy/CyclicHierarchyTest.out b/test/langtools/tools/javac/sealed/erroneous_hierarchy/CyclicHierarchyTest.out new file mode 100644 index 00000000000..c9420a0dc09 --- /dev/null +++ b/test/langtools/tools/javac/sealed/erroneous_hierarchy/CyclicHierarchyTest.out @@ -0,0 +1,3 @@ +CyclicHierarchyTest.java:9:37: compiler.err.invalid.permits.clause: (compiler.misc.doesnt.extend.sealed: CyclicHierarchyTest.Add) +CyclicHierarchyTest.java:11:55: compiler.err.invalid.permits.clause: (compiler.misc.must.not.be.same.class) +2 errors From d6aa7c8ba0e727356562561d939c4965b69d7817 Mon Sep 17 00:00:00 2001 From: Vicente Romero Date: Thu, 16 Nov 2023 16:49:26 +0000 Subject: [PATCH 18/76] 8314621: ClassNotFoundException due to lambda reference to elided anonymous inner class Reviewed-by: jlahoda --- .../com/sun/tools/javac/comp/Lower.java | 20 ++- .../javac/6917288/GraphicalInstallerTest.java | 3 +- .../tools/javac/6917288/T6917288.java | 5 +- .../7199823/InnerClassCannotBeVerified.java | 142 ------------------ ...sNotFoundExceptionDueToPrunedCodeTest.java | 52 +++++++ 5 files changed, 74 insertions(+), 148 deletions(-) delete mode 100644 test/langtools/tools/javac/7199823/InnerClassCannotBeVerified.java create mode 100644 test/langtools/tools/javac/lambda/ClassNotFoundExceptionDueToPrunedCodeTest.java diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java index eb99897ee59..c51b1cb2a7e 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java @@ -1196,6 +1196,18 @@ ClassSymbol accessClass(Symbol sym, boolean protAccess, JCTree tree) { } } + private boolean noClassDefIn(JCTree tree) { + var scanner = new TreeScanner() { + boolean noClassDef = true; + @Override + public void visitClassDef(JCClassDecl tree) { + noClassDef = false; + } + }; + scanner.scan(tree); + return scanner.noClassDef; + } + private void addPrunedInfo(JCTree tree) { List infoList = prunedTree.get(currentClass); infoList = (infoList == null) ? List.of(tree) : infoList.prepend(tree); @@ -3030,10 +3042,10 @@ private Boolean expValueIsNull(boolean eq, JCTree t) { @Override public void visitConditional(JCConditional tree) { JCTree cond = tree.cond = translate(tree.cond, syms.booleanType); - if (isTrue(cond)) { + if (isTrue(cond) && noClassDefIn(tree.falsepart)) { result = convert(translate(tree.truepart, tree.type), tree.type); addPrunedInfo(cond); - } else if (isFalse(cond)) { + } else if (isFalse(cond) && noClassDefIn(tree.truepart)) { result = convert(translate(tree.falsepart, tree.type), tree.type); addPrunedInfo(cond); } else { @@ -3057,10 +3069,10 @@ private JCExpression convert(JCExpression tree, Type pt) { */ public void visitIf(JCIf tree) { JCTree cond = tree.cond = translate(tree.cond, syms.booleanType); - if (isTrue(cond)) { + if (isTrue(cond) && noClassDefIn(tree.elsepart)) { result = translate(tree.thenpart); addPrunedInfo(cond); - } else if (isFalse(cond)) { + } else if (isFalse(cond) && noClassDefIn(tree.thenpart)) { if (tree.elsepart != null) { result = translate(tree.elsepart); } else { diff --git a/test/langtools/tools/javac/6917288/GraphicalInstallerTest.java b/test/langtools/tools/javac/6917288/GraphicalInstallerTest.java index 8dfc0fabc44..3a5bd7d51b4 100644 --- a/test/langtools/tools/javac/6917288/GraphicalInstallerTest.java +++ b/test/langtools/tools/javac/6917288/GraphicalInstallerTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, 2018, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2010, 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 @@ -47,6 +47,7 @@ void run() throws Exception { } check(classes, + "GraphicalInstaller$1.class", "GraphicalInstaller$1X$1.class", "GraphicalInstaller$1X.class", "GraphicalInstaller$BackgroundInstaller.class", diff --git a/test/langtools/tools/javac/6917288/T6917288.java b/test/langtools/tools/javac/6917288/T6917288.java index 08b38449515..07c30e2d436 100644 --- a/test/langtools/tools/javac/6917288/T6917288.java +++ b/test/langtools/tools/javac/6917288/T6917288.java @@ -70,12 +70,15 @@ void test(Kind k) throws Exception { } switch (k) { + case NONE: + check(classesDir, "Test.class", "Test$Inner.class"); + break; case ALWAYS: case TRUE: check(classesDir, "Test.class", "Test$Inner.class", "Test$1.class"); break; default: - check(classesDir, "Test.class", "Test$Inner.class"); + check(classesDir, "Test.class", "Test$Inner.class", "Test$1.class"); } } diff --git a/test/langtools/tools/javac/7199823/InnerClassCannotBeVerified.java b/test/langtools/tools/javac/7199823/InnerClassCannotBeVerified.java deleted file mode 100644 index ae081a8c0a6..00000000000 --- a/test/langtools/tools/javac/7199823/InnerClassCannotBeVerified.java +++ /dev/null @@ -1,142 +0,0 @@ -/* - * Copyright (c) 2012, 2018, 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 7199823 - * @summary javac generates inner class that can't be verified - * @modules java.base/jdk.internal.classfile - * java.base/jdk.internal.classfile.attribute - * java.base/jdk.internal.classfile.constantpool - * java.base/jdk.internal.classfile.instruction - * java.base/jdk.internal.classfile.components - * java.base/jdk.internal.classfile.impl - * @run main InnerClassCannotBeVerified - */ - -import java.nio.file.NoSuchFileException; -import java.util.Arrays; -import javax.tools.JavaFileObject; -import java.net.URI; -import javax.tools.SimpleJavaFileObject; -import javax.tools.ToolProvider; -import javax.tools.JavaCompiler; -import com.sun.source.util.JavacTask; -import jdk.internal.classfile.*; -import java.io.File; -import java.io.IOException; - -public class InnerClassCannotBeVerified { - - enum CompilationKind { - PRE_NESTMATES("-source", "10", "-target", "10"), - POST_NESTMATES(); - - String[] opts; - - CompilationKind(String... opts) { - this.opts = opts; - } - } - - private static final String errorMessage = - "Compile error while compiling the following source:\n"; - - public static void main(String... args) throws Exception { - new InnerClassCannotBeVerified().run(); - } - - void run() throws Exception { - for (CompilationKind ck : CompilationKind.values()) { - File file = new File("Test$1.class"); - if (file.exists()) { - file.delete(); - } - JavaCompiler comp = ToolProvider.getSystemJavaCompiler(); - JavaSource source = new JavaSource(); - JavacTask ct = (JavacTask)comp.getTask(null, null, null, - Arrays.asList(ck.opts), null, Arrays.asList(source)); - try { - if (!ct.call()) { - throw new AssertionError(errorMessage + - source.getCharContent(true)); - } - } catch (Throwable ex) { - throw new AssertionError(errorMessage + - source.getCharContent(true)); - } - check(ck); - } - } - - private void check(CompilationKind ck) throws IOException { - try { - File file = new File("Test$1.class"); - ClassModel classFile = Classfile.of().parse(file.toPath()); - if (ck == CompilationKind.POST_NESTMATES) { - throw new AssertionError("Unexpected constructor tag class!"); - } - boolean inheritsFromObject = - classFile.superclass().orElseThrow().asInternalName().equals("java/lang/Object"); - boolean implementsNoInterface = classFile.interfaces().size() == 0; - boolean noMethods = classFile.methods().size() == 0; - if (!(inheritsFromObject && - implementsNoInterface && - noMethods)) { - throw new AssertionError("The inner classes reused as " + - "access constructor tag for this code must be empty"); - } - } catch (NoSuchFileException ex) { - if (ck == CompilationKind.PRE_NESTMATES) { - throw new AssertionError("Constructor tag class missing!"); - } - } - } - - class JavaSource extends SimpleJavaFileObject { - - String internalSource = - "public class Test {\n" + - " private static class Foo {}\n" + - " public static void main(String[] args){ \n" + - " new Foo();\n" + - " if(false) {\n" + - " new Runnable() {\n" + - " @Override\n" + - " public void run() {\n" + - " System.out.println();\n" + - " }\n" + - " }.run();\n" + - " }\n" + - " }\n" + - "}"; - public JavaSource() { - super(URI.create("Test.java"), JavaFileObject.Kind.SOURCE); - } - - @Override - public CharSequence getCharContent(boolean ignoreEncodingErrors) { - return internalSource; - } - } -} diff --git a/test/langtools/tools/javac/lambda/ClassNotFoundExceptionDueToPrunedCodeTest.java b/test/langtools/tools/javac/lambda/ClassNotFoundExceptionDueToPrunedCodeTest.java new file mode 100644 index 00000000000..d9481a227e4 --- /dev/null +++ b/test/langtools/tools/javac/lambda/ClassNotFoundExceptionDueToPrunedCodeTest.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 8314621 + * @summary ClassNotFoundException due to lambda reference to elided anonymous inner class + */ + +public class ClassNotFoundExceptionDueToPrunedCodeTest { + public static void main(String... args) { + var o1 = false ? new Object() {} : null; + Runnable r = () -> { + System.out.println(o1 == o1); + }; + r.run(); + + var o2 = true ? null : new Object() {}; + r = () -> { + System.out.println(o2 == o2); + }; + r.run(); + + var o3 = switch (0) { default -> { if (false) yield new Object() { }; else yield null; } }; + r = () -> System.out.println(o3); + r.run(); + + var o4 = switch (0) { default -> { if (true) yield null; else yield new Object() { }; } }; + r = () -> System.out.println(o4); + r.run(); + } +} From 9727f4bdddc071e6f59806087339f345405ab004 Mon Sep 17 00:00:00 2001 From: Brian Burkhalter Date: Thu, 16 Nov 2023 16:55:46 +0000 Subject: [PATCH 19/76] 8320199: Fix HTML 5 errors in java.math.BigInteger Reviewed-by: naoto, darcy, lancea, iris --- src/java.base/share/classes/java/math/BigInteger.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/java.base/share/classes/java/math/BigInteger.java b/src/java.base/share/classes/java/math/BigInteger.java index 53db80f0417..43c401dd136 100644 --- a/src/java.base/share/classes/java/math/BigInteger.java +++ b/src/java.base/share/classes/java/math/BigInteger.java @@ -119,7 +119,7 @@ * The range must be at least 1 to 2500000000. * * @apiNote - * As {@code BigInteger} values are + * As {@code BigInteger} values are * arbitrary precision integers, the algorithmic complexity of the * methods of this class varies and may be superlinear in the size of * the input. For example, a method like {@link intValue()} would be @@ -138,7 +138,7 @@ * algorithms between the bounds of the naive and theoretical cases * include the Karatsuba multiplication * (O(n1.585)) and 3-way Toom-Cook - * multiplication (O(n1.465)). + * multiplication (O(n1.465)). * *

A particular implementation of {@link multiply(BigInteger) * multiply} is free to switch between different algorithms for From 1588dd934ce4e00a060e329b80f721d894559597 Mon Sep 17 00:00:00 2001 From: Mandy Chung Date: Thu, 16 Nov 2023 22:40:22 +0000 Subject: [PATCH 20/76] 8319567: Update java/lang/invoke tests to support vm flags 8319568: Update java/lang/reflect/exeCallerAccessTest/CallerAccessTest.java to accept vm flags 8319672: Several classloader tests ignore VM flags 8319676: A couple of jdk/modules/incubator/ tests ignore VM flags 8319677: Test jdk/internal/misc/VM/RuntimeArguments.java should be marked as flagless Reviewed-by: jvernee, lmesnik --- test/jdk/java/lang/ClassLoader/Assert.java | 40 ++-- .../lang/ClassLoader/GetSystemPackage.java | 8 +- .../ClassLoader/getResource/GetResource.java | 22 +-- .../LoadLibraryUnloadTest.java | 26 +-- .../condy/CondyNestedResolutionTest.java | 2 +- .../invoke/findSpecial/FindSpecialTest.java | 20 +- test/jdk/java/lang/invoke/lambda/LUtils.java | 171 ------------------ .../LambdaAccessControlDoPrivilegedTest.java | 85 +++++---- .../lambda/LambdaAccessControlTest.java | 8 +- .../java/lang/invoke/lambda/LambdaAsm.java | 38 ++-- .../lang/invoke/lambda/LambdaStackTrace.java | 35 ++-- .../lambda/LogGeneratedClassesTest.java | 118 ++++++------ .../exeCallerAccessTest/CallerAccessTest.java | 36 +--- .../LargeClasspathWithPkgPrefix.java | 8 +- .../internal/misc/VM/RuntimeArguments.java | 12 +- .../jdk/modules/incubator/DefaultImage.java | 19 +- .../jdk/modules/incubator/ImageModules.java | 5 +- 17 files changed, 219 insertions(+), 434 deletions(-) delete mode 100644 test/jdk/java/lang/invoke/lambda/LUtils.java diff --git a/test/jdk/java/lang/ClassLoader/Assert.java b/test/jdk/java/lang/ClassLoader/Assert.java index d894fef5ef9..d7dc27c7f53 100644 --- a/test/jdk/java/lang/ClassLoader/Assert.java +++ b/test/jdk/java/lang/ClassLoader/Assert.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2020, 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 @@ -24,6 +24,8 @@ /* * @test * @bug 4290640 4785473 + * @requires vm.flagless + * @library /test/lib * @build package1.Class1 package2.Class2 package1.package3.Class3 Assert * @run main/othervm Assert * @summary Test the assertion facility @@ -31,12 +33,17 @@ * @key randomness */ +import jdk.test.lib.process.OutputAnalyzer; import package1.*; import package2.*; import package1.package3.*; -import java.io.*; + +import java.util.ArrayList; +import java.util.List; import java.util.Random; +import static jdk.test.lib.process.ProcessTools.*; + public class Assert { private static Class1 testClass1; @@ -56,7 +63,7 @@ public class Assert { * off at class load time. Once the class is loaded its assertion status * does not change. */ - public static void main(String[] args) throws Exception { + public static void main(String[] args) throws Throwable { // Switch values: 0=don't touch, 1=off, 2 = on int[] switches = new int[7]; @@ -77,28 +84,17 @@ public static void main(String[] args) throws Exception { } // Spawn new VM and load classes - String command = System.getProperty("java.home") + - File.separator + "bin" + File.separator + "java Assert"; - - StringBuffer commandString = new StringBuffer(command); + List commands = new ArrayList<>(); + commands.add("Assert"); for(int j=0; j<7; j++) - commandString.append(" "+switches[j]); - - Process p = null; - p = Runtime.getRuntime().exec(commandString.toString()); - + commands.add(Integer.toString(switches[j])); + OutputAnalyzer outputAnalyzer = executeCommand(createLimitedTestJavaProcessBuilder(commands)); if (debug) { // See output of test VMs - BufferedReader blah = new BufferedReader( - new InputStreamReader(p.getInputStream())); - String outString = blah.readLine(); - while (outString != null) { - System.out.println("from BufferedReader:"+outString); - outString = blah.readLine(); - } + outputAnalyzer.asLines() + .stream() + .forEach(s -> System.out.println(s)); } - - p.waitFor(); - int result = p.exitValue(); + int result = outputAnalyzer.getExitValue(); if (debug) { // See which switch configs failed if (result == 0) { for(int k=6; k>=0; k--) diff --git a/test/jdk/java/lang/ClassLoader/GetSystemPackage.java b/test/jdk/java/lang/ClassLoader/GetSystemPackage.java index 2af81f3c416..0b3653f2f98 100644 --- a/test/jdk/java/lang/ClassLoader/GetSystemPackage.java +++ b/test/jdk/java/lang/ClassLoader/GetSystemPackage.java @@ -24,6 +24,7 @@ /* * @test * @bug 8060130 + * @requires vm.flagless * @library /test/lib * @build package2.Class2 GetSystemPackage * @summary Test if getSystemPackage() return consistent values for cases @@ -41,6 +42,8 @@ import java.util.jar.JarEntry; import java.util.jar.JarOutputStream; import java.util.jar.Manifest; + +import jdk.test.lib.process.OutputAnalyzer; import jdk.test.lib.process.ProcessTools; public class GetSystemPackage { @@ -118,8 +121,9 @@ private static void buildJar(String name, Manifest man) throws Exception { private static void runSubProcess(String messageOnError, String ... args) throws Exception { - ProcessBuilder pb = ProcessTools.createLimitedTestJavaProcessBuilder(args); - int res = pb.directory(tmpFolder).inheritIO().start().waitFor(); + ProcessBuilder pb = ProcessTools.createLimitedTestJavaProcessBuilder(args) + .directory(tmpFolder); + int res = ProcessTools.executeProcess(pb).getExitValue(); if (res != 0) { throw new RuntimeException(messageOnError); } diff --git a/test/jdk/java/lang/ClassLoader/getResource/GetResource.java b/test/jdk/java/lang/ClassLoader/getResource/GetResource.java index 8c97657853d..2a1b76e7d4f 100644 --- a/test/jdk/java/lang/ClassLoader/getResource/GetResource.java +++ b/test/jdk/java/lang/ClassLoader/getResource/GetResource.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2018, 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 @@ -40,11 +40,9 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.List; -import java.util.Map; import java.util.stream.Collectors; import java.util.stream.Stream; -import jdk.test.lib.JDKToolFinder; import static jdk.test.lib.process.ProcessTools.*; import org.testng.annotations.BeforeTest; @@ -144,26 +142,14 @@ public void testCurrentDirA(List options, String expected) throws Throwa private void runTest(Path dir, List options, String expected) throws Throwable { - String javapath = JDKToolFinder.getJDKTool("java"); - List cmdLine = new ArrayList<>(); - cmdLine.add(javapath); options.forEach(cmdLine::add); cmdLine.add("GetResource"); cmdLine.add(expected); - - System.out.println("Command line: " + cmdLine); - ProcessBuilder pb = - new ProcessBuilder(cmdLine.stream().toArray(String[]::new)); - - // change working directory - pb.directory(dir.toFile()); - - // remove CLASSPATH environment variable - Map env = pb.environment(); - String value = env.remove("CLASSPATH"); - + ProcessBuilder pb = createTestJavaProcessBuilder(cmdLine); + pb.directory(dir.toFile()); // change working directory + pb.environment().remove("CLASSPATH"); // remove CLASSPATH environment variable executeCommand(pb).shouldHaveExitValue(0); } diff --git a/test/jdk/java/lang/ClassLoader/loadLibraryUnload/LoadLibraryUnloadTest.java b/test/jdk/java/lang/ClassLoader/loadLibraryUnload/LoadLibraryUnloadTest.java index 1cbde7b943f..c358fa64214 100644 --- a/test/jdk/java/lang/ClassLoader/loadLibraryUnload/LoadLibraryUnloadTest.java +++ b/test/jdk/java/lang/ClassLoader/loadLibraryUnload/LoadLibraryUnloadTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2021, BELLSOFT. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * @@ -38,31 +38,15 @@ */ import jdk.test.lib.Asserts; -import jdk.test.lib.JDKToolFinder; import jdk.test.lib.process.OutputAnalyzer; -import java.lang.ProcessBuilder; -import java.lang.Process; -import java.io.File; -import java.util.*; +import static jdk.test.lib.process.ProcessTools.*; public class LoadLibraryUnloadTest { private static String testClassPath = System.getProperty("test.classes"); private static String testLibraryPath = System.getProperty("test.nativepath"); - private static Process runJavaCommand(String... command) throws Throwable { - String java = JDKToolFinder.getJDKTool("java"); - List commands = new ArrayList<>(); - Collections.addAll(commands, java); - Collections.addAll(commands, command); - System.out.println("COMMAND: " + String.join(" ", commands)); - return new ProcessBuilder(commands.toArray(new String[0])) - .redirectErrorStream(true) - .directory(new File(testClassPath)) - .start(); - } - private final static long countLines(OutputAnalyzer output, String string) { return output.asLines() .stream() @@ -78,12 +62,10 @@ private final static void dump(OutputAnalyzer output) { public static void main(String[] args) throws Throwable { - Process process = runJavaCommand( + OutputAnalyzer outputAnalyzer = executeCommand(createTestJavaProcessBuilder( "-Dtest.classes=" + testClassPath, "-Djava.library.path=" + testLibraryPath, - "LoadLibraryUnload"); - - OutputAnalyzer outputAnalyzer = new OutputAnalyzer(process); + "LoadLibraryUnload")); dump(outputAnalyzer); Asserts.assertTrue( diff --git a/test/jdk/java/lang/invoke/condy/CondyNestedResolutionTest.java b/test/jdk/java/lang/invoke/condy/CondyNestedResolutionTest.java index 914e90650da..f7a36a344b1 100644 --- a/test/jdk/java/lang/invoke/condy/CondyNestedResolutionTest.java +++ b/test/jdk/java/lang/invoke/condy/CondyNestedResolutionTest.java @@ -44,7 +44,7 @@ */ public class CondyNestedResolutionTest { public static void main(String args[]) throws Throwable { - ProcessBuilder pb = ProcessTools.createLimitedTestJavaProcessBuilder("CondyNestedResolution"); + ProcessBuilder pb = ProcessTools.createTestJavaProcessBuilder("CondyNestedResolution"); OutputAnalyzer oa = new OutputAnalyzer(pb.start()); oa.shouldContain("StackOverflowError"); oa.shouldContain("bsm1arg"); diff --git a/test/jdk/java/lang/invoke/findSpecial/FindSpecialTest.java b/test/jdk/java/lang/invoke/findSpecial/FindSpecialTest.java index 129951b79d0..9b6622a7e9c 100644 --- a/test/jdk/java/lang/invoke/findSpecial/FindSpecialTest.java +++ b/test/jdk/java/lang/invoke/findSpecial/FindSpecialTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 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 @@ -37,13 +37,11 @@ import java.nio.file.Path; import java.nio.file.Paths; -import jdk.test.lib.JDKToolFinder; -import jdk.test.lib.process.ProcessTools; +import static jdk.test.lib.process.ProcessTools.*; import org.testng.annotations.Test; public class FindSpecialTest { - static final String JAVA_LAUNCHER = JDKToolFinder.getJDKTool("java"); static final String TEST_CLASSES = System.getProperty("test.classes", "."); static final String TEST_CLASS_PATH = System.getProperty("test.class.path"); static final String TEST_MAIN_CLASS = "test.FindSpecial"; @@ -59,8 +57,9 @@ public static void callerInUnnamedModule() throws Throwable { throw new Error(m1 + " not exist"); } String classpath = m1.toString() + File.pathSeparator + TEST_CLASS_PATH; - ProcessTools.executeCommand(JAVA_LAUNCHER, "-cp", classpath, TEST_MAIN_CLASS) - .shouldHaveExitValue(0); + executeCommand(createTestJavaProcessBuilder("-cp", classpath, + TEST_MAIN_CLASS)) + .shouldHaveExitValue(0); } /* @@ -72,10 +71,9 @@ public static void callerInNamedModule() throws Throwable { if (Files.notExists(modules)) { throw new Error(modules + " not exist"); } - ProcessTools.executeCommand(JAVA_LAUNCHER, - "-cp", TEST_CLASS_PATH, - "-p", modules.toString(), - "-m", TEST_MODULE + "/" + TEST_MAIN_CLASS) - .shouldHaveExitValue(0); + executeCommand(createTestJavaProcessBuilder("-cp", TEST_CLASS_PATH, + "-p", modules.toString(), + "-m", TEST_MODULE + "/" + TEST_MAIN_CLASS)) + .shouldHaveExitValue(0); } } diff --git a/test/jdk/java/lang/invoke/lambda/LUtils.java b/test/jdk/java/lang/invoke/lambda/LUtils.java deleted file mode 100644 index cc052e43e90..00000000000 --- a/test/jdk/java/lang/invoke/lambda/LUtils.java +++ /dev/null @@ -1,171 +0,0 @@ -/* - * Copyright (c) 2012, 2013, 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.io.BufferedReader; -import java.io.File; -import java.io.IOException; -import java.io.InputStreamReader; -import java.io.PrintWriter; -import java.io.StringWriter; -import java.nio.charset.Charset; -import java.nio.file.Files; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; - -/* - * support infrastructure to invoke a java class from the command line - */ -class LUtils { - static final com.sun.tools.javac.Main javac = - new com.sun.tools.javac.Main(); - static final File cwd = new File(".").getAbsoluteFile(); - static final String JAVAHOME = System.getProperty("java.home"); - static final boolean isWindows = - System.getProperty("os.name", "unknown").startsWith("Windows"); - static final File JAVA_BIN_FILE = new File(JAVAHOME, "bin"); - static final File JAVA_CMD = new File(JAVA_BIN_FILE, - isWindows ? "java.exe" : "java"); - static final File JAR_BIN_FILE = new File(JAVAHOME, "bin"); - static final File JAR_CMD = new File(JAR_BIN_FILE, - isWindows ? "jar.exe" : "jar"); - - protected LUtils() { - } - - public static void compile(String... args) { - if (javac.compile(args) != 0) { - throw new RuntimeException("compilation fails"); - } - } - - static void createFile(File outFile, List content) { - try { - Files.write(outFile.getAbsoluteFile().toPath(), content, - Charset.defaultCharset()); - } catch (IOException ex) { - throw new RuntimeException(ex); - } - } - - static File getClassFile(File javaFile) { - return javaFile.getName().endsWith(".java") - ? new File(javaFile.getName().replace(".java", ".class")) - : null; - } - - static String getSimpleName(File inFile) { - String fname = inFile.getName(); - return fname.substring(0, fname.indexOf(".")); - } - - static TestResult doExec(String... cmds) { - return doExec(null, null, cmds); - } - - /* - * A method which executes a java cmd and returns the results in a container - */ - static TestResult doExec(Map envToSet, - java.util.Set envToRemove, String... cmds) { - String cmdStr = ""; - for (String x : cmds) { - cmdStr = cmdStr.concat(x + " "); - } - ProcessBuilder pb = new ProcessBuilder(cmds); - Map env = pb.environment(); - if (envToRemove != null) { - for (String key : envToRemove) { - env.remove(key); - } - } - if (envToSet != null) { - env.putAll(envToSet); - } - BufferedReader rdr = null; - try { - List outputList = new ArrayList<>(); - pb.redirectErrorStream(true); - Process p = pb.start(); - rdr = new BufferedReader(new InputStreamReader(p.getInputStream())); - String in = rdr.readLine(); - while (in != null) { - outputList.add(in); - in = rdr.readLine(); - } - p.waitFor(); - p.destroy(); - - return new TestResult(cmdStr, p.exitValue(), outputList, - env, new Throwable("current stack of the test")); - } catch (Exception ex) { - ex.printStackTrace(); - throw new RuntimeException(ex.getMessage()); - } - } - - static class TestResult { - String cmd; - int exitValue; - List testOutput; - Map env; - Throwable t; - - public TestResult(String str, int rv, List oList, - Map env, Throwable t) { - cmd = str; - exitValue = rv; - testOutput = oList; - this.env = env; - this.t = t; - } - - void assertZero(String message) { - if (exitValue != 0) { - System.err.println(this); - throw new RuntimeException(message); - } - } - - @Override - public String toString() { - StringWriter sw = new StringWriter(); - PrintWriter status = new PrintWriter(sw); - status.println("Cmd: " + cmd); - status.println("Return code: " + exitValue); - status.println("Environment variable:"); - for (String x : env.keySet()) { - status.println("\t" + x + "=" + env.get(x)); - } - status.println("Output:"); - for (String x : testOutput) { - status.println("\t" + x); - } - status.println("Exception:"); - status.println(t.getMessage()); - t.printStackTrace(status); - - return sw.getBuffer().toString(); - } - } -} diff --git a/test/jdk/java/lang/invoke/lambda/LambdaAccessControlDoPrivilegedTest.java b/test/jdk/java/lang/invoke/lambda/LambdaAccessControlDoPrivilegedTest.java index 37be43d0426..fc6fd91e5ac 100644 --- a/test/jdk/java/lang/invoke/lambda/LambdaAccessControlDoPrivilegedTest.java +++ b/test/jdk/java/lang/invoke/lambda/LambdaAccessControlDoPrivilegedTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 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 @@ -24,19 +24,28 @@ /* * @test * @bug 8003881 - * @summary tests DoPrivileged action (implemented as lambda expressions) by - * inserting them into the BootClassPath. + * @library /test/lib/ * @modules jdk.compiler * jdk.zipfs - * @compile -XDignore.symbol.file LambdaAccessControlDoPrivilegedTest.java LUtils.java + * @compile LambdaAccessControlDoPrivilegedTest.java * @run main/othervm -Djava.security.manager=allow LambdaAccessControlDoPrivilegedTest + * @summary tests DoPrivileged action (implemented as lambda expressions) by + * inserting them into the BootClassPath. */ -import java.io.File; +import jdk.test.lib.process.OutputAnalyzer; + +import java.io.IOException; +import java.nio.charset.Charset; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import java.util.List; +import java.util.spi.ToolProvider; + +import static jdk.test.lib.process.ProcessTools.*; -public class LambdaAccessControlDoPrivilegedTest extends LUtils { - public static void main(String... args) { +public class LambdaAccessControlDoPrivilegedTest { + public static void main(String... args) throws Exception { final List scratch = new ArrayList(); scratch.clear(); scratch.add("import java.security.*;"); @@ -47,9 +56,9 @@ public static void main(String... args) { scratch.add("});"); scratch.add("}"); scratch.add("}"); - File doprivJava = new File("DoPriv.java"); - File doprivClass = getClassFile(doprivJava); - createFile(doprivJava, scratch); + Path doprivJava = Path.of("DoPriv.java"); + Path doprivClass = Path.of("DoPriv.class"); + Files.write(doprivJava, scratch, Charset.defaultCharset()); scratch.clear(); scratch.add("public class Bar {"); @@ -59,30 +68,40 @@ public static void main(String... args) { scratch.add("}"); scratch.add("}"); - File barJava = new File("Bar.java"); - File barClass = getClassFile(barJava); - createFile(barJava, scratch); + Path barJava = Path.of("Bar.java"); + Path barClass = Path.of("Bar.class"); + Files.write(barJava, scratch, Charset.defaultCharset()); + + compile(barJava.toString(), doprivJava.toString()); + + jar("cvf", "foo.jar", doprivClass.toString()); + Files.delete(doprivJava); + Files.delete(doprivClass); + + ProcessBuilder pb = createTestJavaProcessBuilder( + "-Xbootclasspath/a:foo.jar", + "-cp", ".", + "-Djava.security.manager=allow", + "Bar"); + executeProcess(pb).shouldHaveExitValue(0); + + Files.delete(barJava); + Files.delete(barClass); + Files.delete(Path.of("foo.jar")); + } + + static final ToolProvider JAR_TOOL = ToolProvider.findFirst("jar").orElseThrow(); + static final ToolProvider JAVAC = ToolProvider.findFirst("javac").orElseThrow(); + static void compile(String... args) throws IOException { + if (JAVAC.run(System.out, System.err, args) != 0) { + throw new RuntimeException("compilation fails"); + } + } - String[] javacArgs = {barJava.getName(), doprivJava.getName()}; - compile(javacArgs); - File jarFile = new File("foo.jar"); - String[] jargs = {"cvf", jarFile.getName(), doprivClass.getName()}; - TestResult tr = doExec(JAR_CMD.getAbsolutePath(), - "cvf", jarFile.getName(), - doprivClass.getName()); - if (tr.exitValue != 0){ - throw new RuntimeException(tr.toString()); + static void jar(String... args) { + int rc = JAR_TOOL.run(System.out, System.err, args); + if (rc != 0){ + throw new RuntimeException("fail to create JAR file"); } - doprivJava.delete(); - doprivClass.delete(); - tr = doExec(JAVA_CMD.getAbsolutePath(), - "-Xbootclasspath/a:foo.jar", - "-cp", ".", - "-Djava.security.manager=allow", - "Bar"); - tr.assertZero("testDoPrivileged fails"); - barJava.delete(); - barClass.delete(); - jarFile.delete(); } } diff --git a/test/jdk/java/lang/invoke/lambda/LambdaAccessControlTest.java b/test/jdk/java/lang/invoke/lambda/LambdaAccessControlTest.java index 2ae76b91111..2588d30be23 100644 --- a/test/jdk/java/lang/invoke/lambda/LambdaAccessControlTest.java +++ b/test/jdk/java/lang/invoke/lambda/LambdaAccessControlTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 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 @@ -24,14 +24,12 @@ /* * @test * @bug 8003881 - * @summary tests Lambda expression with a security manager at top level * @modules jdk.compiler - * @compile -XDignore.symbol.file LambdaAccessControlTest.java LUtils.java - * * @run main/othervm -Djava.security.manager=allow LambdaAccessControlTest + * @summary tests Lambda expression with a security manager at top level */ -public class LambdaAccessControlTest extends LUtils { +public class LambdaAccessControlTest { public static void main(String... args) { System.setSecurityManager(new SecurityManager()); JJ iii = (new CC())::impl; diff --git a/test/jdk/java/lang/invoke/lambda/LambdaAsm.java b/test/jdk/java/lang/invoke/lambda/LambdaAsm.java index 647f906cba8..f0d79988783 100644 --- a/test/jdk/java/lang/invoke/lambda/LambdaAsm.java +++ b/test/jdk/java/lang/invoke/lambda/LambdaAsm.java @@ -24,13 +24,14 @@ /* * @test * @bug 8027232 - * @summary ensures that j.l.i.InvokerByteCodeGenerator and ASM visitMethodInsn - * generate bytecodes with correct constant pool references + * @library /test/lib/ * @modules java.base/jdk.internal.org.objectweb.asm * jdk.jdeps/com.sun.tools.classfile * jdk.zipfs - * @compile -XDignore.symbol.file LambdaAsm.java LUtils.java + * @compile LambdaAsm.java * @run main/othervm LambdaAsm + * @summary ensures that j.l.i.InvokerByteCodeGenerator and ASM visitMethodInsn + * generate bytecodes with correct constant pool references */ import com.sun.tools.classfile.Attribute; import com.sun.tools.classfile.ClassFile; @@ -40,34 +41,36 @@ import com.sun.tools.classfile.Instruction; import com.sun.tools.classfile.Method; import java.io.ByteArrayInputStream; -import java.io.File; +import java.io.IOException; +import java.nio.charset.Charset; +import java.nio.file.Files; import java.util.ArrayList; import java.nio.file.DirectoryStream; import java.nio.file.Path; import jdk.internal.org.objectweb.asm.ClassWriter; import jdk.internal.org.objectweb.asm.MethodVisitor; +import jdk.test.lib.compiler.CompilerUtils; +import jdk.test.lib.process.OutputAnalyzer; import static java.nio.file.Files.*; import static jdk.internal.org.objectweb.asm.Opcodes.*; +import static jdk.test.lib.process.ProcessTools.*; public class LambdaAsm { static final Path DUMP_LAMBDA_PROXY_CLASS_FILES = Path.of("DUMP_LAMBDA_PROXY_CLASS_FILES"); + static final Path SRC = Path.of("src"); + static final Path CLASSES = Path.of("classes"); - static final File TestFile = new File("A.java"); - - static void init() { + static void init() throws Exception { emitCode(); - LUtils.compile(TestFile.getName()); - LUtils.TestResult tr = LUtils.doExec(LUtils.JAVA_CMD.getAbsolutePath(), + CompilerUtils.compile(SRC, CLASSES); + OutputAnalyzer outputAnalyzer = executeProcess(createTestJavaProcessBuilder( "-Djdk.invoke.LambdaMetafactory.dumpProxyClassFiles=true", - "-cp", ".", "A"); - if (tr.exitValue != 0) { - System.out.println("Error: " + tr.toString()); - throw new RuntimeException("could not create proxy classes"); - } + "-cp", CLASSES.toString(), "A")); + outputAnalyzer.shouldHaveExitValue(0); } - static void emitCode() { + static void emitCode() throws IOException { ArrayList scratch = new ArrayList<>(); scratch.add("import java.util.function.*;"); scratch.add("class A {"); @@ -89,7 +92,10 @@ static void emitCode() { scratch.add(" I.d();"); scratch.add(" }"); scratch.add("}"); - LUtils.createFile(TestFile, scratch); + + Path testFile = SRC.resolve("A.java"); + Files.createDirectories(SRC); + Files.write(testFile, scratch, Charset.defaultCharset()); } static void checkMethod(String cname, String mname, ConstantPool cp, diff --git a/test/jdk/java/lang/invoke/lambda/LambdaStackTrace.java b/test/jdk/java/lang/invoke/lambda/LambdaStackTrace.java index f27b56b72b3..6ae63e4b7d9 100644 --- a/test/jdk/java/lang/invoke/lambda/LambdaStackTrace.java +++ b/test/jdk/java/lang/invoke/lambda/LambdaStackTrace.java @@ -24,20 +24,23 @@ /* * @test * @bug 8025636 - * @summary Synthetic frames should be hidden in exceptions + * @library /test/lib/ * @modules java.base/jdk.internal.org.objectweb.asm * jdk.compiler - * @compile -XDignore.symbol.file LUtils.java LambdaStackTrace.java + * @compile LambdaStackTrace.java * @run main LambdaStackTrace + * @summary Synthetic frames should be hidden in exceptions */ import jdk.internal.org.objectweb.asm.ClassWriter; +import jdk.test.lib.compiler.CompilerUtils; -import java.io.File; -import java.io.FileOutputStream; import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.nio.charset.Charset; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.ArrayList; import static jdk.internal.org.objectweb.asm.Opcodes.ACC_ABSTRACT; @@ -47,7 +50,7 @@ public class LambdaStackTrace { - static File classes = new File(System.getProperty("test.classes")); + static Path CLASSES = Path.of(System.getProperty("test.classes", ".")); public static void main(String[] args) throws Exception { testBasic(); @@ -121,12 +124,8 @@ private static void generateInterfaces() throws IOException { // We can't let javac compile these interfaces because in > 1.8 it will insert // bridge methods into the interfaces - we want code that looks like <= 1.7, // so we generate it. - try (FileOutputStream fw = new FileOutputStream(new File(classes, "Maker.class"))) { - fw.write(generateMaker()); - } - try (FileOutputStream fw = new FileOutputStream(new File(classes, "StringMaker.class"))) { - fw.write(generateStringMaker()); - } + Files.write(CLASSES.resolve("Maker.class"), generateMaker()); + Files.write(CLASSES.resolve("StringMaker.class"), generateStringMaker()); } private static byte[] generateMaker() { @@ -154,7 +153,7 @@ private static byte[] generateStringMaker() { } - static void emitCode(File f) { + static void emitCode(Path file) throws IOException { ArrayList scratch = new ArrayList<>(); scratch.add("public class Caller {"); scratch.add(" public static void callStringMaker() {"); @@ -166,13 +165,17 @@ static void emitCode(File f) { scratch.add(" ((Maker) sm).make();"); // <-- This will call the bridge method scratch.add(" }"); scratch.add("}"); - LUtils.createFile(f, scratch); + + Files.write(file, scratch, Charset.defaultCharset()); } - static void compileCaller() { - File caller = new File(classes, "Caller.java"); + static void compileCaller() throws IOException { + Path src = Path.of("src"); + Files.createDirectories(src); + + Path caller = src.resolve("Caller.java"); emitCode(caller); - LUtils.compile("-cp", classes.getAbsolutePath(), "-d", classes.getAbsolutePath(), caller.getAbsolutePath()); + CompilerUtils.compile(src, CLASSES, "-cp", CLASSES.toAbsolutePath().toString()); } private static void verifyFrames(StackTraceElement[] stack, String... patterns) throws Exception { diff --git a/test/jdk/java/lang/invoke/lambda/LogGeneratedClassesTest.java b/test/jdk/java/lang/invoke/lambda/LogGeneratedClassesTest.java index 26ed7b0058e..91ea4b932ca 100644 --- a/test/jdk/java/lang/invoke/lambda/LogGeneratedClassesTest.java +++ b/test/jdk/java/lang/invoke/lambda/LogGeneratedClassesTest.java @@ -24,14 +24,16 @@ /* * @test * @bug 8023524 8304846 - * @summary tests logging generated classes for lambda + * @requires vm.flagless + * @library /test/lib/ * @library /java/nio/file * @modules jdk.compiler * jdk.zipfs * @run testng LogGeneratedClassesTest + * @summary tests logging generated classes for lambda */ -import java.io.File; import java.io.IOException; +import java.nio.charset.Charset; import java.util.ArrayList; import java.util.List; import java.util.function.Predicate; @@ -40,18 +42,21 @@ import java.nio.file.Paths; import java.nio.file.attribute.PosixFileAttributeView; +import jdk.test.lib.compiler.CompilerUtils; +import jdk.test.lib.process.OutputAnalyzer; import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; -import org.testng.SkipException; import static java.nio.file.attribute.PosixFilePermissions.*; +import static jdk.test.lib.process.ProcessTools.*; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; -public class LogGeneratedClassesTest extends LUtils { +public class LogGeneratedClassesTest { static final Path DUMP_LAMBDA_PROXY_CLASS_FILES = Path.of("DUMP_LAMBDA_PROXY_CLASS_FILES"); + static final Path CLASSES = Path.of("classes").toAbsolutePath(); String longFQCN; @BeforeClass @@ -74,9 +79,8 @@ public void setup() throws IOException { scratch.add(" }"); scratch.add("}"); - File test = new File("TestLambda.java"); - createFile(test, scratch); - compile("-d", ".", test.getName()); + Path testLambda = Path.of("TestLambda.java"); + Files.write(testLambda, scratch, Charset.defaultCharset()); scratch.remove(0); scratch.remove(0); @@ -91,9 +95,10 @@ public void setup() throws IOException { sb.append(";"); sb.insert(0, "package "); scratch.add(0, sb.toString()); - test = new File("LongPackageName.java"); - createFile(test, scratch); - compile("-d", ".", test.getName()); + Path lpnTest = Path.of("LongPackageName.java"); + Files.write(lpnTest, scratch, Charset.defaultCharset()); + + CompilerUtils.compile(Path.of("."), CLASSES); } @AfterClass @@ -107,49 +112,49 @@ public void cleanup() throws IOException { } @Test - public void testNotLogging() { - TestResult tr = doExec(JAVA_CMD.getAbsolutePath(), - "-cp", ".", + public void testNotLogging() throws Exception { + ProcessBuilder pb = createLimitedTestJavaProcessBuilder( + "-cp", CLASSES.toString(), "-Djava.security.manager=allow", "com.example.TestLambda"); - tr.assertZero("Should still return 0"); + executeProcess(pb).shouldHaveExitValue(0); } @Test - public void testLogging() throws IOException { + public void testLogging() throws Exception { Path testDir = Path.of("dump"); Path dumpDir = testDir.resolve(DUMP_LAMBDA_PROXY_CLASS_FILES); Files.createDirectory(testDir); - TestResult tr = doExec(JAVA_CMD.getAbsolutePath(), - "-cp", "..", - "-Duser.dir=" + testDir.toAbsolutePath(), + ProcessBuilder pb = createLimitedTestJavaProcessBuilder( + "-cp", CLASSES.toString(), "-Djava.security.manager=allow", "-Djdk.invoke.LambdaMetafactory.dumpProxyClassFiles", - "com.example.TestLambda"); + "com.example.TestLambda").directory(testDir.toFile()); + executeProcess(pb).shouldHaveExitValue(0); + // 2 our own class files. We don't care about the others assertEquals(Files.find( dumpDir, 99, (p, a) -> p.startsWith(dumpDir.resolve("com/example")) && a.isRegularFile()).count(), - 2, "Two lambda captured"); - tr.assertZero("Should still return 0"); + 2, "Two lambda captured"); } @Test - public void testDumpDirNotExist() throws IOException { + public void testDumpDirNotExist() throws Exception { Path testDir = Path.of("NotExist"); Path dumpDir = testDir.resolve(DUMP_LAMBDA_PROXY_CLASS_FILES); Files.createDirectory(testDir); TestUtil.removeAll(dumpDir); - assertFalse(Files.exists(dumpDir)); - TestResult tr = doExec(JAVA_CMD.getAbsolutePath(), - "-cp", "..", - "-Duser.dir=" + testDir.toAbsolutePath(), - "-Djava.security.manager=allow", - "-Djdk.invoke.LambdaMetafactory.dumpProxyClassFiles", - "com.example.TestLambda"); + + ProcessBuilder pb = createLimitedTestJavaProcessBuilder( + "-cp", CLASSES.toString(), + "-Djava.security.manager=allow", + "-Djdk.invoke.LambdaMetafactory.dumpProxyClassFiles", + "com.example.TestLambda").directory(testDir.toFile()); + executeProcess(pb).shouldHaveExitValue(0); // The dump directory will be created if not exist assertEquals(Files.find( @@ -157,28 +162,24 @@ public void testDumpDirNotExist() throws IOException { 99, (p, a) -> p.startsWith(dumpDir.resolve("com/example")) && a.isRegularFile()).count(), - 2, "Two lambda captured"); - tr.assertZero("Should still return 0"); + 2, "Two lambda captured"); } @Test - public void testDumpDirIsFile() throws IOException { + public void testDumpDirIsFile() throws Exception { Path testDir = Path.of("notDir"); Path dumpFile = testDir.resolve(DUMP_LAMBDA_PROXY_CLASS_FILES); Files.createDirectory(testDir); Files.createFile(dumpFile); assertTrue(Files.isRegularFile(dumpFile)); - TestResult tr = doExec(JAVA_CMD.getAbsolutePath(), - "-cp", "..", - "-Duser.dir=" + testDir.toAbsolutePath(), - "-Djava.security.manager=allow", - "-Djdk.invoke.LambdaMetafactory.dumpProxyClassFiles", - "com.example.TestLambda"); - assertEquals(tr.testOutput.stream() - .filter(s -> s.contains("DUMP_LAMBDA_PROXY_CLASS_FILES is not a directory")) - .count(), - 1, "only show error once"); - assertTrue(tr.exitValue != 0); + ProcessBuilder pb = createLimitedTestJavaProcessBuilder( + "-cp", CLASSES.toString(), + "-Djava.security.manager=allow", + "-Djdk.invoke.LambdaMetafactory.dumpProxyClassFiles", + "com.example.TestLambda").directory(testDir.toFile()); + executeProcess(pb) + .shouldContain("DUMP_LAMBDA_PROXY_CLASS_FILES is not a directory") + .shouldNotHaveExitValue(0); } private static boolean isWriteableDirectory(Path p) { @@ -205,7 +206,7 @@ private static boolean isWriteableDirectory(Path p) { } @Test - public void testDumpDirNotWritable() throws IOException { + public void testDumpDirNotWritable() throws Exception { if (!Files.getFileStore(Paths.get(".")) .supportsFileAttributeView(PosixFileAttributeView.class)) { // No easy way to setup readonly directory without POSIX @@ -230,35 +231,33 @@ public void testDumpDirNotWritable() throws IOException { return; } - TestResult tr = doExec(JAVA_CMD.getAbsolutePath(), - "-cp", "..", - "-Duser.dir=" + testDir.toAbsolutePath(), + ProcessBuilder pb = createLimitedTestJavaProcessBuilder( + "-cp", CLASSES.toString(), "-Djava.security.manager=allow", "-Djdk.invoke.LambdaMetafactory.dumpProxyClassFiles", - "com.example.TestLambda"); - assertEquals(tr.testOutput.stream() - .filter(s -> s.contains("DUMP_LAMBDA_PROXY_CLASS_FILES is not writable")) - .count(), - 1, "only show error once"); - assertTrue(tr.exitValue != 0); + "com.example.TestLambda").directory(testDir.toFile()); + executeProcess(pb) + .shouldContain("DUMP_LAMBDA_PROXY_CLASS_FILES is not writable") + .shouldNotHaveExitValue(0); } finally { TestUtil.removeAll(testDir); } } @Test - public void testLoggingException() throws IOException { + public void testLoggingException() throws Exception { Path testDir = Path.of("dumpLong"); Path dumpDir = testDir.resolve(DUMP_LAMBDA_PROXY_CLASS_FILES); Files.createDirectories(dumpDir.resolve("com/example/nonsense")); Files.createFile(dumpDir.resolve("com/example/nonsense/nonsense")); - TestResult tr = doExec(JAVA_CMD.getAbsolutePath(), - "-cp", "..", - "-Duser.dir=" + testDir.toAbsolutePath(), + ProcessBuilder pb = createLimitedTestJavaProcessBuilder( + "-cp", CLASSES.toString(), "-Djava.security.manager=allow", "-Djdk.invoke.LambdaMetafactory.dumpProxyClassFiles", - longFQCN); - assertEquals(tr.testOutput.stream() + longFQCN).directory(testDir.toFile()); + OutputAnalyzer outputAnalyzer = executeProcess(pb); + outputAnalyzer.shouldHaveExitValue(0); + assertEquals(outputAnalyzer.asLines().stream() .filter(s -> s.startsWith("WARNING: Exception")) .count(), 2, "show error each capture"); @@ -279,6 +278,5 @@ public void testLoggingException() throws IOException { assertEquals(Files.walk(dumpDir) .filter(filter) .count(), 5, "Two lambda captured failed to log"); - tr.assertZero("Should still return 0"); } } diff --git a/test/jdk/java/lang/reflect/exeCallerAccessTest/CallerAccessTest.java b/test/jdk/java/lang/reflect/exeCallerAccessTest/CallerAccessTest.java index a6d17f90632..749133bb581 100644 --- a/test/jdk/java/lang/reflect/exeCallerAccessTest/CallerAccessTest.java +++ b/test/jdk/java/lang/reflect/exeCallerAccessTest/CallerAccessTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 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 @@ -31,37 +31,15 @@ // Test disabled on AIX since we cannot invoke the JVM on the primordial thread. -import java.io.File; -import java.util.Map; import jdk.test.lib.Platform; -import jdk.test.lib.process.OutputAnalyzer; - -import java.io.IOException; -import java.nio.file.Path; -import java.nio.file.Paths; +import jdk.test.lib.process.ProcessTools; public class CallerAccessTest { - public static void main(String[] args) throws IOException { - Path launcher = Paths.get(System.getProperty("test.nativepath"), "CallerAccessTest"); - ProcessBuilder pb = new ProcessBuilder(launcher.toString()); - Map env = pb.environment(); - - String libDir = Platform.libDir().toString(); - String vmDir = Platform.jvmLibDir().toString(); - - // set up shared library path - String sharedLibraryPathEnvName = Platform.sharedLibraryPathVariableName(); - env.compute(sharedLibraryPathEnvName, - (k, v) -> (v == null) ? libDir : v + File.pathSeparator + libDir); - env.compute(sharedLibraryPathEnvName, - (k, v) -> (v == null) ? vmDir : v + File.pathSeparator + vmDir); - - System.out.println("Launching: " + launcher + " shared library path: " + - env.get(sharedLibraryPathEnvName)); - new OutputAnalyzer(pb.start()) - .outputTo(System.out) - .errorTo(System.err) - .shouldHaveExitValue(0); + public static void main(String[] args) throws Exception { + ProcessBuilder pb = ProcessTools.createNativeTestProcessBuilder("CallerAccessTest"); + System.out.println("Launching: " + pb.command() + " shared library path: " + + pb.environment().get(Platform.sharedLibraryPathVariableName())); + ProcessTools.executeProcess(pb).shouldHaveExitValue(0); } } diff --git a/test/jdk/jdk/internal/loader/URLClassPath/LargeClasspathWithPkgPrefix.java b/test/jdk/jdk/internal/loader/URLClassPath/LargeClasspathWithPkgPrefix.java index 17def65a6ba..0e532497a46 100644 --- a/test/jdk/jdk/internal/loader/URLClassPath/LargeClasspathWithPkgPrefix.java +++ b/test/jdk/jdk/internal/loader/URLClassPath/LargeClasspathWithPkgPrefix.java @@ -25,7 +25,6 @@ import java.nio.file.Files; import java.nio.file.Path; -import jdk.test.lib.JDKToolFinder; import jdk.test.lib.compiler.CompilerUtils; import jdk.test.lib.process.OutputAnalyzer; import jdk.test.lib.process.ProcessTools; @@ -34,12 +33,12 @@ /* * @test * @bug 8308184 - * @summary Verify that an application can be launched when the classpath contains large number of - * jars and the java.protocol.handler.pkgs system property is set * @library /test/lib/ * @build jdk.test.lib.util.JarBuilder jdk.test.lib.compiler.CompilerUtils * jdk.test.lib.process.ProcessTools * @run driver LargeClasspathWithPkgPrefix + * @summary Verify that an application can be launched when the classpath contains large number of + * jars and the java.protocol.handler.pkgs system property is set */ public class LargeClasspathWithPkgPrefix { @@ -126,8 +125,7 @@ private static void compile(Path javaFile, Path destDir) throws Exception { // java -Djava.protocol.handler.pkgs=foo.bar.some.nonexistent.pkg -cp Foo private static void launchApplication(String classPath) throws Exception { - String java = JDKToolFinder.getJDKTool("java"); - ProcessBuilder pb = new ProcessBuilder(java, + ProcessBuilder pb = ProcessTools.createTestJavaProcessBuilder( "-Djava.protocol.handler.pkgs=foo.bar.some.nonexistent.pkg", "-cp", classPath, "Foo"); diff --git a/test/jdk/jdk/internal/misc/VM/RuntimeArguments.java b/test/jdk/jdk/internal/misc/VM/RuntimeArguments.java index c48de900ec3..dbcb30255a8 100644 --- a/test/jdk/jdk/internal/misc/VM/RuntimeArguments.java +++ b/test/jdk/jdk/internal/misc/VM/RuntimeArguments.java @@ -23,11 +23,12 @@ /** * @test - * @summary Basic test of VM::getRuntimeArguments + * @requires vm.flagless * @library /test/lib * @modules java.base/jdk.internal.misc * jdk.zipfs * @run testng RuntimeArguments + * @summary Basic test of VM::getRuntimeArguments */ import java.io.IOException; @@ -46,7 +47,6 @@ import static org.testng.Assert.*; public class RuntimeArguments { - static final String TEST_CLASSES = System.getProperty("test.classes"); static final List VM_OPTIONS = getInitialOptions(); /* @@ -112,11 +112,9 @@ public static void main(String... expected) { @Test(dataProvider = "options") public void test(List args, List expected) throws Exception { - // launch a test program - // $ java -classpath RuntimeArguments - Stream options = Stream.concat(args.stream(), - Stream.of("-classpath", TEST_CLASSES, "RuntimeArguments")); - + // launch a test program with classpath set by ProcessTools::createLimitedTestJavaProcessBuilder + // $ java RuntimeArguments + Stream options = Stream.concat(args.stream(), Stream.of("RuntimeArguments")); ProcessBuilder pb = ProcessTools.createLimitedTestJavaProcessBuilder( // The runtime image may be created with jlink --add-options // The initial VM options will be included in the result diff --git a/test/jdk/jdk/modules/incubator/DefaultImage.java b/test/jdk/jdk/modules/incubator/DefaultImage.java index 25ff746529b..63bc46b26ad 100644 --- a/test/jdk/jdk/modules/incubator/DefaultImage.java +++ b/test/jdk/jdk/modules/incubator/DefaultImage.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 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 @@ -24,11 +24,12 @@ /* * @test * @bug 8170859 - * @summary Ensure no incubator modules are resolved by default in the image + * @requires vm.flagless * @library /test/lib * @modules jdk.compiler * @build jdk.test.lib.compiler.CompilerUtils * @run testng DefaultImage + * @summary Ensure no incubator modules are resolved by default in the image */ import java.io.ByteArrayOutputStream; @@ -43,6 +44,7 @@ import java.util.stream.Stream; import jdk.test.lib.compiler.CompilerUtils; +import jdk.test.lib.process.ProcessTools; import org.testng.annotations.BeforeTest; import org.testng.annotations.Test; @@ -108,10 +110,8 @@ public void testAllSystem() throws Throwable { static ToolResult java(String... opts) throws Throwable { ByteArrayOutputStream baos = new ByteArrayOutputStream(); PrintStream ps = new PrintStream(baos); - String[] options = Stream.concat(Stream.of(getJava()), Stream.of(opts)) - .toArray(String[]::new); - ProcessBuilder pb = new ProcessBuilder(options); + ProcessBuilder pb = ProcessTools.createLimitedTestJavaProcessBuilder(opts); int exitValue = executeCommand(pb).outputTo(ps) .errorTo(ps) .getExitValue(); @@ -155,15 +155,6 @@ ToolResult assertOutputDoesNotContain(String subString) { } } - static String getJava() { - Path image = Paths.get(JAVA_HOME); - boolean isWindows = System.getProperty("os.name").startsWith("Windows"); - Path java = image.resolve("bin").resolve(isWindows ? "java.exe" : "java"); - if (Files.notExists(java)) - throw new RuntimeException(java + " not found"); - return java.toAbsolutePath().toString(); - } - static boolean isExplodedBuild() { Path modulesPath = Paths.get(JAVA_HOME).resolve("lib").resolve("modules"); return Files.notExists(modulesPath); diff --git a/test/jdk/jdk/modules/incubator/ImageModules.java b/test/jdk/jdk/modules/incubator/ImageModules.java index b69c4022f13..b48a969669e 100644 --- a/test/jdk/jdk/modules/incubator/ImageModules.java +++ b/test/jdk/jdk/modules/incubator/ImageModules.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 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 @@ -24,7 +24,7 @@ /* * @test * @bug 8170859 - * @summary Basic test for incubator modules in jmods and images + * @requires vm.flagless * @library /test/lib * @key intermittent * @modules jdk.compiler jdk.jartool jdk.jlink @@ -32,6 +32,7 @@ * jdk.test.lib.util.FileUtils * jdk.test.lib.compiler.CompilerUtils * @run testng/othervm ImageModules + * @summary Basic test for incubator modules in jmods and images */ import java.io.ByteArrayOutputStream; From 369bbecc0dab389b523c09bc332fe1cf6394cb26 Mon Sep 17 00:00:00 2001 From: Stefan Karlsson Date: Fri, 17 Nov 2023 07:04:13 +0000 Subject: [PATCH 21/76] 8319896: Remove monitor deflation from final audit Reviewed-by: dholmes, dcubed --- src/hotspot/share/runtime/objectMonitor.cpp | 10 --- src/hotspot/share/runtime/synchronizer.cpp | 79 +++++++++++---------- src/hotspot/share/runtime/synchronizer.hpp | 6 +- src/hotspot/share/runtime/vmOperations.cpp | 2 +- 4 files changed, 47 insertions(+), 50 deletions(-) diff --git a/src/hotspot/share/runtime/objectMonitor.cpp b/src/hotspot/share/runtime/objectMonitor.cpp index 1266514c8ee..627f24102db 100644 --- a/src/hotspot/share/runtime/objectMonitor.cpp +++ b/src/hotspot/share/runtime/objectMonitor.cpp @@ -518,16 +518,6 @@ bool ObjectMonitor::deflate_monitor() { return false; } - if (ObjectSynchronizer::is_final_audit() && owner_is_DEFLATER_MARKER()) { - // The final audit can see an already deflated ObjectMonitor on the - // in-use list because MonitorList::unlink_deflated() might have - // blocked for the final safepoint before unlinking all the deflated - // monitors. - assert(contentions() < 0, "must be negative: contentions=%d", contentions()); - // Already returned 'true' when it was originally deflated. - return false; - } - const oop obj = object_peek(); if (obj == nullptr) { diff --git a/src/hotspot/share/runtime/synchronizer.cpp b/src/hotspot/share/runtime/synchronizer.cpp index e23eb267668..0d27376fdff 100644 --- a/src/hotspot/share/runtime/synchronizer.cpp +++ b/src/hotspot/share/runtime/synchronizer.cpp @@ -1060,27 +1060,34 @@ JavaThread* ObjectSynchronizer::get_lock_owner(ThreadsList * t_list, Handle h_ob // Visitors ... +// Iterate over all ObjectMonitors. +template +void ObjectSynchronizer::monitors_iterate(Function function) { + MonitorList::Iterator iter = _in_use_list.iterator(); + while (iter.has_next()) { + ObjectMonitor* monitor = iter.next(); + function(monitor); + } +} + // Iterate ObjectMonitors owned by any thread and where the owner `filter` // returns true. template void ObjectSynchronizer::owned_monitors_iterate_filtered(MonitorClosure* closure, OwnerFilter filter) { - MonitorList::Iterator iter = _in_use_list.iterator(); - while (iter.has_next()) { - ObjectMonitor* mid = iter.next(); - + monitors_iterate([&](ObjectMonitor* monitor) { // This function is only called at a safepoint or when the // target thread is suspended or when the target thread is // operating on itself. The current closures in use today are // only interested in an owned ObjectMonitor and ownership // cannot be dropped under the calling contexts so the // ObjectMonitor cannot be async deflated. - if (mid->has_owner() && filter(mid->owner_raw())) { - assert(!mid->is_being_async_deflated(), "Owned monitors should not be deflating"); - assert(mid->object_peek() != nullptr, "Owned monitors should not have a dead object"); + if (monitor->has_owner() && filter(monitor->owner_raw())) { + assert(!monitor->is_being_async_deflated(), "Owned monitors should not be deflating"); + assert(monitor->object_peek() != nullptr, "Owned monitors should not have a dead object"); - closure->do_monitor(mid); + closure->do_monitor(monitor); } - } + }); } // Iterate ObjectMonitors where the owner == thread; this does NOT include @@ -1585,7 +1592,7 @@ static size_t delete_monitors(GrowableArray* delete_list) { } // This function is called by the MonitorDeflationThread to deflate -// ObjectMonitors. It is also called via do_final_audit_and_print_stats(). +// ObjectMonitors. size_t ObjectSynchronizer::deflate_idle_monitors() { Thread* current = Thread::current(); if (current->is_Java_thread()) { @@ -1614,11 +1621,8 @@ size_t ObjectSynchronizer::deflate_idle_monitors() { size_t deflated_count = deflate_monitor_list(current, ls, &timer); size_t unlinked_count = 0; size_t deleted_count = 0; - if (deflated_count > 0 || is_final_audit()) { - // There are ObjectMonitors that have been deflated or this is the - // final audit and all the remaining ObjectMonitors have been - // deflated, BUT the MonitorDeflationThread blocked for the final - // safepoint during unlinking. + if (deflated_count > 0) { + // There are ObjectMonitors that have been deflated. // Unlink deflated ObjectMonitors from the in-use list. ResourceMark rm; @@ -1799,12 +1803,6 @@ void ObjectSynchronizer::do_final_audit_and_print_stats() { log_info(monitorinflation)("Starting the final audit."); if (log_is_enabled(Info, monitorinflation)) { - // Do deflations in order to reduce the in-use monitor population - // that is reported by ObjectSynchronizer::log_in_use_monitor_details() - // which is called by ObjectSynchronizer::audit_and_print_stats(). - while (deflate_idle_monitors() > 0) { - ; // empty - } // The other audit_and_print_stats() call is done at the Debug // level at a safepoint in SafepointSynchronize::do_cleanup_tasks. audit_and_print_stats(true /* on_exit */); @@ -1853,7 +1851,7 @@ void ObjectSynchronizer::audit_and_print_stats(bool on_exit) { // When exiting this log output is at the Info level. When called // at a safepoint, this log output is at the Trace level since // there can be a lot of it. - log_in_use_monitor_details(ls); + log_in_use_monitor_details(ls, !on_exit /* log_all */); } ls->flush(); @@ -1933,29 +1931,34 @@ void ObjectSynchronizer::chk_in_use_entry(ObjectMonitor* n, outputStream* out, // Log details about ObjectMonitors on the in_use_list. The 'BHL' // flags indicate why the entry is in-use, 'object' and 'object type' // indicate the associated object and its type. -void ObjectSynchronizer::log_in_use_monitor_details(outputStream* out) { - stringStream ss; +void ObjectSynchronizer::log_in_use_monitor_details(outputStream* out, bool log_all) { if (_in_use_list.count() > 0) { + stringStream ss; out->print_cr("In-use monitor info:"); out->print_cr("(B -> is_busy, H -> has hash code, L -> lock status)"); out->print_cr("%18s %s %18s %18s", "monitor", "BHL", "object", "object type"); out->print_cr("================== === ================== =================="); - MonitorList::Iterator iter = _in_use_list.iterator(); - while (iter.has_next()) { - ObjectMonitor* mid = iter.next(); - const oop obj = mid->object_peek(); - const markWord mark = mid->header(); - ResourceMark rm; - out->print(INTPTR_FORMAT " %d%d%d " INTPTR_FORMAT " %s", p2i(mid), - mid->is_busy(), mark.hash() != 0, mid->owner() != nullptr, - p2i(obj), obj == nullptr ? "" : obj->klass()->external_name()); - if (mid->is_busy()) { - out->print(" (%s)", mid->is_busy_to_string(&ss)); - ss.reset(); + + auto is_interesting = [&](ObjectMonitor* monitor) { + return log_all || monitor->has_owner() || monitor->is_busy(); + }; + + monitors_iterate([&](ObjectMonitor* monitor) { + if (is_interesting(monitor)) { + const oop obj = monitor->object_peek(); + const markWord mark = monitor->header(); + ResourceMark rm; + out->print(INTPTR_FORMAT " %d%d%d " INTPTR_FORMAT " %s", p2i(monitor), + monitor->is_busy(), mark.hash() != 0, monitor->owner() != nullptr, + p2i(obj), obj == nullptr ? "" : obj->klass()->external_name()); + if (monitor->is_busy()) { + out->print(" (%s)", monitor->is_busy_to_string(&ss)); + ss.reset(); + } + out->cr(); } - out->cr(); - } + }); } out->flush(); diff --git a/src/hotspot/share/runtime/synchronizer.hpp b/src/hotspot/share/runtime/synchronizer.hpp index 68c177d3b05..d65a1e14bfa 100644 --- a/src/hotspot/share/runtime/synchronizer.hpp +++ b/src/hotspot/share/runtime/synchronizer.hpp @@ -123,6 +123,10 @@ class ObjectSynchronizer : AllStatic { // JNI detach support static void release_monitors_owned_by_thread(JavaThread* current); + // Iterate over all ObjectMonitors. + template + static void monitors_iterate(Function function); + // Iterate ObjectMonitors owned by any thread and where the owner `filter` // returns true. template @@ -167,7 +171,7 @@ class ObjectSynchronizer : AllStatic { static void chk_in_use_entry(ObjectMonitor* n, outputStream* out, int* error_cnt_p); static void do_final_audit_and_print_stats(); - static void log_in_use_monitor_details(outputStream* out); + static void log_in_use_monitor_details(outputStream* out, bool log_all); private: friend class SynchronizerTest; diff --git a/src/hotspot/share/runtime/vmOperations.cpp b/src/hotspot/share/runtime/vmOperations.cpp index fadbcb799f7..7bc84bfa285 100644 --- a/src/hotspot/share/runtime/vmOperations.cpp +++ b/src/hotspot/share/runtime/vmOperations.cpp @@ -344,7 +344,7 @@ class ObjectMonitorsDump : public MonitorClosure, public ObjectMonitorsView { if (monitor->is_owner_anonymous()) { // There's no need to collect anonymous owned monitors - // because the callers of this code is only interested + // because the caller of this code is only interested // in JNI owned monitors. return; } From 129c4708b428bd98c5e8b1f43819bc31c3c9cb0b Mon Sep 17 00:00:00 2001 From: Roland Westrelin Date: Fri, 17 Nov 2023 07:54:10 +0000 Subject: [PATCH 22/76] 8311932: Suboptimal compiled code of nested loop over memory segment Reviewed-by: thartmann, chagedorn --- src/hotspot/share/opto/loopnode.cpp | 22 +++++++ .../c2/irTests/TestLongRangeChecks.java | 62 +++++++++++++++++-- 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/src/hotspot/share/opto/loopnode.cpp b/src/hotspot/share/opto/loopnode.cpp index c41399c2ee3..e9655b329e1 100644 --- a/src/hotspot/share/opto/loopnode.cpp +++ b/src/hotspot/share/opto/loopnode.cpp @@ -855,6 +855,28 @@ bool PhaseIdealLoop::create_loop_nest(IdealLoopTree* loop, Node_List &old_new) { // not a loop after all return false; } + + if (range_checks.size() > 0) { + // This transformation requires peeling one iteration. Also, if it has range checks and they are eliminated by Loop + // Predication, then 2 Hoisted Check Predicates are added for one range check. Finally, transforming a long range + // check requires extra logic to be executed before the loop is entered and for the outer loop. As a result, the + // transformations can't pay off for a small number of iterations: roughly, if the loop runs for 3 iterations, it's + // going to execute as many range checks once transformed with range checks eliminated (1 peeled iteration with + // range checks + 2 predicates per range checks) as it would have not transformed. It also has to pay for the extra + // logic on loop entry and for the outer loop. + loop->compute_trip_count(this); + if (head->is_CountedLoop() && head->as_CountedLoop()->has_exact_trip_count()) { + if (head->as_CountedLoop()->trip_count() <= 3) { + return false; + } + } else { + loop->compute_profile_trip_cnt(this); + if (!head->is_profile_trip_failed() && head->profile_trip_cnt() <= 3) { + return false; + } + } + } + julong orig_iters = (julong)hi->hi_as_long() - lo->lo_as_long(); iters_limit = checked_cast(MIN2((julong)iters_limit, orig_iters)); diff --git a/test/hotspot/jtreg/compiler/c2/irTests/TestLongRangeChecks.java b/test/hotspot/jtreg/compiler/c2/irTests/TestLongRangeChecks.java index 696d7beef34..479a2a45cb9 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/TestLongRangeChecks.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/TestLongRangeChecks.java @@ -29,7 +29,7 @@ /* * @test - * @bug 8259609 8276116 + * @bug 8259609 8276116 8311932 * @summary C2: optimize long range checks in long counted loops * @library /test/lib / * @requires vm.compiler2.enabled @@ -38,9 +38,9 @@ public class TestLongRangeChecks { public static void main(String[] args) { - TestFramework.runWithFlags("-XX:-UseCountedLoopSafepoints"); - TestFramework.runWithFlags("-XX:+UseCountedLoopSafepoints", "-XX:LoopStripMiningIter=1"); - TestFramework.runWithFlags("-XX:+UseCountedLoopSafepoints", "-XX:LoopStripMiningIter=1000"); + TestFramework.runWithFlags("-XX:+TieredCompilation", "-XX:-UseCountedLoopSafepoints", "-XX:LoopUnrollLimit=0"); + TestFramework.runWithFlags("-XX:+TieredCompilation", "-XX:+UseCountedLoopSafepoints", "-XX:LoopStripMiningIter=1", "-XX:LoopUnrollLimit=0"); + TestFramework.runWithFlags("-XX:+TieredCompilation", "-XX:+UseCountedLoopSafepoints", "-XX:LoopStripMiningIter=1000", "-XX:LoopUnrollLimit=0"); } @@ -246,4 +246,58 @@ public static void testStridePosScaleNegInIntLoop2(int start, int stop, long len private void testStridePosScaleNegInIntLoop2_runner() { testStridePosScaleNegInIntLoop2(0, 100, 200, 198); } + + @Test + @IR(counts = { IRNode.LONG_COUNTED_LOOP, "1" }) + @IR(failOn = { IRNode.COUNTED_LOOP, IRNode.LOOP }) + public static void testStridePosScalePosShortLoop(long start, long stop, long length, long offset) { + final long scale = 1; + final long stride = 1; + + // Loop runs for too few iterations. Transforming it wouldn't pay off. + for (long i = start; i < stop; i += stride) { + Objects.checkIndex(scale * i + offset, length); + } + } + + @Run(test = "testStridePosScalePosShortLoop") + private void testStridePosScalePosShortLoop_runner() { + testStridePosScalePosShortLoop(0, 2, 2, 0); + } + + @Test + @IR(counts = { IRNode.COUNTED_LOOP, "1" }) + @IR(failOn = { IRNode.LOOP }) + public static void testStridePosScalePosInIntLoopShortLoop1(int start, int stop, long length, long offset) { + final long scale = 2; + final int stride = 1; + + // Same but with int loop + for (int i = start; i < stop; i += stride) { + Objects.checkIndex(scale * i + offset, length); + } + } + + @Run(test = "testStridePosScalePosInIntLoopShortLoop1") + private void testStridePosScalePosInIntLoopShortLoop1_runner() { + testStridePosScalePosInIntLoopShortLoop1(0, 2, 4, 0); + } + + @Test + @IR(counts = { IRNode.COUNTED_LOOP, "1" }) + @IR(failOn = { IRNode.LOOP }) + public static void testStridePosScalePosInIntLoopShortLoop2(long length, long offset) { + final long scale = 2; + final int stride = 1; + + // Same but with int loop + for (int i = 0; i < 3; i += stride) { + Objects.checkIndex(scale * i + offset, length); + } + } + + @Run(test = "testStridePosScalePosInIntLoopShortLoop2") + private void testStridePosScalePosInIntLoopShortLoop2_runner() { + testStridePosScalePosInIntLoopShortLoop2(6, 0); + } } From bbf52e0e4cb76b4c6425e7d1266dcdbb4df556ea Mon Sep 17 00:00:00 2001 From: Stefan Karlsson Date: Fri, 17 Nov 2023 08:38:21 +0000 Subject: [PATCH 23/76] 8319897: Move StackWatermark handling out of LockStack::contains Reviewed-by: eosterlund, dholmes, dcubed --- src/hotspot/share/runtime/lockStack.inline.hpp | 14 ++++---------- src/hotspot/share/runtime/threads.cpp | 7 +++++++ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/hotspot/share/runtime/lockStack.inline.hpp b/src/hotspot/share/runtime/lockStack.inline.hpp index 9dd04d8f7fe..b36be2f72de 100644 --- a/src/hotspot/share/runtime/lockStack.inline.hpp +++ b/src/hotspot/share/runtime/lockStack.inline.hpp @@ -104,16 +104,10 @@ inline void LockStack::remove(oop o) { inline bool LockStack::contains(oop o) const { verify("pre-contains"); - if (!SafepointSynchronize::is_at_safepoint() && !is_owning_thread()) { - // When a foreign thread inspects this thread's lock-stack, it may see - // bad references here when a concurrent collector has not gotten - // to processing the lock-stack, yet. Call StackWaterMark::start_processing() - // to ensure that all references are valid. - StackWatermark* watermark = StackWatermarkSet::get(get_thread(), StackWatermarkKind::gc); - if (watermark != nullptr) { - watermark->start_processing(); - } - } + + // Can't poke around in thread oops without having started stack watermark processing. + assert(StackWatermarkSet::processing_started(get_thread()), "Processing must have started!"); + int end = to_index(_top); for (int i = end - 1; i >= 0; i--) { if (_base[i] == o) { diff --git a/src/hotspot/share/runtime/threads.cpp b/src/hotspot/share/runtime/threads.cpp index b07f6e21d20..b6ef42cd1d2 100644 --- a/src/hotspot/share/runtime/threads.cpp +++ b/src/hotspot/share/runtime/threads.cpp @@ -81,6 +81,7 @@ #include "runtime/safepointVerifiers.hpp" #include "runtime/serviceThread.hpp" #include "runtime/sharedRuntime.hpp" +#include "runtime/stackWatermarkSet.inline.hpp" #include "runtime/statSampler.hpp" #include "runtime/stubCodeGenerator.hpp" #include "runtime/thread.inline.hpp" @@ -1230,6 +1231,12 @@ JavaThread *Threads::owning_thread_from_monitor_owner(ThreadsList * t_list, JavaThread* Threads::owning_thread_from_object(ThreadsList * t_list, oop obj) { assert(LockingMode == LM_LIGHTWEIGHT, "Only with new lightweight locking"); for (JavaThread* q : *t_list) { + // Need to start processing before accessing oops in the thread. + StackWatermark* watermark = StackWatermarkSet::get(q, StackWatermarkKind::gc); + if (watermark != nullptr) { + watermark->start_processing(); + } + if (q->lock_stack().contains(obj)) { return q; } From 8ec6b8de3bb3d7aeebdcb45d761b18cce3bab75e Mon Sep 17 00:00:00 2001 From: "yibo.yl" Date: Fri, 17 Nov 2023 08:43:18 +0000 Subject: [PATCH 24/76] 8319876: Reduce memory consumption of VM_ThreadDump::doit Reviewed-by: dholmes, stefank --- src/hotspot/share/runtime/vmOperations.cpp | 2 -- src/hotspot/share/services/threadService.cpp | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/hotspot/share/runtime/vmOperations.cpp b/src/hotspot/share/runtime/vmOperations.cpp index 7bc84bfa285..7438814255c 100644 --- a/src/hotspot/share/runtime/vmOperations.cpp +++ b/src/hotspot/share/runtime/vmOperations.cpp @@ -230,7 +230,6 @@ VM_ThreadDump::VM_ThreadDump(ThreadDumpResult* result, _result = result; _num_threads = 0; // 0 indicates all threads _threads = nullptr; - _result = result; _max_depth = max_depth; _with_locked_monitors = with_locked_monitors; _with_locked_synchronizers = with_locked_synchronizers; @@ -245,7 +244,6 @@ VM_ThreadDump::VM_ThreadDump(ThreadDumpResult* result, _result = result; _num_threads = num_threads; _threads = threads; - _result = result; _max_depth = max_depth; _with_locked_monitors = with_locked_monitors; _with_locked_synchronizers = with_locked_synchronizers; diff --git a/src/hotspot/share/services/threadService.cpp b/src/hotspot/share/services/threadService.cpp index e95c832894f..bf9979fa3b4 100644 --- a/src/hotspot/share/services/threadService.cpp +++ b/src/hotspot/share/services/threadService.cpp @@ -696,7 +696,7 @@ void ThreadStackTrace::dump_stack_at_safepoint(int maxDepth, ObjectMonitorsView* RegisterMap::UpdateMap::include, RegisterMap::ProcessFrames::include, RegisterMap::WalkContinuation::skip); - + ResourceMark rm(VMThread::vm_thread()); // If full, we want to print both vthread and carrier frames vframe* start_vf = !full && _thread->is_vthread_mounted() ? _thread->carrier_last_java_vframe(®_map) From 368e4f60a937f5cf6919c1dd41fc791b1f7bf205 Mon Sep 17 00:00:00 2001 From: Martin Doerr Date: Fri, 17 Nov 2023 11:49:21 +0000 Subject: [PATCH 25/76] 8315801: [PPC64] JNI code should be more similar to the Panama implementation Reviewed-by: rrich, lucy --- .../cpu/aarch64/sharedRuntime_aarch64.cpp | 7 +- src/hotspot/cpu/arm/sharedRuntime_arm.cpp | 5 +- src/hotspot/cpu/ppc/assembler_ppc.hpp | 20 ++-- src/hotspot/cpu/ppc/foreignGlobals_ppc.cpp | 3 +- src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp | 108 ++++-------------- .../ppc/templateInterpreterGenerator_ppc.cpp | 24 +--- src/hotspot/cpu/riscv/sharedRuntime_riscv.cpp | 4 +- src/hotspot/cpu/s390/sharedRuntime_s390.cpp | 4 +- src/hotspot/cpu/x86/sharedRuntime_x86_32.cpp | 5 +- src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp | 5 +- src/hotspot/cpu/zero/sharedRuntime_zero.cpp | 1 - src/hotspot/share/c1/c1_FrameMap.cpp | 2 +- src/hotspot/share/opto/callnode.cpp | 2 +- src/hotspot/share/runtime/sharedRuntime.hpp | 3 +- 14 files changed, 53 insertions(+), 140 deletions(-) diff --git a/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp b/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp index 90b426deb90..fa2faddd714 100644 --- a/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp @@ -801,9 +801,7 @@ AdapterHandlerEntry* SharedRuntime::generate_i2c2i_adapters(MacroAssembler *masm static int c_calling_convention_priv(const BasicType *sig_bt, VMRegPair *regs, - VMRegPair *regs2, int total_args_passed) { - assert(regs2 == nullptr, "not needed on AArch64"); // We return the amount of VMRegImpl stack slots we need to reserve for all // the arguments NOT counting out_preserve_stack_slots. @@ -897,10 +895,9 @@ int SharedRuntime::vector_calling_convention(VMRegPair *regs, int SharedRuntime::c_calling_convention(const BasicType *sig_bt, VMRegPair *regs, - VMRegPair *regs2, int total_args_passed) { - int result = c_calling_convention_priv(sig_bt, regs, regs2, total_args_passed); + int result = c_calling_convention_priv(sig_bt, regs, total_args_passed); guarantee(result >= 0, "Unsupported arguments configuration"); return result; } @@ -1457,7 +1454,7 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm, // Now figure out where the args must be stored and how much stack space // they require. int out_arg_slots; - out_arg_slots = c_calling_convention_priv(out_sig_bt, out_regs, nullptr, total_c_args); + out_arg_slots = c_calling_convention_priv(out_sig_bt, out_regs, total_c_args); if (out_arg_slots < 0) { return nullptr; diff --git a/src/hotspot/cpu/arm/sharedRuntime_arm.cpp b/src/hotspot/cpu/arm/sharedRuntime_arm.cpp index e4f4107da0f..9a65f9f09fe 100644 --- a/src/hotspot/cpu/arm/sharedRuntime_arm.cpp +++ b/src/hotspot/cpu/arm/sharedRuntime_arm.cpp @@ -254,10 +254,7 @@ bool SharedRuntime::is_wide_vector(int size) { int SharedRuntime::c_calling_convention(const BasicType *sig_bt, VMRegPair *regs, - VMRegPair *regs2, int total_args_passed) { - assert(regs2 == nullptr, "not needed on arm"); - int slot = 0; int ireg = 0; #ifdef __ABI_HARD__ @@ -795,7 +792,7 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm, out_sig_bt[argc++] = in_sig_bt[i]; } - int out_arg_slots = c_calling_convention(out_sig_bt, out_regs, nullptr, total_c_args); + int out_arg_slots = c_calling_convention(out_sig_bt, out_regs, total_c_args); int stack_slots = SharedRuntime::out_preserve_stack_slots() + out_arg_slots; // Since object arguments need to be wrapped, we must preserve space // for those object arguments which come in registers (GPR_PARAMS maximum) diff --git a/src/hotspot/cpu/ppc/assembler_ppc.hpp b/src/hotspot/cpu/ppc/assembler_ppc.hpp index 8f9c8cb5cb4..61a5d6425ee 100644 --- a/src/hotspot/cpu/ppc/assembler_ppc.hpp +++ b/src/hotspot/cpu/ppc/assembler_ppc.hpp @@ -126,20 +126,20 @@ class Argument { int _number; // The number of the argument. public: enum { - // Only 8 registers may contain integer parameters. - n_register_parameters = 8, - // Can have up to 8 floating registers. - n_float_register_parameters = 8, - // PPC C calling conventions. // The first eight arguments are passed in int regs if they are int. n_int_register_parameters_c = 8, // The first thirteen float arguments are passed in float regs. n_float_register_parameters_c = 13, - // Only the first 8 parameters are not placed on the stack. Aix disassembly - // shows that xlC places all float args after argument 8 on the stack AND - // in a register. This is not documented, but we follow this convention, too. - n_regs_not_on_stack_c = 8, + +#ifdef VM_LITTLE_ENDIAN + // Floats are in the least significant word of an argument slot. + float_on_stack_offset_in_bytes_c = 0, +#else + // Although AIX runs on big endian CPU, float is in the most + // significant word of an argument slot. + float_on_stack_offset_in_bytes_c = AIX_ONLY(0) NOT_AIX(4), +#endif n_int_register_parameters_j = 8, // duplicates num_java_iarg_registers n_float_register_parameters_j = 13, // num_java_farg_registers @@ -150,7 +150,7 @@ class Argument { int number() const { return _number; } // Locating register-based arguments: - bool is_register() const { return _number < n_register_parameters; } + bool is_register() const { return _number < n_int_register_parameters_c; } Register as_register() const { assert(is_register(), "must be a register argument"); diff --git a/src/hotspot/cpu/ppc/foreignGlobals_ppc.cpp b/src/hotspot/cpu/ppc/foreignGlobals_ppc.cpp index 0a37552cd98..2143d139499 100644 --- a/src/hotspot/cpu/ppc/foreignGlobals_ppc.cpp +++ b/src/hotspot/cpu/ppc/foreignGlobals_ppc.cpp @@ -179,7 +179,7 @@ static void move_float(MacroAssembler* masm, int out_stk_bias, case StorageType::STACK: if (from_reg.segment_mask() == REG32_MASK) { assert(to_reg.stack_size() == 4, "size should match"); - // TODO: Check if AIX needs 4 Byte offset + // Note: Argument::float_on_stack_offset_in_bytes_c is handled by CallArranger __ stfs(as_FloatRegister(from_reg), reg2offset(to_reg, out_stk_bias), R1_SP); } else { assert(to_reg.stack_size() == 8, "size should match"); @@ -204,6 +204,7 @@ static void move_stack(MacroAssembler* masm, Register callerSP, int in_stk_bias, case StorageType::FLOAT: switch (from_reg.stack_size()) { case 8: __ lfd(as_FloatRegister(to_reg), reg2offset(from_reg, in_stk_bias), callerSP); break; + // Note: Argument::float_on_stack_offset_in_bytes_c is handled by CallArranger case 4: __ lfs(as_FloatRegister(to_reg), reg2offset(from_reg, in_stk_bias), callerSP); break; default: ShouldNotReachHere(); } diff --git a/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp b/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp index 80c00c713d7..824996168a9 100644 --- a/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp +++ b/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp @@ -741,7 +741,6 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt, // Calling convention for calling C code. int SharedRuntime::c_calling_convention(const BasicType *sig_bt, VMRegPair *regs, - VMRegPair *regs2, int total_args_passed) { // Calling conventions for C runtime calls and calls to JNI native methods. // @@ -782,35 +781,20 @@ int SharedRuntime::c_calling_convention(const BasicType *sig_bt, sizeof(farg_reg) / sizeof(farg_reg[0]) == Argument::n_float_register_parameters_c, "consistency"); - // `Stk' counts stack slots. Due to alignment, 32 bit values occupy - // 2 such slots, like 64 bit values do. - const int inc_stk_for_intfloat = 2; // 2 slots for ints and floats - const int inc_stk_for_longdouble = 2; // 2 slots for longs and doubles + const int additional_frame_header_slots = ((frame::native_abi_minframe_size - frame::jit_out_preserve_size) + / VMRegImpl::stack_slot_size); + const int float_offset_in_slots = Argument::float_on_stack_offset_in_bytes_c / VMRegImpl::stack_slot_size; - int i; VMReg reg; - // Leave room for C-compatible ABI_REG_ARGS. - int stk = (frame::native_abi_reg_args_size - frame::jit_out_preserve_size) / VMRegImpl::stack_slot_size; int arg = 0; int freg = 0; + bool stack_used = false; - // Avoid passing C arguments in the wrong stack slots. -#if defined(ABI_ELFv2) - assert((SharedRuntime::out_preserve_stack_slots() + stk) * VMRegImpl::stack_slot_size == 96, - "passing C arguments in wrong stack slots"); -#else - assert((SharedRuntime::out_preserve_stack_slots() + stk) * VMRegImpl::stack_slot_size == 112, - "passing C arguments in wrong stack slots"); -#endif - // We fill-out regs AND regs2 if an argument must be passed in a - // register AND in a stack slot. If regs2 is null in such a - // situation, we bail-out with a fatal error. for (int i = 0; i < total_args_passed; ++i, ++arg) { - // Initialize regs2 to BAD. - if (regs2 != nullptr) regs2[i].set_bad(); + // Each argument corresponds to a slot in the Parameter Save Area (if not omitted) + int stk = (arg * 2) + additional_frame_header_slots; switch(sig_bt[i]) { - // // If arguments 0-7 are integers, they are passed in integer registers. // Argument i is placed in iarg_reg[i]. @@ -832,7 +816,7 @@ int SharedRuntime::c_calling_convention(const BasicType *sig_bt, reg = iarg_reg[arg]; } else { reg = VMRegImpl::stack2reg(stk); - stk += inc_stk_for_longdouble; + stack_used = true; } regs[i].set2(reg); break; @@ -844,43 +828,14 @@ int SharedRuntime::c_calling_convention(const BasicType *sig_bt, // in farg_reg[j] if argument i is the j-th float argument of this call. // case T_FLOAT: -#if defined(LINUX) - // Linux uses ELF ABI. Both original ELF and ELFv2 ABIs have float - // in the least significant word of an argument slot. -#if defined(VM_LITTLE_ENDIAN) -#define FLOAT_WORD_OFFSET_IN_SLOT 0 -#else -#define FLOAT_WORD_OFFSET_IN_SLOT 1 -#endif -#elif defined(AIX) - // Although AIX runs on big endian CPU, float is in the most - // significant word of an argument slot. -#define FLOAT_WORD_OFFSET_IN_SLOT 0 -#else -#error "unknown OS" -#endif if (freg < Argument::n_float_register_parameters_c) { // Put float in register ... reg = farg_reg[freg]; ++freg; - - // Argument i for i > 8 is placed on the stack even if it's - // placed in a register (if it's a float arg). Aix disassembly - // shows that xlC places these float args on the stack AND in - // a register. This is not documented, but we follow this - // convention, too. - if (arg >= Argument::n_regs_not_on_stack_c) { - // ... and on the stack. - guarantee(regs2 != nullptr, "must pass float in register and stack slot"); - VMReg reg2 = VMRegImpl::stack2reg(stk + FLOAT_WORD_OFFSET_IN_SLOT); - regs2[i].set1(reg2); - stk += inc_stk_for_intfloat; - } - } else { // Put float on stack. - reg = VMRegImpl::stack2reg(stk + FLOAT_WORD_OFFSET_IN_SLOT); - stk += inc_stk_for_intfloat; + reg = VMRegImpl::stack2reg(stk + float_offset_in_slots); + stack_used = true; } regs[i].set1(reg); break; @@ -890,23 +845,10 @@ int SharedRuntime::c_calling_convention(const BasicType *sig_bt, // Put double in register ... reg = farg_reg[freg]; ++freg; - - // Argument i for i > 8 is placed on the stack even if it's - // placed in a register (if it's a double arg). Aix disassembly - // shows that xlC places these float args on the stack AND in - // a register. This is not documented, but we follow this - // convention, too. - if (arg >= Argument::n_regs_not_on_stack_c) { - // ... and on the stack. - guarantee(regs2 != nullptr, "must pass float in register and stack slot"); - VMReg reg2 = VMRegImpl::stack2reg(stk); - regs2[i].set2(reg2); - stk += inc_stk_for_longdouble; - } } else { // Put double on stack. reg = VMRegImpl::stack2reg(stk); - stk += inc_stk_for_longdouble; + stack_used = true; } regs[i].set2(reg); break; @@ -921,7 +863,17 @@ int SharedRuntime::c_calling_convention(const BasicType *sig_bt, } } - return align_up(stk, 2); + // Return size of the stack frame excluding the jit_out_preserve part in single-word slots. +#if defined(ABI_ELFv2) + assert(additional_frame_header_slots == 0, "ABIv2 shouldn't use extra slots"); + // ABIv2 allows omitting the Parameter Save Area if the callee's prototype + // indicates that all parameters can be passed in registers. + return stack_used ? (arg * 2) : 0; +#else + // The Parameter Save Area needs to be at least 8 double-word slots for ABIv1. + // We have to add extra slots because ABIv1 uses a larger header. + return MAX2(arg, 8) * 2 + additional_frame_header_slots; +#endif } #endif // COMPILER2 @@ -2140,7 +2092,6 @@ nmethod *SharedRuntime::generate_native_wrapper(MacroAssembler *masm, BasicType *out_sig_bt = NEW_RESOURCE_ARRAY(BasicType, total_c_args); VMRegPair *out_regs = NEW_RESOURCE_ARRAY(VMRegPair, total_c_args); - VMRegPair *out_regs2 = NEW_RESOURCE_ARRAY(VMRegPair, total_c_args); BasicType* in_elem_bt = nullptr; // Create the signature for the C call: @@ -2193,7 +2144,7 @@ nmethod *SharedRuntime::generate_native_wrapper(MacroAssembler *masm, // - *_slot_offset Indicates offset from SP in number of stack slots. // - *_offset Indicates offset from SP in bytes. - int stack_slots = c_calling_convention(out_sig_bt, out_regs, out_regs2, total_c_args) + // 1+2) + int stack_slots = c_calling_convention(out_sig_bt, out_regs, total_c_args) + // 1+2) SharedRuntime::out_preserve_stack_slots(); // See c_calling_convention. // Now the space for the inbound oop handle area. @@ -2358,11 +2309,6 @@ nmethod *SharedRuntime::generate_native_wrapper(MacroAssembler *masm, } else if (out_regs[out].first()->is_FloatRegister()) { freg_destroyed[out_regs[out].first()->as_FloatRegister()->encoding()] = true; } - if (out_regs2[out].first()->is_Register()) { - reg_destroyed[out_regs2[out].first()->as_Register()->encoding()] = true; - } else if (out_regs2[out].first()->is_FloatRegister()) { - freg_destroyed[out_regs2[out].first()->as_FloatRegister()->encoding()] = true; - } #endif // ASSERT switch (in_sig_bt[in]) { @@ -2389,15 +2335,9 @@ nmethod *SharedRuntime::generate_native_wrapper(MacroAssembler *masm, break; case T_FLOAT: float_move(masm, in_regs[in], out_regs[out], r_callers_sp, r_temp_1); - if (out_regs2[out].first()->is_valid()) { - float_move(masm, in_regs[in], out_regs2[out], r_callers_sp, r_temp_1); - } break; case T_DOUBLE: double_move(masm, in_regs[in], out_regs[out], r_callers_sp, r_temp_1); - if (out_regs2[out].first()->is_valid()) { - double_move(masm, in_regs[in], out_regs2[out], r_callers_sp, r_temp_1); - } break; case T_ADDRESS: fatal("found type (T_ADDRESS) in java args"); @@ -2474,7 +2414,7 @@ nmethod *SharedRuntime::generate_native_wrapper(MacroAssembler *masm, // Save argument registers and leave room for C-compatible ABI_REG_ARGS. int frame_size = frame::native_abi_reg_args_size + align_up(total_c_args * wordSize, frame::alignment_in_bytes); __ mr(R11_scratch1, R1_SP); - RegisterSaver::push_frame_and_save_argument_registers(masm, R12_scratch2, frame_size, total_c_args, out_regs, out_regs2); + RegisterSaver::push_frame_and_save_argument_registers(masm, R12_scratch2, frame_size, total_c_args, out_regs); // Do the call. __ set_last_Java_frame(R11_scratch1, r_return_pc); @@ -2482,7 +2422,7 @@ nmethod *SharedRuntime::generate_native_wrapper(MacroAssembler *masm, __ call_VM_leaf(CAST_FROM_FN_PTR(address, SharedRuntime::complete_monitor_locking_C), r_oop, r_box, R16_thread); __ reset_last_Java_frame(); - RegisterSaver::restore_argument_registers_and_pop_frame(masm, frame_size, total_c_args, out_regs, out_regs2); + RegisterSaver::restore_argument_registers_and_pop_frame(masm, frame_size, total_c_args, out_regs); __ asm_assert_mem8_is_zero(thread_(pending_exception), "no pending exception allowed on exit from SharedRuntime::complete_monitor_locking_C"); diff --git a/src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp b/src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp index c7e52b47861..db49ca5cf22 100644 --- a/src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp +++ b/src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp @@ -291,21 +291,7 @@ address TemplateInterpreterGenerator::generate_slow_signature_handler() { __ bind(do_float); __ lfs(floatSlot, 0, arg_java); -#if defined(LINUX) - // Linux uses ELF ABI. Both original ELF and ELFv2 ABIs have float - // in the least significant word of an argument slot. -#if defined(VM_LITTLE_ENDIAN) - __ stfs(floatSlot, 0, arg_c); -#else - __ stfs(floatSlot, 4, arg_c); -#endif -#elif defined(AIX) - // Although AIX runs on big endian CPU, float is in most significant - // word of an argument slot. - __ stfs(floatSlot, 0, arg_c); -#else -#error "unknown OS" -#endif + __ stfs(floatSlot, Argument::float_on_stack_offset_in_bytes_c, arg_c); __ addi(arg_java, arg_java, -BytesPerWord); __ addi(arg_c, arg_c, BytesPerWord); __ cmplwi(CCR0, fpcnt, max_fp_register_arguments); @@ -951,14 +937,14 @@ void TemplateInterpreterGenerator::generate_fixed_frame(bool native_call, Regist in_bytes(ConstMethod::size_of_parameters_offset()), Rconst_method); if (native_call) { // If we're calling a native method, we reserve space for the worst-case signature - // handler varargs vector, which is max(Argument::n_register_parameters, parameter_count+2). + // handler varargs vector, which is max(Argument::n_int_register_parameters_c, parameter_count+2). // We add two slots to the parameter_count, one for the jni // environment and one for a possible native mirror. Label skip_native_calculate_max_stack; __ addi(Rtop_frame_size, Rsize_of_parameters, 2); - __ cmpwi(CCR0, Rtop_frame_size, Argument::n_register_parameters); + __ cmpwi(CCR0, Rtop_frame_size, Argument::n_int_register_parameters_c); __ bge(CCR0, skip_native_calculate_max_stack); - __ li(Rtop_frame_size, Argument::n_register_parameters); + __ li(Rtop_frame_size, Argument::n_int_register_parameters_c); __ bind(skip_native_calculate_max_stack); __ sldi(Rsize_of_parameters, Rsize_of_parameters, Interpreter::logStackElementSize); __ sldi(Rtop_frame_size, Rtop_frame_size, Interpreter::logStackElementSize); @@ -1355,7 +1341,7 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) { // outgoing argument area. // // Not needed on PPC64. - //__ add(SP, SP, Argument::n_register_parameters*BytesPerWord); + //__ add(SP, SP, Argument::n_int_register_parameters_c*BytesPerWord); assert(result_handler_addr->is_nonvolatile(), "result_handler_addr must be in a non-volatile register"); // Save across call to native method. diff --git a/src/hotspot/cpu/riscv/sharedRuntime_riscv.cpp b/src/hotspot/cpu/riscv/sharedRuntime_riscv.cpp index 287c422d64f..e98f714ae0c 100644 --- a/src/hotspot/cpu/riscv/sharedRuntime_riscv.cpp +++ b/src/hotspot/cpu/riscv/sharedRuntime_riscv.cpp @@ -694,9 +694,7 @@ int SharedRuntime::vector_calling_convention(VMRegPair *regs, int SharedRuntime::c_calling_convention(const BasicType *sig_bt, VMRegPair *regs, - VMRegPair *regs2, int total_args_passed) { - assert(regs2 == nullptr, "not needed on riscv"); // We return the amount of VMRegImpl stack slots we need to reserve for all // the arguments NOT counting out_preserve_stack_slots. @@ -1345,7 +1343,7 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm, // Now figure out where the args must be stored and how much stack space // they require. - int out_arg_slots = c_calling_convention(out_sig_bt, out_regs, nullptr, total_c_args); + int out_arg_slots = c_calling_convention(out_sig_bt, out_regs, total_c_args); // Compute framesize for the wrapper. We need to handlize all oops in // incoming registers diff --git a/src/hotspot/cpu/s390/sharedRuntime_s390.cpp b/src/hotspot/cpu/s390/sharedRuntime_s390.cpp index 05b607ec03c..4798f35f19d 100644 --- a/src/hotspot/cpu/s390/sharedRuntime_s390.cpp +++ b/src/hotspot/cpu/s390/sharedRuntime_s390.cpp @@ -760,9 +760,7 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt, int SharedRuntime::c_calling_convention(const BasicType *sig_bt, VMRegPair *regs, - VMRegPair *regs2, int total_args_passed) { - assert(regs2 == nullptr, "second VMRegPair array not used on this platform"); // Calling conventions for C runtime calls and calls to JNI native methods. const VMReg z_iarg_reg[5] = { @@ -1457,7 +1455,7 @@ nmethod *SharedRuntime::generate_native_wrapper(MacroAssembler *masm, // *_slot_offset indicates offset from SP in #stack slots // *_offset indicates offset from SP in #bytes - int stack_slots = c_calling_convention(out_sig_bt, out_regs, /*regs2=*/nullptr, total_c_args) + // 1+2 + int stack_slots = c_calling_convention(out_sig_bt, out_regs, total_c_args) + // 1+2 SharedRuntime::out_preserve_stack_slots(); // see c_calling_convention // Now the space for the inbound oop handle area. diff --git a/src/hotspot/cpu/x86/sharedRuntime_x86_32.cpp b/src/hotspot/cpu/x86/sharedRuntime_x86_32.cpp index c391349cfa3..3ae539bac8d 100644 --- a/src/hotspot/cpu/x86/sharedRuntime_x86_32.cpp +++ b/src/hotspot/cpu/x86/sharedRuntime_x86_32.cpp @@ -978,9 +978,8 @@ AdapterHandlerEntry* SharedRuntime::generate_i2c2i_adapters(MacroAssembler *masm int SharedRuntime::c_calling_convention(const BasicType *sig_bt, VMRegPair *regs, - VMRegPair *regs2, int total_args_passed) { - assert(regs2 == nullptr, "not needed on x86"); + // We return the amount of VMRegImpl stack slots we need to reserve for all // the arguments NOT counting out_preserve_stack_slots. @@ -1366,7 +1365,7 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm, // Now figure out where the args must be stored and how much stack space // they require. int out_arg_slots; - out_arg_slots = c_calling_convention(out_sig_bt, out_regs, nullptr, total_c_args); + out_arg_slots = c_calling_convention(out_sig_bt, out_regs, total_c_args); // Compute framesize for the wrapper. We need to handlize all oops in // registers a max of 2 on x86. diff --git a/src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp b/src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp index 56f66284324..bf15dab0703 100644 --- a/src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp +++ b/src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp @@ -1053,9 +1053,8 @@ AdapterHandlerEntry* SharedRuntime::generate_i2c2i_adapters(MacroAssembler *masm int SharedRuntime::c_calling_convention(const BasicType *sig_bt, VMRegPair *regs, - VMRegPair *regs2, int total_args_passed) { - assert(regs2 == nullptr, "not needed on x86"); + // We return the amount of VMRegImpl stack slots we need to reserve for all // the arguments NOT counting out_preserve_stack_slots. @@ -1803,7 +1802,7 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm, // Now figure out where the args must be stored and how much stack space // they require. int out_arg_slots; - out_arg_slots = c_calling_convention(out_sig_bt, out_regs, nullptr, total_c_args); + out_arg_slots = c_calling_convention(out_sig_bt, out_regs, total_c_args); // Compute framesize for the wrapper. We need to handlize all oops in // incoming registers diff --git a/src/hotspot/cpu/zero/sharedRuntime_zero.cpp b/src/hotspot/cpu/zero/sharedRuntime_zero.cpp index d55b7aead3f..4244b5817db 100644 --- a/src/hotspot/cpu/zero/sharedRuntime_zero.cpp +++ b/src/hotspot/cpu/zero/sharedRuntime_zero.cpp @@ -118,7 +118,6 @@ RuntimeStub* SharedRuntime::generate_resolve_blob(address destination, const cha int SharedRuntime::c_calling_convention(const BasicType *sig_bt, VMRegPair *regs, - VMRegPair *regs2, int total_args_passed) { ShouldNotCallThis(); return 0; diff --git a/src/hotspot/share/c1/c1_FrameMap.cpp b/src/hotspot/share/c1/c1_FrameMap.cpp index be72b1c1459..c55e41f4583 100644 --- a/src/hotspot/share/c1/c1_FrameMap.cpp +++ b/src/hotspot/share/c1/c1_FrameMap.cpp @@ -118,7 +118,7 @@ CallingConvention* FrameMap::c_calling_convention(const BasicTypeArray* signatur } } - intptr_t out_preserve = SharedRuntime::c_calling_convention(sig_bt, regs, nullptr, sizeargs); + intptr_t out_preserve = SharedRuntime::c_calling_convention(sig_bt, regs, sizeargs); LIR_OprList* args = new LIR_OprList(signature->length()); for (i = 0; i < sizeargs;) { BasicType t = sig_bt[i]; diff --git a/src/hotspot/share/opto/callnode.cpp b/src/hotspot/share/opto/callnode.cpp index 6268c84e046..71f78057707 100644 --- a/src/hotspot/share/opto/callnode.cpp +++ b/src/hotspot/share/opto/callnode.cpp @@ -1232,7 +1232,7 @@ bool CallLeafVectorNode::cmp( const Node &n ) const { //------------------------------calling_convention----------------------------- void CallRuntimeNode::calling_convention(BasicType* sig_bt, VMRegPair *parm_regs, uint argcnt) const { - SharedRuntime::c_calling_convention(sig_bt, parm_regs, /*regs2=*/nullptr, argcnt); + SharedRuntime::c_calling_convention(sig_bt, parm_regs, argcnt); } void CallLeafVectorNode::calling_convention( BasicType* sig_bt, VMRegPair *parm_regs, uint argcnt ) const { diff --git a/src/hotspot/share/runtime/sharedRuntime.hpp b/src/hotspot/share/runtime/sharedRuntime.hpp index b557d9cda9c..663714dc7fc 100644 --- a/src/hotspot/share/runtime/sharedRuntime.hpp +++ b/src/hotspot/share/runtime/sharedRuntime.hpp @@ -387,8 +387,7 @@ class SharedRuntime: AllStatic { // to be filled by the c_calling_convention method. On other architectures, // null is being passed as the second VMRegPair array, so arguments are either // passed in a register OR in a stack slot. - static int c_calling_convention(const BasicType *sig_bt, VMRegPair *regs, VMRegPair *regs2, - int total_args_passed); + static int c_calling_convention(const BasicType *sig_bt, VMRegPair *regs, int total_args_passed); static int vector_calling_convention(VMRegPair *regs, uint num_bits, From 9194d2c71410c377aa70372dc4f51235f6ba967c Mon Sep 17 00:00:00 2001 From: Magnus Ihse Bursie Date: Fri, 17 Nov 2023 12:06:55 +0000 Subject: [PATCH 26/76] 8317357: Update links in building.md to use https rather than http Reviewed-by: iris, erikj, jwaters --- doc/building.html | 40 ++++++++++++++++++++-------------------- doc/building.md | 32 ++++++++++++++++---------------- 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/doc/building.html b/doc/building.html index d76aa090baf..79a683c0a98 100644 --- a/doc/building.html +++ b/doc/building.html @@ -248,7 +248,7 @@

Introduction

software, and reasonably powerful hardware.

If you just want to use the JDK and not build it yourself, this document is not for you. See for instance OpenJDK installation for some +href="https://openjdk.org/install">OpenJDK installation for some methods of installing a prebuilt JDK.

Getting the Source Code

Make sure you are getting the correct version. As of JDK 10, the @@ -405,9 +405,9 @@

Windows

Note: The Windows 32-bit x86 port is deprecated and may be removed in a future release.

Cygwin

-

A functioning Cygwin environment -is required for building the JDK on Windows. If you have a 64-bit OS, we -strongly recommend using the 64-bit version of Cygwin.

+

A functioning Cygwin +environment is required for building the JDK on Windows. If you have a +64-bit OS, we strongly recommend using the 64-bit version of Cygwin.

Note: Cygwin has a model of continuously updating all packages without any easy way to install or revert to a specific version of a package. This means that whenever you add or update a @@ -635,9 +635,9 @@

Boot JDK Requirements

picked, use --with-boot-jdk to point to the JDK to use.

Getting JDK binaries

JDK binaries for Linux, Windows and macOS can be downloaded from jdk.java.net. An alternative is to +href="https://jdk.java.net">jdk.java.net. An alternative is to download the Oracle +href="https://www.oracle.com/technetwork/java/javase/downloads">Oracle JDK. Another is the Adopt OpenJDK Project, which publishes experimental prebuilt binaries for various platforms.

@@ -663,7 +663,7 @@

External Library

As a fallback, the second version allows you to point to the include directory and the lib directory separately.

FreeType

-

FreeType2 from The FreeType +

FreeType2 from The FreeType Project is not required on any platform. The exception is on Unix-based platforms when configuring such that the build artifacts will reference a system installed library, rather than bundling the JDK's own @@ -682,7 +682,7 @@

FreeType

--with-freetype-lib=<path> if configure does not automatically locate the platform FreeType files.

Fontconfig

-

Fontconfig from freedesktop.org +

Fontconfig from freedesktop.org Fontconfig is required on all platforms except Windows and macOS.