Commit 4f781d72 authored by jgruber's avatar jgruber Committed by Commit bot

Fix LookupCode for the DatePrototype_GetField builtin

This was exposed on win64 and manifested as a negative offset during
stack frame collection, i.e. pc < Code::instruction_start() for a
BUILTIN frame.

This happened because StackFrame::LookupCode returns the wrong code
object when call is the last instruction in a code object:
* pc is actually the return address for all but the topmost frame.
* pc points at the next instruction after the call.
* This is beyond the current code object if call is the last
  instruction.
* Lookup itself is naive in that it just returns the first code object
  for which (next_code_obj_addr > pc). It does not check that pc is
  actually within [instruction_start, instruction_end[.
* In this specific case, the pc (== return address) actually pointed
  at the beginning of the header of the next code object.
* We finally calculated offset as (code->instruction_start() - pc),
  but with the wrong code object.

This should be followed up by a proper fix at some point. For instance,
this could be setting pc to (return address - 1) for all but the topmost
frame.

BUG=v8:5311

Review-Url: https://codereview.chromium.org/2284673002
Cr-Commit-Position: refs/heads/master@{#38996}
parent 3b8a2e51
......@@ -1867,6 +1867,16 @@ void Builtins::Generate_DatePrototype_GetField(MacroAssembler* masm,
__ Move(r0, Smi::FromInt(0));
__ EnterBuiltinFrame(cp, r1, r0);
__ CallRuntime(Runtime::kThrowNotDateError);
// It's far from obvious, but this final trailing instruction after the call
// is required for StackFrame::LookupCode to work correctly. To illustrate
// why: if call were the final instruction in the code object, then the pc
// (== return address) would point beyond the code object when the stack is
// traversed. When we then try to look up the code object through
// StackFrame::LookupCode, we actually return the next code object that
// happens to be on the same page in memory.
// TODO(jgruber): A proper fix for this would be nice.
__ nop();
}
}
......
......@@ -1870,6 +1870,16 @@ void Builtins::Generate_DatePrototype_GetField(MacroAssembler* masm,
__ Mov(x0, Smi::FromInt(0));
__ EnterBuiltinFrame(cp, x1, x0);
__ CallRuntime(Runtime::kThrowNotDateError);
// It's far from obvious, but this final trailing instruction after the call
// is required for StackFrame::LookupCode to work correctly. To illustrate
// why: if call were the final instruction in the code object, then the pc
// (== return address) would point beyond the code object when the stack is
// traversed. When we then try to look up the code object through
// StackFrame::LookupCode, we actually return the next code object that
// happens to be on the same page in memory.
// TODO(jgruber): A proper fix for this would be nice.
__ nop();
}
}
......
......@@ -1273,6 +1273,16 @@ void Builtins::Generate_DatePrototype_GetField(MacroAssembler* masm,
__ Move(ebx, Immediate(0));
__ EnterBuiltinFrame(esi, edi, ebx);
__ CallRuntime(Runtime::kThrowNotDateError);
// It's far from obvious, but this final trailing instruction after the call
// is required for StackFrame::LookupCode to work correctly. To illustrate
// why: if call were the final instruction in the code object, then the pc
// (== return address) would point beyond the code object when the stack is
// traversed. When we then try to look up the code object through
// StackFrame::LookupCode, we actually return the next code object that
// happens to be on the same page in memory.
// TODO(jgruber): A proper fix for this would be nice.
__ nop();
}
}
......
......@@ -1856,6 +1856,16 @@ void Builtins::Generate_DatePrototype_GetField(MacroAssembler* masm,
__ Move(a0, Smi::FromInt(0));
__ EnterBuiltinFrame(cp, a1, a0);
__ CallRuntime(Runtime::kThrowNotDateError);
// It's far from obvious, but this final trailing instruction after the call
// is required for StackFrame::LookupCode to work correctly. To illustrate
// why: if call were the final instruction in the code object, then the pc
// (== return address) would point beyond the code object when the stack is
// traversed. When we then try to look up the code object through
// StackFrame::LookupCode, we actually return the next code object that
// happens to be on the same page in memory.
// TODO(jgruber): A proper fix for this would be nice.
__ nop();
}
}
......
......@@ -1850,6 +1850,16 @@ void Builtins::Generate_DatePrototype_GetField(MacroAssembler* masm,
__ Move(a0, Smi::FromInt(0));
__ EnterBuiltinFrame(cp, a1, a0);
__ CallRuntime(Runtime::kThrowNotDateError);
// It's far from obvious, but this final trailing instruction after the call
// is required for StackFrame::LookupCode to work correctly. To illustrate
// why: if call were the final instruction in the code object, then the pc
// (== return address) would point beyond the code object when the stack is
// traversed. When we then try to look up the code object through
// StackFrame::LookupCode, we actually return the next code object that
// happens to be on the same page in memory.
// TODO(jgruber): A proper fix for this would be nice.
__ nop();
}
}
......
......@@ -1893,6 +1893,16 @@ void Builtins::Generate_DatePrototype_GetField(MacroAssembler* masm,
__ LoadSmiLiteral(r3, Smi::FromInt(0));
__ EnterBuiltinFrame(cp, r4, r3);
__ CallRuntime(Runtime::kThrowNotDateError);
// It's far from obvious, but this final trailing instruction after the call
// is required for StackFrame::LookupCode to work correctly. To illustrate
// why: if call were the final instruction in the code object, then the pc
// (== return address) would point beyond the code object when the stack is
// traversed. When we then try to look up the code object through
// StackFrame::LookupCode, we actually return the next code object that
// happens to be on the same page in memory.
// TODO(jgruber): A proper fix for this would be nice.
__ nop();
}
}
......
......@@ -1896,6 +1896,16 @@ void Builtins::Generate_DatePrototype_GetField(MacroAssembler* masm,
__ LoadSmiLiteral(r2, Smi::FromInt(0));
__ EnterBuiltinFrame(cp, r3, r2);
__ CallRuntime(Runtime::kThrowNotDateError);
// It's far from obvious, but this final trailing instruction after the call
// is required for StackFrame::LookupCode to work correctly. To illustrate
// why: if call were the final instruction in the code object, then the pc
// (== return address) would point beyond the code object when the stack is
// traversed. When we then try to look up the code object through
// StackFrame::LookupCode, we actually return the next code object that
// happens to be on the same page in memory.
// TODO(jgruber): A proper fix for this would be nice.
__ nop();
}
}
......
......@@ -1325,6 +1325,16 @@ void Builtins::Generate_DatePrototype_GetField(MacroAssembler* masm,
__ Move(rbx, Smi::FromInt(0));
__ EnterBuiltinFrame(rsi, rdi, rbx);
__ CallRuntime(Runtime::kThrowNotDateError);
// It's far from obvious, but this final trailing instruction after the call
// is required for StackFrame::LookupCode to work correctly. To illustrate
// why: if call were the final instruction in the code object, then the pc
// (== return address) would point beyond the code object when the stack is
// traversed. When we then try to look up the code object through
// StackFrame::LookupCode, we actually return the next code object that
// happens to be on the same page in memory.
// TODO(jgruber): A proper fix for this would be nice.
__ int3();
}
}
......
......@@ -1274,6 +1274,16 @@ void Builtins::Generate_DatePrototype_GetField(MacroAssembler* masm,
__ Move(ebx, Immediate(0));
__ EnterBuiltinFrame(esi, edi, ebx);
__ CallRuntime(Runtime::kThrowNotDateError);
// It's far from obvious, but this final trailing instruction after the call
// is required for StackFrame::LookupCode to work correctly. To illustrate
// why: if call were the final instruction in the code object, then the pc
// (== return address) would point beyond the code object when the stack is
// traversed. When we then try to look up the code object through
// StackFrame::LookupCode, we actually return the next code object that
// happens to be on the same page in memory.
// TODO(jgruber): A proper fix for this would be nice.
__ nop();
}
}
......
......@@ -63,6 +63,8 @@ inline StackHandler* StackFrame::top_handler() const {
inline Code* StackFrame::LookupCode() const {
// TODO(jgruber): This should really check that pc is within the returned
// code's instruction range [instruction_start(), instruction_end()[.
return GetContainingCode(isolate(), pc());
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment