Reland "[regexp] Protect against reentrant RegExpStack use"

This is a reland of e2408c25

Changes since last time: also accept CRASH test results. For some
reason, the CHECK failure is detected as a CRASH on mac bots.

Original change's description:
> [regexp] Protect against reentrant RegExpStack use
>
> Irregexp, and in particular the RegExpStack, are not reentrant.
> Explicitly guard against reentrancy.
>
> Bug: chromium:1125934
> Change-Id: I0fc295f6986a89221982e6a2ccefed46193974f6
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2460820
> Commit-Queue: Yang Guo <yangguo@chromium.org>
> Auto-Submit: Jakob Gruber <jgruber@chromium.org>
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#70436}

Tbr: yangguo@chromium.org
Bug: chromium:1125934
Change-Id: I2116ca5944c49f6114228d4402847bdd426bdd7f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2465823Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70441}

src/regexp/regexp-stack.cc

@@ -14,12 +14,18 @@ RegExpStackScope::RegExpStackScope(Isolate* isolate)
     : regexp_stack_(isolate->regexp_stack()) {
   // Initialize, if not already initialized.
   regexp_stack_->EnsureCapacity(0);
+  // Irregexp is not reentrant in several ways; in particular, the
+  // RegExpStackScope is not reentrant since the destructor frees allocated
+  // memory. Protect against reentrancy here.
+  CHECK(!regexp_stack_->is_in_use());
+  regexp_stack_->set_is_in_use(true);
 }
 
 
 RegExpStackScope::~RegExpStackScope() {
   // Reset the buffer if it has grown.
   regexp_stack_->Reset();
+  DCHECK(!regexp_stack_->is_in_use());
 }
 
 RegExpStack::RegExpStack() : thread_local_(this), isolate_(nullptr) {}
@@ -36,17 +42,15 @@ char* RegExpStack::ArchiveStack(char* to) {
     DCHECK(thread_local_.owns_memory_);
   }
 
-  size_t size = sizeof(thread_local_);
-  MemCopy(reinterpret_cast<void*>(to), &thread_local_, size);
+  MemCopy(reinterpret_cast<void*>(to), &thread_local_, kThreadLocalSize);
   thread_local_ = ThreadLocal(this);
-  return to + size;
+  return to + kThreadLocalSize;
 }
 
 
 char* RegExpStack::RestoreStack(char* from) {
-  size_t size = sizeof(thread_local_);
-  MemCopy(&thread_local_, reinterpret_cast<void*>(from), size);
-  return from + size;
+  MemCopy(&thread_local_, reinterpret_cast<void*>(from), kThreadLocalSize);
+  return from + kThreadLocalSize;
 }
 
 void RegExpStack::Reset() { thread_local_.ResetToStaticStack(this); }
@@ -60,6 +64,7 @@ void RegExpStack::ThreadLocal::ResetToStaticStack(RegExpStack* regexp_stack) {
   limit_ = reinterpret_cast<Address>(regexp_stack->static_stack_) +
            kStackLimitSlack * kSystemPointerSize;
   owns_memory_ = false;
+  is_in_use_ = false;
 }
 
 void RegExpStack::ThreadLocal::FreeAndInvalidate() {

src/regexp/regexp-stack.h

@@ -68,9 +68,12 @@ class RegExpStack {
   // If passing zero, the default/minimum size buffer is allocated.
   Address EnsureCapacity(size_t size);
 
+  bool is_in_use() const { return thread_local_.is_in_use_; }
+  void set_is_in_use(bool v) { thread_local_.is_in_use_ = v; }
+
   // Thread local archiving.
   static constexpr int ArchiveSpacePerThread() {
-    return static_cast<int>(sizeof(ThreadLocal));
+    return static_cast<int>(kThreadLocalSize);
   }
   char* ArchiveStack(char* to);
   char* RestoreStack(char* from);
@@ -112,10 +115,12 @@ class RegExpStack {
     size_t memory_size_ = 0;
     Address limit_ = kNullAddress;
     bool owns_memory_ = false;  // Whether memory_ is owned and must be freed.
+    bool is_in_use_ = false;    // To guard against reentrancy.
 
     void ResetToStaticStack(RegExpStack* regexp_stack);
     void FreeAndInvalidate();
   };
+  static constexpr size_t kThreadLocalSize = sizeof(ThreadLocal);
 
   // Address of top of memory used as stack.
   Address memory_top_address_address() {

test/cctest/cctest.status

@@ -41,6 +41,10 @@
   'test-serialize/TestThatAlwaysFails': [FAIL],
   'test-api/SealHandleScope': [FAIL],
 
+  # This test is expected to hit a CHECK (i.e. a FAIL result actually means the
+  # test passed).
+  'test-api/RegExpInterruptAndReenterIrregexp': [FAIL, CRASH],
+
   # This test always fails.  It tests that LiveEdit causes abort when turned off.
   'test-debug/LiveEditDisabled': [FAIL],
 

test/cctest/test-api.cc

@@ -21435,6 +21435,13 @@ class RegExpInterruptTest {
     string->MakeExternal(&two_byte_string_resource);
   }
 
+  static void ReenterIrregexp(v8::Isolate* isolate, void* data) {
+    v8::HandleScope scope(isolate);
+    v8::TryCatch try_catch(isolate);
+    // Irregexp is not reentrant. This should crash.
+    CompileRun("/((a*)*)*b/.exec('aaaaab')");
+  }
+
  private:
   static void SignalSemaphore(v8::Isolate* isolate, void* data) {
     reinterpret_cast<RegExpInterruptTest*>(data)->sem_.Signal();
@@ -21541,6 +21548,16 @@ TEST(RegExpInterruptAndMakeSubjectTwoByteExternal) {
   test.RunTest(RegExpInterruptTest::MakeSubjectTwoByteExternal);
 }
 
+TEST(RegExpInterruptAndReenterIrregexp) {
+  // We only check in the runtime entry to irregexp, so make sure we don't hit
+  // an interpreter.
+  i::FLAG_regexp_tier_up_ticks = 0;
+  i::FLAG_regexp_interpret_all = false;
+  i::FLAG_enable_experimental_regexp_engine = false;
+  RegExpInterruptTest test;
+  test.RunTest(RegExpInterruptTest::ReenterIrregexp);
+}
+
 class RequestInterruptTestBase {
  public:
   RequestInterruptTestBase()