From 03204600c596214895ef86581eba9722f76d39b3 Mon Sep 17 00:00:00 2001 From: Andrew Haley Date: Mon, 12 Aug 2024 07:38:43 +0000 Subject: [PATCH] 8337958: Out-of-bounds array access in secondary_super_cache Reviewed-by: vlivanov, shade --- src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp | 4 ++-- src/hotspot/cpu/riscv/macroAssembler_riscv.cpp | 4 ++-- src/hotspot/cpu/x86/macroAssembler_x86.cpp | 5 ++--- src/hotspot/share/oops/klass.cpp | 13 ++++++++----- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp index 25dfaca6dca..9dd4371cf69 100644 --- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp @@ -1730,8 +1730,8 @@ void MacroAssembler::lookup_secondary_supers_table_slow_path(Register r_super_kl // The bitmap is full to bursting. // Implicit invariant: BITMAP_FULL implies (length > 0) assert(Klass::SECONDARY_SUPERS_BITMAP_FULL == ~uintx(0), ""); - cmn(r_bitmap, (u1)1); - br(EQ, L_huge); + cmpw(r_array_length, (u1)(Klass::SECONDARY_SUPERS_TABLE_SIZE - 2)); + br(GT, L_huge); // NB! Our caller has checked bits 0 and 1 in the bitmap. The // current slot (at secondary_supers[r_array_index]) has not yet diff --git a/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp b/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp index e0b24b7fd66..c6624cb45bb 100644 --- a/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp +++ b/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp @@ -3973,8 +3973,8 @@ void MacroAssembler::lookup_secondary_supers_table_slow_path(Register r_super_kl // Check if bitmap is SECONDARY_SUPERS_BITMAP_FULL assert(Klass::SECONDARY_SUPERS_BITMAP_FULL == ~uintx(0), "Adjust this code"); - addi(t0, r_bitmap, (u1)1); - beqz(t0, L_bitmap_full); + subw(t0, r_array_length, Klass::SECONDARY_SUPERS_TABLE_SIZE - 2); + bgtz(t0, L_bitmap_full); // NB! Our caller has checked bits 0 and 1 in the bitmap. The // current slot (at secondary_supers[r_array_index]) has not yet diff --git a/src/hotspot/cpu/x86/macroAssembler_x86.cpp b/src/hotspot/cpu/x86/macroAssembler_x86.cpp index c8234d6b9b8..78bcabb2000 100644 --- a/src/hotspot/cpu/x86/macroAssembler_x86.cpp +++ b/src/hotspot/cpu/x86/macroAssembler_x86.cpp @@ -4945,9 +4945,8 @@ void MacroAssembler::lookup_secondary_supers_table_slow_path(Register r_super_kl // The bitmap is full to bursting. // Implicit invariant: BITMAP_FULL implies (length > 0) - assert(Klass::SECONDARY_SUPERS_BITMAP_FULL == ~uintx(0), ""); - cmpq(r_bitmap, (int32_t)-1); // sign-extends immediate to 64-bit value - jcc(Assembler::equal, L_huge); + cmpl(r_array_length, (int32_t)Klass::SECONDARY_SUPERS_TABLE_SIZE - 2); + jcc(Assembler::greater, L_huge); // NB! Our caller has checked bits 0 and 1 in the bitmap. The // current slot (at secondary_supers[r_array_index]) has not yet diff --git a/src/hotspot/share/oops/klass.cpp b/src/hotspot/share/oops/klass.cpp index 1fed317a860..964bb030b96 100644 --- a/src/hotspot/share/oops/klass.cpp +++ b/src/hotspot/share/oops/klass.cpp @@ -306,6 +306,7 @@ void Klass::set_secondary_supers(Array* secondaries, uintx bitmap) { if (UseSecondarySupersTable && secondaries != nullptr) { uintx real_bitmap = compute_secondary_supers_bitmap(secondaries); assert(bitmap == real_bitmap, "must be"); + assert(secondaries->length() >= (int)population_count(bitmap), "must be"); } #endif _bitmap = bitmap; @@ -344,11 +345,12 @@ uintx Klass::hash_secondary_supers(Array* secondaries, bool rewrite) { return uintx(1) << hash_slot; } - // For performance reasons we don't use a hashed table unless there - // are at least two empty slots in it. If there were only one empty - // slot it'd take a long time to create the table and the resulting - // search would be no faster than linear probing. - if (length > SECONDARY_SUPERS_TABLE_SIZE - 2) { + // Invariant: _secondary_supers.length >= population_count(_secondary_supers_bitmap) + + // Don't attempt to hash a table that's completely full, because in + // the case of an absent interface linear probing would not + // terminate. + if (length >= SECONDARY_SUPERS_TABLE_SIZE) { return SECONDARY_SUPERS_BITMAP_FULL; } @@ -788,6 +790,7 @@ void Klass::remove_java_mirror() { void Klass::restore_unshareable_info(ClassLoaderData* loader_data, Handle protection_domain, TRAPS) { assert(is_klass(), "ensure C++ vtable is restored"); assert(is_shared(), "must be set"); + assert(secondary_supers()->length() >= (int)population_count(_bitmap), "must be"); JFR_ONLY(RESTORE_ID(this);) if (log_is_enabled(Trace, cds, unshareable)) { ResourceMark rm(THREAD);