[vm, test] Fix vm/cc/DartAPI_StackOverflowStackTrace* under MSAN.

The old code would unpoison the stack in one operation, based on the requested size of the stack for threads created by the VM. This may be smaller than the actual size of the stack, leaving some of the stack still poisoned.

The new code unpoisons the stack frame by frame, and only frames created by generated code. For exit frames, it guesses the frame size, but for all other frames it is precise.

Change-Id: Ice16480feca19727fcfadf19c9ec6cf205607946
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/137725
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Régis Crelier <regis@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>

runtime/vm/deopt_instructions.cc

@@ -358,6 +358,9 @@ static void FillDeferredSlots(DeoptContext* deopt_context,
 // Materializes all deferred objects.  Returns the total number of
 // artificial arguments used during deoptimization.
 intptr_t DeoptContext::MaterializeDeferredObjects() {
+  // This region is initialized by a mixture of C++ and generated code.
+  MSAN_UNPOISON(dest_frame_, dest_frame_size_ * kWordSize);
+
   // Populate slots with references to all unboxed "primitive" values (doubles,
   // mints, simd) and deferred objects. Deferred objects are only allocated
   // but not filled with data. This is done later because deferred objects

runtime/vm/stack_frame.cc

@@ -571,9 +571,12 @@ void StackFrameIterator::SetupLastExitFrameData() {
   ASSERT(thread_ != NULL);
   uword exit_marker = thread_->top_exit_frame_info();
   frames_.fp_ = exit_marker;
+  frames_.sp_ = 0;
+  frames_.pc_ = 0;
   if (FLAG_enable_interpreter) {
     frames_.CheckIfInterpreted(exit_marker);
   }
+  frames_.Unpoison();
 }
 
 void StackFrameIterator::SetupNextExitFrameData() {
@@ -589,14 +592,7 @@ void StackFrameIterator::SetupNextExitFrameData() {
   if (FLAG_enable_interpreter) {
     frames_.CheckIfInterpreted(exit_marker);
   }
-}
-
-// Tell MemorySanitizer that generated code initializes part of the stack.
-// TODO(koda): Limit to frames that are actually written by generated code.
-static void UnpoisonStack(uword fp) {
-  ASSERT(fp != 0);
-  uword size = OSThread::GetSpecifiedStackSize();
-  MSAN_UNPOISON(reinterpret_cast<void*>(fp - size), 2 * size);
+  frames_.Unpoison();
 }
 
 StackFrameIterator::StackFrameIterator(ValidationPolicy validation_policy,
@@ -631,6 +627,7 @@ StackFrameIterator::StackFrameIterator(uword last_fp,
   if (FLAG_enable_interpreter) {
     frames_.CheckIfInterpreted(last_fp);
   }
+  frames_.Unpoison();
 }
 
 StackFrameIterator::StackFrameIterator(uword fp,
@@ -653,6 +650,7 @@ StackFrameIterator::StackFrameIterator(uword fp,
   if (FLAG_enable_interpreter) {
     frames_.CheckIfInterpreted(fp);
   }
+  frames_.Unpoison();
 }
 
 StackFrame* StackFrameIterator::NextFrame() {
@@ -672,7 +670,6 @@ StackFrame* StackFrameIterator::NextFrame() {
     if (!HasNextFrame()) {
       return NULL;
     }
-    UnpoisonStack(frames_.fp_);
     if (frames_.pc_ == 0) {
       // Iteration starts from an exit frame given by its fp.
       current_frame_ = NextExitFrame();
@@ -721,6 +718,29 @@ void StackFrameIterator::FrameSetIterator::CheckIfInterpreted(
 #endif  // !defined(DART_PRECOMPILED_RUNTIME)
 }
 
+// Tell MemorySanitizer that generated code initializes part of the stack.
+void StackFrameIterator::FrameSetIterator::Unpoison() {
+  // When using a simulator, all writes to the stack happened from MSAN
+  // instrumented C++, so there is nothing to unpoison. Additionally,
+  // fp_ will be somewhere in the simulator's stack instead of the OSThread's
+  // stack.
+#if !defined(USING_SIMULATOR)
+  if (fp_ == 0) return;
+  ASSERT(is_interpreted_ || ((thread_->os_thread()->stack_limit() < fp_) &&
+                             (thread_->os_thread()->stack_base() > fp_)));
+  uword lower;
+  if (sp_ == 0) {
+    // Exit frame: guess sp.
+    lower = fp_ - kDartFrameFixedSize * kWordSize;
+  } else {
+    lower = sp_;
+  }
+  uword upper = fp_ + kSavedCallerPcSlotFromFp * kWordSize;
+  // Both lower and upper are inclusive, so we add one word when computing size.
+  MSAN_UNPOISON(reinterpret_cast<void*>(lower), upper - lower + kWordSize);
+#endif  // !defined(USING_SIMULATOR)
+}
+
 StackFrame* StackFrameIterator::FrameSetIterator::NextFrame(bool validate) {
   StackFrame* frame;
   ASSERT(HasNext());
@@ -732,6 +752,7 @@ StackFrame* StackFrameIterator::FrameSetIterator::NextFrame(bool validate) {
   sp_ = frame->GetCallerSp();
   fp_ = frame->GetCallerFp();
   pc_ = frame->GetCallerPc();
+  Unpoison();
   ASSERT(is_interpreted_ == frame->is_interpreted_);
   ASSERT(!validate || frame->IsValid());
   return frame;
@@ -745,6 +766,7 @@ ExitFrame* StackFrameIterator::NextExitFrame() {
   frames_.sp_ = exit_.GetCallerSp();
   frames_.fp_ = exit_.GetCallerFp();
   frames_.pc_ = exit_.GetCallerPc();
+  frames_.Unpoison();
   ASSERT(frames_.is_interpreted_ == exit_.is_interpreted_);
   ASSERT(!validate_ || exit_.IsValid());
   return &exit_;

runtime/vm/stack_frame.h

@@ -315,6 +315,7 @@ class StackFrameIterator : public ValueObject {
           is_interpreted_(false) {}
     bool is_interpreted() const { return is_interpreted_; }
     void CheckIfInterpreted(uword exit_marker);
+    void Unpoison();
 
     uword fp_;
     uword sp_;