Revert "[regexp] Protect against reentrant RegExpStack use"

This reverts commit e2408c2521.

Reason for revert: https://ci.chromium.org/p/v8/builders/ci/V8%20Mac64/36733?1

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,jgruber@chromium.org

Change-Id: I7b51659d21fe2d49ff343f4de0f6bb9720281b86
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:1125934
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2465822
Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Commit-Queue: Nico Hartmann <nicohartmann@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70437}
This commit is contained in:
Nico Hartmann 2020-10-12 08:55:11 +00:00 committed by Commit Bot
parent e2408c2521
commit f9a31e424c
4 changed files with 7 additions and 38 deletions

View File

@ -14,18 +14,12 @@ RegExpStackScope::RegExpStackScope(Isolate* isolate)
: regexp_stack_(isolate->regexp_stack()) { : regexp_stack_(isolate->regexp_stack()) {
// Initialize, if not already initialized. // Initialize, if not already initialized.
regexp_stack_->EnsureCapacity(0); 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() { RegExpStackScope::~RegExpStackScope() {
// Reset the buffer if it has grown. // Reset the buffer if it has grown.
regexp_stack_->Reset(); regexp_stack_->Reset();
DCHECK(!regexp_stack_->is_in_use());
} }
RegExpStack::RegExpStack() : thread_local_(this), isolate_(nullptr) {} RegExpStack::RegExpStack() : thread_local_(this), isolate_(nullptr) {}
@ -42,15 +36,17 @@ char* RegExpStack::ArchiveStack(char* to) {
DCHECK(thread_local_.owns_memory_); DCHECK(thread_local_.owns_memory_);
} }
MemCopy(reinterpret_cast<void*>(to), &thread_local_, kThreadLocalSize); size_t size = sizeof(thread_local_);
MemCopy(reinterpret_cast<void*>(to), &thread_local_, size);
thread_local_ = ThreadLocal(this); thread_local_ = ThreadLocal(this);
return to + kThreadLocalSize; return to + size;
} }
char* RegExpStack::RestoreStack(char* from) { char* RegExpStack::RestoreStack(char* from) {
MemCopy(&thread_local_, reinterpret_cast<void*>(from), kThreadLocalSize); size_t size = sizeof(thread_local_);
return from + kThreadLocalSize; MemCopy(&thread_local_, reinterpret_cast<void*>(from), size);
return from + size;
} }
void RegExpStack::Reset() { thread_local_.ResetToStaticStack(this); } void RegExpStack::Reset() { thread_local_.ResetToStaticStack(this); }
@ -64,7 +60,6 @@ void RegExpStack::ThreadLocal::ResetToStaticStack(RegExpStack* regexp_stack) {
limit_ = reinterpret_cast<Address>(regexp_stack->static_stack_) + limit_ = reinterpret_cast<Address>(regexp_stack->static_stack_) +
kStackLimitSlack * kSystemPointerSize; kStackLimitSlack * kSystemPointerSize;
owns_memory_ = false; owns_memory_ = false;
is_in_use_ = false;
} }
void RegExpStack::ThreadLocal::FreeAndInvalidate() { void RegExpStack::ThreadLocal::FreeAndInvalidate() {

View File

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

View File

@ -41,10 +41,6 @@
'test-serialize/TestThatAlwaysFails': [FAIL], 'test-serialize/TestThatAlwaysFails': [FAIL],
'test-api/SealHandleScope': [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],
# This test always fails. It tests that LiveEdit causes abort when turned off. # This test always fails. It tests that LiveEdit causes abort when turned off.
'test-debug/LiveEditDisabled': [FAIL], 'test-debug/LiveEditDisabled': [FAIL],

View File

@ -21435,13 +21435,6 @@ class RegExpInterruptTest {
string->MakeExternal(&two_byte_string_resource); 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: private:
static void SignalSemaphore(v8::Isolate* isolate, void* data) { static void SignalSemaphore(v8::Isolate* isolate, void* data) {
reinterpret_cast<RegExpInterruptTest*>(data)->sem_.Signal(); reinterpret_cast<RegExpInterruptTest*>(data)->sem_.Signal();
@ -21548,16 +21541,6 @@ TEST(RegExpInterruptAndMakeSubjectTwoByteExternal) {
test.RunTest(RegExpInterruptTest::MakeSubjectTwoByteExternal); 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 { class RequestInterruptTestBase {
public: public:
RequestInterruptTestBase() RequestInterruptTestBase()