From c2498fe8fe2caca9cb8e42e8e6e3ff20b3daafa7 Mon Sep 17 00:00:00 2001 From: Peter Marshall Date: Fri, 5 Apr 2019 16:00:53 +0200 Subject: [PATCH] [unwinder] Remove final FP bounds check which is invalid on Windows Bug: v8:9092 Change-Id: I1839651c0a47dbbefa93c7441597c98653132ff8 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1554692 Auto-Submit: Peter Marshall Reviewed-by: Michael Starzinger Commit-Queue: Peter Marshall Cr-Commit-Position: refs/heads/master@{#60748} --- src/unwinder.cc | 5 ++++- test/cctest/test-unwinder.cc | 10 ++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/unwinder.cc b/src/unwinder.cc index b0b6ee0504..c3fcd1a6ae 100644 --- a/src/unwinder.cc +++ b/src/unwinder.cc @@ -80,8 +80,11 @@ bool Unwinder::TryUnwindV8Frames(const UnwindState& unwind_state, if (!AddressIsInStack(final_sp, stack_base, stack_top)) return false; register_state->sp = final_sp; + // We don't check that the final FP value is within the stack bounds because + // this is just the rbp value that JSEntryStub pushed. On platforms like + // Win64 this is not used as a dedicated FP register, and could contain + // anything. void* final_fp = GetCallerFPFromFP(current_fp); - if (!AddressIsInStack(final_fp, stack_base, stack_top)) return false; register_state->fp = final_fp; register_state->pc = next_pc; diff --git a/test/cctest/test-unwinder.cc b/test/cctest/test-unwinder.cc index 63ce17d57c..5b3f3ef98e 100644 --- a/test/cctest/test-unwinder.cc +++ b/test/cctest/test-unwinder.cc @@ -423,11 +423,13 @@ TEST(Unwind_StackBounds_WithUnwinding) { stack_base); CHECK(!unwound); - // Change the return address so that it is not in range. + // Change the return address so that it is not in range. We will not range + // check the stack[9] FP value because we have finished unwinding and the + // contents of rbp does not necessarily have to be the FP in this case. stack[10] = 202; unwound = v8::Unwinder::TryUnwindV8Frames(unwind_state, ®ister_state, stack_base); - CHECK(!unwound); + CHECK(unwound); } TEST(PCIsInV8_BadState_Fail) { @@ -482,8 +484,8 @@ TEST(PCIsInV8_InCodeOrEmbeddedRange) { embedded_range_length); } -// PCIsInV8 doesn't check if the PC is in JSEntrydirectly. It's assumed that the -// CodeRange or EmbeddedCodeRange contain JSEntry. +// PCIsInV8 doesn't check if the PC is in JSEntry directly. It's assumed that +// the CodeRange or EmbeddedCodeRange contain JSEntry. TEST(PCIsInV8_InJSEntryRange) { LocalContext env; v8::Isolate* isolate = env->GetIsolate();