[parser] More PreParsedScopeData cleanup

- Use overflow-proof HasRemainingBytes
- Add DCHECK to catch index_ OOB in RemainingBytes

Change-Id: I65c47c42438c93c9b4673f09fed4b8ef4685f257
Reviewed-on: https://chromium-review.googlesource.com/c/1388535Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58441}

src/parsing/preparsed-scope-data-impl.h

@@ -93,16 +93,24 @@ class BaseConsumedPreParsedScopeData : public ConsumedPreParsedScopeData {
       DISALLOW_HEAP_ALLOCATION(no_gc);
     };
 
-    void SetPosition(int position) { index_ = position; }
+    void SetPosition(int position) {
+      DCHECK_LE(position, data_.length());
+      index_ = position;
+    }
 
     size_t RemainingBytes() const {
       DCHECK(has_data_);
+      DCHECK_LE(index_, data_.length());
       return data_.length() - index_;
     }
 
-    int32_t ReadUint32() {
+    bool HasRemainingBytes(size_t bytes) const {
       DCHECK(has_data_);
-      DCHECK_GE(RemainingBytes(), kUint32Size);
+      return index_ <= data_.length() && bytes <= RemainingBytes();
+    }
+
+    int32_t ReadUint32() {
+      DCHECK(HasRemainingBytes(kUint32Size));
       // Check that there indeed is an integer following.
       DCHECK_EQ(data_.get(index_++), kUint32Size);
       int32_t result = data_.get(index_) + (data_.get(index_ + 1) << 8) +
@@ -115,7 +123,7 @@ class BaseConsumedPreParsedScopeData : public ConsumedPreParsedScopeData {
 
     uint8_t ReadUint8() {
       DCHECK(has_data_);
-      DCHECK_GE(RemainingBytes(), kUint8Size);
+      DCHECK(HasRemainingBytes(kUint8Size));
       // Check that there indeed is a byte following.
       DCHECK_EQ(data_.get(index_++), kUint8Size);
       stored_quarters_ = 0;
@@ -125,7 +133,7 @@ class BaseConsumedPreParsedScopeData : public ConsumedPreParsedScopeData {
     uint8_t ReadQuarter() {
       DCHECK(has_data_);
       if (stored_quarters_ == 0) {
-        DCHECK_GE(RemainingBytes(), kUint8Size);
+        DCHECK(HasRemainingBytes(kUint8Size));
         // Check that there indeed are quarters following.
         DCHECK_EQ(data_.get(index_++), kQuarterMarker);
         stored_byte_ = data_.get(index_++);

src/parsing/preparsed-scope-data.cc

@@ -152,9 +152,7 @@ PreParsedScopeDataBuilder::PreParsedScopeDataBuilder(
       byte_data_(new (zone) ByteData(zone)),
       data_for_inner_functions_(zone),
       bailed_out_(false) {
-  if (parent != nullptr) {
-    parent->data_for_inner_functions_.push_back(this);
-  }
+  if (parent != nullptr) parent->data_for_inner_functions_.push_back(this);
 #ifdef DEBUG
   // Reserve space for scope_data_start, written later:
   byte_data_->WriteUint32(0);
@@ -208,8 +206,6 @@ void PreParsedScopeDataBuilder::SaveScopeAllocationData(
             ByteData::kPlaceholderSize);
 
   if (bailed_out_) return;
-
-
   // If there are no skippable inner functions, we don't need to save anything.
   if (!ContainsInnerFunctions()) return;
 
@@ -234,13 +230,8 @@ bool PreParsedScopeDataBuilder::ContainsInnerFunctions() const {
 MaybeHandle<PreParsedScopeData> PreParsedScopeDataBuilder::Serialize(
     Isolate* isolate) {
   if (bailed_out_) return MaybeHandle<PreParsedScopeData>();
-
   DCHECK(!ThisOrParentBailedOut());
-
-  if (!ContainsInnerFunctions()) {
-    // The data contains only the placeholder.
-    return MaybeHandle<PreParsedScopeData>();
-  }
+  if (!ContainsInnerFunctions()) return MaybeHandle<PreParsedScopeData>();
 
   int child_data_length = static_cast<int>(data_for_inner_functions_.size());
   Handle<PreParsedScopeData> data =
@@ -265,9 +256,7 @@ MaybeHandle<PreParsedScopeData> PreParsedScopeDataBuilder::Serialize(
 
 ZonePreParsedScopeData* PreParsedScopeDataBuilder::Serialize(Zone* zone) {
   if (bailed_out_) return nullptr;
-
   DCHECK(!ThisOrParentBailedOut());
-
   if (!ContainsInnerFunctions()) return nullptr;
 
   int child_length = static_cast<int>(data_for_inner_functions_.size());
@@ -331,15 +320,11 @@ void PreParsedScopeDataBuilder::SaveDataForScope(Scope* scope) {
 
   if (scope->scope_type() == ScopeType::FUNCTION_SCOPE) {
     Variable* function = scope->AsDeclarationScope()->function_var();
-    if (function != nullptr) {
-      SaveDataForVariable(function);
-    }
+    if (function != nullptr) SaveDataForVariable(function);
   }
 
   for (Variable* var : *scope->locals()) {
-    if (IsDeclaredVariableMode(var->mode())) {
-      SaveDataForVariable(var);
-    }
+    if (IsDeclaredVariableMode(var->mode())) SaveDataForVariable(var);
   }
 
   SaveDataForInnerScopes(scope);
@@ -457,7 +442,7 @@ BaseConsumedPreParsedScopeData<Data>::GetDataForSkippableFunction(
   // The skippable function *must* be the next function in the data. Use the
   // start position as a sanity check.
   typename ByteData::ReadingScope reading_scope(this);
-  CHECK_GE(scope_data_->RemainingBytes(), ByteData::kSkippableFunctionDataSize);
+  CHECK(scope_data_->HasRemainingBytes(ByteData::kSkippableFunctionDataSize));
   int start_position_from_data = scope_data_->ReadUint32();
   CHECK_EQ(start_position, start_position_from_data);
 
@@ -512,16 +497,12 @@ void BaseConsumedPreParsedScopeData<Data>::RestoreDataForScope(Scope* scope) {
   if (!PreParsedScopeDataBuilder::ScopeNeedsData(scope)) return;
 
   // scope_type is stored only in debug mode.
-  CHECK_GE(scope_data_->RemainingBytes(), ByteData::kUint8Size);
   DCHECK_EQ(scope_data_->ReadUint8(), scope->scope_type());
 
+  CHECK(scope_data_->HasRemainingBytes(ByteData::kUint8Size));
   uint32_t eval = scope_data_->ReadUint8();
-  if (ScopeCallsSloppyEvalField::decode(eval)) {
-    scope->RecordEvalCall();
-  }
-  if (InnerScopeCallsEvalField::decode(eval)) {
-    scope->RecordInnerScopeEvalCall();
-  }
+  if (ScopeCallsSloppyEvalField::decode(eval)) scope->RecordEvalCall();
+  if (InnerScopeCallsEvalField::decode(eval)) scope->RecordInnerScopeEvalCall();
 
   if (scope->scope_type() == ScopeType::FUNCTION_SCOPE) {
     Variable* function = scope->AsDeclarationScope()->function_var();
@@ -529,9 +510,7 @@ void BaseConsumedPreParsedScopeData<Data>::RestoreDataForScope(Scope* scope) {
   }
 
   for (Variable* var : *scope->locals()) {
-    if (IsDeclaredVariableMode(var->mode())) {
-      RestoreDataForVariable(var);
-    }
+    if (IsDeclaredVariableMode(var->mode())) RestoreDataForVariable(var);
   }
 
   RestoreDataForInnerScopes(scope);
@@ -602,9 +581,7 @@ ProducedPreParsedScopeData* OnHeapConsumedPreParsedScopeData::GetChildData(
     Zone* zone, int child_index) {
   CHECK_GT(data_->length(), child_index);
   Object* child_data = data_->child_data(child_index);
-  if (!child_data->IsPreParsedScopeData()) {
-    return nullptr;
-  }
+  if (!child_data->IsPreParsedScopeData()) return nullptr;
   Handle<PreParsedScopeData> child_data_handle(
       PreParsedScopeData::cast(child_data), isolate_);
   return ProducedPreParsedScopeData::For(child_data_handle, zone);