Make Isolate::GetIncumbentContext() work well with MSan

https://crrev.com/c/1343709 fixed GetIncumbentContext to work
with ASan, however, GetIncumbentContext didn't work well with
MSan because MSan uses a simulator which supports yet another
separate stack frame.

This patch fixes GetIncumbentContext so that it works well
with not only ASan but also MSan simply following the same way
as v8::TryCatch does.

i::GetCurrentStackPosition() solves the issue of ASan and
SafeStack (native but separate stack frame), and
i::SimulatorStack solves the issue of MSan (simulator stack
frame).

Bug: chromium:888867, chromium:866610
Change-Id: Id803cbfd17fb1b1d9b8ee34c4802768f3a2f8e79
Reviewed-on: https://chromium-review.googlesource.com/c/1356691
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58096}

include/v8.h

@@ -9096,7 +9096,7 @@ class V8_EXPORT Context {
    * stack.
    * https://html.spec.whatwg.org/multipage/webappapis.html#backup-incumbent-settings-object-stack
    */
-  class V8_EXPORT BackupIncumbentScope {
+  class V8_EXPORT BackupIncumbentScope final {
    public:
     /**
      * |backup_incumbent_context| is pushed onto the backup incumbent settings
@@ -9105,10 +9105,20 @@ class V8_EXPORT Context {
     explicit BackupIncumbentScope(Local<Context> backup_incumbent_context);
     ~BackupIncumbentScope();
 
+    /**
+     * Returns address that is comparable with JS stack address.  Note that JS
+     * stack may be allocated separately from the native stack.  See also
+     * |TryCatch::JSStackComparableAddress| for details.
+     */
+    uintptr_t JSStackComparableAddress() const {
+      return js_stack_comparable_address_;
+    }
+
    private:
     friend class internal::Isolate;
 
     Local<Context> backup_incumbent_context_;
+    uintptr_t js_stack_comparable_address_ = 0;
     const BackupIncumbentScope* prev_ = nullptr;
   };
 

src/api.cc

@@ -1182,6 +1182,10 @@ Context::BackupIncumbentScope::BackupIncumbentScope(
 
   i::Handle<i::Context> env = Utils::OpenHandle(*backup_incumbent_context_);
   i::Isolate* isolate = env->GetIsolate();
+
+  js_stack_comparable_address_ =
+      i::SimulatorStack::RegisterJSStackComparableAddress(isolate);
+
   prev_ = isolate->top_backup_incumbent_scope();
   isolate->set_top_backup_incumbent_scope(this);
 }
@@ -1189,6 +1193,9 @@ Context::BackupIncumbentScope::BackupIncumbentScope(
 Context::BackupIncumbentScope::~BackupIncumbentScope() {
   i::Handle<i::Context> env = Utils::OpenHandle(*backup_incumbent_context_);
   i::Isolate* isolate = env->GetIsolate();
+
+  i::SimulatorStack::UnregisterJSStackComparableAddress(isolate);
+
   isolate->set_top_backup_incumbent_scope(prev_);
 }
 
@@ -2629,9 +2636,8 @@ v8::TryCatch::TryCatch(v8::Isolate* isolate)
       has_terminated_(false) {
   ResetInternal();
   // Special handling for simulators which have a separate JS stack.
-  js_stack_comparable_address_ =
-      reinterpret_cast<void*>(i::SimulatorStack::RegisterCTryCatch(
-          isolate_, i::GetCurrentStackPosition()));
+  js_stack_comparable_address_ = reinterpret_cast<void*>(
+      i::SimulatorStack::RegisterJSStackComparableAddress(isolate_));
   isolate_->RegisterTryCatchHandler(this);
 }
 
@@ -2650,7 +2656,7 @@ v8::TryCatch::~TryCatch() {
       isolate_->RestorePendingMessageFromTryCatch(this);
     }
     isolate_->UnregisterTryCatchHandler(this);
-    i::SimulatorStack::UnregisterCTryCatch(isolate_);
+    i::SimulatorStack::UnregisterJSStackComparableAddress(isolate_);
     reinterpret_cast<Isolate*>(isolate_)->ThrowException(exc);
     DCHECK(!isolate_->thread_local_top()->rethrowing_message_);
   } else {
@@ -2661,7 +2667,7 @@ v8::TryCatch::~TryCatch() {
       isolate_->CancelScheduledExceptionFromTryCatch(this);
     }
     isolate_->UnregisterTryCatchHandler(this);
-    i::SimulatorStack::UnregisterCTryCatch(isolate_);
+    i::SimulatorStack::UnregisterJSStackComparableAddress(isolate_);
   }
 }
 

src/isolate.cc

@@ -76,10 +76,6 @@
 #include "unicode/uobject.h"
 #endif  // V8_INTL_SUPPORT
 
-#if defined(V8_USE_ADDRESS_SANITIZER)
-#include <sanitizer/asan_interface.h>
-#endif
-
 extern "C" const uint8_t* v8_Default_embedded_blob_;
 extern "C" uint32_t v8_Default_embedded_blob_size_;
 
@@ -2506,21 +2502,10 @@ Handle<Context> Isolate::GetIncumbentContext() {
   // if it's newer than the last Context::BackupIncumbentScope entry.
   //
   // NOTE: This code assumes that the stack grows downward.
-#if defined(V8_USE_ADDRESS_SANITIZER)
-  // |it.frame()->sp()| points to an address in the real stack frame, but
-  // |top_backup_incumbent_scope()| points to an address in a fake stack frame.
-  // In order to compare them, convert the latter into the address in the real
-  // stack frame.
-  void* maybe_fake_top = const_cast<void*>(
-      reinterpret_cast<const void*>(top_backup_incumbent_scope()));
-  void* maybe_real_top = __asan_addr_is_in_fake_stack(
-      __asan_get_current_fake_stack(), maybe_fake_top, nullptr, nullptr);
-  Address top_backup_incumbent = reinterpret_cast<Address>(
-      maybe_real_top ? maybe_real_top : maybe_fake_top);
-#else
   Address top_backup_incumbent =
-      reinterpret_cast<Address>(top_backup_incumbent_scope());
-#endif
+      top_backup_incumbent_scope()
+          ? top_backup_incumbent_scope()->JSStackComparableAddress()
+          : 0;
   if (!it.done() &&
       (!top_backup_incumbent || it.frame()->sp() < top_backup_incumbent)) {
     Context context = Context::cast(it.frame()->context());

src/simulator.h

@@ -8,6 +8,10 @@
 #include "src/globals.h"
 #include "src/objects/code.h"
 
+#if !defined(USE_SIMULATOR)
+#include "src/utils.h"
+#endif
+
 #if V8_TARGET_ARCH_IA32
 #include "src/ia32/simulator-ia32.h"
 #elif V8_TARGET_ARCH_X64
@@ -45,12 +49,18 @@ class SimulatorStack : public v8::internal::AllStatic {
     return Simulator::current(isolate)->StackLimit(c_limit);
   }
 
-  static inline uintptr_t RegisterCTryCatch(v8::internal::Isolate* isolate,
-                                            uintptr_t try_catch_address) {
-    return Simulator::current(isolate)->PushAddress(try_catch_address);
+  // Returns the current stack address on the simulator stack frame.
+  // The returned address is comparable with JS stack address.
+  static inline uintptr_t RegisterJSStackComparableAddress(
+      v8::internal::Isolate* isolate) {
+    // The value of |kPlaceHolder| is actually not used.  It just occupies a
+    // single word on the stack frame of the simulator.
+    const uintptr_t kPlaceHolder = 0x4A535350u;  // "JSSP" in ASCII
+    return Simulator::current(isolate)->PushAddress(kPlaceHolder);
   }
 
-  static inline void UnregisterCTryCatch(v8::internal::Isolate* isolate) {
+  static inline void UnregisterJSStackComparableAddress(
+      v8::internal::Isolate* isolate) {
     Simulator::current(isolate)->PopAddress();
   }
 };
@@ -69,13 +79,16 @@ class SimulatorStack : public v8::internal::AllStatic {
     return c_limit;
   }
 
-  static inline uintptr_t RegisterCTryCatch(v8::internal::Isolate* isolate,
-                                            uintptr_t try_catch_address) {
+  // Returns the current stack address on the native stack frame.
+  // The returned address is comparable with JS stack address.
+  static inline uintptr_t RegisterJSStackComparableAddress(
+      v8::internal::Isolate* isolate) {
     USE(isolate);
-    return try_catch_address;
+    return internal::GetCurrentStackPosition();
   }
 
-  static inline void UnregisterCTryCatch(v8::internal::Isolate* isolate) {
+  static inline void UnregisterJSStackComparableAddress(
+      v8::internal::Isolate* isolate) {
     USE(isolate);
   }
 };