Revert "[parser] Move some PPSD sanity checks to debug"

This reverts commit f45045cc.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> [parser] Move some PPSD sanity checks to debug
> 
> Move some of PreParsedScopeData's santity checks, such as the magic
> value separating skippable function data from scope data, to be debug
> only, to save memory.
> 
> Start position of inner skippable functions is still kept, because it's
> too good at catching bugs, but we may want to remove it in the future
> as well.
> 
> Bug: chromium:818642
> Change-Id: If86ff1b9845e8dd3b015b4e554d0033328b145bf
> Reviewed-on: https://chromium-review.googlesource.com/1127046
> Commit-Queue: Leszek Swirski <leszeks@chromium.org>
> Reviewed-by: Marja Hölttä <marja@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#54263}

TBR=marja@chromium.org,leszeks@chromium.org

Change-Id: I15ceedd66d9ecb66cf65f5834d09975b41d3ed27
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:818642
Reviewed-on: https://chromium-review.googlesource.com/1127859Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54290}

src/parsing/preparsed-scope-data.cc

@@ -24,21 +24,19 @@ class VariableMaybeAssignedField : public BitField8<bool, 0, 1> {};
 class VariableContextAllocatedField
     : public BitField8<bool, VariableMaybeAssignedField::kNext, 1> {};
 
-
-#ifdef DEBUG
 const int kMagicValue = 0xC0DE0DE;
 
+#ifdef DEBUG
 const size_t kUint32Size = 5;
 const size_t kUint8Size = 2;
 const size_t kQuarterMarker = 0;
-const size_t kPlaceholderSize = kUint32Size;
 #else
 const size_t kUint32Size = 4;
 const size_t kUint8Size = 1;
-const size_t kPlaceholderSize = 0;
 #endif
 
-const size_t kSkippableFunctionDataSize = 4 * kUint32Size + 1 * kUint8Size;
+const int kPlaceholderSize = kUint32Size;
+const int kSkippableFunctionDataSize = 4 * kUint32Size + 1 * kUint8Size;
 
 class LanguageField : public BitField8<LanguageMode, 0, 1> {};
 class UsesSuperField : public BitField8<bool, LanguageField::kNext, 1> {};
@@ -53,7 +51,7 @@ STATIC_ASSERT(LanguageModeSize <= LanguageField::kNumValues);
 
   (Skippable function data:)
   ------------------------------------
-  | scope_data_start (debug only)    |
+  | scope_data_start                 |
   ------------------------------------
   | data for inner function 1        |
   | ...                              |
@@ -61,11 +59,11 @@ STATIC_ASSERT(LanguageModeSize <= LanguageField::kNumValues);
   | data for inner function n        |
   | ...                              |
   ------------------------------------
-  (Scope allocation data:)             << scope_data_start points here in debug
+  (Scope allocation data:)             << scope_data_start points here
   ------------------------------------
-  magic value (debug only)
+  magic value
   ------------------------------------
-  scope positions (debug only)
+  scope positions
   ------------------------------------
   | scope type << only in debug      |
   | eval                             |
@@ -103,19 +101,19 @@ void ProducedPreParsedScopeData::ByteData::WriteUint32(uint32_t data) {
   free_quarters_in_last_byte_ = 0;
 }
 
-#ifdef DEBUG
 void ProducedPreParsedScopeData::ByteData::OverwriteFirstUint32(uint32_t data) {
   auto it = backing_store_.begin();
+#ifdef DEBUG
   // Check that that position already holds an item of the expected size.
   DCHECK_GE(backing_store_.size(), kUint32Size);
   DCHECK_EQ(*it, kUint32Size);
   ++it;
+#endif
   const uint8_t* d = reinterpret_cast<uint8_t*>(&data);
   for (size_t i = 0; i < 4; ++i) {
     *it++ = *d++;
   }
 }
-#endif
 
 void ProducedPreParsedScopeData::ByteData::WriteUint8(uint8_t data) {
 #ifdef DEBUG
@@ -168,10 +166,8 @@ ProducedPreParsedScopeData::ProducedPreParsedScopeData(
   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);
-#endif
 }
 
 // Create a ProducedPreParsedScopeData which is just a proxy for a previous
@@ -231,9 +227,6 @@ void ProducedPreParsedScopeData::AddSkippableFunction(
     return;
   }
 
-  // Start position is used for a sanity check when consuming the data, we could
-  // remove it in the future if we're very pressed for space but it's been good
-  // at catching bugs in the wild so far.
   byte_data_->WriteUint32(start_position);
   byte_data_->WriteUint32(end_position);
   byte_data_->WriteUint32(num_parameters);
@@ -266,7 +259,6 @@ void ProducedPreParsedScopeData::SaveScopeAllocationData(
     return;
   }
 
-#ifdef DEBUG
   byte_data_->OverwriteFirstUint32(scope_data_start);
 
   // For a data integrity check, write a value between data about skipped inner
@@ -274,7 +266,6 @@ void ProducedPreParsedScopeData::SaveScopeAllocationData(
   byte_data_->WriteUint32(kMagicValue);
   byte_data_->WriteUint32(scope->start_position());
   byte_data_->WriteUint32(scope->end_position());
-#endif
 
   SaveDataForScope(scope);
 }
@@ -505,9 +496,9 @@ void ConsumedPreParsedScopeData::SetData(Isolate* isolate,
   int scope_data_start = scope_data_->ReadUint32();
   scope_data_->SetPosition(scope_data_start);
   DCHECK_EQ(scope_data_->ReadUint32(), kMagicValue);
+#endif
   // The first data item is scope_data_start. Skip over it.
   scope_data_->SetPosition(kPlaceholderSize);
-#endif
 }
 
 ProducedPreParsedScopeData*
@@ -553,16 +544,14 @@ void ConsumedPreParsedScopeData::RestoreScopeAllocationData(
 
   ByteData::ReadingScope reading_scope(this);
 
-#ifdef DEBUG
   int magic_value_from_data = scope_data_->ReadUint32();
   // Check that we've consumed all inner function data.
-  DCHECK_EQ(magic_value_from_data, kMagicValue);
+  CHECK_EQ(magic_value_from_data, kMagicValue);
 
   int start_position_from_data = scope_data_->ReadUint32();
   int end_position_from_data = scope_data_->ReadUint32();
-  DCHECK_EQ(start_position_from_data, scope->start_position());
-  DCHECK_EQ(end_position_from_data, scope->end_position());
-#endif
+  CHECK_EQ(start_position_from_data, scope->start_position());
+  CHECK_EQ(end_position_from_data, scope->end_position());
 
   RestoreData(scope);
 

src/parsing/preparsed-scope-data.h

@@ -76,10 +76,8 @@ class ProducedPreParsedScopeData : public ZoneObject {
     void WriteUint8(uint8_t data);
     void WriteQuarter(uint8_t data);
 
-#ifdef DEBUG
     // For overwriting previously written data at position 0.
     void OverwriteFirstUint32(uint32_t data);
-#endif
 
     Handle<PodArray<uint8_t>> Serialize(Isolate* isolate);
 

test/cctest/parsing/test-preparser.cc

@@ -820,9 +820,7 @@ TEST(ProducingAndConsumingByteData) {
   bytes.WriteUint8(255);
   bytes.WriteUint32(0);
   bytes.WriteUint8(0);
-#ifdef DEBUG
   bytes.OverwriteFirstUint32(2017);
-#endif
   bytes.WriteUint8(100);
   // Write quarter bytes between uint8s and uint32s to verify they're stored
   // correctly.
@@ -845,11 +843,7 @@ TEST(ProducingAndConsumingByteData) {
       &bytes_for_reading, *data_on_heap);
 
   // Read the data back.
-#ifdef DEBUG
   CHECK_EQ(bytes_for_reading.ReadUint32(), 2017);
-#else
-  CHECK_EQ(bytes_for_reading.ReadUint32(), 1983);
-#endif
   CHECK_EQ(bytes_for_reading.ReadUint32(), 2147483647);
   CHECK_EQ(bytes_for_reading.ReadUint8(), 4);
   CHECK_EQ(bytes_for_reading.ReadUint8(), 255);