Skip to content

Commit 677556d

Browse files
shipilevshipilev
committed
8320275: assert(_chunk->bitmap().at(index)) failed: Bit not set at index
8323595: is_aligned(p, alignof(OopT))) assertion fails in Jetty without compressed OOPs Reviewed-by: phh Backport-of: e9e694f4ef7b080d7fe1ad5b2f2daa2fccd0456e
1 parent 8ac4313 commit 677556d

17 files changed

+72
-43
lines changed

‎src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp

+8-4
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
310310

311311
uint int_args = 0;
312312
uint fp_args = 0;
313-
uint stk_args = 0; // inc by 2 each time
313+
uint stk_args = 0;
314314

315315
for (int i = 0; i < total_args_passed; i++) {
316316
switch (sig_bt[i]) {
@@ -322,8 +322,9 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
322322
if (int_args < Argument::n_int_register_parameters_j) {
323323
regs[i].set1(INT_ArgReg[int_args++]->as_VMReg());
324324
} else {
325+
stk_args = align_up(stk_args, 2);
325326
regs[i].set1(VMRegImpl::stack2reg(stk_args));
326-
stk_args += 2;
327+
stk_args += 1;
327328
}
328329
break;
329330
case T_VOID:
@@ -340,6 +341,7 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
340341
if (int_args < Argument::n_int_register_parameters_j) {
341342
regs[i].set2(INT_ArgReg[int_args++]->as_VMReg());
342343
} else {
344+
stk_args = align_up(stk_args, 2);
343345
regs[i].set2(VMRegImpl::stack2reg(stk_args));
344346
stk_args += 2;
345347
}
@@ -348,15 +350,17 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
348350
if (fp_args < Argument::n_float_register_parameters_j) {
349351
regs[i].set1(FP_ArgReg[fp_args++]->as_VMReg());
350352
} else {
353+
stk_args = align_up(stk_args, 2);
351354
regs[i].set1(VMRegImpl::stack2reg(stk_args));
352-
stk_args += 2;
355+
stk_args += 1;
353356
}
354357
break;
355358
case T_DOUBLE:
356359
assert((i + 1) < total_args_passed && sig_bt[i + 1] == T_VOID, "expecting half");
357360
if (fp_args < Argument::n_float_register_parameters_j) {
358361
regs[i].set2(FP_ArgReg[fp_args++]->as_VMReg());
359362
} else {
363+
stk_args = align_up(stk_args, 2);
360364
regs[i].set2(VMRegImpl::stack2reg(stk_args));
361365
stk_args += 2;
362366
}
@@ -367,7 +371,7 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
367371
}
368372
}
369373

370-
return align_up(stk_args, 2);
374+
return stk_args;
371375
}
372376

373377
// Patch the callers callsite with entry to compiled code if it exists.

‎src/hotspot/cpu/arm/sharedRuntime_arm.cpp

-1
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,6 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
444444
}
445445
}
446446

447-
if (slot & 1) slot++;
448447
return slot;
449448
}
450449

‎src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,7 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
734734
ShouldNotReachHere();
735735
}
736736
}
737-
return align_up(stk, 2);
737+
return stk;
738738
}
739739

740740
#if defined(COMPILER1) || defined(COMPILER2)

‎src/hotspot/cpu/riscv/sharedRuntime_riscv.cpp

+8-4
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
266266

267267
uint int_args = 0;
268268
uint fp_args = 0;
269-
uint stk_args = 0; // inc by 2 each time
269+
uint stk_args = 0;
270270

271271
for (int i = 0; i < total_args_passed; i++) {
272272
switch (sig_bt[i]) {
@@ -278,8 +278,9 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
278278
if (int_args < Argument::n_int_register_parameters_j) {
279279
regs[i].set1(INT_ArgReg[int_args++]->as_VMReg());
280280
} else {
281+
stk_args = align_up(stk_args, 2);
281282
regs[i].set1(VMRegImpl::stack2reg(stk_args));
282-
stk_args += 2;
283+
stk_args += 1;
283284
}
284285
break;
285286
case T_VOID:
@@ -295,6 +296,7 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
295296
if (int_args < Argument::n_int_register_parameters_j) {
296297
regs[i].set2(INT_ArgReg[int_args++]->as_VMReg());
297298
} else {
299+
stk_args = align_up(stk_args, 2);
298300
regs[i].set2(VMRegImpl::stack2reg(stk_args));
299301
stk_args += 2;
300302
}
@@ -303,15 +305,17 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
303305
if (fp_args < Argument::n_float_register_parameters_j) {
304306
regs[i].set1(FP_ArgReg[fp_args++]->as_VMReg());
305307
} else {
308+
stk_args = align_up(stk_args, 2);
306309
regs[i].set1(VMRegImpl::stack2reg(stk_args));
307-
stk_args += 2;
310+
stk_args += 1;
308311
}
309312
break;
310313
case T_DOUBLE:
311314
assert((i + 1) < total_args_passed && sig_bt[i + 1] == T_VOID, "expecting half");
312315
if (fp_args < Argument::n_float_register_parameters_j) {
313316
regs[i].set2(FP_ArgReg[fp_args++]->as_VMReg());
314317
} else {
318+
stk_args = align_up(stk_args, 2);
315319
regs[i].set2(VMRegImpl::stack2reg(stk_args));
316320
stk_args += 2;
317321
}
@@ -321,7 +325,7 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
321325
}
322326
}
323327

324-
return align_up(stk_args, 2);
328+
return stk_args;
325329
}
326330

327331
// Patch the callers callsite with entry to compiled code if it exists.

‎src/hotspot/cpu/s390/sharedRuntime_s390.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -755,7 +755,7 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
755755
ShouldNotReachHere();
756756
}
757757
}
758-
return align_up(stk, 2);
758+
return stk;
759759
}
760760

761761
int SharedRuntime::c_calling_convention(const BasicType *sig_bt,

‎src/hotspot/cpu/x86/sharedRuntime_x86_32.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -528,8 +528,7 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
528528
}
529529
}
530530

531-
// return value can be odd number of VMRegImpl stack slots make multiple of 2
532-
return align_up(stack, 2);
531+
return stack;
533532
}
534533

535534
// Patch the callers callsite with entry to compiled code if it exists.

‎src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp

+8-4
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,7 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
497497

498498
uint int_args = 0;
499499
uint fp_args = 0;
500-
uint stk_args = 0; // inc by 2 each time
500+
uint stk_args = 0;
501501

502502
for (int i = 0; i < total_args_passed; i++) {
503503
switch (sig_bt[i]) {
@@ -509,8 +509,9 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
509509
if (int_args < Argument::n_int_register_parameters_j) {
510510
regs[i].set1(INT_ArgReg[int_args++]->as_VMReg());
511511
} else {
512+
stk_args = align_up(stk_args, 2);
512513
regs[i].set1(VMRegImpl::stack2reg(stk_args));
513-
stk_args += 2;
514+
stk_args += 1;
514515
}
515516
break;
516517
case T_VOID:
@@ -527,6 +528,7 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
527528
if (int_args < Argument::n_int_register_parameters_j) {
528529
regs[i].set2(INT_ArgReg[int_args++]->as_VMReg());
529530
} else {
531+
stk_args = align_up(stk_args, 2);
530532
regs[i].set2(VMRegImpl::stack2reg(stk_args));
531533
stk_args += 2;
532534
}
@@ -535,15 +537,17 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
535537
if (fp_args < Argument::n_float_register_parameters_j) {
536538
regs[i].set1(FP_ArgReg[fp_args++]->as_VMReg());
537539
} else {
540+
stk_args = align_up(stk_args, 2);
538541
regs[i].set1(VMRegImpl::stack2reg(stk_args));
539-
stk_args += 2;
542+
stk_args += 1;
540543
}
541544
break;
542545
case T_DOUBLE:
543546
assert((i + 1) < total_args_passed && sig_bt[i + 1] == T_VOID, "expecting half");
544547
if (fp_args < Argument::n_float_register_parameters_j) {
545548
regs[i].set2(FP_ArgReg[fp_args++]->as_VMReg());
546549
} else {
550+
stk_args = align_up(stk_args, 2);
547551
regs[i].set2(VMRegImpl::stack2reg(stk_args));
548552
stk_args += 2;
549553
}
@@ -554,7 +558,7 @@ int SharedRuntime::java_calling_convention(const BasicType *sig_bt,
554558
}
555559
}
556560

557-
return align_up(stk_args, 2);
561+
return stk_args;
558562
}
559563

560564
// Patch the callers callsite with entry to compiled code if it exists.

‎src/hotspot/share/c1/c1_FrameMap.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ CallingConvention* FrameMap::java_calling_convention(const BasicTypeArray* signa
7272
}
7373
}
7474

75-
intptr_t out_preserve = SharedRuntime::java_calling_convention(sig_bt, regs, sizeargs);
75+
intptr_t out_preserve = align_up(SharedRuntime::java_calling_convention(sig_bt, regs, sizeargs), 2);
7676
LIR_OprList* args = new LIR_OprList(signature->length());
7777
for (i = 0; i < sizeargs;) {
7878
BasicType t = sig_bt[i];

‎src/hotspot/share/code/nmethod.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -3059,7 +3059,7 @@ void nmethod::print_nmethod_labels(outputStream* stream, address block_begin, bo
30593059
assert(sig_index == sizeargs, "");
30603060
}
30613061
const char* spname = "sp"; // make arch-specific?
3062-
intptr_t out_preserve = SharedRuntime::java_calling_convention(sig_bt, regs, sizeargs);
3062+
SharedRuntime::java_calling_convention(sig_bt, regs, sizeargs);
30633063
int stack_slot_offset = this->frame_size() * wordSize;
30643064
int tab1 = 14, tab2 = 24;
30653065
int sig_index = 0;

‎src/hotspot/share/oops/method.hpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,8 @@ class Method : public Metadata {
444444
void remove_unshareable_flags() NOT_CDS_RETURN;
445445

446446
// the number of argument reg slots that the compiled method uses on the stack.
447-
int num_stack_arg_slots() const { return constMethod()->num_stack_arg_slots(); }
447+
int num_stack_arg_slots(bool rounded = true) const {
448+
return rounded ? align_up(constMethod()->num_stack_arg_slots(), 2) : constMethod()->num_stack_arg_slots(); }
448449

449450
virtual void metaspace_pointers_do(MetaspaceClosure* iter);
450451
virtual MetaspaceObj::Type type() const { return MethodType; }

‎src/hotspot/share/oops/stackChunkOop.hpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ class stackChunkOopDesc : public instanceOopDesc {
155155

156156
inline void* gc_data() const;
157157
inline BitMapView bitmap() const;
158-
inline BitMap::idx_t bit_index_for(intptr_t* p) const;
158+
inline BitMap::idx_t bit_index_for(address p) const;
159159
inline intptr_t* address_for_bit(BitMap::idx_t index) const;
160160
template <typename OopT> inline BitMap::idx_t bit_index_for(OopT* p) const;
161161
template <typename OopT> inline OopT* address_for_bit(BitMap::idx_t index) const;

‎src/hotspot/share/oops/stackChunkOop.inline.hpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -256,12 +256,13 @@ inline BitMapView stackChunkOopDesc::bitmap() const {
256256
return bitmap;
257257
}
258258

259-
inline BitMap::idx_t stackChunkOopDesc::bit_index_for(intptr_t* p) const {
259+
inline BitMap::idx_t stackChunkOopDesc::bit_index_for(address p) const {
260260
return UseCompressedOops ? bit_index_for((narrowOop*)p) : bit_index_for((oop*)p);
261261
}
262262

263263
template <typename OopT>
264264
inline BitMap::idx_t stackChunkOopDesc::bit_index_for(OopT* p) const {
265+
assert(is_aligned(p, alignof(OopT)), "should be aligned: " PTR_FORMAT, p2i(p));
265266
assert(p >= (OopT*)start_address(), "Address not in chunk");
266267
return p - (OopT*)start_address();
267268
}

‎src/hotspot/share/prims/foreignGlobals.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ int NativeCallingConvention::calling_convention(const BasicType* sig_bt, VMStora
170170

171171
int JavaCallingConvention::calling_convention(const BasicType* sig_bt, VMStorage* regs, int num_args) const {
172172
VMRegPair* vm_regs = NEW_RESOURCE_ARRAY(VMRegPair, num_args);
173-
int slots = SharedRuntime::java_calling_convention(sig_bt, vm_regs, num_args);
173+
int slots = align_up(SharedRuntime::java_calling_convention(sig_bt, vm_regs, num_args), 2);
174174
for (int i = 0; i < num_args; i++) {
175175
VMRegPair pair = vm_regs[i];
176176
// note, we ignore second here. Signature should consist of register-size values. So there should be

‎src/hotspot/share/runtime/continuationFreezeThaw.cpp

+22-8
Original file line numberDiff line numberDiff line change
@@ -1741,7 +1741,7 @@ class ThawBase : public StackObj {
17411741
inline void before_thaw_java_frame(const frame& hf, const frame& caller, bool bottom, int num_frame);
17421742
inline void after_thaw_java_frame(const frame& f, bool bottom);
17431743
inline void patch(frame& f, const frame& caller, bool bottom);
1744-
void clear_bitmap_bits(intptr_t* start, int range);
1744+
void clear_bitmap_bits(address start, address end);
17451745

17461746
NOINLINE void recurse_thaw_interpreted_frame(const frame& hf, frame& caller, int num_frames);
17471747
void recurse_thaw_compiled_frame(const frame& hf, frame& caller, int num_frames, bool stub_caller);
@@ -2122,13 +2122,22 @@ inline void ThawBase::patch(frame& f, const frame& caller, bool bottom) {
21222122
assert(!bottom || (_cont.is_empty() != Continuation::is_cont_barrier_frame(f)), "");
21232123
}
21242124

2125-
void ThawBase::clear_bitmap_bits(intptr_t* start, int range) {
2125+
void ThawBase::clear_bitmap_bits(address start, address end) {
2126+
assert(is_aligned(start, wordSize), "should be aligned: " PTR_FORMAT, p2i(start));
2127+
assert(is_aligned(end, VMRegImpl::stack_slot_size), "should be aligned: " PTR_FORMAT, p2i(end));
2128+
21262129
// we need to clear the bits that correspond to arguments as they reside in the caller frame
2127-
// or they will keep objects that are otherwise unreachable alive
2128-
log_develop_trace(continuations)("clearing bitmap for " INTPTR_FORMAT " - " INTPTR_FORMAT, p2i(start), p2i(start+range));
2130+
// or they will keep objects that are otherwise unreachable alive.
2131+
2132+
// Align `end` if UseCompressedOops is not set to avoid UB when calculating the bit index, since
2133+
// `end` could be at an odd number of stack slots from `start`, i.e might not be oop aligned.
2134+
// If that's the case the bit range corresponding to the last stack slot should not have bits set
2135+
// anyways and we assert that before returning.
2136+
address effective_end = UseCompressedOops ? end : align_down(end, wordSize);
2137+
log_develop_trace(continuations)("clearing bitmap for " INTPTR_FORMAT " - " INTPTR_FORMAT, p2i(start), p2i(effective_end));
21292138
stackChunkOop chunk = _cont.tail();
2130-
chunk->bitmap().clear_range(chunk->bit_index_for(start),
2131-
chunk->bit_index_for(start+range));
2139+
chunk->bitmap().clear_range(chunk->bit_index_for(start), chunk->bit_index_for(effective_end));
2140+
assert(effective_end == end || !chunk->bitmap().at(chunk->bit_index_for(effective_end)), "bit should not be set");
21322141
}
21332142

21342143
NOINLINE void ThawBase::recurse_thaw_interpreted_frame(const frame& hf, frame& caller, int num_frames) {
@@ -2181,7 +2190,9 @@ NOINLINE void ThawBase::recurse_thaw_interpreted_frame(const frame& hf, frame& c
21812190
_cont.tail()->fix_thawed_frame(caller, SmallRegisterMap::instance);
21822191
} else if (_cont.tail()->has_bitmap() && locals > 0) {
21832192
assert(hf.is_heap_frame(), "should be");
2184-
clear_bitmap_bits(heap_frame_bottom - locals, locals);
2193+
address start = (address)(heap_frame_bottom - locals);
2194+
address end = (address)heap_frame_bottom;
2195+
clear_bitmap_bits(start, end);
21852196
}
21862197

21872198
DEBUG_ONLY(after_thaw_java_frame(f, is_bottom_frame);)
@@ -2254,7 +2265,10 @@ void ThawBase::recurse_thaw_compiled_frame(const frame& hf, frame& caller, int n
22542265
// can only fix caller once this frame is thawed (due to callee saved regs); this happens on the stack
22552266
_cont.tail()->fix_thawed_frame(caller, SmallRegisterMap::instance);
22562267
} else if (_cont.tail()->has_bitmap() && added_argsize > 0) {
2257-
clear_bitmap_bits(heap_frame_top + ContinuationHelper::CompiledFrame::size(hf) + frame::metadata_words_at_top, added_argsize);
2268+
address start = (address)(heap_frame_top + ContinuationHelper::CompiledFrame::size(hf) + frame::metadata_words_at_top);
2269+
int stack_args_slots = f.cb()->as_compiled_method()->method()->num_stack_arg_slots(false /* rounded */);
2270+
int argsize_in_bytes = stack_args_slots * VMRegImpl::stack_slot_size;
2271+
clear_bitmap_bits(start, start + argsize_in_bytes);
22582272
}
22592273

22602274
DEBUG_ONLY(after_thaw_java_frame(f, is_bottom_frame);)

‎src/hotspot/share/runtime/frame.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1439,7 +1439,7 @@ void frame::describe(FrameValues& values, int frame_no, const RegisterMap* reg_m
14391439
assert(sig_index == sizeargs, "");
14401440
}
14411441
int stack_arg_slots = SharedRuntime::java_calling_convention(sig_bt, regs, sizeargs);
1442-
assert(stack_arg_slots == m->num_stack_arg_slots(), "");
1442+
assert(stack_arg_slots == m->num_stack_arg_slots(false /* rounded */), "");
14431443
int out_preserve = SharedRuntime::out_preserve_stack_slots();
14441444
int sig_index = 0;
14451445
int arg_index = (m->is_static() ? 0 : -1);

‎src/hotspot/share/runtime/sharedRuntime.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -1991,7 +1991,7 @@ void SharedRuntime::check_member_name_argument_is_last_argument(const methodHand
19911991
assert(member_arg_pos >= 0 && member_arg_pos < total_args_passed, "oob");
19921992
assert(sig_bt[member_arg_pos] == T_OBJECT, "dispatch argument must be an object");
19931993

1994-
int comp_args_on_stack = java_calling_convention(sig_bt, regs_without_member_name, total_args_passed - 1);
1994+
java_calling_convention(sig_bt, regs_without_member_name, total_args_passed - 1);
19951995

19961996
for (int i = 0; i < member_arg_pos; i++) {
19971997
VMReg a = regs_with_member_name[i].first();
@@ -3089,7 +3089,7 @@ void AdapterHandlerLibrary::create_native_wrapper(const methodHandle& method) {
30893089
BasicType ret_type = si.return_type();
30903090

30913091
// Now get the compiled-Java arguments layout.
3092-
int comp_args_on_stack = SharedRuntime::java_calling_convention(sig_bt, regs, total_args_passed);
3092+
SharedRuntime::java_calling_convention(sig_bt, regs, total_args_passed);
30933093

30943094
// Generate the compiled-to-native wrapper code
30953095
nm = SharedRuntime::generate_native_wrapper(&_masm, method, compile_id, sig_bt, regs, ret_type);

0 commit comments

Comments
 (0)