From 0461d9a7d67230736ce6370ff8146a60f3bd9cf3 Mon Sep 17 00:00:00 2001 From: Thomas Stuefe Date: Wed, 1 Nov 2023 08:12:39 +0000 Subject: [PATCH 01/12] 8318016: Per-compilation memory ceiling Reviewed-by: roland, thartmann --- src/hotspot/share/c1/c1_Compilation.cpp | 8 +- src/hotspot/share/c1/c1_Compilation.hpp | 5 + src/hotspot/share/ci/ciEnv.hpp | 4 +- .../compiler/compilationMemoryStatistic.cpp | 165 ++++++++++++++++-- .../compiler/compilationMemoryStatistic.hpp | 20 ++- src/hotspot/share/compiler/compileTask.cpp | 2 +- src/hotspot/share/compiler/compileTask.hpp | 2 +- .../share/compiler/compilerDirectives.cpp | 13 +- .../share/compiler/compilerDirectives.hpp | 3 + src/hotspot/share/compiler/compilerOracle.cpp | 49 +++++- src/hotspot/share/compiler/compilerOracle.hpp | 1 + src/hotspot/share/opto/compile.cpp | 6 + src/hotspot/share/opto/compile.hpp | 11 ++ src/hotspot/share/utilities/debug.hpp | 3 +- .../print/CompileCommandMemLimit.java | 154 ++++++++++++++++ .../print/CompileCommandPrintMemStat.java | 8 +- .../compiler/CompilerMemoryStatisticTest.java | 6 +- 17 files changed, 426 insertions(+), 34 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/print/CompileCommandMemLimit.java diff --git a/src/hotspot/share/c1/c1_Compilation.cpp b/src/hotspot/share/c1/c1_Compilation.cpp index 864510da371..74fd825a07a 100644 --- a/src/hotspot/share/c1/c1_Compilation.cpp +++ b/src/hotspot/share/c1/c1_Compilation.cpp @@ -397,6 +397,7 @@ int Compilation::compile_java_method() { PhaseTraceTime timeit(_t_buildIR); build_hir(); } + CHECK_BAILOUT_(no_frame_size); if (BailoutAfterHIR) { BAILOUT_("Bailing out because of -XX:+BailoutAfterHIR", no_frame_size); } @@ -446,13 +447,13 @@ void Compilation::install_code(int frame_size) { void Compilation::compile_method() { - CompilationMemoryStatisticMark cmsm(env()->task()->directive()); - { PhaseTraceTime timeit(_t_setup); // setup compilation initialize(); + CHECK_BAILOUT(); + } if (!method()->can_be_compiled()) { @@ -605,6 +606,9 @@ Compilation::Compilation(AbstractCompiler* compiler, ciEnv* env, ciMethod* metho _cfg_printer_output = new CFGPrinterOutput(this); } #endif + + CompilationMemoryStatisticMark cmsm(directive); + compile_method(); if (bailed_out()) { _env->record_method_not_compilable(bailout_msg()); diff --git a/src/hotspot/share/c1/c1_Compilation.hpp b/src/hotspot/share/c1/c1_Compilation.hpp index 4d310c088e0..e9de43e8146 100644 --- a/src/hotspot/share/c1/c1_Compilation.hpp +++ b/src/hotspot/share/c1/c1_Compilation.hpp @@ -85,6 +85,7 @@ class Compilation: public StackObj { bool _has_monitors; // Fastpath monitors detection for Continuations bool _install_code; const char* _bailout_msg; + bool _oom; ExceptionInfoList* _exception_info_list; ExceptionHandlerTable _exception_handler_table; ImplicitExceptionTable _implicit_exception_table; @@ -203,6 +204,10 @@ class Compilation: public StackObj { } #endif // PRODUCT + // MemLimit handling + bool oom() const { return _oom; } + void set_oom() { _oom = true; } + // error handling void bailout(const char* msg); bool bailed_out() const { return _bailout_msg != nullptr; } diff --git a/src/hotspot/share/ci/ciEnv.hpp b/src/hotspot/share/ci/ciEnv.hpp index added1ae358..5c39cb58f9f 100644 --- a/src/hotspot/share/ci/ciEnv.hpp +++ b/src/hotspot/share/ci/ciEnv.hpp @@ -319,10 +319,10 @@ class ciEnv : StackObj { // This is true if the compilation is not going to produce code. // (It is reasonable to retry failed compilations.) - bool failing() { return _failure_reason != nullptr; } + bool failing() const { return _failure_reason != nullptr; } // Reason this compilation is failing, such as "too many basic blocks". - const char* failure_reason() { return _failure_reason; } + const char* failure_reason() const { return _failure_reason; } // Return state of appropriate compatibility int compilable() { return _compilable; } diff --git a/src/hotspot/share/compiler/compilationMemoryStatistic.cpp b/src/hotspot/share/compiler/compilationMemoryStatistic.cpp index 8395d221c92..33e797d2353 100644 --- a/src/hotspot/share/compiler/compilationMemoryStatistic.cpp +++ b/src/hotspot/share/compiler/compilationMemoryStatistic.cpp @@ -26,6 +26,9 @@ #include "precompiled.hpp" #include "logging/log.hpp" #include "logging/logStream.hpp" +#ifdef COMPILER1 +#include "c1/c1_Compilation.hpp" +#endif #include "compiler/abstractCompiler.hpp" #include "compiler/compilationMemoryStatistic.hpp" #include "compiler/compilerDirectives.hpp" @@ -42,15 +45,16 @@ #endif #include "runtime/mutexLocker.hpp" #include "runtime/os.hpp" +#include "utilities/debug.hpp" #include "utilities/globalDefinitions.hpp" #include "utilities/ostream.hpp" #include "utilities/quickSort.hpp" #include "utilities/resourceHash.hpp" - ArenaStatCounter::ArenaStatCounter() : _current(0), _start(0), _peak(0), _na(0), _ra(0), + _limit(0), _hit_limit(false), _na_at_peak(0), _ra_at_peak(0), _live_nodes_at_peak(0) {} @@ -58,8 +62,15 @@ size_t ArenaStatCounter::peak_since_start() const { return _peak > _start ? _peak - _start : 0; } -void ArenaStatCounter::start() { +void ArenaStatCounter::start(size_t limit) { _peak = _start = _current; + _limit = limit; + _hit_limit = false; +} + +void ArenaStatCounter::end(){ + _limit = 0; + _hit_limit = false; } void ArenaStatCounter::update_c2_node_count() { @@ -104,6 +115,10 @@ bool ArenaStatCounter::account(ssize_t delta, int tag) { _ra_at_peak = _ra; update_c2_node_count(); rc = true; + // Did we hit the memory limit? + if (!_hit_limit && _limit > 0 && peak_since_start() > _limit) { + _hit_limit = true; + } } return rc; } @@ -125,7 +140,8 @@ class FullMethodName { public: - FullMethodName(Symbol* k, Symbol* m, Symbol* s) : _k(k), _m(m), _s(s) {} + FullMethodName(const Method* m) : + _k(m->klass_name()), _m(m->name()), _s(m->signature()) {}; FullMethodName(const FullMethodName& o) : _k(o._k), _m(o._m), _s(o._s) {} void make_permanent() { @@ -173,13 +189,15 @@ class MemStatEntry : public CHeapObj { size_t _na_at_peak; size_t _ra_at_peak; unsigned _live_nodes_at_peak; + const char* _result; public: MemStatEntry(FullMethodName method) : _method(method), _comptype(compiler_c1), _time(0), _num_recomp(0), _thread(nullptr), - _total(0), _na_at_peak(0), _ra_at_peak(0), _live_nodes_at_peak(0) { + _total(0), _na_at_peak(0), _ra_at_peak(0), _live_nodes_at_peak(0), + _result(nullptr) { } void set_comptype(CompilerType comptype) { _comptype = comptype; } @@ -192,6 +210,8 @@ class MemStatEntry : public CHeapObj { void set_ra_at_peak(size_t n) { _ra_at_peak = n; } void set_live_nodes_at_peak(unsigned n) { _live_nodes_at_peak = n; } + void set_result(const char* s) { _result = s; } + size_t total() const { return _total; } static void print_legend(outputStream* st) { @@ -199,7 +219,8 @@ class MemStatEntry : public CHeapObj { st->print_cr(" total : memory allocated via arenas while compiling"); st->print_cr(" NA : ...how much in node arenas (if c2)"); st->print_cr(" RA : ...how much in resource areas"); - st->print_cr(" #nodes : ...how many nodes (if c2)"); + st->print_cr(" result : Result: 'ok' finished successfully, 'oom' hit memory limit, 'err' compilation failed"); + st->print_cr(" #nodes : ...how many nodes (c2 only)"); st->print_cr(" time : time of last compilation (sec)"); st->print_cr(" type : compiler type"); st->print_cr(" #rc : how often recompiled"); @@ -207,7 +228,7 @@ class MemStatEntry : public CHeapObj { } static void print_header(outputStream* st) { - st->print_cr("total NA RA #nodes time type #rc thread method"); + st->print_cr("total NA RA result #nodes time type #rc thread method"); } void print_on(outputStream* st, bool human_readable) const { @@ -237,6 +258,10 @@ class MemStatEntry : public CHeapObj { } col += 10; st->fill_to(col); + // result? + st->print("%s ", _result ? _result : ""); + col += 8; st->fill_to(col); + // Number of Nodes when memory peaked st->print("%u ", _live_nodes_at_peak); col += 8; st->fill_to(col); @@ -281,7 +306,7 @@ class MemStatTable : void add(const FullMethodName& fmn, CompilerType comptype, size_t total, size_t na_at_peak, size_t ra_at_peak, - unsigned live_nodes_at_peak) { + unsigned live_nodes_at_peak, const char* result) { assert_lock_strong(NMTCompilationCostHistory_lock); MemStatEntry** pe = get(fmn); @@ -302,6 +327,7 @@ class MemStatTable : e->set_na_at_peak(na_at_peak); e->set_ra_at_peak(ra_at_peak); e->set_live_nodes_at_peak(live_nodes_at_peak); + e->set_result(result); } // Returns a C-heap-allocated SortMe array containing all entries from the table, @@ -341,20 +367,21 @@ void CompilationMemoryStatistic::initialize() { log_info(compilation, alloc)("Compilation memory statistic enabled"); } -void CompilationMemoryStatistic::on_start_compilation() { +void CompilationMemoryStatistic::on_start_compilation(const DirectiveSet* directive) { assert(enabled(), "Not enabled?"); - Thread::current()->as_Compiler_thread()->arena_stat()->start(); + const size_t limit = directive->mem_limit(); + Thread::current()->as_Compiler_thread()->arena_stat()->start(limit); } void CompilationMemoryStatistic::on_end_compilation() { assert(enabled(), "Not enabled?"); ResourceMark rm; CompilerThread* const th = Thread::current()->as_Compiler_thread(); - const ArenaStatCounter* const arena_stat = th->arena_stat(); + ArenaStatCounter* const arena_stat = th->arena_stat(); const CompilerType ct = th->task()->compiler()->type(); const Method* const m = th->task()->method(); - FullMethodName fmn(m->klass_name(), m->name(), m->signature()); + FullMethodName fmn(m); fmn.make_permanent(); const DirectiveSet* directive = th->task()->directive(); @@ -368,6 +395,20 @@ void CompilationMemoryStatistic::on_end_compilation() { arena_stat->print_on(tty); tty->cr(); } + + // Store result + // For this to work, we must call on_end_compilation() at a point where + // Compile|Compilation already handed over the failure string to ciEnv, + // but ciEnv must still be alive. + const char* result = "ok"; // ok + const ciEnv* const env = th->env(); + if (env) { + const char* const failure_reason = env->failure_reason(); + if (failure_reason != nullptr) { + result = (failure_reason == failure_reason_memlimit()) ? "oom" : "err"; + } + } + { MutexLocker ml(NMTCompilationCostHistory_lock, Mutex::_no_safepoint_check_flag); assert(_the_table != nullptr, "not initialized"); @@ -376,14 +417,105 @@ void CompilationMemoryStatistic::on_end_compilation() { arena_stat->peak_since_start(), // total arena_stat->na_at_peak(), arena_stat->ra_at_peak(), - arena_stat->live_nodes_at_peak()); + arena_stat->live_nodes_at_peak(), + result); + } + + arena_stat->end(); // reset things +} + +static void inform_compilation_about_oom(CompilerType ct) { + // Inform C1 or C2 that an OOM happened. They will take delayed action + // and abort the compilation in progress. Note that this is not instantaneous, + // since the compiler has to actively bailout, which may take a while, during + // which memory usage may rise further. + // + // The mechanism differs slightly between C1 and C2: + // - With C1, we directly set the bailout string, which will cause C1 to + // bailout at the typical BAILOUT places. + // - With C2, the corresponding mechanism would be the failure string; but + // bailout paths in C2 are not complete and therefore it is dangerous to + // set the failure string at - for C2 - seemingly random places. Instead, + // upon OOM C2 sets the failure string next time it checks the node limit. + if (ciEnv::current() != nullptr) { + void* compiler_data = ciEnv::current()->compiler_data(); +#ifdef COMPILER1 + if (ct == compiler_c1) { + Compilation* C = static_cast(compiler_data); + if (C != nullptr) { + C->bailout(CompilationMemoryStatistic::failure_reason_memlimit()); + C->set_oom(); + } + } +#endif +#ifdef COMPILER2 + if (ct == compiler_c2) { + Compile* C = static_cast(compiler_data); + if (C != nullptr) { + C->set_oom(); + } + } +#endif // COMPILER2 } } void CompilationMemoryStatistic::on_arena_change(ssize_t diff, const Arena* arena) { assert(enabled(), "Not enabled?"); CompilerThread* const th = Thread::current()->as_Compiler_thread(); - th->arena_stat()->account(diff, (int)arena->get_tag()); + + ArenaStatCounter* const arena_stat = th->arena_stat(); + bool hit_limit_before = arena_stat->hit_limit(); + + if (arena_stat->account(diff, (int)arena->get_tag())) { // new peak? + + // Limit handling + if (arena_stat->hit_limit()) { + + char name[1024] = ""; + bool print = false; + bool crash = false; + CompilerType ct = compiler_none; + + // get some more info + const CompileTask* task = th->task(); + if (task != nullptr) { + ct = task->compiler()->type(); + const DirectiveSet* directive = task->directive(); + print = directive->should_print_memstat(); + crash = directive->should_crash_at_mem_limit(); + const Method* m = th->task()->method(); + if (m != nullptr) { + FullMethodName(m).as_C_string(name, sizeof(name)); + } + } + + char message[1024] = ""; + + // build up message if we need it later + if (print || crash) { + stringStream ss(message, sizeof(message)); + if (ct != compiler_none && name[0] != '\0') { + ss.print("%s %s: ", compilertype2name(ct), name); + } + ss.print("Hit MemLimit %s (limit: %zu now: %zu)", + (hit_limit_before ? "again" : ""), + arena_stat->limit(), arena_stat->peak_since_start()); + } + + // log if needed + if (print) { + tty->print_raw(message); + tty->cr(); + } + + // Crash out if needed + if (crash) { + report_fatal(OOM_HOTSPOT_ARENA, __FILE__, __LINE__, "%s", message); + } else { + inform_compilation_about_oom(ct); + } + } + } } static inline ssize_t diff_entries_by_size(const MemStatEntry* e1, const MemStatEntry* e2) { @@ -438,10 +570,15 @@ void CompilationMemoryStatistic::print_all_by_size(outputStream* st, bool human_ FREE_C_HEAP_ARRAY(Entry, filtered); } +const char* CompilationMemoryStatistic::failure_reason_memlimit() { + static const char* const s = "hit memory limit while compiling"; + return s; +} + CompilationMemoryStatisticMark::CompilationMemoryStatisticMark(const DirectiveSet* directive) : _active(directive->should_collect_memstat()) { if (_active) { - CompilationMemoryStatistic::on_start_compilation(); + CompilationMemoryStatistic::on_start_compilation(directive); } } CompilationMemoryStatisticMark::~CompilationMemoryStatisticMark() { diff --git a/src/hotspot/share/compiler/compilationMemoryStatistic.hpp b/src/hotspot/share/compiler/compilationMemoryStatistic.hpp index 06ac9382199..625875f05f4 100644 --- a/src/hotspot/share/compiler/compilationMemoryStatistic.hpp +++ b/src/hotspot/share/compiler/compilationMemoryStatistic.hpp @@ -47,6 +47,9 @@ class ArenaStatCounter : public CHeapObj { size_t _na; // Current bytes used for resource areas size_t _ra; + // MemLimit handling + size_t _limit; + bool _hit_limit; // Peak composition: // Size of node arena when total peaked (c2 only) @@ -69,15 +72,20 @@ class ArenaStatCounter : public CHeapObj { size_t ra_at_peak() const { return _ra_at_peak; } unsigned live_nodes_at_peak() const { return _live_nodes_at_peak; } - // Mark the start of a compilation. - void start(); + // Mark the start and end of a compilation. + void start(size_t limit); + void end(); // Account an arena allocation or de-allocation. // Returns true if new peak reached bool account(ssize_t delta, int tag); void set_live_nodes_at_peak(unsigned i) { _live_nodes_at_peak = i; } + void print_on(outputStream* st) const; + + size_t limit() const { return _limit; } + bool hit_limit() const { return _hit_limit; } }; class CompilationMemoryStatistic : public AllStatic { @@ -86,10 +94,16 @@ class CompilationMemoryStatistic : public AllStatic { static void initialize(); // true if CollectMemStat or PrintMemStat has been enabled for any method static bool enabled() { return _enabled; } - static void on_start_compilation(); + static void on_start_compilation(const DirectiveSet* directive); + + // Called at end of compilation. Records the arena usage peak. Also takes over + // status information from ciEnv (compilation failed, oom'ed or went okay). ciEnv::_failure_reason + // must be set at this point (so place CompilationMemoryStatisticMark correctly). static void on_end_compilation(); static void on_arena_change(ssize_t diff, const Arena* arena); static void print_all_by_size(outputStream* st, bool human_readable, size_t minsize); + // For compilers + static const char* failure_reason_memlimit(); }; // RAII object to wrap one compilation diff --git a/src/hotspot/share/compiler/compileTask.cpp b/src/hotspot/share/compiler/compileTask.cpp index f6b6b698eaf..4690cd5c134 100644 --- a/src/hotspot/share/compiler/compileTask.cpp +++ b/src/hotspot/share/compiler/compileTask.cpp @@ -142,7 +142,7 @@ void CompileTask::initialize(int compile_id, /** * Returns the compiler for this task. */ -AbstractCompiler* CompileTask::compiler() { +AbstractCompiler* CompileTask::compiler() const { return CompileBroker::compiler(_comp_level); } diff --git a/src/hotspot/share/compiler/compileTask.hpp b/src/hotspot/share/compiler/compileTask.hpp index e5ab8ba3eca..43beacb03d2 100644 --- a/src/hotspot/share/compiler/compileTask.hpp +++ b/src/hotspot/share/compiler/compileTask.hpp @@ -180,7 +180,7 @@ class CompileTask : public CHeapObj { int comp_level() { return _comp_level;} void set_comp_level(int comp_level) { _comp_level = comp_level;} - AbstractCompiler* compiler(); + AbstractCompiler* compiler() const; CompileTask* select_for_compilation(); int num_inlined_bytecodes() const { return _num_inlined_bytecodes; } diff --git a/src/hotspot/share/compiler/compilerDirectives.cpp b/src/hotspot/share/compiler/compilerDirectives.cpp index 20c576fce66..c76ef6de01d 100644 --- a/src/hotspot/share/compiler/compilerDirectives.cpp +++ b/src/hotspot/share/compiler/compilerDirectives.cpp @@ -203,13 +203,24 @@ bool DirectiveSet::is_c2(CompilerDirectives* directive) const { } bool DirectiveSet::should_collect_memstat() const { - return MemStatOption > 0; + // MemLimit requires the memory statistic to be active + return MemStatOption > 0 || MemLimitOption != 0; } bool DirectiveSet::should_print_memstat() const { return MemStatOption == (uintx)MemStatAction::print; } +size_t DirectiveSet::mem_limit() const { + return MemLimitOption < 0 ? -MemLimitOption : MemLimitOption; +} + +bool DirectiveSet::should_crash_at_mem_limit() const { + // The sign encodes the action to be taken when reaching + // the memory limit (+ stop - crash) + return MemLimitOption < 0; +} + // In the list of Control/disabled intrinsics, the ID of the control intrinsics can separated: // - by ',' (if -XX:Control/DisableIntrinsic is used once when invoking the VM) or // - by '\n' (if -XX:Control/DisableIntrinsic is used multiple times when invoking the VM) or diff --git a/src/hotspot/share/compiler/compilerDirectives.hpp b/src/hotspot/share/compiler/compilerDirectives.hpp index 439a4c2f925..a252ad02889 100644 --- a/src/hotspot/share/compiler/compilerDirectives.hpp +++ b/src/hotspot/share/compiler/compilerDirectives.hpp @@ -41,6 +41,7 @@ cflags(BreakAtExecute, bool, false, BreakAtExecute) \ cflags(BreakAtCompile, bool, false, BreakAtCompile) \ cflags(Log, bool, LogCompilation, Unknown) \ + cflags(MemLimit, intx, 0, MemLimit) \ cflags(MemStat, uintx, 0, MemStat) \ cflags(PrintAssembly, bool, PrintAssembly, PrintAssembly) \ cflags(PrintCompilation, bool, PrintCompilation, PrintCompilation) \ @@ -150,6 +151,8 @@ class DirectiveSet : public CHeapObj { bool is_c2(CompilerDirectives* directive) const; bool should_collect_memstat() const; bool should_print_memstat() const; + size_t mem_limit() const; + bool should_crash_at_mem_limit() const; // true: crash false: stop compilation typedef enum { #define enum_of_flags(name, type, dvalue, cc_flag) name##Index, diff --git a/src/hotspot/share/compiler/compilerOracle.cpp b/src/hotspot/share/compiler/compilerOracle.cpp index e1fb019bdbf..56fed1394c2 100644 --- a/src/hotspot/share/compiler/compilerOracle.cpp +++ b/src/hotspot/share/compiler/compilerOracle.cpp @@ -39,6 +39,7 @@ #include "runtime/handles.inline.hpp" #include "runtime/jniHandles.hpp" #include "runtime/os.hpp" +#include "utilities/parseInteger.hpp" static const char* optiontype_names[] = { #define enum_of_types(type, name) name, @@ -459,7 +460,7 @@ bool CompilerOracle::should_print_methods() { // Tells whether there are any methods to collect memory statistics for bool CompilerOracle::should_collect_memstat() { - return has_command(CompileCommand::MemStat); + return has_command(CompileCommand::MemStat) || has_command(CompileCommand::MemLimit); } bool CompilerOracle::should_print_final_memstat_report() { @@ -634,6 +635,44 @@ void skip_comma(char* &line) { } } +static bool parseMemLimit(const char* line, intx& value, int& bytes_read, char* errorbuf, const int buf_size) { + // Format: + // "['~' ]" + // can have units, e.g. M + // one of "crash" "stop", if omitted, "stop" is implied. + // + // Examples: + // -XX:CompileCommand='memlimit,*.*,20m' + // -XX:CompileCommand='memlimit,*.*,20m~stop' + // -XX:CompileCommand='memlimit,Option::toString,1m~crash' + // + // The resulting intx carries the size and whether we are to stop or crash: + // - neg. value means crash + // - pos. value (default) means stop + size_t s = 0; + char* end; + if (!parse_integer(line, &end, &s)) { + jio_snprintf(errorbuf, buf_size, "MemLimit: invalid value"); + } + bytes_read = (int)(end - line); + + intx v = (intx)s; + if ((*end) != '\0') { + if (strncasecmp(end, "~crash", 6) == 0) { + v = -v; + bytes_read += 6; + } else if (strncasecmp(end, "~stop", 5) == 0) { + // ok, this is the default + bytes_read += 5; + } else { + jio_snprintf(errorbuf, buf_size, "MemLimit: invalid option"); + return true; + } + } + value = v; + return true; +} + static bool parseEnumValueAsUintx(enum CompileCommand option, const char* line, uintx& value, int& bytes_read, char* errorbuf, const int buf_size) { if (option == CompileCommand::MemStat) { if (strncasecmp(line, "collect", 7) == 0) { @@ -659,7 +698,13 @@ static void scan_value(enum OptionType type, char* line, int& total_bytes_read, total_bytes_read += skipped; if (type == OptionType::Intx) { intx value; - if (sscanf(line, "" INTX_FORMAT "%n", &value, &bytes_read) == 1) { + // Special handling for memlimit + bool success = (option == CompileCommand::MemLimit) && parseMemLimit(line, value, bytes_read, errorbuf, buf_size); + if (!success) { + // Is it a raw number? + success = sscanf(line, "" INTX_FORMAT "%n", &value, &bytes_read) == 1; + } + if (success) { total_bytes_read += bytes_read; line += bytes_read; register_command(matcher, option, value); diff --git a/src/hotspot/share/compiler/compilerOracle.hpp b/src/hotspot/share/compiler/compilerOracle.hpp index 251f29fda38..be1a270f3c9 100644 --- a/src/hotspot/share/compiler/compilerOracle.hpp +++ b/src/hotspot/share/compiler/compilerOracle.hpp @@ -57,6 +57,7 @@ class methodHandle; option(Break, "break", Bool) \ option(BreakAtExecute, "BreakAtExecute", Bool) \ option(BreakAtCompile, "BreakAtCompile", Bool) \ + option(MemLimit, "MemLimit", Intx) \ option(MemStat, "MemStat", Uintx) \ option(PrintAssembly, "PrintAssembly", Bool) \ option(PrintCompilation, "PrintCompilation", Bool) \ diff --git a/src/hotspot/share/opto/compile.cpp b/src/hotspot/share/opto/compile.cpp index c66c32fe973..ac2d65feca3 100644 --- a/src/hotspot/share/opto/compile.cpp +++ b/src/hotspot/share/opto/compile.cpp @@ -29,6 +29,7 @@ #include "classfile/javaClasses.hpp" #include "code/exceptionHandlerTable.hpp" #include "code/nmethod.hpp" +#include "compiler/compilationMemoryStatistic.hpp" #include "compiler/compileBroker.hpp" #include "compiler/compileLog.hpp" #include "compiler/disassembler.hpp" @@ -661,6 +662,7 @@ Compile::Compile( ciEnv* ci_env, ciMethod* target, int osr_bci, _vector_reboxing_late_inlines(comp_arena(), 2, 0, nullptr), _late_inlines_pos(0), _number_of_mh_late_inlines(0), + _oom(false), _print_inlining_stream(new (mtCompiler) stringStream()), _print_inlining_list(nullptr), _print_inlining_idx(0), @@ -938,6 +940,7 @@ Compile::Compile( ciEnv* ci_env, _types(nullptr), _node_hash(nullptr), _number_of_mh_late_inlines(0), + _oom(false), _print_inlining_stream(new (mtCompiler) stringStream()), _print_inlining_list(nullptr), _print_inlining_idx(0), @@ -5261,3 +5264,6 @@ Node* Compile::narrow_value(BasicType bt, Node* value, const Type* type, PhaseGV return result; } +void Compile::record_method_not_compilable_oom() { + record_method_not_compilable(CompilationMemoryStatistic::failure_reason_memlimit()); +} diff --git a/src/hotspot/share/opto/compile.hpp b/src/hotspot/share/opto/compile.hpp index 47638dfa6bf..67e790ffa23 100644 --- a/src/hotspot/share/opto/compile.hpp +++ b/src/hotspot/share/opto/compile.hpp @@ -460,6 +460,9 @@ class Compile : public Phase { int _late_inlines_pos; // Where in the queue should the next late inlining candidate go (emulate depth first inlining) uint _number_of_mh_late_inlines; // number of method handle late inlining still pending + // "MemLimit" directive was specified and the memory limit was hit during compilation + bool _oom; + // Inlining may not happen in parse order which would make // PrintInlining output confusing. Keep track of PrintInlining // pieces in order. @@ -503,6 +506,8 @@ class Compile : public Phase { void log_late_inline_failure(CallGenerator* cg, const char* msg); DEBUG_ONLY(bool _exception_backedge;) + void record_method_not_compilable_oom(); + public: void* barrier_set_state() const { return _barrier_set_state; } @@ -814,6 +819,10 @@ class Compile : public Phase { record_failure(reason); } bool check_node_count(uint margin, const char* reason) { + if (oom()) { + record_method_not_compilable_oom(); + return true; + } if (live_nodes() + margin > max_node_limit()) { record_method_not_compilable(reason); return true; @@ -821,6 +830,8 @@ class Compile : public Phase { return false; } } + bool oom() const { return _oom; } + void set_oom() { _oom = true; } // Node management uint unique() const { return _unique; } diff --git a/src/hotspot/share/utilities/debug.hpp b/src/hotspot/share/utilities/debug.hpp index 93af7bdd0fa..be0ee035d08 100644 --- a/src/hotspot/share/utilities/debug.hpp +++ b/src/hotspot/share/utilities/debug.hpp @@ -250,7 +250,8 @@ enum VMErrorType : unsigned int { OOM_MALLOC_ERROR = 0xe0000001, OOM_MMAP_ERROR = 0xe0000002, OOM_MPROTECT_ERROR = 0xe0000003, - OOM_JAVA_HEAP_FATAL = 0xe0000004 + OOM_JAVA_HEAP_FATAL = 0xe0000004, + OOM_HOTSPOT_ARENA = 0xe0000005 }; // error reporting helper functions diff --git a/test/hotspot/jtreg/compiler/print/CompileCommandMemLimit.java b/test/hotspot/jtreg/compiler/print/CompileCommandMemLimit.java new file mode 100644 index 00000000000..f10834298f9 --- /dev/null +++ b/test/hotspot/jtreg/compiler/print/CompileCommandMemLimit.java @@ -0,0 +1,154 @@ +/* + * 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 id=c1_crash + * @requires vm.compiler1.enabled + * @summary Checks that -XX:CompileCommand=MemLimit,...,crash causes compilation to crash + * @library /test/lib + * @run driver compiler.print.CompileCommandMemLimit crash false + */ + +/* + * @test id=c2_crash + * @requires vm.compiler2.enabled + * @summary Checks that -XX:CompileCommand=MemLimit,...,crash causes compilation to crash + * @library /test/lib + * @run driver compiler.print.CompileCommandMemLimit crash true + */ + +/* + * @test id=c1_stop + * @requires vm.compiler1.enabled + * @summary Checks that -XX:CompileCommand=MemLimit,...,stop causes compilation to stop + * @library /test/lib + * @run driver compiler.print.CompileCommandMemLimit stop false + */ + +/* + * @test id=c2_stop + * @requires vm.compiler2.enabled + * @summary Checks that -XX:CompileCommand=MemLimit,...,stop causes compilation to stop + * @library /test/lib + * @run driver compiler.print.CompileCommandMemLimit stop true + */ + +package compiler.print; + +import jdk.test.lib.process.OutputAnalyzer; +import jdk.test.lib.process.ProcessTools; + +import java.util.ArrayList; +import java.util.List; + +public class CompileCommandMemLimit { + + final static String METHOD1 = "method1"; + final static String METHOD2 = "method2"; + + static boolean c2; + static boolean test_crash; + + public static void main(String[] args) throws Exception { + switch (args[0]) { + case "crash" : test_crash = true; break; + case "stop" : test_crash = false; break; + default: throw new RuntimeException("invalid argument"); + } + c2 = Boolean.parseBoolean(args[1]); + test(METHOD1, METHOD2); + test(METHOD2, METHOD1); + } + + private static void test(String include, String exclude) throws Exception { + + // A method that is known to cost compilers a bit of memory to compile + + List options = new ArrayList(); + options.add("-Xcomp"); + options.add("-XX:-Inline"); + options.add("-XX:CompileCommand=compileonly," + getTestClass() + "::*"); + // We pass a very small size to guarantee the crash + options.add("-XX:CompileCommand=MemStat," + getTestMethod(include) + ",print"); + options.add("-XX:CompileCommand=MemLimit," + getTestMethod(include) + ",4k" + (test_crash ? "~crash" : "")); + if (c2) { + options.add("-XX:-TieredCompilation"); + } else { + options.add("-XX:TieredStopAtLevel=1"); + } + options.add(getTestClass()); + + OutputAnalyzer oa = ProcessTools.executeTestJvm(options); + + oa.reportDiagnosticSummary(); + + String expectedNameIncl = getTestMethod(include) + .replace('.', '/') + .replace("$", "\\$"); + String expectedNameExcl = getTestMethod(exclude) + .replace('.', '/') + .replace("$", "\\$"); + + String ct = c2 ? "c2" : "c1"; + + if (test_crash) { + oa.shouldNotHaveExitValue(0); + oa.shouldMatch("# *Internal Error.*"); + oa.shouldMatch("# *fatal error: " + ct + " *" + expectedNameIncl + ".*: Hit MemLimit .*limit: 4096.*"); + oa.shouldNotMatch(".*" + expectedNameExcl + ".*"); + } else { + // Should see trace output when methods are compiled + oa.shouldHaveExitValue(0) + .shouldMatch(".*" + expectedNameIncl + ".*") + .shouldNotMatch(".*" + expectedNameExcl + ".*"); + + // Expect this log line + oa.shouldMatch(".*" + expectedNameIncl + ".*Hit MemLimit.*"); + + // Expect final output to contain "oom" + oa.shouldMatch(".*oom.*" + expectedNameIncl + ".*"); + } + } + + // Test class that is invoked by the sub process + public static String getTestClass() { + return TestMain.class.getName(); + } + + public static String getTestMethod(String method) { + return getTestClass() + "::" + method; + } + + public static class TestMain { + public static void main(String[] args) { + method1(); + method2(); + } + + static long method1() { + return System.currentTimeMillis(); + } + static void method2() {} + } +} + diff --git a/test/hotspot/jtreg/compiler/print/CompileCommandPrintMemStat.java b/test/hotspot/jtreg/compiler/print/CompileCommandPrintMemStat.java index 6ac4b493790..90682278760 100644 --- a/test/hotspot/jtreg/compiler/print/CompileCommandPrintMemStat.java +++ b/test/hotspot/jtreg/compiler/print/CompileCommandPrintMemStat.java @@ -73,11 +73,11 @@ private static void test(String include, String exclude) throws Exception { // Should see final report // Looks like this: - // total NA RA #nodes time type #rc thread method - // 621832 0 589104 0 0,025 c1 1 0x00007f5ccc1951a0 java/util/zip/ZipFile$Source::checkAndAddEntry((II)I) + // total NA RA result #nodes time type #rc thread method + // 211488 66440 77624 ok 13 0.057 c2 2 0x00007fb49428db70 compiler/print/CompileCommandPrintMemStat$TestMain::method1(()V) oa.shouldMatch("total.*method"); - oa.shouldMatch("\\d+ +\\d+ +\\d+ +\\d+.*" + expectedNameIncl + ".*"); - oa.shouldNotMatch("\\d+ +\\d+ +\\d+ +\\d+.*" + expectedNameExcl + ".*"); + oa.shouldMatch("\\d+ +\\d+ +\\d+ +\\S+ +\\d+.*" + expectedNameIncl + ".*"); + oa.shouldNotMatch("\\d+ +\\d+ +\\d+ +\\S+ +\\d+.*" + expectedNameExcl + ".*"); } // Test class that is invoked by the sub process diff --git a/test/hotspot/jtreg/serviceability/dcmd/compiler/CompilerMemoryStatisticTest.java b/test/hotspot/jtreg/serviceability/dcmd/compiler/CompilerMemoryStatisticTest.java index afc79b57142..3a4f34c2c99 100644 --- a/test/hotspot/jtreg/serviceability/dcmd/compiler/CompilerMemoryStatisticTest.java +++ b/test/hotspot/jtreg/serviceability/dcmd/compiler/CompilerMemoryStatisticTest.java @@ -59,9 +59,9 @@ public static void main(String args[]) throws Exception { out.shouldHaveExitValue(0); // Looks like this: - // total NA RA #nodes time type #rc thread method - // 621832 0 589104 0 0,025 c1 1 0x00007f5ccc1951a0 java/util/zip/ZipFile$Source.checkAndAddEntry((II)I) + // total NA RA result #nodes time type #rc thread method + // 211488 66440 77624 ok 13 0.057 c2 2 0x00007fb49428db70 compiler/print/CompileCommandPrintMemStat$TestMain::method1(()V) out.shouldMatch("total.*method"); - out.shouldMatch("\\d+ +\\d+ +\\d+ +\\d+.*java.*\\(.*\\)"); + out.shouldMatch("\\d+ +\\d+ +\\d+ +\\S+ +\\d+.*java.*\\(.*\\)"); } } From b4f5379d50db9412208552fd69bc316e7730aedd Mon Sep 17 00:00:00 2001 From: Julian Waters Date: Wed, 1 Nov 2023 10:42:23 +0000 Subject: [PATCH 02/12] 8304939: os::win32::exit_process_or_thread should be marked noreturn Reviewed-by: dholmes, kbarrett --- src/hotspot/os/windows/os_windows.cpp | 26 +++++++++++++--------- src/hotspot/os/windows/os_windows.hpp | 7 ------ src/hotspot/os/windows/vmError_windows.cpp | 1 + src/hotspot/share/utilities/vmError.hpp | 2 +- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/hotspot/os/windows/os_windows.cpp b/src/hotspot/os/windows/os_windows.cpp index 802054ff9a2..bb3e47050b2 100644 --- a/src/hotspot/os/windows/os_windows.cpp +++ b/src/hotspot/os/windows/os_windows.cpp @@ -505,12 +505,17 @@ struct tm* os::gmtime_pd(const time_t* clock, struct tm* res) { return nullptr; } +enum Ept { EPT_THREAD, EPT_PROCESS, EPT_PROCESS_DIE }; +// Wrapper around _endthreadex(), exit() and _exit() +[[noreturn]] +static void exit_process_or_thread(Ept what, int code); + JNIEXPORT LONG WINAPI topLevelExceptionFilter(struct _EXCEPTION_POINTERS* exceptionInfo); // Thread start routine for all newly created threads. // Called with the associated Thread* as the argument. -unsigned __stdcall os::win32::thread_native_entry(void* t) { +static unsigned __stdcall thread_native_entry(void* t) { Thread* thread = static_cast(t); thread->record_stack_base_and_size(); @@ -558,7 +563,8 @@ unsigned __stdcall os::win32::thread_native_entry(void* t) { // Thread must not return from exit_process_or_thread(), but if it does, // let it proceed to exit normally - return (unsigned)os::win32::exit_process_or_thread(os::win32::EPT_THREAD, res); + exit_process_or_thread(EPT_THREAD, res); + return res; } static OSThread* create_os_thread(Thread* thread, HANDLE thread_handle, @@ -745,7 +751,7 @@ bool os::create_thread(Thread* thread, ThreadType thr_type, thread_handle = (HANDLE)_beginthreadex(nullptr, (unsigned)stack_size, - &os::win32::thread_native_entry, + &thread_native_entry, thread, initflag, &thread_id); @@ -1202,7 +1208,7 @@ void os::abort(bool dump_core, void* siginfo, const void* context) { if (dumpFile != nullptr) { CloseHandle(dumpFile); } - win32::exit_process_or_thread(win32::EPT_PROCESS, 1); + exit_process_or_thread(EPT_PROCESS, 1); } dumpType = (MINIDUMP_TYPE)(MiniDumpWithFullMemory | MiniDumpWithHandleData | @@ -1226,12 +1232,12 @@ void os::abort(bool dump_core, void* siginfo, const void* context) { jio_fprintf(stderr, "Call to MiniDumpWriteDump() failed (Error 0x%x)\n", GetLastError()); } CloseHandle(dumpFile); - win32::exit_process_or_thread(win32::EPT_PROCESS, 1); + exit_process_or_thread(EPT_PROCESS, 1); } // Die immediately, no exit hook, no abort hook, no cleanup. void os::die() { - win32::exit_process_or_thread(win32::EPT_PROCESS_DIE, -1); + exit_process_or_thread(EPT_PROCESS_DIE, -1); } void os::dll_unload(void *lib) { @@ -4097,7 +4103,7 @@ static BOOL CALLBACK init_crit_sect_call(PINIT_ONCE, PVOID pcrit_sect, PVOID*) { return TRUE; } -int os::win32::exit_process_or_thread(Ept what, int exit_code) { +static void exit_process_or_thread(Ept what, int exit_code) { // Basic approach: // - Each exiting thread registers its intent to exit and then does so. // - A thread trying to terminate the process must wait for all @@ -4275,7 +4281,7 @@ int os::win32::exit_process_or_thread(Ept what, int exit_code) { } // Should not reach here - return exit_code; + os::infinite_sleep(); } #undef EXIT_TIMEOUT @@ -4853,11 +4859,11 @@ ssize_t os::pd_write(int fd, const void *buf, size_t nBytes) { } void os::exit(int num) { - win32::exit_process_or_thread(win32::EPT_PROCESS, num); + exit_process_or_thread(EPT_PROCESS, num); } void os::_exit(int num) { - win32::exit_process_or_thread(win32::EPT_PROCESS_DIE, num); + exit_process_or_thread(EPT_PROCESS_DIE, num); } // Is a (classpath) directory empty? diff --git a/src/hotspot/os/windows/os_windows.hpp b/src/hotspot/os/windows/os_windows.hpp index 2f90da71554..5bf402069f3 100644 --- a/src/hotspot/os/windows/os_windows.hpp +++ b/src/hotspot/os/windows/os_windows.hpp @@ -70,13 +70,6 @@ class os::win32 { static HINSTANCE load_Windows_dll(const char* name, char *ebuf, int ebuflen); private: - // The handler passed to _beginthreadex(). - // Called with the associated Thread* as the argument. - static unsigned __stdcall thread_native_entry(void*); - - enum Ept { EPT_THREAD, EPT_PROCESS, EPT_PROCESS_DIE }; - // Wrapper around _endthreadex(), exit() and _exit() - static int exit_process_or_thread(Ept what, int exit_code); static void initialize_performance_counter(); diff --git a/src/hotspot/os/windows/vmError_windows.cpp b/src/hotspot/os/windows/vmError_windows.cpp index 2443246f5e5..c149a7f4c37 100644 --- a/src/hotspot/os/windows/vmError_windows.cpp +++ b/src/hotspot/os/windows/vmError_windows.cpp @@ -71,4 +71,5 @@ void VMError::raise_fail_fast(void* exrecord, void* context) { RaiseFailFastException(static_cast(exrecord), static_cast(context), flags); + os::infinite_sleep(); } diff --git a/src/hotspot/share/utilities/vmError.hpp b/src/hotspot/share/utilities/vmError.hpp index 88ba476891e..d5d4279bb0b 100644 --- a/src/hotspot/share/utilities/vmError.hpp +++ b/src/hotspot/share/utilities/vmError.hpp @@ -144,7 +144,7 @@ class VMError : public AllStatic { static jlong get_step_start_time(); static void clear_step_start_time(); - WINDOWS_ONLY(ATTRIBUTE_NORETURN static void raise_fail_fast(void* exrecord, void* context);) + WINDOWS_ONLY([[noreturn]] static void raise_fail_fast(void* exrecord, void* context);) public: From ab1934848b2680aff86631e7a68e5ef22857742f Mon Sep 17 00:00:00 2001 From: Albert Mingkun Yang Date: Wed, 1 Nov 2023 11:50:52 +0000 Subject: [PATCH 03/12] 8318647: Serial: Refactor BlockOffsetTable Reviewed-by: tschatzl, iwalulya --- .../gc/serial/serialBlockOffsetTable.cpp | 461 ++++-------------- .../gc/serial/serialBlockOffsetTable.hpp | 357 ++------------ .../serial/serialBlockOffsetTable.inline.hpp | 27 +- .../share/gc/serial/tenuredGeneration.cpp | 7 +- .../share/gc/serial/tenuredGeneration.hpp | 4 +- .../share/gc/serial/vmStructs_serial.hpp | 64 +-- src/hotspot/share/gc/shared/space.cpp | 58 +-- src/hotspot/share/gc/shared/space.hpp | 32 +- src/hotspot/share/gc/shared/space.inline.hpp | 21 +- 9 files changed, 205 insertions(+), 826 deletions(-) diff --git a/src/hotspot/share/gc/serial/serialBlockOffsetTable.cpp b/src/hotspot/share/gc/serial/serialBlockOffsetTable.cpp index 721a8de5abc..a94cfbc44de 100644 --- a/src/hotspot/share/gc/serial/serialBlockOffsetTable.cpp +++ b/src/hotspot/share/gc/serial/serialBlockOffsetTable.cpp @@ -26,7 +26,6 @@ #include "gc/serial/serialBlockOffsetTable.inline.hpp" #include "gc/shared/blockOffsetTable.hpp" #include "gc/shared/collectedHeap.inline.hpp" -#include "gc/shared/space.inline.hpp" #include "logging/log.hpp" #include "memory/iterator.hpp" #include "memory/universe.hpp" @@ -34,14 +33,9 @@ #include "oops/oop.inline.hpp" #include "runtime/java.hpp" -////////////////////////////////////////////////////////////////////// -// BlockOffsetSharedArray -////////////////////////////////////////////////////////////////////// - -BlockOffsetSharedArray::BlockOffsetSharedArray(MemRegion reserved, - size_t init_word_size): - _reserved(reserved), _end(nullptr) -{ +SerialBlockOffsetSharedArray::SerialBlockOffsetSharedArray(MemRegion reserved, + size_t init_word_size): + _reserved(reserved) { size_t size = compute_size(reserved.word_size()); ReservedSpace rs(size); if (!rs.is_reserved()) { @@ -53,27 +47,25 @@ BlockOffsetSharedArray::BlockOffsetSharedArray(MemRegion reserved, if (!_vs.initialize(rs, 0)) { vm_exit_during_initialization("Could not reserve enough space for heap offset array"); } - _offset_array = (u_char*)_vs.low_boundary(); + _offset_array = (uint8_t*)_vs.low_boundary(); resize(init_word_size); - log_trace(gc, bot)("BlockOffsetSharedArray::BlockOffsetSharedArray: "); + log_trace(gc, bot)("SerialBlockOffsetSharedArray::SerialBlockOffsetSharedArray: "); log_trace(gc, bot)(" rs.base(): " PTR_FORMAT " rs.size(): " SIZE_FORMAT_X_0 " rs end(): " PTR_FORMAT, p2i(rs.base()), rs.size(), p2i(rs.base() + rs.size())); log_trace(gc, bot)(" _vs.low_boundary(): " PTR_FORMAT " _vs.high_boundary(): " PTR_FORMAT, p2i(_vs.low_boundary()), p2i(_vs.high_boundary())); } -void BlockOffsetSharedArray::resize(size_t new_word_size) { +void SerialBlockOffsetSharedArray::resize(size_t new_word_size) { assert(new_word_size <= _reserved.word_size(), "Resize larger than reserved"); size_t new_size = compute_size(new_word_size); size_t old_size = _vs.committed_size(); size_t delta; char* high = _vs.high(); - _end = _reserved.start() + new_word_size; if (new_size > old_size) { delta = ReservedSpace::page_align_size_up(new_size - old_size); assert(delta > 0, "just checking"); if (!_vs.expand_by(delta)) { - // Do better than this for Merlin vm_exit_out_of_memory(delta, OOM_MMAP_ERROR, "offset table expansion"); } assert(_vs.high() == high + delta, "invalid expansion"); @@ -85,384 +77,109 @@ void BlockOffsetSharedArray::resize(size_t new_word_size) { } } -bool BlockOffsetSharedArray::is_card_boundary(HeapWord* p) const { - assert(p >= _reserved.start(), "just checking"); - size_t delta = pointer_delta(p, _reserved.start()); - return (delta & right_n_bits((int)BOTConstants::log_card_size_in_words())) == (size_t)NoBits; -} - - -////////////////////////////////////////////////////////////////////// -// BlockOffsetArray -////////////////////////////////////////////////////////////////////// - -BlockOffsetArray::BlockOffsetArray(BlockOffsetSharedArray* array, - MemRegion mr, bool init_to_zero_) : - BlockOffsetTable(mr.start(), mr.end()), - _array(array) -{ - assert(_bottom <= _end, "arguments out of order"); - set_init_to_zero(init_to_zero_); - if (!init_to_zero_) { - // initialize cards to point back to mr.start() - set_remainder_to_point_to_start(mr.start() + BOTConstants::card_size_in_words(), mr.end()); - _array->set_offset_array(0, 0); // set first card to 0 - } -} - - -// The arguments follow the normal convention of denoting -// a right-open interval: [start, end) -void -BlockOffsetArray:: -set_remainder_to_point_to_start(HeapWord* start, HeapWord* end, bool reducing) { - - check_reducing_assertion(reducing); - if (start >= end) { - // The start address is equal to the end address (or to - // the right of the end address) so there are not cards - // that need to be updated.. - return; - } - - // Write the backskip value for each region. - // - // offset - // card 2nd 3rd - // | +- 1st | | - // v v v v - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +-+-+-+-+-+-+-+-+-+-+- - // |x|0|0|0|0|0|0|0|1|1|1|1|1|1| ... |1|1|1|1|2|2|2|2|2|2| ... - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +-+-+-+-+-+-+-+-+-+-+- - // 11 19 75 - // 12 - // - // offset card is the card that points to the start of an object - // x - offset value of offset card - // 1st - start of first logarithmic region - // 0 corresponds to logarithmic value N_words + 0 and 2**(3 * 0) = 1 - // 2nd - start of second logarithmic region - // 1 corresponds to logarithmic value N_words + 1 and 2**(3 * 1) = 8 - // 3rd - start of third logarithmic region - // 2 corresponds to logarithmic value N_words + 2 and 2**(3 * 2) = 64 - // - // integer below the block offset entry is an example of - // the index of the entry - // - // Given an address, - // Find the index for the address - // Find the block offset table entry - // Convert the entry to a back slide - // (e.g., with today's, offset = 0x81 => - // back slip = 2**(3*(0x81 - N_words)) = 2**3) = 8 - // Move back N (e.g., 8) entries and repeat with the - // value of the new entry - // - size_t start_card = _array->index_for(start); - size_t end_card = _array->index_for(end-1); - assert(start ==_array->address_for_index(start_card), "Precondition"); - assert(end ==_array->address_for_index(end_card)+BOTConstants::card_size_in_words(), "Precondition"); - set_remainder_to_point_to_start_incl(start_card, end_card, reducing); // closed interval -} - - -// Unlike the normal convention in this code, the argument here denotes -// a closed, inclusive interval: [start_card, end_card], cf set_remainder_to_point_to_start() -// above. -void -BlockOffsetArray::set_remainder_to_point_to_start_incl(size_t start_card, size_t end_card, bool reducing) { - - check_reducing_assertion(reducing); - if (start_card > end_card) { - return; - } - assert(start_card > _array->index_for(_bottom), "Cannot be first card"); - assert(_array->offset_array(start_card-1) <= BOTConstants::card_size_in_words(), - "Offset card has an unexpected value"); - size_t start_card_for_region = start_card; - u_char offset = max_jubyte; - for (uint i = 0; i < BOTConstants::N_powers; i++) { - // -1 so that the card with the actual offset is counted. Another -1 - // so that the reach ends in this region and not at the start - // of the next. - size_t reach = start_card - 1 + (BOTConstants::power_to_cards_back(i+1) - 1); - offset = BOTConstants::card_size_in_words() + i; - if (reach >= end_card) { - _array->set_offset_array(start_card_for_region, end_card, offset, reducing); +// Write the backskip value for each logarithmic region (array slots containing the same entry value). +// +// offset +// card 2nd 3rd +// | +- 1st | | +// v v v v +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +-+-+-+-+-+-+-+-+-+-+- +// |x|0|0|0|0|0|0|0|1|1|1|1|1|1| ... |1|1|1|1|2|2|2|2|2|2| ... +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +-+-+-+-+-+-+-+-+-+-+- +// 11 19 75 +// 12 +// +// offset card is the card that points to the start of an object +// x - offset value of offset card +// 1st - start of first logarithmic region +// 0 corresponds to logarithmic value N_words + 0 and 2**(3 * 0) = 1 +// 2nd - start of second logarithmic region +// 1 corresponds to logarithmic value N_words + 1 and 2**(3 * 1) = 8 +// 3rd - start of third logarithmic region +// 2 corresponds to logarithmic value N_words + 2 and 2**(3 * 2) = 64 +// +// integer below the block offset entry is an example of +// the index of the entry +// +// Given an address, +// Find the index for the address +// Find the block offset table entry +// Convert the entry to a back slide +// (e.g., with today's, offset = 0x81 => +// back slip = 2**(3*(0x81 - N_words)) = 2**3) = 8 +// Move back N (e.g., 8) entries and repeat with the +// value of the new entry +// +void SerialBlockOffsetTable::update_for_block_work(HeapWord* blk_start, + HeapWord* blk_end) { + HeapWord* const cur_card_boundary = align_up_by_card_size(blk_start); + size_t const offset_card = _array->index_for(cur_card_boundary); + + // The first card holds the actual offset. + _array->set_offset_array(offset_card, cur_card_boundary, blk_start); + + // Check if this block spans over other cards. + size_t end_card = _array->index_for(blk_end - 1); + assert(offset_card <= end_card, "inv"); + + if (offset_card != end_card) { + // Handling remaining cards. + size_t start_card_for_region = offset_card + 1; + for (uint i = 0; i < BOTConstants::N_powers; i++) { + // -1 so that the reach ends in this region and not at the start + // of the next. + size_t reach = offset_card + BOTConstants::power_to_cards_back(i + 1) - 1; + uint8_t value = checked_cast(BOTConstants::card_size_in_words() + i); + + _array->set_offset_array(start_card_for_region, MIN2(reach, end_card), value); start_card_for_region = reach + 1; - break; - } - _array->set_offset_array(start_card_for_region, reach, offset, reducing); - start_card_for_region = reach + 1; - } - assert(start_card_for_region > end_card, "Sanity check"); - DEBUG_ONLY(check_all_cards(start_card, end_card);) -} - -// The card-interval [start_card, end_card] is a closed interval; this -// is an expensive check -- use with care and only under protection of -// suitable flag. -void BlockOffsetArray::check_all_cards(size_t start_card, size_t end_card) const { - - if (end_card < start_card) { - return; - } - guarantee(_array->offset_array(start_card) == BOTConstants::card_size_in_words(), "Wrong value in second card"); - u_char last_entry = BOTConstants::card_size_in_words(); - for (size_t c = start_card + 1; c <= end_card; c++ /* yeah! */) { - u_char entry = _array->offset_array(c); - guarantee(entry >= last_entry, "Monotonicity"); - if (c - start_card > BOTConstants::power_to_cards_back(1)) { - guarantee(entry > BOTConstants::card_size_in_words(), "Should be in logarithmic region"); - } - size_t backskip = BOTConstants::entry_to_cards_back(entry); - size_t landing_card = c - backskip; - guarantee(landing_card >= (start_card - 1), "Inv"); - if (landing_card >= start_card) { - guarantee(_array->offset_array(landing_card) <= entry, "Monotonicity"); - } else { - guarantee(landing_card == (start_card - 1), "Tautology"); - // Note that N_words is the maximum offset value - guarantee(_array->offset_array(landing_card) <= BOTConstants::card_size_in_words(), "Offset value"); - } - last_entry = entry; // remember for monotonicity test - } -} - - -void -BlockOffsetArray::alloc_block(HeapWord* blk_start, HeapWord* blk_end) { - assert(blk_start != nullptr && blk_end > blk_start, - "phantom block"); - single_block(blk_start, blk_end); -} -void -BlockOffsetArray::do_block_internal(HeapWord* blk_start, - HeapWord* blk_end, - bool reducing) { - assert(_sp->is_in_reserved(blk_start), - "reference must be into the space"); - assert(_sp->is_in_reserved(blk_end-1), - "limit must be within the space"); - // This is optimized to make the test fast, assuming we only rarely - // cross boundaries. - uintptr_t end_ui = (uintptr_t)(blk_end - 1); - uintptr_t start_ui = (uintptr_t)blk_start; - // Calculate the last card boundary preceding end of blk - intptr_t boundary_before_end = (intptr_t)end_ui; - clear_bits(boundary_before_end, right_n_bits((int)BOTConstants::log_card_size())); - if (start_ui <= (uintptr_t)boundary_before_end) { - // blk starts at or crosses a boundary - // Calculate index of card on which blk begins - size_t start_index = _array->index_for(blk_start); - // Index of card on which blk ends - size_t end_index = _array->index_for(blk_end - 1); - // Start address of card on which blk begins - HeapWord* boundary = _array->address_for_index(start_index); - assert(boundary <= blk_start, "blk should start at or after boundary"); - if (blk_start != boundary) { - // blk starts strictly after boundary - // adjust card boundary and start_index forward to next card - boundary += BOTConstants::card_size_in_words(); - start_index++; - } - assert(start_index <= end_index, "monotonicity of index_for()"); - assert(boundary <= (HeapWord*)boundary_before_end, "tautology"); - _array->set_offset_array(start_index, boundary, blk_start, reducing); - // We have finished marking the "offset card". We need to now - // mark the subsequent cards that this blk spans. - if (start_index < end_index) { - HeapWord* rem_st = _array->address_for_index(start_index) + BOTConstants::card_size_in_words(); - HeapWord* rem_end = _array->address_for_index(end_index) + BOTConstants::card_size_in_words(); - set_remainder_to_point_to_start(rem_st, rem_end, reducing); + if (reach >= end_card) { + break; + } } + assert(start_card_for_region > end_card, "Sanity check"); } -} -// The range [blk_start, blk_end) represents a single contiguous block -// of storage; modify the block offset table to represent this -// information; Right-open interval: [blk_start, blk_end) -// NOTE: this method does _not_ adjust _unallocated_block. -void -BlockOffsetArray::single_block(HeapWord* blk_start, - HeapWord* blk_end) { - do_block_internal(blk_start, blk_end); + debug_only(verify_for_block(blk_start, blk_end);) } -void BlockOffsetArray::verify() const { - // For each entry in the block offset table, verify that - // the entry correctly finds the start of an object at the - // first address covered by the block or to the left of that - // first address. - - size_t next_index = 1; - size_t last_index = last_active_index(); +HeapWord* SerialBlockOffsetTable::block_start_reaching_into_card(const void* addr) const { + size_t index = _array->index_for(addr); - // Use for debugging. Initialize to null to distinguish the - // first iteration through the while loop. - HeapWord* last_p = nullptr; - HeapWord* last_start = nullptr; - oop last_o = nullptr; + uint8_t offset; + while (true) { + offset = _array->offset_array(index); - while (next_index <= last_index) { - // Use an address past the start of the address for - // the entry. - HeapWord* p = _array->address_for_index(next_index) + 1; - if (p >= _end) { - // That's all of the allocated block table. - return; + if (offset < BOTConstants::card_size_in_words()) { + break; } - // block_start() asserts that start <= p. - HeapWord* start = block_start(p); - // First check if the start is an allocated block and only - // then if it is a valid object. - oop o = cast_to_oop(start); - assert(!Universe::is_fully_initialized() || - _sp->is_free_block(start) || - oopDesc::is_oop_or_null(o), "Bad object was found"); - next_index++; - last_p = p; - last_start = start; - last_o = o; - } -} -////////////////////////////////////////////////////////////////////// -// BlockOffsetArrayContigSpace -////////////////////////////////////////////////////////////////////// - -HeapWord* BlockOffsetArrayContigSpace::block_start_unsafe(const void* addr) const { - assert(_array->offset_array(0) == 0, "objects can't cross covered areas"); - - // Otherwise, find the block start using the table. - assert(_bottom <= addr && addr < _end, - "addr must be covered by this Array"); - size_t index = _array->index_for(addr); - // We must make sure that the offset table entry we use is valid. If - // "addr" is past the end, start at the last known one and go forward. - index = MIN2(index, _next_offset_index-1); - HeapWord* q = _array->address_for_index(index); - - uint offset = _array->offset_array(index); // Extend u_char to uint. - while (offset > BOTConstants::card_size_in_words()) { // The excess of the offset from N_words indicates a power of Base // to go back by. size_t n_cards_back = BOTConstants::entry_to_cards_back(offset); - q -= (BOTConstants::card_size_in_words() * n_cards_back); - assert(q >= _sp->bottom(), "Went below bottom!"); index -= n_cards_back; - offset = _array->offset_array(index); - } - while (offset == BOTConstants::card_size_in_words()) { - assert(q >= _sp->bottom(), "Went below bottom!"); - q -= BOTConstants::card_size_in_words(); - index--; - offset = _array->offset_array(index); } - assert(offset < BOTConstants::card_size_in_words(), "offset too large"); - q -= offset; - HeapWord* n = q; - while (n <= addr) { - debug_only(HeapWord* last = q); // for debugging - q = n; - n += _sp->block_size(n); - } - assert(q <= addr, "wrong order for current and arg"); - assert(addr <= n, "wrong order for arg and next"); - return q; + HeapWord* q = _array->address_for_index(index); + return q - offset; } -// -// _next_offset_threshold -// | _next_offset_index -// v v -// +-------+-------+-------+-------+-------+ -// | i-1 | i | i+1 | i+2 | i+3 | -// +-------+-------+-------+-------+-------+ -// ( ^ ] -// block-start -// - -void BlockOffsetArrayContigSpace::alloc_block_work(HeapWord* blk_start, - HeapWord* blk_end) { - assert(blk_start != nullptr && blk_end > blk_start, - "phantom block"); - assert(blk_end > _next_offset_threshold, - "should be past threshold"); - assert(blk_start <= _next_offset_threshold, - "blk_start should be at or before threshold"); - assert(pointer_delta(_next_offset_threshold, blk_start) <= BOTConstants::card_size_in_words(), - "offset should be <= BlockOffsetSharedArray::N"); - assert(_sp->is_in_reserved(blk_start), - "reference must be into the space"); - assert(_sp->is_in_reserved(blk_end-1), - "limit must be within the space"); - assert(_next_offset_threshold == - _array->_reserved.start() + _next_offset_index*BOTConstants::card_size_in_words(), - "index must agree with threshold"); - - debug_only(size_t orig_next_offset_index = _next_offset_index;) +void SerialBlockOffsetTable::verify_for_block(HeapWord* blk_start, HeapWord* blk_end) const { + assert(is_crossing_card_boundary(blk_start, blk_end), "precondition"); - // Mark the card that holds the offset into the block. Note - // that _next_offset_index and _next_offset_threshold are not - // updated until the end of this method. - _array->set_offset_array(_next_offset_index, - _next_offset_threshold, - blk_start); - - // We need to now mark the subsequent cards that this blk spans. - - // Index of card on which blk ends. - size_t end_index = _array->index_for(blk_end - 1); - - // Are there more cards left to be updated? - if (_next_offset_index + 1 <= end_index) { - HeapWord* rem_st = _array->address_for_index(_next_offset_index + 1); - // Calculate rem_end this way because end_index - // may be the last valid index in the covered region. - HeapWord* rem_end = _array->address_for_index(end_index) + BOTConstants::card_size_in_words(); - set_remainder_to_point_to_start(rem_st, rem_end); - } + const size_t start_card = _array->index_for(align_up_by_card_size(blk_start)); + const size_t end_card = _array->index_for(blk_end - 1); + // Check cards in [start_card, end_card] + assert(_array->offset_array(start_card) < BOTConstants::card_size_in_words(), "offset card"); - // _next_offset_index and _next_offset_threshold updated here. - _next_offset_index = end_index + 1; - // Calculate _next_offset_threshold this way because end_index - // may be the last valid index in the covered region. - _next_offset_threshold = _array->address_for_index(end_index) + BOTConstants::card_size_in_words(); - assert(_next_offset_threshold >= blk_end, "Incorrect offset threshold"); - -#ifdef ASSERT - // The offset can be 0 if the block starts on a boundary. That - // is checked by an assertion above. - size_t start_index = _array->index_for(blk_start); - HeapWord* boundary = _array->address_for_index(start_index); - assert((_array->offset_array(orig_next_offset_index) == 0 && - blk_start == boundary) || - (_array->offset_array(orig_next_offset_index) > 0 && - _array->offset_array(orig_next_offset_index) <= BOTConstants::card_size_in_words()), - "offset array should have been set"); - for (size_t j = orig_next_offset_index + 1; j <= end_index; j++) { - assert(_array->offset_array(j) > 0 && - _array->offset_array(j) <= (u_char) (BOTConstants::card_size_in_words()+BOTConstants::N_powers-1), - "offset array should have been set"); + for (size_t i = start_card + 1; i <= end_card; ++i) { + const uint8_t prev = _array->offset_array(i-1); + const uint8_t value = _array->offset_array(i); + if (prev != value) { + assert(value >= prev, "monotonic"); + size_t n_cards_back = BOTConstants::entry_to_cards_back(value); + assert(start_card == (i - n_cards_back), "inv"); + } } -#endif -} - -void BlockOffsetArrayContigSpace::initialize_threshold() { - _next_offset_index = _array->index_for(_bottom); - _next_offset_index++; - _next_offset_threshold = - _array->address_for_index(_next_offset_index); -} - -void BlockOffsetArrayContigSpace::zero_bottom_entry() { - size_t bottom_index = _array->index_for(_bottom); - _array->set_offset_array(bottom_index, 0); -} - -size_t BlockOffsetArrayContigSpace::last_active_index() const { - return _next_offset_index == 0 ? 0 : _next_offset_index - 1; } diff --git a/src/hotspot/share/gc/serial/serialBlockOffsetTable.hpp b/src/hotspot/share/gc/serial/serialBlockOffsetTable.hpp index 6b2215cf945..3a416c29fc0 100644 --- a/src/hotspot/share/gc/serial/serialBlockOffsetTable.hpp +++ b/src/hotspot/share/gc/serial/serialBlockOffsetTable.hpp @@ -36,168 +36,34 @@ #include "utilities/globalDefinitions.hpp" #include "utilities/macros.hpp" -// The CollectedHeap type requires subtypes to implement a method -// "block_start". For some subtypes, notably generational -// systems using card-table-based write barriers, the efficiency of this -// operation may be important. Implementations of the "BlockOffsetArray" -// class may be useful in providing such efficient implementations. -// -// BlockOffsetTable (abstract) -// - BlockOffsetArray (abstract) -// - BlockOffsetArrayContigSpace -// - -class ContiguousSpace; - -////////////////////////////////////////////////////////////////////////// -// The BlockOffsetTable "interface" -////////////////////////////////////////////////////////////////////////// -class BlockOffsetTable { - friend class VMStructs; -protected: - // These members describe the region covered by the table. - - // The space this table is covering. - HeapWord* _bottom; // == reserved.start - HeapWord* _end; // End of currently allocated region. - -public: - // Initialize the table to cover the given space. - // The contents of the initial table are undefined. - BlockOffsetTable(HeapWord* bottom, HeapWord* end): - _bottom(bottom), _end(end) { - assert(_bottom <= _end, "arguments out of order"); - assert(BOTConstants::card_size() == CardTable::card_size(), "sanity"); - } - - // Note that the committed size of the covered space may have changed, - // so the table size might also wish to change. - virtual void resize(size_t new_word_size) = 0; - - virtual void set_bottom(HeapWord* new_bottom) { - assert(new_bottom <= _end, "new_bottom > _end"); - _bottom = new_bottom; - resize(pointer_delta(_end, _bottom)); - } - - // Requires "addr" to be contained by a block, and returns the address of - // the start of that block. - virtual HeapWord* block_start_unsafe(const void* addr) const = 0; - - // Returns the address of the start of the block containing "addr", or - // else "null" if it is covered by no block. - HeapWord* block_start(const void* addr) const; -}; - -////////////////////////////////////////////////////////////////////////// -// One implementation of "BlockOffsetTable," the BlockOffsetArray, -// divides the covered region into "N"-word subregions (where -// "N" = 2^"LogN". An array with an entry for each such subregion -// indicates how far back one must go to find the start of the -// chunk that includes the first word of the subregion. -// -// Each BlockOffsetArray is owned by a Space. However, the actual array -// may be shared by several BlockOffsetArrays; this is useful -// when a single resizable area (such as a generation) is divided up into -// several spaces in which contiguous allocation takes place. (Consider, -// for example, the garbage-first generation.) - -// Here is the shared array type. -////////////////////////////////////////////////////////////////////////// -// BlockOffsetSharedArray -////////////////////////////////////////////////////////////////////////// -class BlockOffsetSharedArray: public CHeapObj { - friend class BlockOffsetArray; - friend class BlockOffsetArrayNonContigSpace; - friend class BlockOffsetArrayContigSpace; +class SerialBlockOffsetSharedArray: public CHeapObj { friend class VMStructs; + friend class SerialBlockOffsetTable; - private: - bool _init_to_zero; - - // The reserved region covered by the shared array. + // The reserved heap (i.e. old-gen) covered by the shared array. MemRegion _reserved; - // End of the current committed region. - HeapWord* _end; - // Array for keeping offsets for retrieving object start fast given an // address. VirtualSpace _vs; - u_char* _offset_array; // byte array keeping backwards offsets + uint8_t* _offset_array; // byte array keeping backwards offsets - void fill_range(size_t start, size_t num_cards, u_char offset) { + void fill_range(size_t start, size_t num_cards, uint8_t offset) { void* start_ptr = &_offset_array[start]; - // If collector is concurrent, special handling may be needed. - G1GC_ONLY(assert(!UseG1GC, "Shouldn't be here when using G1");) memset(start_ptr, offset, num_cards); } - protected: - // Bounds checking accessors: - // For performance these have to devolve to array accesses in product builds. - u_char offset_array(size_t index) const { + uint8_t offset_array(size_t index) const { assert(index < _vs.committed_size(), "index out of range"); return _offset_array[index]; } - // An assertion-checking helper method for the set_offset_array() methods below. - void check_reducing_assertion(bool reducing); - - void set_offset_array(size_t index, u_char offset, bool reducing = false) { - check_reducing_assertion(reducing); - assert(index < _vs.committed_size(), "index out of range"); - assert(!reducing || _offset_array[index] >= offset, "Not reducing"); - _offset_array[index] = offset; - } - - void set_offset_array(size_t index, HeapWord* high, HeapWord* low, bool reducing = false) { - check_reducing_assertion(reducing); - assert(index < _vs.committed_size(), "index out of range"); - assert(high >= low, "addresses out of order"); - assert(pointer_delta(high, low) <= BOTConstants::card_size_in_words(), "offset too large"); - assert(!reducing || _offset_array[index] >= (u_char)pointer_delta(high, low), - "Not reducing"); - _offset_array[index] = (u_char)pointer_delta(high, low); - } - - void set_offset_array(HeapWord* left, HeapWord* right, u_char offset, bool reducing = false) { - check_reducing_assertion(reducing); - assert(index_for(right - 1) < _vs.committed_size(), - "right address out of range"); - assert(left < right, "Heap addresses out of order"); - size_t num_cards = pointer_delta(right, left) >> BOTConstants::log_card_size_in_words(); - - fill_range(index_for(left), num_cards, offset); - } - - void set_offset_array(size_t left, size_t right, u_char offset, bool reducing = false) { - check_reducing_assertion(reducing); - assert(right < _vs.committed_size(), "right address out of range"); - assert(left <= right, "indexes out of order"); - size_t num_cards = right - left + 1; - - fill_range(left, num_cards, offset); - } - - void check_offset_array(size_t index, HeapWord* high, HeapWord* low) const { - assert(index < _vs.committed_size(), "index out of range"); - assert(high >= low, "addresses out of order"); - assert(pointer_delta(high, low) <= BOTConstants::card_size_in_words(), "offset too large"); - assert(_offset_array[index] == pointer_delta(high, low), - "Wrong offset"); - } - - bool is_card_boundary(HeapWord* p) const; // Return the number of slots needed for an offset array // that covers mem_region_words words. - // We always add an extra slot because if an object - // ends on a card boundary we put a 0 in the next - // offset array slot, so we want that slot always - // to be reserved. + static size_t compute_size(size_t mem_region_words) { + assert(mem_region_words % BOTConstants::card_size_in_words() == 0, "precondition"); - size_t compute_size(size_t mem_region_words) { - size_t number_of_slots = (mem_region_words / BOTConstants::card_size_in_words()) + 1; + size_t number_of_slots = mem_region_words / BOTConstants::card_size_in_words(); return ReservedSpace::allocation_align_size_up(number_of_slots); } @@ -207,198 +73,79 @@ class BlockOffsetSharedArray: public CHeapObj { // (see "resize" below) up to the size of "_reserved" (which must be at // least "init_word_size".) The contents of the initial table are // undefined; it is the responsibility of the constituent - // BlockOffsetTable(s) to initialize cards. - BlockOffsetSharedArray(MemRegion reserved, size_t init_word_size); + // SerialBlockOffsetTable(s) to initialize cards. + SerialBlockOffsetSharedArray(MemRegion reserved, size_t init_word_size); // Notes a change in the committed size of the region covered by the // table. The "new_word_size" may not be larger than the size of the // reserved region this table covers. void resize(size_t new_word_size); - void set_bottom(HeapWord* new_bottom); - - // Whether entries should be initialized to zero. Used currently only for - // error checking. - void set_init_to_zero(bool val) { _init_to_zero = val; } - bool init_to_zero() { return _init_to_zero; } - - // Updates all the BlockOffsetArray's sharing this shared array to - // reflect the current "top"'s of their spaces. - void update_offset_arrays(); // Not yet implemented! - // Return the appropriate index into "_offset_array" for "p". size_t index_for(const void* p) const; // Return the address indicating the start of the region corresponding to // "index" in "_offset_array". HeapWord* address_for_index(size_t index) const; -}; - -class Space; - -////////////////////////////////////////////////////////////////////////// -// The BlockOffsetArray whose subtypes use the BlockOffsetSharedArray. -////////////////////////////////////////////////////////////////////////// -class BlockOffsetArray: public BlockOffsetTable { - friend class VMStructs; - protected: - // The shared array, which is shared with other BlockOffsetArray's - // corresponding to different spaces within a generation or span of - // memory. - BlockOffsetSharedArray* _array; - - // The space that owns this subregion. - Space* _sp; - // If true, array entries are initialized to 0; otherwise, they are - // initialized to point backwards to the beginning of the covered region. - bool _init_to_zero; - - // An assertion-checking helper method for the set_remainder*() methods below. - void check_reducing_assertion(bool reducing) { _array->check_reducing_assertion(reducing); } - - // Sets the entries - // corresponding to the cards starting at "start" and ending at "end" - // to point back to the card before "start": the interval [start, end) - // is right-open. The last parameter, reducing, indicates whether the - // updates to individual entries always reduce the entry from a higher - // to a lower value. (For example this would hold true during a temporal - // regime during which only block splits were updating the BOT. - void set_remainder_to_point_to_start(HeapWord* start, HeapWord* end, bool reducing = false); - // Same as above, except that the args here are a card _index_ interval - // that is closed: [start_index, end_index] - void set_remainder_to_point_to_start_incl(size_t start, size_t end, bool reducing = false); - - // A helper function for BOT adjustment/verification work - void do_block_internal(HeapWord* blk_start, HeapWord* blk_end, bool reducing = false); - - public: - // The space may not have its bottom and top set yet, which is why the - // region is passed as a parameter. If "init_to_zero" is true, the - // elements of the array are initialized to zero. Otherwise, they are - // initialized to point backwards to the beginning. - BlockOffsetArray(BlockOffsetSharedArray* array, MemRegion mr, - bool init_to_zero_); - - // Note: this ought to be part of the constructor, but that would require - // "this" to be passed as a parameter to a member constructor for - // the containing concrete subtype of Space. - // This would be legal C++, but MS VC++ doesn't allow it. - void set_space(Space* sp) { _sp = sp; } - - // Resets the covered region to the given "mr". - void set_region(MemRegion mr) { - _bottom = mr.start(); - _end = mr.end(); - } - - // Note that the committed size of the covered space may have changed, - // so the table size might also wish to change. - virtual void resize(size_t new_word_size) { - HeapWord* new_end = _bottom + new_word_size; - if (_end < new_end && !init_to_zero()) { - // verify that the old and new boundaries are also card boundaries - assert(_array->is_card_boundary(_end), - "_end not a card boundary"); - assert(_array->is_card_boundary(new_end), - "new _end would not be a card boundary"); - // set all the newly added cards - _array->set_offset_array(_end, new_end, BOTConstants::card_size_in_words()); - } - _end = new_end; // update _end - } - - // Adjust the BOT to show that it has a single block in the - // range [blk_start, blk_start + size). All necessary BOT - // cards are adjusted, but _unallocated_block isn't. - void single_block(HeapWord* blk_start, HeapWord* blk_end); - void single_block(HeapWord* blk, size_t size) { - single_block(blk, blk + size); + void set_offset_array(size_t index, HeapWord* high, HeapWord* low) { + assert(index < _vs.committed_size(), "index out of range"); + assert(high >= low, "addresses out of order"); + assert(pointer_delta(high, low) < BOTConstants::card_size_in_words(), "offset too large"); + _offset_array[index] = checked_cast(pointer_delta(high, low)); } - // When the alloc_block() call returns, the block offset table should - // have enough information such that any subsequent block_start() call - // with an argument equal to an address that is within the range - // [blk_start, blk_end) would return the value blk_start, provided - // there have been no calls in between that reset this information - // (e.g. see BlockOffsetArrayNonContigSpace::single_block() call - // for an appropriate range covering the said interval). - // These methods expect to be called with [blk_start, blk_end) - // representing a block of memory in the heap. - virtual void alloc_block(HeapWord* blk_start, HeapWord* blk_end); - void alloc_block(HeapWord* blk, size_t size) { - alloc_block(blk, blk + size); - } + void set_offset_array(size_t left, size_t right, uint8_t offset) { + assert(right < _vs.committed_size(), "right address out of range"); + assert(left <= right, "precondition"); + size_t num_cards = right - left + 1; - // If true, initialize array slots with no allocated blocks to zero. - // Otherwise, make them point back to the front. - bool init_to_zero() { return _init_to_zero; } - // Corresponding setter - void set_init_to_zero(bool val) { - _init_to_zero = val; - assert(_array != nullptr, "_array should be non-null"); - _array->set_init_to_zero(val); + fill_range(left, num_cards, offset); } - - // Debugging - // Return the index of the last entry in the "active" region. - virtual size_t last_active_index() const = 0; - // Verify the block offset table - void verify() const; - void check_all_cards(size_t left_card, size_t right_card) const; }; -//////////////////////////////////////////////////////////////////////////// -// A subtype of BlockOffsetArray that takes advantage of the fact -// that its underlying space is a ContiguousSpace, so that its "active" -// region can be more efficiently tracked (than for a non-contiguous space). -//////////////////////////////////////////////////////////////////////////// -class BlockOffsetArrayContigSpace: public BlockOffsetArray { +// SerialBlockOffsetTable divides the covered region into "N"-word subregions (where +// "N" = 2^"LogN". An array with an entry for each such subregion indicates +// how far back one must go to find the start of the chunk that includes the +// first word of the subregion. +class SerialBlockOffsetTable { friend class VMStructs; - private: - // allocation boundary at which offset array must be updated - HeapWord* _next_offset_threshold; - size_t _next_offset_index; // index corresponding to that boundary - // Work function when allocation start crosses threshold. - void alloc_block_work(HeapWord* blk_start, HeapWord* blk_end); + // The array that contains offset values. Its reacts to heap resizing. + SerialBlockOffsetSharedArray* _array; - public: - BlockOffsetArrayContigSpace(BlockOffsetSharedArray* array, MemRegion mr): - BlockOffsetArray(array, mr, true) { - _next_offset_threshold = nullptr; - _next_offset_index = 0; + void update_for_block_work(HeapWord* blk_start, HeapWord* blk_end); + + static HeapWord* align_up_by_card_size(HeapWord* const addr) { + return align_up(addr, BOTConstants::card_size()); } - void set_contig_space(ContiguousSpace* sp) { set_space((Space*)sp); } + void verify_for_block(HeapWord* blk_start, HeapWord* blk_end) const; - // Initialize the threshold for an empty heap. - void initialize_threshold(); - // Zero out the entry for _bottom (offset will be zero) - void zero_bottom_entry(); +public: + // Initialize the table to cover the given space. + // The contents of the initial table are undefined. + SerialBlockOffsetTable(SerialBlockOffsetSharedArray* array) : _array(array) { + assert(BOTConstants::card_size() == CardTable::card_size(), "sanity"); + } - // Return the next threshold, the point at which the table should be - // updated. - HeapWord* threshold() const { return _next_offset_threshold; } + static bool is_crossing_card_boundary(HeapWord* const obj_start, + HeapWord* const obj_end) { + HeapWord* cur_card_boundary = align_up_by_card_size(obj_start); + // Strictly greater-than, since we check if this block *crosses* card boundary. + return obj_end > cur_card_boundary; + } + + // Returns the address of the start of the block reaching into the card containing + // "addr". + HeapWord* block_start_reaching_into_card(const void* addr) const; - // In general, these methods expect to be called with // [blk_start, blk_end) representing a block of memory in the heap. - // In this implementation, however, we are OK even if blk_start and/or - // blk_end are null because null is represented as 0, and thus - // never exceeds the "_next_offset_threshold". - void alloc_block(HeapWord* blk_start, HeapWord* blk_end) { - if (blk_end > _next_offset_threshold) { - alloc_block_work(blk_start, blk_end); + void update_for_block(HeapWord* blk_start, HeapWord* blk_end) { + if (is_crossing_card_boundary(blk_start, blk_end)) { + update_for_block_work(blk_start, blk_end); } } - void alloc_block(HeapWord* blk, size_t size) { - alloc_block(blk, blk + size); - } - - HeapWord* block_start_unsafe(const void* addr) const; - - // Debugging support - virtual size_t last_active_index() const; }; #endif // SHARE_GC_SERIAL_SERIALBLOCKOFFSETTABLE_HPP diff --git a/src/hotspot/share/gc/serial/serialBlockOffsetTable.inline.hpp b/src/hotspot/share/gc/serial/serialBlockOffsetTable.inline.hpp index c635e8cc19e..81c384cfc65 100644 --- a/src/hotspot/share/gc/serial/serialBlockOffsetTable.inline.hpp +++ b/src/hotspot/share/gc/serial/serialBlockOffsetTable.inline.hpp @@ -27,24 +27,7 @@ #include "gc/serial/serialBlockOffsetTable.hpp" -#include "gc/shared/space.hpp" -#include "runtime/safepoint.hpp" - -////////////////////////////////////////////////////////////////////////// -// BlockOffsetTable inlines -////////////////////////////////////////////////////////////////////////// -inline HeapWord* BlockOffsetTable::block_start(const void* addr) const { - if (addr >= _bottom && addr < _end) { - return block_start_unsafe(addr); - } else { - return nullptr; - } -} - -////////////////////////////////////////////////////////////////////////// -// BlockOffsetSharedArray inlines -////////////////////////////////////////////////////////////////////////// -inline size_t BlockOffsetSharedArray::index_for(const void* p) const { +inline size_t SerialBlockOffsetSharedArray::index_for(const void* p) const { char* pc = (char*)p; assert(pc >= (char*)_reserved.start() && pc < (char*)_reserved.end(), @@ -55,7 +38,7 @@ inline size_t BlockOffsetSharedArray::index_for(const void* p) const { return result; } -inline HeapWord* BlockOffsetSharedArray::address_for_index(size_t index) const { +inline HeapWord* SerialBlockOffsetSharedArray::address_for_index(size_t index) const { assert(index < _vs.committed_size(), "bad index"); HeapWord* result = _reserved.start() + (index << BOTConstants::log_card_size_in_words()); assert(result >= _reserved.start() && result < _reserved.end(), @@ -63,10 +46,4 @@ inline HeapWord* BlockOffsetSharedArray::address_for_index(size_t index) const { return result; } -inline void BlockOffsetSharedArray::check_reducing_assertion(bool reducing) { - assert(reducing || !SafepointSynchronize::is_at_safepoint() || init_to_zero() || - Thread::current()->is_VM_thread() || - Thread::current()->is_ConcurrentGC_thread(), "Crack"); -} - #endif // SHARE_GC_SERIAL_SERIALBLOCKOFFSETTABLE_INLINE_HPP diff --git a/src/hotspot/share/gc/serial/tenuredGeneration.cpp b/src/hotspot/share/gc/serial/tenuredGeneration.cpp index 28bb00fe2a3..f42973691f3 100644 --- a/src/hotspot/share/gc/serial/tenuredGeneration.cpp +++ b/src/hotspot/share/gc/serial/tenuredGeneration.cpp @@ -295,8 +295,8 @@ TenuredGeneration::TenuredGeneration(ReservedSpace rs, assert((uintptr_t(start) & 3) == 0, "bad alignment"); assert((reserved_byte_size & 3) == 0, "bad alignment"); MemRegion reserved_mr(start, heap_word_size(reserved_byte_size)); - _bts = new BlockOffsetSharedArray(reserved_mr, - heap_word_size(initial_byte_size)); + _bts = new SerialBlockOffsetSharedArray(reserved_mr, + heap_word_size(initial_byte_size)); MemRegion committed_mr(start, heap_word_size(initial_byte_size)); _rs->resize_covered_region(committed_mr); @@ -474,11 +474,10 @@ void TenuredGeneration::object_iterate(ObjectClosure* blk) { void TenuredGeneration::complete_loaded_archive_space(MemRegion archive_space) { // Create the BOT for the archive space. TenuredSpace* space = _the_space; - space->initialize_threshold(); HeapWord* start = archive_space.start(); while (start < archive_space.end()) { size_t word_size = cast_to_oop(start)->size();; - space->alloc_block(start, start + word_size); + space->update_for_block(start, start + word_size); start += word_size; } } diff --git a/src/hotspot/share/gc/serial/tenuredGeneration.hpp b/src/hotspot/share/gc/serial/tenuredGeneration.hpp index 5a0e296e019..75e8bf84860 100644 --- a/src/hotspot/share/gc/serial/tenuredGeneration.hpp +++ b/src/hotspot/share/gc/serial/tenuredGeneration.hpp @@ -31,7 +31,7 @@ #include "gc/shared/generationCounters.hpp" #include "utilities/macros.hpp" -class BlockOffsetSharedArray; +class SerialBlockOffsetSharedArray; class CardTableRS; class ContiguousSpace; @@ -50,7 +50,7 @@ class TenuredGeneration: public Generation { // This is shared with other generations. CardTableRS* _rs; // This is local to this generation. - BlockOffsetSharedArray* _bts; + SerialBlockOffsetSharedArray* _bts; // Current shrinking effect: this damps shrinking when the heap gets empty. size_t _shrink_factor; diff --git a/src/hotspot/share/gc/serial/vmStructs_serial.hpp b/src/hotspot/share/gc/serial/vmStructs_serial.hpp index c1879b0856f..c99b90aa7d6 100644 --- a/src/hotspot/share/gc/serial/vmStructs_serial.hpp +++ b/src/hotspot/share/gc/serial/vmStructs_serial.hpp @@ -29,38 +29,31 @@ #include "gc/serial/serialHeap.hpp" #include "gc/serial/tenuredGeneration.hpp" -#define VM_STRUCTS_SERIALGC(nonstatic_field, \ - volatile_nonstatic_field, \ - static_field) \ - nonstatic_field(TenuredGeneration, _rs, CardTableRS*) \ - nonstatic_field(TenuredGeneration, _bts, BlockOffsetSharedArray*) \ - nonstatic_field(TenuredGeneration, _shrink_factor, size_t) \ - nonstatic_field(TenuredGeneration, _capacity_at_prologue, size_t) \ - nonstatic_field(TenuredGeneration, _used_at_prologue, size_t) \ - nonstatic_field(TenuredGeneration, _min_heap_delta_bytes, size_t) \ - nonstatic_field(TenuredGeneration, _the_space, TenuredSpace*) \ - \ - nonstatic_field(DefNewGeneration, _old_gen, Generation*) \ - nonstatic_field(DefNewGeneration, _tenuring_threshold, uint) \ - nonstatic_field(DefNewGeneration, _age_table, AgeTable) \ - nonstatic_field(DefNewGeneration, _eden_space, ContiguousSpace*) \ - nonstatic_field(DefNewGeneration, _from_space, ContiguousSpace*) \ - nonstatic_field(DefNewGeneration, _to_space, ContiguousSpace*) \ - \ - nonstatic_field(BlockOffsetTable, _bottom, HeapWord*) \ - nonstatic_field(BlockOffsetTable, _end, HeapWord*) \ - \ - nonstatic_field(BlockOffsetSharedArray, _reserved, MemRegion) \ - nonstatic_field(BlockOffsetSharedArray, _end, HeapWord*) \ - nonstatic_field(BlockOffsetSharedArray, _vs, VirtualSpace) \ - nonstatic_field(BlockOffsetSharedArray, _offset_array, u_char*) \ - \ - nonstatic_field(BlockOffsetArray, _array, BlockOffsetSharedArray*) \ - nonstatic_field(BlockOffsetArray, _sp, Space*) \ - nonstatic_field(BlockOffsetArrayContigSpace, _next_offset_threshold, HeapWord*) \ - nonstatic_field(BlockOffsetArrayContigSpace, _next_offset_index, size_t) \ - \ - nonstatic_field(TenuredSpace, _offsets, BlockOffsetArray) +#define VM_STRUCTS_SERIALGC(nonstatic_field, \ + volatile_nonstatic_field, \ + static_field) \ + nonstatic_field(TenuredGeneration, _rs, CardTableRS*) \ + nonstatic_field(TenuredGeneration, _bts, SerialBlockOffsetSharedArray*) \ + nonstatic_field(TenuredGeneration, _shrink_factor, size_t) \ + nonstatic_field(TenuredGeneration, _capacity_at_prologue, size_t) \ + nonstatic_field(TenuredGeneration, _used_at_prologue, size_t) \ + nonstatic_field(TenuredGeneration, _min_heap_delta_bytes, size_t) \ + nonstatic_field(TenuredGeneration, _the_space, TenuredSpace*) \ + \ + nonstatic_field(DefNewGeneration, _old_gen, Generation*) \ + nonstatic_field(DefNewGeneration, _tenuring_threshold, uint) \ + nonstatic_field(DefNewGeneration, _age_table, AgeTable) \ + nonstatic_field(DefNewGeneration, _eden_space, ContiguousSpace*) \ + nonstatic_field(DefNewGeneration, _from_space, ContiguousSpace*) \ + nonstatic_field(DefNewGeneration, _to_space, ContiguousSpace*) \ + \ + nonstatic_field(SerialBlockOffsetTable, _array, SerialBlockOffsetSharedArray*) \ + \ + nonstatic_field(SerialBlockOffsetSharedArray, _reserved, MemRegion) \ + nonstatic_field(SerialBlockOffsetSharedArray, _vs, VirtualSpace) \ + nonstatic_field(SerialBlockOffsetSharedArray, _offset_array, u_char*) \ + \ + nonstatic_field(TenuredSpace, _offsets, SerialBlockOffsetTable) #define VM_TYPES_SERIALGC(declare_type, \ declare_toplevel_type, \ @@ -73,11 +66,8 @@ declare_type(CardTableRS, CardTable) \ \ declare_toplevel_type(TenuredGeneration*) \ - declare_toplevel_type(BlockOffsetSharedArray) \ - declare_toplevel_type(BlockOffsetTable) \ - declare_type(BlockOffsetArray, BlockOffsetTable) \ - declare_type(BlockOffsetArrayContigSpace, BlockOffsetArray) \ - declare_toplevel_type(BlockOffsetSharedArray*) + declare_toplevel_type(SerialBlockOffsetSharedArray) \ + declare_toplevel_type(SerialBlockOffsetTable) #define VM_INT_CONSTANTS_SERIALGC(declare_constant, \ declare_constant_with_value) diff --git a/src/hotspot/share/gc/shared/space.cpp b/src/hotspot/share/gc/shared/space.cpp index 763ff8c616d..0745141e950 100644 --- a/src/hotspot/share/gc/shared/space.cpp +++ b/src/hotspot/share/gc/shared/space.cpp @@ -87,25 +87,6 @@ bool ContiguousSpace::is_free_block(const HeapWord* p) const { return p >= _top; } -#if INCLUDE_SERIALGC -void TenuredSpace::clear(bool mangle_space) { - ContiguousSpace::clear(mangle_space); - _offsets.initialize_threshold(); -} - -void TenuredSpace::set_bottom(HeapWord* new_bottom) { - Space::set_bottom(new_bottom); - _offsets.set_bottom(new_bottom); -} - -void TenuredSpace::set_end(HeapWord* new_end) { - // Space should not advertise an increase in size - // until after the underlying offset table has been enlarged. - _offsets.resize(pointer_delta(new_end, bottom())); - Space::set_end(new_end); -} -#endif // INCLUDE_SERIALGC - #ifndef PRODUCT void ContiguousSpace::set_top_for_allocations(HeapWord* v) { @@ -152,7 +133,6 @@ HeapWord* ContiguousSpace::forward(oop q, size_t size, } compact_top = cp->space->bottom(); cp->space->set_compaction_top(compact_top); - cp->space->initialize_threshold(); compaction_max_size = pointer_delta(cp->space->end(), compact_top); } @@ -172,7 +152,7 @@ HeapWord* ContiguousSpace::forward(oop q, size_t size, // We need to update the offset table so that the beginnings of objects can be // found during scavenge. Note that we are updating the offset table based on // where the object will be once the compaction phase finishes. - cp->space->alloc_block(compact_top - size, compact_top); + cp->space->update_for_block(compact_top - size, compact_top); return compact_top; } @@ -190,7 +170,6 @@ void ContiguousSpace::prepare_for_compaction(CompactPoint* cp) { assert(cp->gen != nullptr, "need a generation"); assert(cp->gen->first_compaction_space() == this, "just checking"); cp->space = cp->gen->first_compaction_space(); - cp->space->initialize_threshold(); cp->space->set_compaction_top(cp->space->bottom()); } @@ -384,9 +363,8 @@ void ContiguousSpace::print_on(outputStream* st) const { #if INCLUDE_SERIALGC void TenuredSpace::print_on(outputStream* st) const { print_short_on(st); - st->print_cr(" [" PTR_FORMAT ", " PTR_FORMAT ", " - PTR_FORMAT ", " PTR_FORMAT ")", - p2i(bottom()), p2i(top()), p2i(_offsets.threshold()), p2i(end())); + st->print_cr(" [" PTR_FORMAT ", " PTR_FORMAT ", " PTR_FORMAT ")", + p2i(bottom()), p2i(top()), p2i(end())); } #endif @@ -510,20 +488,30 @@ HeapWord* ContiguousSpace::par_allocate(size_t size) { } #if INCLUDE_SERIALGC -void TenuredSpace::initialize_threshold() { - _offsets.initialize_threshold(); +void TenuredSpace::update_for_block(HeapWord* start, HeapWord* end) { + _offsets.update_for_block(start, end); } -void TenuredSpace::alloc_block(HeapWord* start, HeapWord* end) { - _offsets.alloc_block(start, end); +HeapWord* TenuredSpace::block_start_const(const void* addr) const { + HeapWord* cur_block = _offsets.block_start_reaching_into_card(addr); + + while (true) { + HeapWord* next_block = cur_block + cast_to_oop(cur_block)->size(); + if (next_block > addr) { + assert(cur_block <= addr, "postcondition"); + return cur_block; + } + cur_block = next_block; + // Because the BOT is precise, we should never step into the next card + // (i.e. crossing the card boundary). + assert(!SerialBlockOffsetTable::is_crossing_card_boundary(cur_block, (HeapWord*)addr), "must be"); + } } -TenuredSpace::TenuredSpace(BlockOffsetSharedArray* sharedOffsetArray, +TenuredSpace::TenuredSpace(SerialBlockOffsetSharedArray* sharedOffsetArray, MemRegion mr) : - _offsets(sharedOffsetArray, mr), - _par_alloc_lock(Mutex::safepoint, "TenuredSpaceParAlloc_lock", true) + _offsets(sharedOffsetArray) { - _offsets.set_contig_space(this); initialize(mr, SpaceDecorator::Clear, SpaceDecorator::Mangle); } @@ -536,10 +524,6 @@ void TenuredSpace::verify() const { int objs = 0; int blocks = 0; - if (VerifyObjectStartArray) { - _offsets.verify(); - } - while (p < top()) { size_t size = cast_to_oop(p)->size(); // For a sampling of objects in the space, find it using the diff --git a/src/hotspot/share/gc/shared/space.hpp b/src/hotspot/share/gc/shared/space.hpp index d978aabc186..f609c7bad43 100644 --- a/src/hotspot/share/gc/shared/space.hpp +++ b/src/hotspot/share/gc/shared/space.hpp @@ -47,11 +47,6 @@ // Forward decls. class Space; class ContiguousSpace; -#if INCLUDE_SERIALGC -class BlockOffsetArray; -class BlockOffsetArrayContigSpace; -class BlockOffsetTable; -#endif class Generation; class ContiguousSpace; class CardTableRS; @@ -241,7 +236,7 @@ class ContiguousSpace: public Space { // This the function to invoke when an allocation of an object covering // "start" to "end" occurs to update other internal data structures. - virtual void alloc_block(HeapWord* start, HeapWord* the_end) { } + virtual void update_for_block(HeapWord* start, HeapWord* the_end) { } GenSpaceMangler* mangler() { return _mangler; } @@ -308,11 +303,6 @@ class ContiguousSpace: public Space { // live part of a compacted space ("deadwood" support.) virtual size_t allowed_dead_ratio() const { return 0; }; - // Some contiguous spaces may maintain some data structures that should - // be updated whenever an allocation crosses a boundary. This function - // initializes these data structures for further updates. - virtual void initialize_threshold() { } - // "q" is an object of the given "size" that should be forwarded; // "cp" names the generation ("gen") and containing "this" (which must // also equal "cp->space"). "compact_top" is where in "this" the @@ -322,7 +312,7 @@ class ContiguousSpace: public Space { // be one, since compaction must succeed -- we go to the first space of // the previous generation if necessary, updating "cp"), reset compact_top // and then forward. In either case, returns the new value of "compact_top". - // Invokes the "alloc_block" function of the then-current compaction + // Invokes the "update_for_block" function of the then-current compaction // space. virtual HeapWord* forward(oop q, size_t size, CompactPoint* cp, HeapWord* compact_top); @@ -412,36 +402,28 @@ class ContiguousSpace: public Space { #if INCLUDE_SERIALGC // Class TenuredSpace is used by TenuredGeneration; it supports an efficient -// "block_start" operation via a BlockOffsetArray (whose BlockOffsetSharedArray -// may be shared with other spaces.) +// "block_start" operation via a SerialBlockOffsetTable. class TenuredSpace: public ContiguousSpace { friend class VMStructs; protected: - BlockOffsetArrayContigSpace _offsets; - Mutex _par_alloc_lock; + SerialBlockOffsetTable _offsets; // Mark sweep support size_t allowed_dead_ratio() const override; public: // Constructor - TenuredSpace(BlockOffsetSharedArray* sharedOffsetArray, + TenuredSpace(SerialBlockOffsetSharedArray* sharedOffsetArray, MemRegion mr); - void set_bottom(HeapWord* value) override; - void set_end(HeapWord* value) override; - - void clear(bool mangle_space) override; - - inline HeapWord* block_start_const(const void* p) const override; + HeapWord* block_start_const(const void* addr) const override; // Add offset table update. inline HeapWord* allocate(size_t word_size) override; inline HeapWord* par_allocate(size_t word_size) override; // MarkSweep support phase3 - void initialize_threshold() override; - void alloc_block(HeapWord* start, HeapWord* end) override; + void update_for_block(HeapWord* start, HeapWord* end) override; void print_on(outputStream* st) const override; diff --git a/src/hotspot/share/gc/shared/space.inline.hpp b/src/hotspot/share/gc/shared/space.inline.hpp index e1a3e66c146..9c4c2864ced 100644 --- a/src/hotspot/share/gc/shared/space.inline.hpp +++ b/src/hotspot/share/gc/shared/space.inline.hpp @@ -47,36 +47,19 @@ inline HeapWord* Space::block_start(const void* p) { inline HeapWord* TenuredSpace::allocate(size_t size) { HeapWord* res = ContiguousSpace::allocate(size); if (res != nullptr) { - _offsets.alloc_block(res, size); + _offsets.update_for_block(res, res + size); } return res; } -// Because of the requirement of keeping "_offsets" up to date with the -// allocations, we sequentialize these with a lock. Therefore, best if -// this is used for larger LAB allocations only. inline HeapWord* TenuredSpace::par_allocate(size_t size) { - MutexLocker x(&_par_alloc_lock); - // This ought to be just "allocate", because of the lock above, but that - // ContiguousSpace::allocate asserts that either the allocating thread - // holds the heap lock or it is the VM thread and we're at a safepoint. - // The best I (dld) could figure was to put a field in ContiguousSpace - // meaning "locking at safepoint taken care of", and set/reset that - // here. But this will do for now, especially in light of the comment - // above. Perhaps in the future some lock-free manner of keeping the - // coordination. HeapWord* res = ContiguousSpace::par_allocate(size); if (res != nullptr) { - _offsets.alloc_block(res, size); + _offsets.update_for_block(res, res + size); } return res; } -inline HeapWord* -TenuredSpace::block_start_const(const void* p) const { - return _offsets.block_start(p); -} - class DeadSpacer : StackObj { size_t _allowed_deadspace_words; bool _active; From 36de19d4622e38b6c00644b0035521808574e255 Mon Sep 17 00:00:00 2001 From: Aggelos Biboudis Date: Wed, 1 Nov 2023 13:38:10 +0000 Subject: [PATCH 04/12] 8317048: VerifyError with unnamed pattern variable and more than one components Reviewed-by: jlahoda --- .../sun/tools/javac/comp/TransPatterns.java | 1 + .../tools/javac/patterns/T8317048.java | 88 +++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 test/langtools/tools/javac/patterns/T8317048.java diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java index 59568aef605..aedafa20c3e 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java @@ -907,6 +907,7 @@ public void resolve(VarSymbol commonBinding, } } else if (currentBinding != null && commonBinding.type.tsym == currentBinding.type.tsym && + commonBinding.isUnnamedVariable() == currentBinding.isUnnamedVariable() && !previousNullable && new TreeDiffer(List.of(commonBinding), List.of(currentBinding)) .scan(commonNestedExpression, currentNestedExpression)) { diff --git a/test/langtools/tools/javac/patterns/T8317048.java b/test/langtools/tools/javac/patterns/T8317048.java new file mode 100644 index 00000000000..988f51ad2f0 --- /dev/null +++ b/test/langtools/tools/javac/patterns/T8317048.java @@ -0,0 +1,88 @@ +/* + * 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 8317048 + * @summary VerifyError with unnamed pattern variable and more than one components + * @enablePreview + * @compile T8317048.java + * @run main T8317048 + */ + +public class T8317048 { + record Tuple(T1 first, T2 second) {} + record R1(Integer value) implements R {} + record R2(Integer value) implements R {} + sealed interface R {} + + static int meth1(Tuple, R> o) { + return switch (o) { + case Tuple, R>(R1 _, R1 _) -> -1; + case Tuple, R>(R1 _, R2 _) -> -1; + case Tuple, R>(R2 _, R1 _) -> -1; + case Tuple, R>(R2 fst, R2 snd) -> fst.value().compareTo(snd.value()); + }; + } + + static int meth2(Tuple, R> o) { + return switch (o) { + case Tuple, R>(R1 _, R1 _) -> -1; + case Tuple, R>(R1 fst, R2 snd) -> fst.value().compareTo(snd.value()); + case Tuple, R>(R2 _, R1 _) -> -1; + case Tuple, R>(R2 fst, R2 snd) -> fst.value().compareTo(snd.value()); + }; + } + + static int meth3(Tuple, R> o) { + return switch (o) { + case Tuple, R>(R1 fst, R1 _) -> fst.value(); + case Tuple, R>(R1 _, R2 snd) -> snd.value(); + case Tuple, R>(R2 _, R1 _) -> -1; + case Tuple, R>(R2 fst, R2 snd) -> fst.value().compareTo(snd.value()); + }; + } + + static int meth4(Tuple, R> o) { + return switch (o) { + case Tuple, R>(R1 _, R1 _) -> -1; + case Tuple, R>(R1 _, R2 _) -> -1; + case Tuple, R>(R2 fst, R2 snd) -> fst.value().compareTo(snd.value()); + case Tuple, R>(R2 _, R1 _) -> -1; + }; + } + + public static void main(String[] args) { + assertEquals(1, meth1(new Tuple, R>(new R2<>(2), new R2<>(1)))); + assertEquals(1, meth2(new Tuple, R>(new R1<>(2), new R2<>(1)))); + assertEquals(0, meth2(new Tuple, R>(new R2<>(1), new R2<>(1)))); + assertEquals(2, meth3(new Tuple, R>(new R1<>(2), new R1<>(1)))); + assertEquals(3, meth3(new Tuple, R>(new R1<>(2), new R2<>(3)))); + assertEquals(1, meth4(new Tuple, R>(new R2<>(2), new R2<>(1)))); + } + + static void assertEquals(int expected, int actual) { + if (expected != actual) { + throw new AssertionError("Expected: " + expected + ", but got: " + actual); + } + } +} \ No newline at end of file From 7f47c51aced9c724dbc9b0d8cbd88c49435da460 Mon Sep 17 00:00:00 2001 From: Alexey Ivanov Date: Wed, 1 Nov 2023 15:27:05 +0000 Subject: [PATCH 05/12] 8316025: Use testUI() method of PassFailJFrame.Builder in FileChooserSymLinkTest.java Reviewed-by: azvegint --- .../JFileChooser/FileChooserSymLinkTest.java | 133 +++++++++--------- 1 file changed, 63 insertions(+), 70 deletions(-) diff --git a/test/jdk/javax/swing/JFileChooser/FileChooserSymLinkTest.java b/test/jdk/javax/swing/JFileChooser/FileChooserSymLinkTest.java index ef29b708536..15ac95bc198 100644 --- a/test/jdk/javax/swing/JFileChooser/FileChooserSymLinkTest.java +++ b/test/jdk/javax/swing/JFileChooser/FileChooserSymLinkTest.java @@ -22,21 +22,21 @@ */ import java.awt.BorderLayout; +import java.awt.Window; import java.awt.event.ActionEvent; import java.awt.event.ActionListener; import java.beans.PropertyChangeEvent; import java.beans.PropertyChangeListener; import java.io.File; -import java.lang.reflect.InvocationTargetException; import java.util.Arrays; +import java.util.List; import javax.swing.JCheckBox; import javax.swing.JFileChooser; import javax.swing.JFrame; import javax.swing.JPanel; -import javax.swing.JTextArea; import javax.swing.JScrollPane; -import javax.swing.SwingUtilities; +import javax.swing.JTextArea; import javax.swing.WindowConstants; /* @@ -51,87 +51,80 @@ * @run main/manual FileChooserSymLinkTest */ public class FileChooserSymLinkTest { + private static final String INSTRUCTIONS = """ + + Instructions to Test: +
    +
  1. Open an elevated Command Prompt. +
  2. Paste the following commands: + 
    cd /d C:\\
+            mkdir FileChooserTest
+            cd FileChooserTest
+            mkdir target
+            mklink /d link target
    + +
  3. Navigate to C:\\FileChooserTest in + the JFileChooser. +
  4. Perform testing in single- and multi-selection modes: +
      +
    • Single-selection: +
        +
      1. Ensure Enable multi-selection is cleared + (the default state). +
      2. Click link directory, + the absolute path of the symbolic + link should be displayed.
        + If it's null, click Fail. +
      3. Click target directory, + its absolute path should be displayed. +
      +
    • Multi-selection: +
        +
      1. Select Enable multi-selection. +
      2. Click link, +
      3. Press Ctrl and + then click target. +
      4. Both should be selected and + their absolute paths should be displayed. +
      5. If link can't be selected or + if its absolute path is null, + click Fail. +
      +
    +

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

    +
  5. When done with testing, paste the following commands to + remove the FileChooserTest directory: + 
    cd \\
+            rmdir /s /q C:\\FileChooserTest
    + + or use File Explorer to clean it up. +
+ """; + static JFrame frame; static JFileChooser jfc; static JPanel panel; static JTextArea pathList; static JCheckBox multiSelection; - static PassFailJFrame passFailJFrame; public static void main(String[] args) throws Exception { - SwingUtilities.invokeAndWait(new Runnable() { - public void run() { - try { - initialize(); - } catch (InterruptedException | InvocationTargetException e) { - throw new RuntimeException(e); - } - } - }); - passFailJFrame.awaitAndCheck(); + PassFailJFrame.builder() + .instructions(INSTRUCTIONS) + .rows(35) + .columns(50) + .testUI(FileChooserSymLinkTest::createTestUI) + .build() + .awaitAndCheck(); } - static void initialize() throws InterruptedException, InvocationTargetException { - //Initialize the components - final String INSTRUCTIONS = """ - - Instructions to Test: -
    -
  1. Open an elevated Command Prompt. -
  2. Paste the following commands: - 
    cd /d C:\\
-                mkdir FileChooserTest
-                cd FileChooserTest
-                mkdir target
-                mklink /d link target
    - -
  3. Navigate to C:\\FileChooserTest in - the JFileChooser. -
  4. Perform testing in single- and multi-selection modes: -
      -
    • Single-selection: -
        -
      1. Ensure Enable multi-selection is cleared - (the default state). -
      2. Click link directory, - the absolute path of the symbolic - link should be displayed.
        - If it's null, click Fail. -
      3. Click target directory, - its absolute path should be displayed. -
      -
    • Multi-selection: -
        -
      1. Select Enable multi-selection. -
      2. Click link, -
      3. Press Ctrl and - then click target. -
      4. Both should be selected and - their absolute paths should be displayed. -
      5. If link can't be selected or - if its absolute path is null, - click Fail. -
      -
    -

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

    -
  5. When done with testing, paste the following commands to - remove the FileChooserTest directory: - 
    cd \\
-                rmdir /s /q C:\\FileChooserTest
    - - or use File Explorer to clean it up. -
- """; + private static List createTestUI() { frame = new JFrame("JFileChooser Symbolic Link test"); panel = new JPanel(new BorderLayout()); multiSelection = new JCheckBox("Enable Multi-Selection"); pathList = new JTextArea(10, 50); jfc = new JFileChooser(new File("C:\\")); - passFailJFrame = new PassFailJFrame("Test Instructions", INSTRUCTIONS, 5L, 35, 50); - PassFailJFrame.addTestWindow(frame); - PassFailJFrame.positionTestWindow(frame, PassFailJFrame.Position.HORIZONTAL); frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE); panel.add(multiSelection, BorderLayout.EAST); panel.add(new JScrollPane(pathList), BorderLayout.WEST); @@ -166,6 +159,6 @@ public void propertyChange(PropertyChangeEvent evt) { frame.add(panel, BorderLayout.NORTH); frame.add(jfc, BorderLayout.CENTER); frame.pack(); - frame.setVisible(true); + return List.of(frame); } } From 3660a90ad8658f86f137de5955c0ae6df2c85c4f Mon Sep 17 00:00:00 2001 From: Jonathan Gibbons Date: Wed, 1 Nov 2023 15:33:53 +0000 Subject: [PATCH 06/12] 8319139: Improve diagnosability of `JavadocTester` output Reviewed-by: hannesw --- .../lib/javadoc/tester/JavadocTester.java | 65 +++++++++++-------- .../testJavadocTester/TestJavadocTester.java | 62 ++++++++++-------- .../TestJavadocTesterCrash.java | 6 +- 3 files changed, 78 insertions(+), 55 deletions(-) diff --git a/test/langtools/jdk/javadoc/lib/javadoc/tester/JavadocTester.java b/test/langtools/jdk/javadoc/lib/javadoc/tester/JavadocTester.java index a409dd2cedb..b4bb8a319df 100644 --- a/test/langtools/jdk/javadoc/lib/javadoc/tester/JavadocTester.java +++ b/test/langtools/jdk/javadoc/lib/javadoc/tester/JavadocTester.java @@ -827,9 +827,9 @@ public void checkFiles(boolean expectedFound, Collection paths) { Path file = outputDir.resolve(path); boolean isFound = Files.exists(file); if (isFound == expectedFound) { - passed(file, "file " + (isFound ? "found:" : "not found:") + "\n"); + passed(file, "file " + (isFound ? "found:" : "not found:")); } else { - failed(file, "file " + (isFound ? "found:" : "not found:") + "\n"); + failed(file, "file " + (isFound ? "found:" : "not found:")); } } } @@ -946,9 +946,10 @@ protected void checking(String message) { * * @param file the file that was the focus of the check * @param message a short description of the outcome + * @param details optional additional details */ - protected void passed(Path file, String message) { - passed(file + ": " + message); + protected void passed(Path file, String message, String... details) { + passed(file + ": " + message, details); } /** @@ -957,10 +958,14 @@ protected void passed(Path file, String message) { *

This method should be called after previously calling {@code checking(...)}. * * @param message a short description of the outcome + * @param details optional additional details */ - protected void passed(String message) { + protected void passed(String message, String... details) { numTestsPassed++; print("Passed", message); + for (var detail: details) { + detail.lines().forEachOrdered(out::println); + } out.println(); } @@ -971,9 +976,10 @@ protected void passed(String message) { * * @param file the file that was the focus of the check * @param message a short description of the outcome + * @param details optional additional details */ - protected void failed(Path file, String message) { - failed(file + ": " + message); + protected void failed(Path file, String message, String... details) { + failed(file + ": " + message, details); } /** @@ -982,8 +988,9 @@ protected void failed(Path file, String message) { *

This method should be called after previously calling {@code checking(...)}. * * @param message a short description of the outcome + * @param details optional additional details */ - protected void failed(String message) { + protected void failed(String message, String... details) { print("FAILED", message); StackWalker.getInstance().walk(s -> { s.dropWhile(f -> f.getMethodName().equals("failed")) @@ -993,6 +1000,9 @@ protected void failed(String message) { + "(" + f.getFileName() + ":" + f.getLineNumber() + ")")); return null; }); + for (var detail: details) { + detail.lines().forEachOrdered(out::println); + } out.println(); } @@ -1002,10 +1012,7 @@ private void print(String prefix, String message) { else { out.print(prefix); out.print(": "); - out.print(message.replace("\n", NL)); - if (!(message.endsWith("\n") || message.endsWith(NL))) { - out.println(); - } + message.lines().forEachOrdered(out::println); } } @@ -1219,25 +1226,28 @@ private void check(Function finder, SearchKind kind, String s) { boolean isFound = r != null; if (isFound == expectFound) { matches.add(lastMatch = r); - passed(name + ": following " + kind + " " + (isFound ? "found:" : "not found:") + "\n" - + s); + passed(name + ": the following " + kind + " was " + (isFound ? "found:" : "not found:"), + s); } else { // item not found in order, so check if the item is found out of order, to determine the best message if (expectFound && expectOrdered && start > 0) { Range r2 = finder.apply(0); if (r2 != null) { - failed(name + ": following " + kind + " was found on line " + failed(name + ": output not as expected", + ">> the following " + kind + " was found on line " + getLineNumber(r2.start) + ", but not in order as expected, on or after line " - + getLineNumber(start) - + ":\n" - + s); + + getLineNumber(start), + ">> " + kind + ":", + s); return; } } - failed(name + ": following " + kind + " " - + (isFound ? "found:" : "not found:") + "\n" - + s + '\n' + "found \n" + content); + failed(name + ": output not as expected", + ">> the following " + kind + " was " + (isFound ? "found:" : "not found:"), + s, + ">> found", + content); } } @@ -1374,8 +1384,9 @@ public OutputChecker checkComplete() { if (uncovered.isEmpty()) { passed("All output matched"); } else { - failed("The following output was not matched: " - + uncovered.stream() + failed("Output not as expected", + ">> The following output was not matched", + uncovered.stream() .map(Range::toIntervalString) .collect(Collectors.joining(", "))); } @@ -1395,8 +1406,9 @@ public OutputChecker checkEmpty() { if (content == null || content.isEmpty()) { passed(name + " is empty, as expected"); } else { - failed(name + " is not empty; contains:\n" - + content); + failed(name + " is not empty", + ">> found:", + content); } return this; } @@ -1444,7 +1456,8 @@ private OutputChecker checkAnyOf(SearchKind kind, List items, BiFunction< if (count == 0) { failed("no match found for any " + kind); } else { - passed(count + " matches found; earliest is " + earliest.toIntervalString()); + passed(count + " matches found", + ">> the earliest is: " + earliest.toIntervalString()); } return this; } diff --git a/test/langtools/jdk/javadoc/testJavadocTester/TestJavadocTester.java b/test/langtools/jdk/javadoc/testJavadocTester/TestJavadocTester.java index 2fcb0c9a958..abebf6fb3ba 100644 --- a/test/langtools/jdk/javadoc/testJavadocTester/TestJavadocTester.java +++ b/test/langtools/jdk/javadoc/testJavadocTester/TestJavadocTester.java @@ -73,9 +73,9 @@ public static void main(String... args) throws Exception { * @param message a short description of the outcome */ @Override - public void passed(String message) { - super.passed(message); - messages.add("Passed: " + message); + public void passed(String message, String... details) { + super.passed(message, details); + messages.add("Passed: " + join(message, details)); } /** @@ -85,9 +85,13 @@ public void passed(String message) { * @param message a short description of the outcome */ @Override - public void failed(String message) { - super.failed(message); - messages.add("FAILED: " + message); + public void failed(String message, String... details) { + super.failed(message, details); + messages.add("FAILED: " + join(message, details)); + } + + private String join(String message, String... details) { + return details.length == 0 ? message : message + "\n" + String.join("\n", details); } /** @@ -138,6 +142,8 @@ void checkMessages(String... expect) { testErrors++; } } + + messages.forEach(m -> out.println("MESSAGES: " + m)); } /** @@ -153,7 +159,7 @@ void checkMessages(String... expect) { * @param message the message to be reported. */ private void report(String message) { - message.lines().forEachOrdered(l -> out.println(">>> " + l)); + message.lines().forEachOrdered(l -> out.println(">>>> " + l)); } //------------------------------------------------- @@ -202,13 +208,13 @@ public void testSimpleStringCheck() { messages.forEach(this::report); checkMessages( """ - Passed: out/p/C.html: following text found: + Passed: out/p/C.html: the following text was found: Second sentence""", """ - Passed: out/p/C.html: following text found: + Passed: out/p/C.html: the following text was found: abc123""", """ - Passed: out/p/C.html: following text found: + Passed: out/p/C.html: the following text was found: def456"""); } @@ -220,7 +226,7 @@ public void testSimpleNegativeStringCheck_expected() { .check("Third sentence."); checkMessages( """ - Passed: out/p/C.html: following text not found: + Passed: out/p/C.html: the following text was not found: Third sentence"""); } @@ -231,7 +237,8 @@ public void testSimpleNegativeStringCheck_unexpected() { .check("Third sentence."); checkMessages( """ - FAILED: out/p/C.html: following text not found: + FAILED: out/p/C.html: output not as expected + >> the following text was not found: Third sentence"""); } @@ -244,13 +251,13 @@ public void testSimpleRegexCheck() { Pattern.compile("d.f4.6")); checkMessages( """ - Passed: out/p/C.html: following pattern found: + Passed: out/p/C.html: the following pattern was found: S.cond s.nt.nc.""", """ - Passed: out/p/C.html: following pattern found: + Passed: out/p/C.html: the following pattern was found: [abc]{3}[123]{3}""", """ - Passed: out/p/C.html: following pattern found: + Passed: out/p/C.html: the following pattern was found: d.f4.6"""); } @@ -271,28 +278,28 @@ public void testOrdered() { checkMessages( """ - Passed: out/p/C.html: following text found: + Passed: out/p/C.html: the following text was found:

Method Summary

""", """ - Passed: out/p/C.html: following text found: + Passed: out/p/C.html: the following text was found: m1""", """ - Passed: out/p/C.html: following text found: + Passed: out/p/C.html: the following text was found: m2""", """ - Passed: out/p/C.html: following text found: + Passed: out/p/C.html: the following text was found: m3""", """ - Passed: out/p/C.html: following text found: + Passed: out/p/C.html: the following text was found:

Method Details

""", """ - Passed: out/p/C.html: following text found: + Passed: out/p/C.html: the following text was found:
""", """ - Passed: out/p/C.html: following text found: + Passed: out/p/C.html: the following text was found:
""", """ - Passed: out/p/C.html: following text found: + Passed: out/p/C.html: the following text was found:
""" ); } @@ -306,10 +313,10 @@ public void testUnordered_expected() { "First sentence"); checkMessages( """ - Passed: out/p/C.html: following text found: + Passed: out/p/C.html: the following text was found: Second sentence""", """ - Passed: out/p/C.html: following text found: + Passed: out/p/C.html: the following text was found: First sentence"""); } @@ -321,10 +328,11 @@ public void testUnordered_unexpected() { "First sentence"); checkMessages( """ - Passed: out/p/C.html: following text found: + Passed: out/p/C.html: the following text was found: Second sentence""", """ - FAILED: out/p/C.html: following text was found on line"""); + FAILED: out/p/C.html: output not as expected + >> the following text was found on line"""); } @Test diff --git a/test/langtools/jdk/javadoc/testJavadocTester/TestJavadocTesterCrash.java b/test/langtools/jdk/javadoc/testJavadocTester/TestJavadocTesterCrash.java index f0a6b0e98eb..bd509023895 100644 --- a/test/langtools/jdk/javadoc/testJavadocTester/TestJavadocTesterCrash.java +++ b/test/langtools/jdk/javadoc/testJavadocTester/TestJavadocTesterCrash.java @@ -86,7 +86,7 @@ public String toString(List tags, Element element) { String s = tags.toString(); if (s.contains("test")) { throw new Error("demo error"); - }; + } return s; } } @@ -118,6 +118,8 @@ public class C { }"""); "1 error"); // verify that JavadocTester detected the crash - checkMessages("FAILED: STDERR: following text found:"); + checkMessages(""" + FAILED: STDERR: output not as expected + >> the following text was found:"""); } } From c86592d38d651beac40f1da43c718a2d4b17bd19 Mon Sep 17 00:00:00 2001 From: Jonathan Gibbons Date: Wed, 1 Nov 2023 15:48:31 +0000 Subject: [PATCH 07/12] 8319046: Execute tests in source/class-file order in JavadocTester Reviewed-by: hannesw --- .../lib/javadoc/tester/JavadocTester.java | 75 +++++++++++++++++-- 1 file changed, 68 insertions(+), 7 deletions(-) diff --git a/test/langtools/jdk/javadoc/lib/javadoc/tester/JavadocTester.java b/test/langtools/jdk/javadoc/lib/javadoc/tester/JavadocTester.java index b4bb8a319df..f31da1a6edd 100644 --- a/test/langtools/jdk/javadoc/lib/javadoc/tester/JavadocTester.java +++ b/test/langtools/jdk/javadoc/lib/javadoc/tester/JavadocTester.java @@ -29,7 +29,6 @@ import java.io.PrintStream; import java.io.PrintWriter; import java.io.StringWriter; -import java.lang.annotation.Annotation; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.ref.SoftReference; @@ -59,8 +58,11 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; + import javax.tools.StandardJavaFileManager; +//import jdk.lang.classfile.Classfile; + /** * Test framework for running javadoc and performing tests on the resulting output. @@ -292,16 +294,75 @@ public void runTests() throws Exception { * @throws Exception if any errors occurred while executing a test method */ public void runTests(Function f) throws Exception { - for (Method m : getClass().getDeclaredMethods()) { - Annotation a = m.getAnnotation(Test.class); - if (a != null) { - runTest(m, f); - out.println(); - } + var methods = List.of(getClass().getDeclaredMethods()).stream() + .filter(m -> m.isAnnotationPresent(Test.class)) + .collect(Collectors.toCollection(() -> new ArrayList<>())); + var methodOrderComparator = getMethodComparator(); + if (methodOrderComparator != null) { + methods.sort(methodOrderComparator); + } + for (Method m : methods) { + runTest(m, f); + out.println(); } printSummary(); } +// The following is for when the Classfile library is generally available. +// private Comparator getClassOrderMethodComparator(Class c) { +// try { +// var url = c.getProtectionDomain().getCodeSource().getLocation(); +// var path = Path.of(url.toURI()).resolve(c.getName().replace(".", "/") + ".class"); +// var cf = Classfile.of().parse(path); +// var map = new HashMap(); +// var index = 0; +// for (var m : cf.methods()) { +// map.putIfAbsent(m.methodName().stringValue(), index++); +// } +// return Comparator.comparingInt(m -> map.getOrDefault(m.getName(), -1)); +// } catch (URISyntaxException | IOException e) { +// throw new Error("Cannot sort methods: " + e, e); +// } +// } + + /** + * {@return the comparator used to sort the default set of methods to be executed, + * or {@code null} if the methods should not be sorted } + * + * @implSpec This implementation returns a source-order comparator. + */ + public Comparator getMethodComparator() { + return getSourceOrderMethodComparator(getClass()); + } + + /** + * {@return the source-order method comparator for methods in the given class} + * @param c the class + */ + public static Comparator getSourceOrderMethodComparator(Class c) { + var path = Path.of(testSrc) + .resolve(c.getName() + .replace(".", "/") + .replaceAll("\\$.*", "") + + ".java"); + try { + var src = Files.readString(path); + // Fuzzy match for test method declarations. + // It doesn't matter if there are false positives, as long as the true positives are in the correct order. + // It doesn't matter too much if there are false negatives: they'll just be executed first. + var isMethodDecl = Pattern.compile("public +void +(?[A-Za-z][A-Za-z0-9_]*)\\("); + var matcher = isMethodDecl.matcher(src); + var map = new HashMap(); + var index = 0; + while (matcher.find()) { + map.putIfAbsent(matcher.group("name"), index++); + } + return Comparator.comparingInt(m -> map.getOrDefault(m.getName(), -1)); + } catch (IOException e) { + throw new Error("Cannot sort methods: " + e, e); + } + } + /** * Run the specified methods annotated with @Test, or all methods annotated * with @Test if none are specified, followed by printSummary. From d354141aa191c80b473dfeee27b51f1562ffeafd Mon Sep 17 00:00:00 2001 From: Doug Simon Date: Wed, 1 Nov 2023 16:27:04 +0000 Subject: [PATCH 08/12] 8318694: [JVMCI] disable can_call_java in most contexts for libjvmci compiler threads Reviewed-by: dholmes, never --- src/hotspot/share/classfile/javaClasses.cpp | 5 +- .../share/classfile/systemDictionary.cpp | 4 +- src/hotspot/share/compiler/compilerThread.cpp | 11 ++-- src/hotspot/share/compiler/compilerThread.hpp | 10 ++-- src/hotspot/share/jvmci/jvmci.cpp | 28 ++++++++++ src/hotspot/share/jvmci/jvmci.hpp | 29 +++++++++++ src/hotspot/share/jvmci/jvmciCompilerToVM.cpp | 42 +++++++++++---- src/hotspot/share/jvmci/jvmciEnv.cpp | 23 +++++++-- src/hotspot/share/jvmci/jvmciRuntime.cpp | 51 ------------------- src/hotspot/share/jvmci/jvmciRuntime.hpp | 4 -- src/hotspot/share/prims/upcallLinker.cpp | 2 +- .../jdk/vm/ci/hotspot/CompilerToVM.java | 3 +- .../vm/ci/hotspot/HotSpotConstantPool.java | 8 +-- 13 files changed, 132 insertions(+), 88 deletions(-) diff --git a/src/hotspot/share/classfile/javaClasses.cpp b/src/hotspot/share/classfile/javaClasses.cpp index c78bdd73a5d..8ff8258b93c 100644 --- a/src/hotspot/share/classfile/javaClasses.cpp +++ b/src/hotspot/share/classfile/javaClasses.cpp @@ -2453,7 +2453,7 @@ void java_lang_Throwable::print_stack_trace(Handle throwable, outputStream* st) BacktraceElement bte = iter.next(THREAD); print_stack_element_to_stream(st, bte._mirror, bte._method_id, bte._version, bte._bci, bte._name); } - { + if (THREAD->can_call_java()) { // Call getCause() which doesn't necessarily return the _cause field. ExceptionMark em(THREAD); JavaValue cause(T_OBJECT); @@ -2475,6 +2475,9 @@ void java_lang_Throwable::print_stack_trace(Handle throwable, outputStream* st) st->cr(); } } + } else { + st->print_raw_cr("<>"); + return; } } } diff --git a/src/hotspot/share/classfile/systemDictionary.cpp b/src/hotspot/share/classfile/systemDictionary.cpp index 3d1674bca99..0af5b5fbcf5 100644 --- a/src/hotspot/share/classfile/systemDictionary.cpp +++ b/src/hotspot/share/classfile/systemDictionary.cpp @@ -611,7 +611,7 @@ InstanceKlass* SystemDictionary::resolve_instance_class_or_null(Symbol* name, InstanceKlass* loaded_class = nullptr; SymbolHandle superclassname; // Keep alive while loading in parallel thread. - assert(THREAD->can_call_java(), + guarantee(THREAD->can_call_java(), "can not load classes with compiler thread: class=%s, classloader=%s", name->as_C_string(), class_loader.is_null() ? "null" : class_loader->klass()->name()->as_C_string()); @@ -2056,7 +2056,7 @@ Method* SystemDictionary::find_method_handle_invoker(Klass* klass, Klass* accessing_klass, Handle* appendix_result, TRAPS) { - assert(THREAD->can_call_java() ,""); + guarantee(THREAD->can_call_java(), ""); Handle method_type = SystemDictionary::find_method_handle_type(signature, accessing_klass, CHECK_NULL); diff --git a/src/hotspot/share/compiler/compilerThread.cpp b/src/hotspot/share/compiler/compilerThread.cpp index 77a5f4b7027..47e3f5a6f49 100644 --- a/src/hotspot/share/compiler/compilerThread.cpp +++ b/src/hotspot/share/compiler/compilerThread.cpp @@ -39,6 +39,7 @@ CompilerThread::CompilerThread(CompileQueue* queue, _queue = queue; _counters = counters; _buffer_blob = nullptr; + _can_call_java = false; _compiler = nullptr; _arena_stat = CompilationMemoryStatistic::enabled() ? new ArenaStatCounter : nullptr; @@ -56,15 +57,17 @@ CompilerThread::~CompilerThread() { delete _arena_stat; } +void CompilerThread::set_compiler(AbstractCompiler* c) { + // Only jvmci compiler threads can call Java + _can_call_java = c != nullptr && c->is_jvmci(); + _compiler = c; +} + void CompilerThread::thread_entry(JavaThread* thread, TRAPS) { assert(thread->is_Compiler_thread(), "must be compiler thread"); CompileBroker::compiler_thread_loop(); } -bool CompilerThread::can_call_java() const { - return _compiler != nullptr && _compiler->is_jvmci(); -} - // Hide native compiler threads from external view. bool CompilerThread::is_hidden_from_external_view() const { return _compiler == nullptr || _compiler->is_hidden_from_external_view(); diff --git a/src/hotspot/share/compiler/compilerThread.hpp b/src/hotspot/share/compiler/compilerThread.hpp index 65bb4481c02..3531fb6d72d 100644 --- a/src/hotspot/share/compiler/compilerThread.hpp +++ b/src/hotspot/share/compiler/compilerThread.hpp @@ -31,18 +31,17 @@ class AbstractCompiler; class ArenaStatCounter; class BufferBlob; class ciEnv; -class CompileThread; +class CompilerThread; class CompileLog; class CompileTask; class CompileQueue; class CompilerCounters; class IdealGraphPrinter; -class JVMCIEnv; -class JVMCIPrimitiveArray; // A thread used for Compilation. class CompilerThread : public JavaThread { friend class VMStructs; + JVMCI_ONLY(friend class CompilerThreadCanCallJava;) private: CompilerCounters* _counters; @@ -51,6 +50,7 @@ class CompilerThread : public JavaThread { CompileTask* volatile _task; // print_threads_compiling can read this concurrently. CompileQueue* _queue; BufferBlob* _buffer_blob; + bool _can_call_java; AbstractCompiler* _compiler; TimeStamp _idle_time; @@ -73,13 +73,13 @@ class CompilerThread : public JavaThread { bool is_Compiler_thread() const { return true; } - virtual bool can_call_java() const; + virtual bool can_call_java() const { return _can_call_java; } // Returns true if this CompilerThread is hidden from JVMTI and FlightRecorder. C1 and C2 are // always hidden but JVMCI compiler threads might be hidden. virtual bool is_hidden_from_external_view() const; - void set_compiler(AbstractCompiler* c) { _compiler = c; } + void set_compiler(AbstractCompiler* c); AbstractCompiler* compiler() const { return _compiler; } CompileQueue* queue() const { return _queue; } diff --git a/src/hotspot/share/jvmci/jvmci.cpp b/src/hotspot/share/jvmci/jvmci.cpp index 12b400478ca..447792b6fca 100644 --- a/src/hotspot/share/jvmci/jvmci.cpp +++ b/src/hotspot/share/jvmci/jvmci.cpp @@ -23,6 +23,7 @@ #include "precompiled.hpp" #include "classfile/systemDictionary.hpp" +#include "compiler/abstractCompiler.hpp" #include "compiler/compileTask.hpp" #include "compiler/compilerThread.hpp" #include "gc/shared/collectedHeap.hpp" @@ -53,6 +54,29 @@ volatile intx JVMCI::_fatal_log_init_thread = -1; volatile int JVMCI::_fatal_log_fd = -1; const char* JVMCI::_fatal_log_filename = nullptr; +CompilerThreadCanCallJava::CompilerThreadCanCallJava(JavaThread* current, bool new_state) { + _current = nullptr; + if (current->is_Compiler_thread()) { + CompilerThread* ct = CompilerThread::cast(current); + if (ct->_can_call_java != new_state && + ct->_compiler != nullptr && + ct->_compiler->is_jvmci()) + { + // Only enter a new context if the ability of the + // current thread to call Java actually changes + _reset_state = ct->_can_call_java; + ct->_can_call_java = new_state; + _current = ct; + } + } +} + +CompilerThreadCanCallJava::~CompilerThreadCanCallJava() { + if (_current != nullptr) { + _current->_can_call_java = _reset_state; + } +} + void jvmci_vmStructs_init() NOT_DEBUG_RETURN; bool JVMCI::can_initialize_JVMCI() { @@ -176,6 +200,10 @@ void JVMCI::ensure_box_caches_initialized(TRAPS) { java_lang_Long_LongCache::symbol() }; + // Class resolution and initialization below + // requires calling into Java + CompilerThreadCanCallJava ccj(THREAD, true); + for (unsigned i = 0; i < sizeof(box_classes) / sizeof(Symbol*); i++) { Klass* k = SystemDictionary::resolve_or_fail(box_classes[i], true, CHECK); InstanceKlass* ik = InstanceKlass::cast(k); diff --git a/src/hotspot/share/jvmci/jvmci.hpp b/src/hotspot/share/jvmci/jvmci.hpp index 200046906f6..1e9fa08898f 100644 --- a/src/hotspot/share/jvmci/jvmci.hpp +++ b/src/hotspot/share/jvmci/jvmci.hpp @@ -29,6 +29,7 @@ #include "utilities/exceptions.hpp" class BoolObjectClosure; +class CompilerThread; class constantPoolHandle; class JavaThread; class JVMCIEnv; @@ -46,6 +47,34 @@ typedef FormatStringEventLog<256> StringEventLog; struct _jmetadata; typedef struct _jmetadata *jmetadata; +// A stack object that manages a scope in which the current thread, if +// it's a CompilerThread, can have its CompilerThread::_can_call_java +// field changed. This allows restricting libjvmci better in terms +// of when it can make Java calls. If a Java call on a CompilerThread +// reaches a clinit, there's a risk of dead-lock when async compilation +// is disabled (e.g. -Xbatch or -Xcomp) as the non-CompilerThread thread +// waiting for the blocking compilation may hold the clinit lock. +// +// This scope is primarily used to disable Java calls when libjvmci enters +// the VM via a C2V (i.e. CompilerToVM) native method. +class CompilerThreadCanCallJava : StackObj { + private: + CompilerThread* _current; // Only non-null if state of thread changed + bool _reset_state; // Value prior to state change, undefined + // if no state change. +public: + // Enters a scope in which the ability of the current CompilerThread + // to call Java is specified by `new_state`. This call only makes a + // change if the current thread is a CompilerThread associated with + // a JVMCI compiler whose CompilerThread::_can_call_java is not + // currently `new_state`. + CompilerThreadCanCallJava(JavaThread* current, bool new_state); + + // Resets CompilerThread::_can_call_java of the current thread if the + // constructor changed it. + ~CompilerThreadCanCallJava(); +}; + class JVMCI : public AllStatic { friend class JVMCIRuntime; friend class JVMCIEnv; diff --git a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp b/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp index 00a3984df8a..f2d58ae3fb3 100644 --- a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp +++ b/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp @@ -165,14 +165,19 @@ Handle JavaArgumentUnboxer::next_arg(BasicType expectedType) { MACOS_AARCH64_ONLY(ThreadWXEnable __wx(WXWrite, thread)); \ ThreadInVMfromNative __tiv(thread); \ HandleMarkCleaner __hm(thread); \ - JavaThread* THREAD = thread; \ + JavaThread* THREAD = thread; \ debug_only(VMNativeEntryWrapper __vew;) // Native method block that transitions current thread to '_thread_in_vm'. -#define C2V_BLOCK(result_type, name, signature) \ - JVMCI_VM_ENTRY_MARK; \ - ResourceMark rm; \ - JVMCIENV_FROM_JNI(JVMCI::compilation_tick(thread), env); +// Note: CompilerThreadCanCallJava must precede JVMCIENV_FROM_JNI so that +// the translation of an uncaught exception in the JVMCIEnv does not make +// a Java call when __is_hotspot == false. +#define C2V_BLOCK(result_type, name, signature) \ + JVMCI_VM_ENTRY_MARK; \ + ResourceMark rm; \ + bool __is_hotspot = env == thread->jni_environment(); \ + CompilerThreadCanCallJava ccj(thread, __is_hotspot); \ + JVMCIENV_FROM_JNI(JVMCI::compilation_tick(thread), env); \ static JavaThread* get_current_thread(bool allow_null=true) { Thread* thread = Thread::current_or_null_safe(); @@ -188,7 +193,7 @@ static JavaThread* get_current_thread(bool allow_null=true) { #define C2V_VMENTRY(result_type, name, signature) \ JNIEXPORT result_type JNICALL c2v_ ## name signature { \ JavaThread* thread = get_current_thread(); \ - if (thread == nullptr) { \ + if (thread == nullptr) { \ env->ThrowNew(JNIJVMCI::InternalError::clazz(), \ err_msg("Cannot call into HotSpot from JVMCI shared library without attaching current thread")); \ return; \ @@ -199,7 +204,7 @@ static JavaThread* get_current_thread(bool allow_null=true) { #define C2V_VMENTRY_(result_type, name, signature, result) \ JNIEXPORT result_type JNICALL c2v_ ## name signature { \ JavaThread* thread = get_current_thread(); \ - if (thread == nullptr) { \ + if (thread == nullptr) { \ env->ThrowNew(JNIJVMCI::InternalError::clazz(), \ err_msg("Cannot call into HotSpot from JVMCI shared library without attaching current thread")); \ return result; \ @@ -221,7 +226,7 @@ static JavaThread* get_current_thread(bool allow_null=true) { #define JNI_THROW(caller, name, msg) do { \ jint __throw_res = env->ThrowNew(JNIJVMCI::name::clazz(), msg); \ if (__throw_res != JNI_OK) { \ - tty->print_cr("Throwing " #name " in " caller " returned %d", __throw_res); \ + JVMCI_event_1("Throwing " #name " in " caller " returned %d", __throw_res); \ } \ return; \ } while (0); @@ -229,7 +234,7 @@ static JavaThread* get_current_thread(bool allow_null=true) { #define JNI_THROW_(caller, name, msg, result) do { \ jint __throw_res = env->ThrowNew(JNIJVMCI::name::clazz(), msg); \ if (__throw_res != JNI_OK) { \ - tty->print_cr("Throwing " #name " in " caller " returned %d", __throw_res); \ + JVMCI_event_1("Throwing " #name " in " caller " returned %d", __throw_res); \ } \ return result; \ } while (0) @@ -579,6 +584,7 @@ C2V_VMENTRY_0(jboolean, shouldInlineMethod,(JNIEnv* env, jobject, ARGUMENT_PAIR( C2V_END C2V_VMENTRY_NULL(jobject, lookupType, (JNIEnv* env, jobject, jstring jname, ARGUMENT_PAIR(accessing_klass), jint accessing_klass_loader, jboolean resolve)) + CompilerThreadCanCallJava canCallJava(thread, resolve); // Resolution requires Java calls JVMCIObject name = JVMCIENV->wrap(jname); const char* str = JVMCIENV->as_utf8_string(name); TempNewSymbol class_name = SymbolTable::new_symbol(str); @@ -592,7 +598,7 @@ C2V_VMENTRY_NULL(jobject, lookupType, (JNIEnv* env, jobject, jstring jname, ARGU if (val != nullptr) { if (strstr(val, "") != nullptr) { tty->print_cr("CompilerToVM.lookupType: %s", str); - } else if (strstr(val, str) != nullptr) { + } else if (strstr(str, val) != nullptr) { THROW_MSG_0(vmSymbols::java_lang_Exception(), err_msg("lookupTypeException: %s", str)); } @@ -938,6 +944,17 @@ C2V_VMENTRY_NULL(jobject, resolveFieldInPool, (JNIEnv* env, jobject, ARGUMENT_PA Bytecodes::Code code = (Bytecodes::Code)(((int) opcode) & 0xFF); fieldDescriptor fd; methodHandle mh(THREAD, UNPACK_PAIR(Method, method)); + + Bytecodes::Code bc = (Bytecodes::Code) (((int) opcode) & 0xFF); + int holder_index = cp->klass_ref_index_at(index, bc); + if (!cp->tag_at(holder_index).is_klass() && !THREAD->can_call_java()) { + // If the holder is not resolved in the constant pool and the current + // thread cannot call Java, return null. This avoids a Java call + // in LinkInfo to load the holder. + Symbol* klass_name = cp->klass_ref_at_noresolve(index, bc); + return nullptr; + } + LinkInfo link_info(cp, index, mh, code, CHECK_NULL); LinkResolver::resolve_field(fd, link_info, Bytecodes::java_code(code), false, CHECK_NULL); JVMCIPrimitiveArray info = JVMCIENV->wrap(info_handle); @@ -2726,6 +2743,7 @@ C2V_VMENTRY_0(jlong, translate, (JNIEnv* env, jobject, jobject obj_handle, jbool return 0L; } PEER_JVMCIENV_FROM_THREAD(THREAD, !JVMCIENV->is_hotspot()); + CompilerThreadCanCallJava canCallJava(thread, PEER_JVMCIENV->is_hotspot()); PEER_JVMCIENV->check_init(JVMCI_CHECK_0); JVMCIEnv* thisEnv = JVMCIENV; @@ -2945,18 +2963,21 @@ static jbyteArray get_encoded_annotation_data(InstanceKlass* holder, AnnotationA C2V_VMENTRY_NULL(jbyteArray, getEncodedClassAnnotationData, (JNIEnv* env, jobject, ARGUMENT_PAIR(klass), jobject filter, jint filter_length, jlong filter_klass_pointers)) + CompilerThreadCanCallJava canCallJava(thread, true); // Requires Java support InstanceKlass* holder = InstanceKlass::cast(UNPACK_PAIR(Klass, klass)); return get_encoded_annotation_data(holder, holder->class_annotations(), true, filter_length, filter_klass_pointers, THREAD, JVMCIENV); C2V_END C2V_VMENTRY_NULL(jbyteArray, getEncodedExecutableAnnotationData, (JNIEnv* env, jobject, ARGUMENT_PAIR(method), jobject filter, jint filter_length, jlong filter_klass_pointers)) + CompilerThreadCanCallJava canCallJava(thread, true); // Requires Java support methodHandle method(THREAD, UNPACK_PAIR(Method, method)); return get_encoded_annotation_data(method->method_holder(), method->annotations(), false, filter_length, filter_klass_pointers, THREAD, JVMCIENV); C2V_END C2V_VMENTRY_NULL(jbyteArray, getEncodedFieldAnnotationData, (JNIEnv* env, jobject, ARGUMENT_PAIR(klass), jint index, jobject filter, jint filter_length, jlong filter_klass_pointers)) + CompilerThreadCanCallJava canCallJava(thread, true); // Requires Java support InstanceKlass* holder = check_field(InstanceKlass::cast(UNPACK_PAIR(Klass, klass)), index, JVMCIENV); fieldDescriptor fd(holder, index); return get_encoded_annotation_data(holder, fd.annotations(), false, filter_length, filter_klass_pointers, THREAD, JVMCIENV); @@ -3013,6 +3034,7 @@ C2V_VMENTRY_0(jboolean, addFailedSpeculation, (JNIEnv* env, jobject, jlong faile C2V_END C2V_VMENTRY(void, callSystemExit, (JNIEnv* env, jobject, jint status)) + CompilerThreadCanCallJava canCallJava(thread, true); JavaValue result(T_VOID); JavaCallArguments jargs(1); jargs.push_int(status); diff --git a/src/hotspot/share/jvmci/jvmciEnv.cpp b/src/hotspot/share/jvmci/jvmciEnv.cpp index 24af7715a5b..f5a3e5b4d4d 100644 --- a/src/hotspot/share/jvmci/jvmciEnv.cpp +++ b/src/hotspot/share/jvmci/jvmciEnv.cpp @@ -448,6 +448,15 @@ class HotSpotToSharedLibraryExceptionTranslation : public ExceptionTranslation { private: const Handle& _throwable; + char* print_throwable_to_buffer(Handle throwable, jlong buffer, int buffer_size) { + char* char_buffer = (char*) buffer + 4; + stringStream st(char_buffer, (size_t) buffer_size - 4); + java_lang_Throwable::print_stack_trace(throwable, &st); + u4 len = (u4) st.size(); + *((u4*) buffer) = len; + return char_buffer; + } + bool handle_pending_exception(JavaThread* THREAD, jlong buffer, int buffer_size) { if (HAS_PENDING_EXCEPTION) { Handle throwable = Handle(THREAD, PENDING_EXCEPTION); @@ -457,11 +466,7 @@ class HotSpotToSharedLibraryExceptionTranslation : public ExceptionTranslation { JVMCI_event_1("error translating exception: OutOfMemoryError"); decode(THREAD, _encode_oome_fail, 0L); } else { - char* char_buffer = (char*) buffer + 4; - stringStream st(char_buffer, (size_t) buffer_size - 4); - java_lang_Throwable::print_stack_trace(throwable, &st); - u4 len = (u4) st.size(); - *((u4*) buffer) = len; + char* char_buffer = print_throwable_to_buffer(throwable, buffer, buffer_size); JVMCI_event_1("error translating exception: %s", char_buffer); decode(THREAD, _encode_fail, buffer); } @@ -471,6 +476,13 @@ class HotSpotToSharedLibraryExceptionTranslation : public ExceptionTranslation { } int encode(JavaThread* THREAD, jlong buffer, int buffer_size) { + if (!THREAD->can_call_java()) { + char* char_buffer = print_throwable_to_buffer(_throwable, buffer, buffer_size); + const char* detail = log_is_enabled(Info, exceptions) ? "" : " (-Xlog:exceptions may give more detail)"; + JVMCI_event_1("cannot call Java to translate exception%s: %s", detail, char_buffer); + decode(THREAD, _encode_fail, buffer); + return 0; + } Klass* vmSupport = SystemDictionary::resolve_or_fail(vmSymbols::jdk_internal_vm_VMSupport(), true, THREAD); if (handle_pending_exception(THREAD, buffer, buffer_size)) { return 0; @@ -1311,6 +1323,7 @@ JVMCIObject JVMCIEnv::get_jvmci_type(const JVMCIKlassHandle& klass, JVMCI_TRAPS) JavaThread* THREAD = JVMCI::compilation_tick(JavaThread::current()); // For exception macros. jboolean exception = false; if (is_hotspot()) { + CompilerThreadCanCallJava ccj(THREAD, true); JavaValue result(T_OBJECT); JavaCallArguments args; args.push_long(pointer); diff --git a/src/hotspot/share/jvmci/jvmciRuntime.cpp b/src/hotspot/share/jvmci/jvmciRuntime.cpp index da87341e5bf..5e9999f2733 100644 --- a/src/hotspot/share/jvmci/jvmciRuntime.cpp +++ b/src/hotspot/share/jvmci/jvmciRuntime.cpp @@ -1819,57 +1819,6 @@ Klass* JVMCIRuntime::get_klass_by_index(const constantPoolHandle& cpool, return result; } -// ------------------------------------------------------------------ -// Implementation of get_field_by_index. -// -// Implementation note: the results of field lookups are cached -// in the accessor klass. -void JVMCIRuntime::get_field_by_index_impl(InstanceKlass* klass, fieldDescriptor& field_desc, - int index, Bytecodes::Code bc) { - JVMCI_EXCEPTION_CONTEXT; - - assert(klass->is_linked(), "must be linked before using its constant-pool"); - - constantPoolHandle cpool(thread, klass->constants()); - - // Get the field's name, signature, and type. - Symbol* name = cpool->name_ref_at(index, bc); - - int nt_index = cpool->name_and_type_ref_index_at(index, bc); - int sig_index = cpool->signature_ref_index_at(nt_index); - Symbol* signature = cpool->symbol_at(sig_index); - - // Get the field's declared holder. - int holder_index = cpool->klass_ref_index_at(index, bc); - bool holder_is_accessible; - Klass* declared_holder = get_klass_by_index(cpool, holder_index, - holder_is_accessible, - klass); - - // The declared holder of this field may not have been loaded. - // Bail out with partial field information. - if (!holder_is_accessible) { - return; - } - - - // Perform the field lookup. - Klass* canonical_holder = - InstanceKlass::cast(declared_holder)->find_field(name, signature, &field_desc); - if (canonical_holder == nullptr) { - return; - } - - assert(canonical_holder == field_desc.field_holder(), "just checking"); -} - -// ------------------------------------------------------------------ -// Get a field by index from a klass's constant pool. -void JVMCIRuntime::get_field_by_index(InstanceKlass* accessor, fieldDescriptor& fd, int index, Bytecodes::Code bc) { - ResourceMark rm; - return get_field_by_index_impl(accessor, fd, index, bc); -} - // ------------------------------------------------------------------ // Perform an appropriate method lookup based on accessor, holder, // name, signature, and bytecode. diff --git a/src/hotspot/share/jvmci/jvmciRuntime.hpp b/src/hotspot/share/jvmci/jvmciRuntime.hpp index d3898be4ce0..c12c18abd78 100644 --- a/src/hotspot/share/jvmci/jvmciRuntime.hpp +++ b/src/hotspot/share/jvmci/jvmciRuntime.hpp @@ -231,8 +231,6 @@ class JVMCIRuntime: public CHeapObj { int klass_index, bool& is_accessible, Klass* loading_klass); - static void get_field_by_index_impl(InstanceKlass* loading_klass, fieldDescriptor& fd, - int field_index, Bytecodes::Code bc); static Method* get_method_by_index_impl(const constantPoolHandle& cpool, int method_index, Bytecodes::Code bc, InstanceKlass* loading_klass); @@ -417,8 +415,6 @@ class JVMCIRuntime: public CHeapObj { int klass_index, bool& is_accessible, Klass* loading_klass); - static void get_field_by_index(InstanceKlass* loading_klass, fieldDescriptor& fd, - int field_index, Bytecodes::Code bc); static Method* get_method_by_index(const constantPoolHandle& cpool, int method_index, Bytecodes::Code bc, InstanceKlass* loading_klass); diff --git a/src/hotspot/share/prims/upcallLinker.cpp b/src/hotspot/share/prims/upcallLinker.cpp index 8358649ac95..acaa3621672 100644 --- a/src/hotspot/share/prims/upcallLinker.cpp +++ b/src/hotspot/share/prims/upcallLinker.cpp @@ -79,7 +79,7 @@ JavaThread* UpcallLinker::on_entry(UpcallStub::FrameData* context, jobject recei guarantee(thread->thread_state() == _thread_in_native, "wrong thread state for upcall"); context->thread = thread; - assert(thread->can_call_java(), "must be able to call Java"); + guarantee(thread->can_call_java(), "must be able to call Java"); // Allocate handle block for Java code. This must be done before we change thread_state to _thread_in_Java, // since it can potentially block. diff --git a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/CompilerToVM.java b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/CompilerToVM.java index f3bf7f7cf1b..239529e0320 100644 --- a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/CompilerToVM.java +++ b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/CompilerToVM.java @@ -565,7 +565,8 @@ HotSpotResolvedObjectTypeImpl resolveTypeInPool(HotSpotConstantPool constantPool * The behavior of this method is undefined if {@code rawIndex} is invalid. * * @param info an array in which the details of the field are returned - * @return the type defining the field if resolution is successful, null otherwise + * @return the type defining the field if resolution is successful, null if the type cannot be resolved + * @throws LinkageError if there were other problems resolving the field */ HotSpotResolvedObjectTypeImpl resolveFieldInPool(HotSpotConstantPool constantPool, int rawIndex, HotSpotResolvedJavaMethodImpl method, byte opcode, int[] info) { long methodPointer = method != null ? method.getMethodPointer() : 0L; diff --git a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java index 98eb0093c09..98615247ce4 100644 --- a/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java +++ b/src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotConstantPool.java @@ -847,10 +847,10 @@ public JavaField lookupField(int rawIndex, ResolvedJavaMethod method, int opcode try { resolvedHolder = compilerToVM().resolveFieldInPool(this, rawIndex, (HotSpotResolvedJavaMethodImpl) method, (byte) opcode, info); } catch (Throwable t) { - /* - * If there was an exception resolving the field we give up and return an unresolved - * field. - */ + resolvedHolder = null; + } + if (resolvedHolder == null) { + // There was an exception resolving the field or it returned null so return an unresolved field. return new UnresolvedJavaField(fieldHolder, lookupUtf8(getNameRefIndexAt(nameAndTypeIndex)), type); } final int flags = info[0]; From bfaf5704e7e71f968b716b5f448860e9cda721b4 Mon Sep 17 00:00:00 2001 From: Ben Perez Date: Wed, 1 Nov 2023 16:49:50 +0000 Subject: [PATCH 09/12] 8311546: Certificate name constraints improperly validated with leading period Reviewed-by: mullan --- .../classes/sun/security/x509/DNSName.java | 12 +- .../security/x509/DNSName/LeadingPeriod.java | 113 ++++++++++++++++++ .../x509/DNSName/certs/generate-certs.sh | 91 ++++++++++++++ .../security/x509/DNSName/certs/openssl.cnf | 40 +++++++ .../DNSName/certs/withLeadingPeriod/ca.pem | 16 +++ .../DNSName/certs/withLeadingPeriod/leaf.pem | 12 ++ .../DNSName/certs/withoutLeadingPeriod/ca.pem | 16 +++ .../certs/withoutLeadingPeriod/leaf.pem | 12 ++ 8 files changed, 303 insertions(+), 9 deletions(-) create mode 100644 test/jdk/sun/security/x509/DNSName/LeadingPeriod.java create mode 100644 test/jdk/sun/security/x509/DNSName/certs/generate-certs.sh create mode 100644 test/jdk/sun/security/x509/DNSName/certs/openssl.cnf create mode 100644 test/jdk/sun/security/x509/DNSName/certs/withLeadingPeriod/ca.pem create mode 100644 test/jdk/sun/security/x509/DNSName/certs/withLeadingPeriod/leaf.pem create mode 100644 test/jdk/sun/security/x509/DNSName/certs/withoutLeadingPeriod/ca.pem create mode 100644 test/jdk/sun/security/x509/DNSName/certs/withoutLeadingPeriod/leaf.pem diff --git a/src/java.base/share/classes/sun/security/x509/DNSName.java b/src/java.base/share/classes/sun/security/x509/DNSName.java index 25443724f5a..597652022ce 100644 --- a/src/java.base/share/classes/sun/security/x509/DNSName.java +++ b/src/java.base/share/classes/sun/security/x509/DNSName.java @@ -200,18 +200,12 @@ public int hashCode() { * . These results are used in checking NameConstraints during * certification path verification. *

- * RFC5280: DNS name restrictions are expressed as host.example.com. + * RFC5280: For DNS names, restrictions MUST use the DNSName syntax in Section 4.2.1.6. * Any DNS name that can be constructed by simply adding zero or more * labels to the left-hand side of the name satisfies the name constraint. * For example, www.host.example.com would satisfy the constraint but * host1.example.com would not. *

- * RFC 5280: DNSName restrictions are expressed as foo.bar.com. - * Any DNSName that - * can be constructed by simply adding to the left-hand side of the name - * satisfies the name constraint. For example, www.foo.bar.com would - * satisfy the constraint but foo1.bar.com would not. - *

* RFC1034: By convention, domain names can be stored with arbitrary case, but * domain name comparisons for all present domain functions are done in a * case-insensitive manner, assuming an ASCII character set, and a high @@ -236,13 +230,13 @@ else if (inputName.getType() != NAME_DNS) constraintType = NAME_MATCH; else if (thisName.endsWith(inName)) { int inNdx = thisName.lastIndexOf(inName); - if (thisName.charAt(inNdx-1) == '.' ) + if (thisName.charAt(inNdx-1) == '.' ^ inName.charAt(0) == '.') constraintType = NAME_WIDENS; else constraintType = NAME_SAME_TYPE; } else if (inName.endsWith(thisName)) { int ndx = inName.lastIndexOf(thisName); - if (inName.charAt(ndx-1) == '.' ) + if (inName.charAt(ndx-1) == '.' ^ thisName.charAt(0) == '.') constraintType = NAME_NARROWS; else constraintType = NAME_SAME_TYPE; diff --git a/test/jdk/sun/security/x509/DNSName/LeadingPeriod.java b/test/jdk/sun/security/x509/DNSName/LeadingPeriod.java new file mode 100644 index 00000000000..50ce8e4b7ad --- /dev/null +++ b/test/jdk/sun/security/x509/DNSName/LeadingPeriod.java @@ -0,0 +1,113 @@ +/* + * 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 8311546 + * @summary Adopt de-facto standards on x509 Name Constraints with leading dot. Certs + * can be generated by running generate-certs.sh + * @library /test/lib + * @modules java.base/sun.security.x509 + */ + +import java.io.*; +import java.nio.file.*; +import java.util.*; +import java.security.Security; +import java.security.cert.*; + +public class LeadingPeriod { + + private static CertPath makeCertPath(String caStr, String targetCertStr) + throws CertificateException { + // generate certificate from cert strings + CertificateFactory cf = CertificateFactory.getInstance("X.509"); + + ByteArrayInputStream is; + + is = new ByteArrayInputStream(targetCertStr.getBytes()); + Certificate targetCert = cf.generateCertificate(is); + + is = new ByteArrayInputStream(caStr.getBytes()); + Certificate ca = cf.generateCertificate(is); + + // generate certification path + List list = List.of(targetCert, ca); + + return cf.generateCertPath(list); + } + + private static PKIXParameters genParams(String caStr) throws Exception { + // generate certificate from cert string + CertificateFactory cf = CertificateFactory.getInstance("X.509"); + + ByteArrayInputStream is = new ByteArrayInputStream(caStr.getBytes()); + Certificate selfSignedCert = cf.generateCertificate(is); + + // generate a trust anchor + TrustAnchor anchor = new TrustAnchor((X509Certificate) selfSignedCert, null); + + Set anchors = Collections.singleton(anchor); + + PKIXParameters params = new PKIXParameters(anchors); + + // disable certificate revocation checking + params.setRevocationEnabled(false); + + return params; + } + + public static void main(String[] args) throws Exception { + + CertPathValidator validator = CertPathValidator.getInstance("PKIX"); + + // Load certs with a NameConstraint where DNS value does not begin with a period + Path targetFromCAWithoutPeriodPath = Paths.get(System.getProperty( + "test.src", "./") + "/certs/withoutLeadingPeriod/leaf.pem"); + String targetFromCAWithoutPeriod = Files.readString(targetFromCAWithoutPeriodPath); + + Path caWithoutLeadingPeriodPath = Paths.get(System.getProperty( + "test.src", "./") + "/certs/withoutLeadingPeriod/ca.pem"); + String caWithoutLeadingPeriod = Files.readString(caWithoutLeadingPeriodPath); + + PKIXParameters paramsForCAWithoutLeadingPeriod = genParams(caWithoutLeadingPeriod); + CertPath pathWithoutLeadingPeriod = makeCertPath(caWithoutLeadingPeriod, + targetFromCAWithoutPeriod); + + validator.validate(pathWithoutLeadingPeriod, paramsForCAWithoutLeadingPeriod); + + // Load certificates with a NameConstraint where the DNS value does begin with a period + Path targetFromCAWithPeriodPath = Paths.get(System.getProperty( + "test.src", "./") + "/certs/withLeadingPeriod/leaf.pem"); + String targetFromCAWithPeriod = Files.readString(targetFromCAWithPeriodPath); + + Path caWithLeadingPeriodPath = Paths.get(System.getProperty( + "test.src", "./") + "/certs/withLeadingPeriod/ca.pem"); + String caWithLeadingPeriod = Files.readString(caWithLeadingPeriodPath); + + PKIXParameters paramsForCAWithLeadingPeriod = genParams(caWithLeadingPeriod); + CertPath pathWithLeadingPeriod = makeCertPath(caWithLeadingPeriod, targetFromCAWithPeriod); + + validator.validate(pathWithLeadingPeriod, paramsForCAWithLeadingPeriod); + } +} \ No newline at end of file diff --git a/test/jdk/sun/security/x509/DNSName/certs/generate-certs.sh b/test/jdk/sun/security/x509/DNSName/certs/generate-certs.sh new file mode 100644 index 00000000000..f000bfc9cda --- /dev/null +++ b/test/jdk/sun/security/x509/DNSName/certs/generate-certs.sh @@ -0,0 +1,91 @@ +#!/usr/bin/env bash + +set -e + +############################################################### +# CA with a leading period in the name constraint # +############################################################### +mkdir -p withLeadingPeriod + +openssl req \ + -newkey rsa:1024 \ + -keyout withLeadingPeriod/ca.key \ + -out withLeadingPeriod/ca.csr \ + -subj "/C=US/O=Example/CN=Example CA with period" \ + -nodes + +openssl x509 \ + -req \ + -in withLeadingPeriod/ca.csr \ + -extfile openssl.cnf \ + -extensions withLeadingPeriod \ + -signkey withLeadingPeriod/ca.key \ + -out withLeadingPeriod/ca.pem + +# leaf certificate +openssl req \ + -newkey rsa:1024 \ + -keyout withLeadingPeriod/leaf.key \ + -out withLeadingPeriod/leaf.csr \ + -subj '/CN=demo.example.com' \ + -addext 'subjectAltName = DNS:demo.example.com' \ + -nodes + +openssl x509 \ + -req \ + -in withLeadingPeriod/leaf.csr \ + -CAcreateserial \ + -CA withLeadingPeriod/ca.pem \ + -CAkey withLeadingPeriod/ca.key \ + -out withLeadingPeriod/leaf.pem + + +# ################################################################## +# # CA without a leading period in the name contraint # +# ################################################################## +mkdir -p withoutLeadingPeriod + +openssl req \ + -newkey rsa:1024 \ + -keyout withoutLeadingPeriod/ca.key \ + -out withoutLeadingPeriod/ca.csr \ + -subj "/C=US/O=Example/CN=Example CA without period" \ + -nodes + +openssl x509 \ + -req \ + -in withoutLeadingPeriod/ca.csr \ + -extfile openssl.cnf \ + -extensions withoutLeadingPeriod \ + -signkey withoutLeadingPeriod/ca.key \ + -out withoutLeadingPeriod/ca.pem + +# leaf certificate +openssl req \ + -newkey rsa:1024 \ + -keyout withoutLeadingPeriod/leaf.key \ + -out withoutLeadingPeriod/leaf.csr \ + -subj '/CN=demo.example.com' \ + -addext 'subjectAltName = DNS:demo.example.com' \ + -nodes + +openssl x509 \ + -req \ + -in withoutLeadingPeriod/leaf.csr \ + -CAcreateserial \ + -CA withoutLeadingPeriod/ca.pem \ + -CAkey withoutLeadingPeriod/ca.key \ + -out withoutLeadingPeriod/leaf.pem + + +# # Verify both leaf certificates + +set +e +openssl verify \ + -CAfile withLeadingPeriod/ca.pem \ + withLeadingPeriod/leaf.pem + +openssl verify \ + -CAfile withoutLeadingPeriod/ca.pem \ + withoutLeadingPeriod/leaf.pem + \ No newline at end of file diff --git a/test/jdk/sun/security/x509/DNSName/certs/openssl.cnf b/test/jdk/sun/security/x509/DNSName/certs/openssl.cnf new file mode 100644 index 00000000000..a69d206a5fc --- /dev/null +++ b/test/jdk/sun/security/x509/DNSName/certs/openssl.cnf @@ -0,0 +1,40 @@ +# +# OpenSSL configuration file. +# + +[ withLeadingPeriod ] +subjectKeyIdentifier = hash +authorityKeyIdentifier = keyid:always,issuer +basicConstraints = critical,CA:true +keyUsage = critical,keyCertSign +nameConstraints = critical,permitted;DNS:.example.com + +[ withoutLeadingPeriod ] +subjectKeyIdentifier = hash +authorityKeyIdentifier = keyid:always,issuer +basicConstraints = critical,CA:true +keyUsage = critical,keyCertSign +nameConstraints = critical,permitted;DNS:example.com + +[ v3_ca ] +subjectKeyIdentifier = hash +authorityKeyIdentifier = keyid:always,issuer +basicConstraints = critical,CA:true +keyUsage = critical,keyCertSign + + +[req] +distinguished_name = req_distinguished_name +x509_extensions = v3_ca # The extentions to add to self signed certs +req_extensions = v3_req # The extensions to add to req's + +prompt = no + +[req_distinguished_name] +C = US +O = Example +CN = example.com + +[v3_req] +keyUsage = keyEncipherment, dataEncipherment +extendedKeyUsage = serverAuth diff --git a/test/jdk/sun/security/x509/DNSName/certs/withLeadingPeriod/ca.pem b/test/jdk/sun/security/x509/DNSName/certs/withLeadingPeriod/ca.pem new file mode 100644 index 00000000000..535a8d9fc7e --- /dev/null +++ b/test/jdk/sun/security/x509/DNSName/certs/withLeadingPeriod/ca.pem @@ -0,0 +1,16 @@ +-----BEGIN CERTIFICATE----- +MIICgzCCAeygAwIBAgIJANBGv+BGZZHtMA0GCSqGSIb3DQEBCwUAMEAxCzAJBgNV +BAYTAlVTMRAwDgYDVQQKDAdFeGFtcGxlMR8wHQYDVQQDDBZFeGFtcGxlIENBIHdp +dGggcGVyaW9kMB4XDTIzMTAxOTIwNTE0NVoXDTIzMTExODIwNTE0NVowQDELMAkG +A1UEBhMCVVMxEDAOBgNVBAoMB0V4YW1wbGUxHzAdBgNVBAMMFkV4YW1wbGUgQ0Eg +d2l0aCBwZXJpb2QwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAPfaISk+Dvzk +m3AY7IDZYrGWpwxHciacalrsrOFl3mj3FQ/kVhofDri3mE7bxNKWyHNcbt+Cteck +TsGKBH85QsIifju7hqlrR+UbYtQF9/REkxX72gzim4xGk9KmKkuGpT5aZgvTE5eg +ZumJ9XxCjGn5nbsdJoqAE/9B96GqXJvlAgMBAAGjgYQwgYEwHQYDVR0OBBYEFG8h +vtka47iFUsc+3wmQ3LQRXUv3MB8GA1UdIwQYMBaAFG8hvtka47iFUsc+3wmQ3LQR +XUv3MA8GA1UdEwEB/wQFMAMBAf8wDgYDVR0PAQH/BAQDAgIEMB4GA1UdHgEB/wQU +MBKgEDAOggwuZXhhbXBsZS5jb20wDQYJKoZIhvcNAQELBQADgYEAkPeCbKokI067 +Dt2bommouO7LhTXivjMsByfZs8i9LZUVJz5Is+mDC36nLy4U3+QhofRLlEkWyOlE +033xBtm4YPsazpur79PJtvZemV0KwwMtKCoJYNlcy2llmdKjUwe4PsPnJPqH2KL5 +Cxios1gil8XL5OCmEUSCT9uBblh+9gk= +-----END CERTIFICATE----- diff --git a/test/jdk/sun/security/x509/DNSName/certs/withLeadingPeriod/leaf.pem b/test/jdk/sun/security/x509/DNSName/certs/withLeadingPeriod/leaf.pem new file mode 100644 index 00000000000..c587ca20c9a --- /dev/null +++ b/test/jdk/sun/security/x509/DNSName/certs/withLeadingPeriod/leaf.pem @@ -0,0 +1,12 @@ +-----BEGIN CERTIFICATE----- +MIIB0jCCATsCCQCgOCS7vOUOXTANBgkqhkiG9w0BAQsFADBAMQswCQYDVQQGEwJV +UzEQMA4GA1UECgwHRXhhbXBsZTEfMB0GA1UEAwwWRXhhbXBsZSBDQSB3aXRoIHBl +cmlvZDAeFw0yMzEwMTkyMDUxNDVaFw0yMzExMTgyMDUxNDVaMBsxGTAXBgNVBAMM +EGRlbW8uZXhhbXBsZS5jb20wgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBALxi +O4r50rNBbHu/blOSRfo9Vqei+QFS7fPwme68FOcvG+uYXf7zluO59V+8O4RPO+ZJ +W6meZvtpOCWFvlSMhBLTRz350LuSPWF+/tnbiyEv487Vi6Tp7kxJytIcMtH/sWkw +hF0Og8YYt3Xm2aLYPxZHGkvOccjau5X1xG1YiULzAgMBAAEwDQYJKoZIhvcNAQEL +BQADgYEA8OXnFO3yZSVmQfYvC2o9amYa7tNUPHgvEjp7xDlPkvL5zF+n8k0hT0kt +pv4BXzRqVIWsZcNi3H1wk6LMeUXi8EWCOR6gclWI0wZkWBhtoh8SCd2VJRmcv+zG +EnInCapszNKN05KEzaFOQv0QayILBUGgHTTXOgbEmryLHXg6zik= +-----END CERTIFICATE----- diff --git a/test/jdk/sun/security/x509/DNSName/certs/withoutLeadingPeriod/ca.pem b/test/jdk/sun/security/x509/DNSName/certs/withoutLeadingPeriod/ca.pem new file mode 100644 index 00000000000..cb886c34a1d --- /dev/null +++ b/test/jdk/sun/security/x509/DNSName/certs/withoutLeadingPeriod/ca.pem @@ -0,0 +1,16 @@ +-----BEGIN CERTIFICATE----- +MIICiDCCAfGgAwIBAgIJALUX88nU2b75MA0GCSqGSIb3DQEBCwUAMEMxCzAJBgNV +BAYTAlVTMRAwDgYDVQQKDAdFeGFtcGxlMSIwIAYDVQQDDBlFeGFtcGxlIENBIHdp +dGhvdXQgcGVyaW9kMB4XDTIzMTAxOTIwNTE0NVoXDTIzMTExODIwNTE0NVowQzEL +MAkGA1UEBhMCVVMxEDAOBgNVBAoMB0V4YW1wbGUxIjAgBgNVBAMMGUV4YW1wbGUg +Q0Egd2l0aG91dCBwZXJpb2QwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBANmH +HqVoDjgoS60aPhQisqEL3as81tPXxXgnp5M0TE+Lb0z/kXS2mkqYYhzcZneBhVyI +oGTnSnTSh6S1r/gE80x+hH4ZrLZR7jJMRDA9Q7RTOZBNgozS4XnE3AV/EjrIzHJ1 +IEt1ezoj2QNdVOv7UHprHGoARl9tdxre4MVUv4S3AgMBAAGjgYMwgYAwHQYDVR0O +BBYEFFG0Mvse4c5C01o8kvKiM4MKMJTCMB8GA1UdIwQYMBaAFFG0Mvse4c5C01o8 +kvKiM4MKMJTCMA8GA1UdEwEB/wQFMAMBAf8wDgYDVR0PAQH/BAQDAgIEMB0GA1Ud +HgEB/wQTMBGgDzANggtleGFtcGxlLmNvbTANBgkqhkiG9w0BAQsFAAOBgQC0CKS0 +JOR8hfUkZHBo52PrF3IKs33wczH5mfV+HLTdSeKtBD0Vj/7DoT0Yx2k5n6vpwA/x +LTSMC4wUo4hb5164ks45iloU0FrA+n9fWbeqwhQb+DW5MSOgoVLkW+rcdKbDatTO +ENcKZsqiG3aKM7pS7mQa+kUUpXWBUgVnhcsYtQ== +-----END CERTIFICATE----- diff --git a/test/jdk/sun/security/x509/DNSName/certs/withoutLeadingPeriod/leaf.pem b/test/jdk/sun/security/x509/DNSName/certs/withoutLeadingPeriod/leaf.pem new file mode 100644 index 00000000000..449176ff8a8 --- /dev/null +++ b/test/jdk/sun/security/x509/DNSName/certs/withoutLeadingPeriod/leaf.pem @@ -0,0 +1,12 @@ +-----BEGIN CERTIFICATE----- +MIIB1TCCAT4CCQDLscehyPPGXjANBgkqhkiG9w0BAQsFADBDMQswCQYDVQQGEwJV +UzEQMA4GA1UECgwHRXhhbXBsZTEiMCAGA1UEAwwZRXhhbXBsZSBDQSB3aXRob3V0 +IHBlcmlvZDAeFw0yMzEwMTkyMDUxNDVaFw0yMzExMTgyMDUxNDVaMBsxGTAXBgNV +BAMMEGRlbW8uZXhhbXBsZS5jb20wgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGB +AMZM9Prp5DkAe4pkLqn4v8DFamMtWVqPlYacJacGzBkuzBn8VNQQw4qnf6wiVHBj +uXLHrUCbldtKFK4XdVukmVyYSLR5BBPxA20fjZcsrBZW7u/14qWmIZW7G0xphezg +6g33qNPq9CPqVHR+IrfEmjWnLjc2KrZ3OQElpJOGp48hAgMBAAEwDQYJKoZIhvcN +AQELBQADgYEAYbIuAQKTXsgaBP3pyMqxPHvklDI7wJjEapbKDsOXYmKMb33pmFSD +ntQFZuOKYNV2rAqQaV/3kiWKyR4vO/gsCfuFeW8kxkhZEXX9CNU0Z6mKcvy274A4 +K0gqYGki8hyCc5IMWTUAX2TLdq8W1hwfbjeiNqTY21e+6lIa3kcuLC0= +-----END CERTIFICATE----- From f262f06c97b9ea94cd6119b3a8beb16bf804d083 Mon Sep 17 00:00:00 2001 From: Maurizio Cimadamore Date: Wed, 1 Nov 2023 17:49:21 +0000 Subject: [PATCH 10/12] 8319211: Regression in LoopOverNonConstantFP Reviewed-by: jvernee --- .../java/lang/invoke/MethodHandleStatics.java | 3 + .../classes/java/lang/invoke/VarHandles.java | 3 +- .../foreign/AbstractMemorySegmentImpl.java | 72 +++++++++---------- .../foreign/TestMemoryAccessInstance.java | 1 + 4 files changed, 42 insertions(+), 37 deletions(-) diff --git a/src/java.base/share/classes/java/lang/invoke/MethodHandleStatics.java b/src/java.base/share/classes/java/lang/invoke/MethodHandleStatics.java index 59b43d0b7d5..0191306881b 100644 --- a/src/java.base/share/classes/java/lang/invoke/MethodHandleStatics.java +++ b/src/java.base/share/classes/java/lang/invoke/MethodHandleStatics.java @@ -62,6 +62,7 @@ private MethodHandleStatics() { } // do not instantiate static final boolean VAR_HANDLE_GUARDS; static final int MAX_ARITY; static final boolean VAR_HANDLE_IDENTITY_ADAPT; + static final boolean VAR_HANDLE_SEGMENT_FORCE_EXACT; static final ClassFileDumper DUMP_CLASS_FILES; static { @@ -91,6 +92,8 @@ private MethodHandleStatics() { } // do not instantiate props.getProperty("java.lang.invoke.VarHandle.VAR_HANDLE_GUARDS", "true")); VAR_HANDLE_IDENTITY_ADAPT = Boolean.parseBoolean( props.getProperty("java.lang.invoke.VarHandle.VAR_HANDLE_IDENTITY_ADAPT", "false")); + VAR_HANDLE_SEGMENT_FORCE_EXACT = Boolean.parseBoolean( + props.getProperty("java.lang.invoke.VarHandle.VAR_HANDLE_SEGMENT_FORCE_EXACT", "false")); // Do not adjust this except for special platforms: MAX_ARITY = Integer.parseInt( diff --git a/src/java.base/share/classes/java/lang/invoke/VarHandles.java b/src/java.base/share/classes/java/lang/invoke/VarHandles.java index c0db5ddb866..0a393200447 100644 --- a/src/java.base/share/classes/java/lang/invoke/VarHandles.java +++ b/src/java.base/share/classes/java/lang/invoke/VarHandles.java @@ -41,6 +41,7 @@ import java.util.stream.Stream; import static java.lang.invoke.MethodHandleStatics.UNSAFE; +import static java.lang.invoke.MethodHandleStatics.VAR_HANDLE_SEGMENT_FORCE_EXACT; import static java.lang.invoke.MethodHandleStatics.VAR_HANDLE_IDENTITY_ADAPT; import static java.lang.invoke.MethodHandleStatics.newIllegalArgumentException; @@ -317,7 +318,7 @@ static VarHandle memorySegmentViewHandle(Class carrier, long alignmentMask, } long size = Utils.byteWidthOfPrimitive(carrier); boolean be = byteOrder == ByteOrder.BIG_ENDIAN; - boolean exact = false; + boolean exact = VAR_HANDLE_SEGMENT_FORCE_EXACT; if (carrier == byte.class) { return maybeAdapt(new VarHandleSegmentAsBytes(be, size, alignmentMask, exact)); diff --git a/src/java.base/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java b/src/java.base/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java index 361a10cf49b..de0ad6603d5 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java +++ b/src/java.base/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java @@ -736,235 +736,235 @@ private static int getScaleFactor(Buffer buffer) { @ForceInline @Override public byte get(ValueLayout.OfByte layout, long offset) { - return (byte) layout.varHandle().get(this, offset); + return (byte) layout.varHandle().get((MemorySegment)this, offset); } @ForceInline @Override public void set(ValueLayout.OfByte layout, long offset, byte value) { - layout.varHandle().set(this, offset, value); + layout.varHandle().set((MemorySegment)this, offset, value); } @ForceInline @Override public boolean get(ValueLayout.OfBoolean layout, long offset) { - return (boolean) layout.varHandle().get(this, offset); + return (boolean) layout.varHandle().get((MemorySegment)this, offset); } @ForceInline @Override public void set(ValueLayout.OfBoolean layout, long offset, boolean value) { - layout.varHandle().set(this, offset, value); + layout.varHandle().set((MemorySegment)this, offset, value); } @ForceInline @Override public char get(ValueLayout.OfChar layout, long offset) { - return (char) layout.varHandle().get(this, offset); + return (char) layout.varHandle().get((MemorySegment)this, offset); } @ForceInline @Override public void set(ValueLayout.OfChar layout, long offset, char value) { - layout.varHandle().set(this, offset, value); + layout.varHandle().set((MemorySegment)this, offset, value); } @ForceInline @Override public short get(ValueLayout.OfShort layout, long offset) { - return (short) layout.varHandle().get(this, offset); + return (short) layout.varHandle().get((MemorySegment)this, offset); } @ForceInline @Override public void set(ValueLayout.OfShort layout, long offset, short value) { - layout.varHandle().set(this, offset, value); + layout.varHandle().set((MemorySegment)this, offset, value); } @ForceInline @Override public int get(ValueLayout.OfInt layout, long offset) { - return (int) layout.varHandle().get(this, offset); + return (int) layout.varHandle().get((MemorySegment)this, offset); } @ForceInline @Override public void set(ValueLayout.OfInt layout, long offset, int value) { - layout.varHandle().set(this, offset, value); + layout.varHandle().set((MemorySegment)this, offset, value); } @ForceInline @Override public float get(ValueLayout.OfFloat layout, long offset) { - return (float) layout.varHandle().get(this, offset); + return (float) layout.varHandle().get((MemorySegment)this, offset); } @ForceInline @Override public void set(ValueLayout.OfFloat layout, long offset, float value) { - layout.varHandle().set(this, offset, value); + layout.varHandle().set((MemorySegment)this, offset, value); } @ForceInline @Override public long get(ValueLayout.OfLong layout, long offset) { - return (long) layout.varHandle().get(this, offset); + return (long) layout.varHandle().get((MemorySegment)this, offset); } @ForceInline @Override public void set(ValueLayout.OfLong layout, long offset, long value) { - layout.varHandle().set(this, offset, value); + layout.varHandle().set((MemorySegment)this, offset, value); } @ForceInline @Override public double get(ValueLayout.OfDouble layout, long offset) { - return (double) layout.varHandle().get(this, offset); + return (double) layout.varHandle().get((MemorySegment)this, offset); } @ForceInline @Override public void set(ValueLayout.OfDouble layout, long offset, double value) { - layout.varHandle().set(this, offset, value); + layout.varHandle().set((MemorySegment)this, offset, value); } @ForceInline @Override public MemorySegment get(AddressLayout layout, long offset) { - return (MemorySegment) layout.varHandle().get(this, offset); + return (MemorySegment) layout.varHandle().get((MemorySegment)this, offset); } @ForceInline @Override public void set(AddressLayout layout, long offset, MemorySegment value) { - layout.varHandle().set(this, offset, value); + layout.varHandle().set((MemorySegment)this, offset, value); } @ForceInline @Override public byte getAtIndex(ValueLayout.OfByte layout, long index) { Utils.checkElementAlignment(layout, "Layout alignment greater than its size"); - return (byte) layout.varHandle().get(this, index * layout.byteSize()); + return (byte) layout.varHandle().get((MemorySegment)this, index * layout.byteSize()); } @ForceInline @Override public boolean getAtIndex(ValueLayout.OfBoolean layout, long index) { Utils.checkElementAlignment(layout, "Layout alignment greater than its size"); - return (boolean) layout.varHandle().get(this, index * layout.byteSize()); + return (boolean) layout.varHandle().get((MemorySegment)this, index * layout.byteSize()); } @ForceInline @Override public char getAtIndex(ValueLayout.OfChar layout, long index) { Utils.checkElementAlignment(layout, "Layout alignment greater than its size"); - return (char) layout.varHandle().get(this, index * layout.byteSize()); + return (char) layout.varHandle().get((MemorySegment)this, index * layout.byteSize()); } @ForceInline @Override public void setAtIndex(ValueLayout.OfChar layout, long index, char value) { Utils.checkElementAlignment(layout, "Layout alignment greater than its size"); - layout.varHandle().set(this, index * layout.byteSize(), value); + layout.varHandle().set((MemorySegment)this, index * layout.byteSize(), value); } @ForceInline @Override public short getAtIndex(ValueLayout.OfShort layout, long index) { Utils.checkElementAlignment(layout, "Layout alignment greater than its size"); - return (short) layout.varHandle().get(this, index * layout.byteSize()); + return (short) layout.varHandle().get((MemorySegment)this, index * layout.byteSize()); } @ForceInline @Override public void setAtIndex(ValueLayout.OfByte layout, long index, byte value) { Utils.checkElementAlignment(layout, "Layout alignment greater than its size"); - layout.varHandle().set(this, index * layout.byteSize(), value); + layout.varHandle().set((MemorySegment)this, index * layout.byteSize(), value); } @ForceInline @Override public void setAtIndex(ValueLayout.OfBoolean layout, long index, boolean value) { Utils.checkElementAlignment(layout, "Layout alignment greater than its size"); - layout.varHandle().set(this, index * layout.byteSize(), value); + layout.varHandle().set((MemorySegment)this, index * layout.byteSize(), value); } @ForceInline @Override public void setAtIndex(ValueLayout.OfShort layout, long index, short value) { Utils.checkElementAlignment(layout, "Layout alignment greater than its size"); - layout.varHandle().set(this, index * layout.byteSize(), value); + layout.varHandle().set((MemorySegment)this, index * layout.byteSize(), value); } @ForceInline @Override public int getAtIndex(ValueLayout.OfInt layout, long index) { Utils.checkElementAlignment(layout, "Layout alignment greater than its size"); - return (int) layout.varHandle().get(this, index * layout.byteSize()); + return (int) layout.varHandle().get((MemorySegment)this, index * layout.byteSize()); } @ForceInline @Override public void setAtIndex(ValueLayout.OfInt layout, long index, int value) { Utils.checkElementAlignment(layout, "Layout alignment greater than its size"); - layout.varHandle().set(this, index * layout.byteSize(), value); + layout.varHandle().set((MemorySegment)this, index * layout.byteSize(), value); } @ForceInline @Override public float getAtIndex(ValueLayout.OfFloat layout, long index) { Utils.checkElementAlignment(layout, "Layout alignment greater than its size"); - return (float) layout.varHandle().get(this, index * layout.byteSize()); + return (float) layout.varHandle().get((MemorySegment)this, index * layout.byteSize()); } @ForceInline @Override public void setAtIndex(ValueLayout.OfFloat layout, long index, float value) { Utils.checkElementAlignment(layout, "Layout alignment greater than its size"); - layout.varHandle().set(this, index * layout.byteSize(), value); + layout.varHandle().set((MemorySegment)this, index * layout.byteSize(), value); } @ForceInline @Override public long getAtIndex(ValueLayout.OfLong layout, long index) { Utils.checkElementAlignment(layout, "Layout alignment greater than its size"); - return (long) layout.varHandle().get(this, index * layout.byteSize()); + return (long) layout.varHandle().get((MemorySegment)this, index * layout.byteSize()); } @ForceInline @Override public void setAtIndex(ValueLayout.OfLong layout, long index, long value) { Utils.checkElementAlignment(layout, "Layout alignment greater than its size"); - layout.varHandle().set(this, index * layout.byteSize(), value); + layout.varHandle().set((MemorySegment)this, index * layout.byteSize(), value); } @ForceInline @Override public double getAtIndex(ValueLayout.OfDouble layout, long index) { Utils.checkElementAlignment(layout, "Layout alignment greater than its size"); - return (double) layout.varHandle().get(this, index * layout.byteSize()); + return (double) layout.varHandle().get((MemorySegment)this, index * layout.byteSize()); } @ForceInline @Override public void setAtIndex(ValueLayout.OfDouble layout, long index, double value) { Utils.checkElementAlignment(layout, "Layout alignment greater than its size"); - layout.varHandle().set(this, index * layout.byteSize(), value); + layout.varHandle().set((MemorySegment)this, index * layout.byteSize(), value); } @ForceInline @Override public MemorySegment getAtIndex(AddressLayout layout, long index) { Utils.checkElementAlignment(layout, "Layout alignment greater than its size"); - return (MemorySegment) layout.varHandle().get(this, index * layout.byteSize()); + return (MemorySegment) layout.varHandle().get((MemorySegment)this, index * layout.byteSize()); } @ForceInline @Override public void setAtIndex(AddressLayout layout, long index, MemorySegment value) { Utils.checkElementAlignment(layout, "Layout alignment greater than its size"); - layout.varHandle().set(this, index * layout.byteSize(), value); + layout.varHandle().set((MemorySegment)this, index * layout.byteSize(), value); } @Override diff --git a/test/jdk/java/foreign/TestMemoryAccessInstance.java b/test/jdk/java/foreign/TestMemoryAccessInstance.java index fbcdc5f89b5..a1c774b72dd 100644 --- a/test/jdk/java/foreign/TestMemoryAccessInstance.java +++ b/test/jdk/java/foreign/TestMemoryAccessInstance.java @@ -24,6 +24,7 @@ /* * @test * @run testng/othervm --enable-native-access=ALL-UNNAMED TestMemoryAccessInstance + * @run testng/othervm -Djava.lang.invoke.VarHandle.VAR_HANDLE_SEGMENT_FORCE_EXACT=true --enable-native-access=ALL-UNNAMED TestMemoryAccessInstance */ import java.lang.foreign.MemoryLayout; From ee57e731d03101ba6508297ef7d895082b04b427 Mon Sep 17 00:00:00 2001 From: Justin Lu Date: Wed, 1 Nov 2023 21:29:45 +0000 Subject: [PATCH 11/12] 8317612: ChoiceFormat and MessageFormat constructors call non-final public method Reviewed-by: naoto, lancea --- .../share/classes/java/text/ChoiceFormat.java | 34 ++++++++++++++----- .../classes/java/text/MessageFormat.java | 33 +++++++++++++----- 2 files changed, 50 insertions(+), 17 deletions(-) diff --git a/src/java.base/share/classes/java/text/ChoiceFormat.java b/src/java.base/share/classes/java/text/ChoiceFormat.java index 6f27137ea0d..510a1d37029 100644 --- a/src/java.base/share/classes/java/text/ChoiceFormat.java +++ b/src/java.base/share/classes/java/text/ChoiceFormat.java @@ -246,6 +246,17 @@ public class ChoiceFormat extends NumberFormat { * @see #ChoiceFormat(String) */ public void applyPattern(String newPattern) { + applyPatternImpl(newPattern); + } + + /** + * Implementation of applying a pattern to this ChoiceFormat. + * This method processes a String pattern in accordance with the ChoiceFormat + * pattern syntax and populates the internal {@code limits} and {@code formats} + * array variables. See the {@linkplain ##patterns} section for + * further understanding of certain special characters: "#", "<", "\u2264", "|". + */ + private void applyPatternImpl(String newPattern) { StringBuilder[] segments = new StringBuilder[2]; for (int i = 0; i < segments.length; ++i) { segments[i] = new StringBuilder(); @@ -326,8 +337,8 @@ public void applyPattern(String newPattern) { } /** - * {@return a pattern {@code string} that represents the the limits and formats - * of this ChoiceFormat object} + * {@return a pattern {@code string} that represents the {@code limits} and + * {@code formats} of this ChoiceFormat object} * * The {@code string} returned is not guaranteed to be the same input * {@code string} passed to either {@link #applyPattern(String)} or @@ -396,7 +407,7 @@ public String toPattern() { * @see #applyPattern */ public ChoiceFormat(String newPattern) { - applyPattern(newPattern); + applyPatternImpl(newPattern); } /** @@ -411,11 +422,12 @@ public ChoiceFormat(String newPattern) { * @see #setChoices */ public ChoiceFormat(double[] limits, String[] formats) { - setChoices(limits, formats); + setChoicesImpl(limits, formats); } /** * Set the choices to be used in formatting. + * * @param limits contains the top value that you want * parsed with that format, and should be in ascending sorted order. When * formatting X, the choice will be the i, where @@ -429,6 +441,14 @@ public ChoiceFormat(double[] limits, String[] formats) { * and {@code formats} are not equal */ public void setChoices(double[] limits, String[] formats) { + setChoicesImpl(limits, formats); + } + + /** + * Implementation of populating the {@code limits} and + * {@code formats} of this ChoiceFormat. Defensive copies are made. + */ + private void setChoicesImpl(double[] limits, String[] formats) { if (limits.length != formats.length) { throw new IllegalArgumentException( "Input arrays must be of the same length."); @@ -441,16 +461,14 @@ public void setChoices(double[] limits, String[] formats) { * {@return the limits of this ChoiceFormat} */ public double[] getLimits() { - double[] newLimits = Arrays.copyOf(choiceLimits, choiceLimits.length); - return newLimits; + return Arrays.copyOf(choiceLimits, choiceLimits.length); } /** * {@return the formats of this ChoiceFormat} */ public Object[] getFormats() { - Object[] newFormats = Arrays.copyOf(choiceFormats, choiceFormats.length); - return newFormats; + return Arrays.copyOf(choiceFormats, choiceFormats.length); } // Overrides diff --git a/src/java.base/share/classes/java/text/MessageFormat.java b/src/java.base/share/classes/java/text/MessageFormat.java index 89acc14cdb1..e03c1d3decf 100644 --- a/src/java.base/share/classes/java/text/MessageFormat.java +++ b/src/java.base/share/classes/java/text/MessageFormat.java @@ -371,7 +371,7 @@ public class MessageFormat extends Format { * The constructor first sets the locale, then parses the pattern and * creates a list of subformats for the format elements contained in it. * Patterns and their interpretation are specified in the - * class description. + * {@linkplain ##patterns class description}. * * @param pattern the pattern for this message format * @throws IllegalArgumentException if the pattern is invalid @@ -380,7 +380,7 @@ public class MessageFormat extends Format { */ public MessageFormat(String pattern) { this.locale = Locale.getDefault(Locale.Category.FORMAT); - applyPattern(pattern); + applyPatternImpl(pattern); } /** @@ -389,7 +389,7 @@ public MessageFormat(String pattern) { * The constructor first sets the locale, then parses the pattern and * creates a list of subformats for the format elements contained in it. * Patterns and their interpretation are specified in the - * class description. + * {@linkplain ##patterns class description}. * * @implSpec The default implementation throws a * {@code NullPointerException} if {@code locale} is {@code null} @@ -408,7 +408,7 @@ public MessageFormat(String pattern) { */ public MessageFormat(String pattern, Locale locale) { this.locale = locale; - applyPattern(pattern); + applyPatternImpl(pattern); } /** @@ -447,15 +447,29 @@ public Locale getLocale() { * The method parses the pattern and creates a list of subformats * for the format elements contained in it. * Patterns and their interpretation are specified in the - * class description. + * {@linkplain ##patterns class description}. * * @param pattern the pattern for this message format * @throws IllegalArgumentException if the pattern is invalid * @throws NullPointerException if {@code pattern} is * {@code null} */ - @SuppressWarnings("fallthrough") // fallthrough in switch is expected, suppress it public void applyPattern(String pattern) { + applyPatternImpl(pattern); + } + + /** + * Implementation of applying a pattern to this MessageFormat. + * This method processes a String pattern in accordance with the MessageFormat + * pattern syntax and sets the internal {@code pattern} variable as well as + * populating the {@code formats} array with the subformats defined in the + * pattern. See the {@linkplain ##patterns} section for further understanding + * of certain special characters: "{", "}", ",". See {@linkplain + * ##makeFormat(int, int, StringBuilder[])} for the implementation of setting + * a subformat. + */ + @SuppressWarnings("fallthrough") // fallthrough in switch is expected, suppress it + private void applyPatternImpl(String pattern) { StringBuilder[] segments = new StringBuilder[4]; // Allocate only segments[SEG_RAW] here. The rest are // allocated on demand. @@ -509,6 +523,7 @@ public void applyPattern(String pattern) { case '}': if (braceStack == 0) { part = SEG_RAW; + // Set the subformat makeFormat(i, formatNumber, segments); formatNumber++; // throw away other segments @@ -1592,7 +1607,7 @@ private void makeFormat(int position, int offsetNumber, formats[offsetNumber] = newFormat; } - private static final int findKeyword(String s, String[] list) { + private static int findKeyword(String s, String[] list) { for (int i = 0; i < list.length; ++i) { if (s.equals(list[i])) return i; @@ -1609,8 +1624,8 @@ private static final int findKeyword(String s, String[] list) { return -1; } - private static final void copyAndFixQuotes(String source, int start, int end, - StringBuilder target) { + private static void copyAndFixQuotes(String source, int start, int end, + StringBuilder target) { boolean quoted = false; for (int i = start; i < end; ++i) { From 5207443b360cfe3ee9c53ece55da3464c13f6a9f Mon Sep 17 00:00:00 2001 From: Mandy Chung Date: Wed, 1 Nov 2023 22:19:57 +0000 Subject: [PATCH 12/12] 8317965: TestLoadLibraryDeadlock.java fails with "Unable to load native library.: expected true, was false" Reviewed-by: rriggs --- .../LoadLibraryDeadlock.java | 15 ++- .../TestLoadLibraryDeadlock.java | 112 ++++-------------- 2 files changed, 35 insertions(+), 92 deletions(-) diff --git a/test/jdk/java/lang/ClassLoader/loadLibraryDeadlock/LoadLibraryDeadlock.java b/test/jdk/java/lang/ClassLoader/loadLibraryDeadlock/LoadLibraryDeadlock.java index 42ecd372aec..bab4d828457 100644 --- a/test/jdk/java/lang/ClassLoader/loadLibraryDeadlock/LoadLibraryDeadlock.java +++ b/test/jdk/java/lang/ClassLoader/loadLibraryDeadlock/LoadLibraryDeadlock.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, 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. * @@ -37,12 +37,14 @@ public class LoadLibraryDeadlock { public static void main(String[] args) { + System.out.println("LoadLibraryDeadlock test started"); Thread t1 = new Thread() { public void run() { try { // an instance of unsigned class that loads a native library Class c1 = Class.forName("Class1"); Object o = c1.newInstance(); + System.out.println("Class1 loaded from " + getLocation(c1)); } catch (ClassNotFoundException | InstantiationException | IllegalAccessException e) { @@ -56,7 +58,7 @@ public void run() { try { // load a class from a signed jar, which locks the JarFile Class c2 = Class.forName("p.Class2"); - System.out.println("Signed jar loaded."); + System.out.println("Class2 loaded from " + getLocation(c2)); } catch (ClassNotFoundException e) { System.out.println("Class Class2 not found."); throw new RuntimeException(e); @@ -68,7 +70,14 @@ public void run() { try { t1.join(); t2.join(); - } catch (InterruptedException ignore) { + } catch (InterruptedException ex) { + throw new RuntimeException(ex); } } + + private static String getLocation(Class c) { + var pd = c.getProtectionDomain(); + var cs = pd != null ? pd.getCodeSource() : null; + return cs != null ? cs.getLocation().getPath() : null; + } } diff --git a/test/jdk/java/lang/ClassLoader/loadLibraryDeadlock/TestLoadLibraryDeadlock.java b/test/jdk/java/lang/ClassLoader/loadLibraryDeadlock/TestLoadLibraryDeadlock.java index 65a485b34a3..0e031c8610b 100644 --- a/test/jdk/java/lang/ClassLoader/loadLibraryDeadlock/TestLoadLibraryDeadlock.java +++ b/test/jdk/java/lang/ClassLoader/loadLibraryDeadlock/TestLoadLibraryDeadlock.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, 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,13 +38,14 @@ import jdk.test.lib.util.FileUtils; import java.lang.ProcessBuilder; -import java.lang.Process; +import java.nio.file.Path; import java.nio.file.Paths; import java.io.*; import java.util.*; -import java.util.concurrent.*; import java.util.spi.ToolProvider; +import static jdk.test.lib.process.ProcessTools.*; + public class TestLoadLibraryDeadlock { private static final ToolProvider JAR = ToolProvider.findFirst("jar") @@ -108,53 +109,6 @@ private static OutputAnalyzer signJar(String jarToSign) throws Throwable { ); } - 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 static OutputAnalyzer jcmd(long pid, String command) throws Throwable { - String jcmd = JDKToolFinder.getJDKTool("jcmd"); - return runCommandInTestClassPath(jcmd, - String.valueOf(pid), - command - ); - } - - private static String readAvailable(final InputStream is) throws Throwable { - final List list = Collections.synchronizedList(new ArrayList()); - ExecutorService executor = Executors.newFixedThreadPool(2); - Future future = executor.submit(new Callable() { - public String call() { - String result = new String(); - BufferedReader reader = new BufferedReader(new InputStreamReader(is)); - try { - while(true) { - String s = reader.readLine(); - if (s.length() > 0) { - list.add(s); - result += s + "\n"; - } - } - } catch (IOException ignore) {} - return result; - } - }); - try { - return future.get(1000, TimeUnit.MILLISECONDS); - } catch (Exception ignoreAll) { - future.cancel(true); - return String.join("\n", list); - } - } - private final static long countLines(OutputAnalyzer output, String string) { return output.asLines() .stream() @@ -162,22 +116,17 @@ private final static long countLines(OutputAnalyzer output, String string) { .count(); } - private final static void dump(OutputAnalyzer output) { - output.asLines() - .stream() - .forEach(s -> System.out.println(s)); - } - public static void main(String[] args) throws Throwable { genKey() .shouldHaveExitValue(0); - FileUtils.deleteFileIfExistsWithRetry( - Paths.get(testClassPath, "a.jar")); - FileUtils.deleteFileIfExistsWithRetry( - Paths.get(testClassPath, "b.jar")); - FileUtils.deleteFileIfExistsWithRetry( - Paths.get(testClassPath, "c.jar")); + Path aJar = Path.of(testClassPath, "a.jar"); + Path bJar = Path.of(testClassPath, "b.jar"); + Path cJar = Path.of(testClassPath, "c.jar"); + + FileUtils.deleteFileIfExistsWithRetry(aJar); + FileUtils.deleteFileIfExistsWithRetry(bJar); + FileUtils.deleteFileIfExistsWithRetry(cJar); createJar("a.jar", "LoadLibraryDeadlock.class", @@ -194,24 +143,13 @@ public static void main(String[] args) throws Throwable { .shouldHaveExitValue(0); // load trigger class - Process process = runJavaCommand("-cp", - "a.jar" + classPathSeparator + - "b.jar" + classPathSeparator + - "c.jar", + OutputAnalyzer outputAnalyzer = executeCommand(createTestJavaProcessBuilder("-cp", + aJar.toString() + classPathSeparator + + bJar.toString() + classPathSeparator + + cJar.toString(), "-Djava.library.path=" + testLibraryPath, - "LoadLibraryDeadlock"); - - // wait for a while to grab some output - process.waitFor(5, TimeUnit.SECONDS); - - // dump available output - String output = readAvailable(process.getInputStream()); - OutputAnalyzer outputAnalyzer = new OutputAnalyzer(output); - dump(outputAnalyzer); - - // if the process is still running, get the thread dump - OutputAnalyzer outputAnalyzerJcmd = jcmd(process.pid(), "Thread.print"); - dump(outputAnalyzerJcmd); + "LoadLibraryDeadlock")); + outputAnalyzer.shouldHaveExitValue(0); Asserts.assertTrue( countLines(outputAnalyzer, "Java-level deadlock") == 0, @@ -231,19 +169,15 @@ public static void main(String[] args) throws Throwable { "Unable to load native library."); Asserts.assertTrue( - countLines(outputAnalyzer, "Signed jar loaded.") > 0, - "Unable to load signed jar."); + countLines(outputAnalyzer, "Class1 loaded from " + bJar) > 0, + "Unable to load b.jar."); + + Asserts.assertTrue( + countLines(outputAnalyzer, "Class2 loaded from " + cJar) > 0, + "Unable to load signed c.jar."); Asserts.assertTrue( countLines(outputAnalyzer, "Signed jar loaded from native library.") > 0, "Unable to load signed jar from native library."); - - if (!process.waitFor(5, TimeUnit.SECONDS)) { - // if the process is still frozen, fail the test even though - // the "deadlock" text hasn't been found - process.destroyForcibly(); - Asserts.assertTrue(process.waitFor() == 0, - "Process frozen."); - } } }