Skip to content

Commit

Permalink
8292694: x86_64 c2i/i2c adapters may use more stack space than necessary
Browse files Browse the repository at this point in the history
Reviewed-by: kvn, thartmann
  • Loading branch information
dean-long committed Aug 29, 2022
1 parent 30def49 commit adb3d4f
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 35 deletions.
16 changes: 16 additions & 0 deletions src/hotspot/cpu/x86/macroAssembler_x86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9453,3 +9453,19 @@ void MacroAssembler::get_thread(Register thread) {


#endif // !WIN32 || _LP64

void MacroAssembler::check_stack_alignment(Register sp, const char* msg, unsigned bias, Register tmp) {
Label L_stack_ok;
if (bias == 0) {
testptr(sp, 2 * wordSize - 1);
} else {
// lea(tmp, Address(rsp, bias);
mov(tmp, sp);
addptr(tmp, bias);
testptr(tmp, 2 * wordSize - 1);
}
jcc(Assembler::equal, L_stack_ok);
block_comment(msg);
stop(msg);
bind(L_stack_ok);
}
3 changes: 3 additions & 0 deletions src/hotspot/cpu/x86/macroAssembler_x86.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2094,6 +2094,9 @@ class MacroAssembler: public Assembler {
#endif // _LP64

void vallones(XMMRegister dst, int vector_len);

void check_stack_alignment(Register sp, const char* msg, unsigned bias = 0, Register tmp = noreg);

};

/**
Expand Down
70 changes: 35 additions & 35 deletions src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -615,25 +615,40 @@ static void gen_c2i_adapter(MacroAssembler *masm,
__ bind(skip_fixup);

// Since all args are passed on the stack, total_args_passed *
// Interpreter::stackElementSize is the space we need. Plus 1 because
// we also account for the return address location since
// we store it first rather than hold it in rax across all the shuffling
// Interpreter::stackElementSize is the space we need.

int extraspace = (total_args_passed * Interpreter::stackElementSize) + wordSize;
assert(total_args_passed >= 0, "total_args_passed is %d", total_args_passed);

int extraspace = (total_args_passed * Interpreter::stackElementSize);

// stack is aligned, keep it that way
// This is not currently needed or enforced by the interpreter, but
// we might as well conform to the ABI.
extraspace = align_up(extraspace, 2*wordSize);

// Get return address
__ pop(rax);

// set senderSP value
__ mov(r13, rsp);
__ lea(r13, Address(rsp, wordSize));

#ifdef ASSERT
__ check_stack_alignment(r13, "sender stack not aligned");
#endif
if (extraspace > 0) {
// Pop the return address
__ pop(rax);

__ subptr(rsp, extraspace);
__ subptr(rsp, extraspace);

// Store the return address in the expected location
__ movptr(Address(rsp, 0), rax);
// Push the return address
__ push(rax);

// Account for the return address location since we store it first rather
// than hold it in a register across all the shuffling
extraspace += wordSize;
}

#ifdef ASSERT
__ check_stack_alignment(rsp, "callee stack not aligned", wordSize, rax);
#endif

// Now write the args into the outgoing interpreter space
for (int i = 0; i < total_args_passed; i++) {
Expand Down Expand Up @@ -779,16 +794,15 @@ void SharedRuntime::gen_i2c_adapter(MacroAssembler *masm,
// If this happens, control eventually transfers back to the compiled
// caller, but with an uncorrected stack, causing delayed havoc.

// Pick up the return address
__ movptr(rax, Address(rsp, 0));

if (VerifyAdapterCalls &&
(Interpreter::code() != NULL || StubRoutines::code1() != NULL)) {
// So, let's test for cascading c2i/i2c adapters right now.
// assert(Interpreter::contains($return_addr) ||
// StubRoutines::contains($return_addr),
// "i2c adapter must return to an interpreter frame");
__ block_comment("verify_i2c { ");
// Pick up the return address
__ movptr(rax, Address(rsp, 0));
Label L_ok;
if (Interpreter::code() != NULL)
range_check(masm, rax, r11,
Expand All @@ -813,22 +827,16 @@ void SharedRuntime::gen_i2c_adapter(MacroAssembler *masm,
// we need to align the outgoing SP for compiled code.
__ movptr(r11, rsp);

// Cut-out for having no stack args. Since up to 2 int/oop args are passed
// in registers, we will occasionally have no stack args.
int comp_words_on_stack = 0;
// Pick up the return address
__ pop(rax);

// Convert 4-byte c2 stack slots to words.
int comp_words_on_stack = align_up(comp_args_on_stack*VMRegImpl::stack_slot_size, wordSize)>>LogBytesPerWord;

if (comp_args_on_stack) {
// Sig words on the stack are greater-than VMRegImpl::stack0. Those in
// registers are below. By subtracting stack0, we either get a negative
// number (all values in registers) or the maximum stack slot accessed.

// Convert 4-byte c2 stack slots to words.
comp_words_on_stack = align_up(comp_args_on_stack*VMRegImpl::stack_slot_size, wordSize)>>LogBytesPerWord;
// Round up to miminum stack alignment, in wordSize
comp_words_on_stack = align_up(comp_words_on_stack, 2);
__ subptr(rsp, comp_words_on_stack * wordSize);
}


// Ensure compiled code always sees stack at proper alignment
__ andptr(rsp, -16);

Expand Down Expand Up @@ -1754,15 +1762,7 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,
}

#ifdef ASSERT
{
Label L;
__ mov(rax, rsp);
__ andptr(rax, -16); // must be 16 byte boundary (see amd64 ABI)
__ cmpptr(rax, rsp);
__ jcc(Assembler::equal, L);
__ stop("improperly aligned stack");
__ bind(L);
}
__ check_stack_alignment(rsp, "improperly aligned stack");
#endif /* ASSERT */


Expand Down

1 comment on commit adb3d4f

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.