[parser] Fix storing has_data bit for inner function preparse data

Drive-by-fix:
- improve PreparseData::Print

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

src/objects-printer.cc

@@ -2254,6 +2254,12 @@ void PreparseData::PreparseDataPrint(std::ostream& os) {  // NOLINT
   PrintHeader(os, "PreparseData");
   os << "\n - data_length: " << data_length();
   os << "\n - children_length: " << children_length();
+  if (data_length() > 0) {
+    os << "\n - data-start: " << (address() + kDataStartOffset);
+  }
+  if (children_length() > 0) {
+    os << "\n - children-start: " << inner_start_offset();
+  }
   for (int i = 0; i < children_length(); ++i) {
     os << "\n - [" << i << "]: " << Brief(get_child(i));
   }

src/parsing/parser.cc

@@ -2440,9 +2440,7 @@ bool Parser::SkipFunction(const AstRawString* function_name, FunctionKind kind,
     LanguageMode language_mode;
     int num_inner_functions;
     bool uses_super_property;
-    if (stack_overflow()) {
-      return true;
-    }
+    if (stack_overflow()) return true;
     *produced_preparse_data =
         consumed_preparse_data_->GetDataForSkippableFunction(
             main_zone(), function_scope->start_position(), &end_position,

src/parsing/preparse-data-impl.h

@@ -158,7 +158,7 @@ class BaseConsumedPreparseData : public ConsumedPreparseData {
   void RestoreScopeAllocationData(DeclarationScope* scope) final;
 
 #ifdef DEBUG
-  void VerifyDataStart();
+  bool VerifyDataStart();
 #endif
 
  private:

src/parsing/preparse-data.cc

@@ -252,7 +252,6 @@ bool PreparseDataBuilder::ScopeNeedsData(Scope* scope) {
 
 bool PreparseDataBuilder::SaveDataForSkippableFunction(
     PreparseDataBuilder* builder) {
-  if (builder->bailed_out_) return false;
   DeclarationScope* function_scope = builder->function_scope_;
   // 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
@@ -260,7 +259,7 @@ bool PreparseDataBuilder::SaveDataForSkippableFunction(
   byte_data_.WriteVarint32(function_scope->start_position());
   byte_data_.WriteVarint32(function_scope->end_position());
 
-  bool has_data = builder->has_data_;
+  bool has_data = builder->HasData();
   uint32_t has_data_and_num_parameters =
       HasDataField::encode(has_data) |
       NumberOfParametersField::encode(function_scope->num_parameters());
@@ -276,7 +275,7 @@ bool PreparseDataBuilder::SaveDataForSkippableFunction(
 
 void PreparseDataBuilder::SaveScopeAllocationData(DeclarationScope* scope,
                                                   Parser* parser) {
-  if (!HasData()) return;
+  if (!has_data_) return;
   DCHECK(HasInnerFunctions());
 
   byte_data_.Start(parser->preparse_data_buffer());
@@ -292,6 +291,8 @@ void PreparseDataBuilder::SaveScopeAllocationData(DeclarationScope* scope,
     if (SaveDataForSkippableFunction(builder)) num_inner_with_data_++;
   }
 
+  // Don't save imcoplete scope information when bailed out.
+  if (!bailed_out_) {
 #ifdef DEBUG
   // function data items, kSkippableMinFunctionDataSize each.
   CHECK_GE(byte_data_.length(), kPlaceholderSize);
@@ -306,6 +307,7 @@ void PreparseDataBuilder::SaveScopeAllocationData(DeclarationScope* scope,
 #endif
 
   if (ScopeNeedsData(scope)) SaveDataForScope(scope);
+  }
   byte_data_.Finalize(parser->factory()->zone());
 }
 
@@ -630,14 +632,15 @@ void BaseConsumedPreparseData<Data>::RestoreDataForInnerScopes(Scope* scope) {
 
 #ifdef DEBUG
 template <class Data>
-void BaseConsumedPreparseData<Data>::VerifyDataStart() {
+bool BaseConsumedPreparseData<Data>::VerifyDataStart() {
   typename ByteData::ReadingScope reading_scope(this);
   // The first uint32 contains the size of the skippable function data.
   int scope_data_start = scope_data_->ReadUint32();
   scope_data_->SetPosition(scope_data_start);
-  DCHECK_EQ(scope_data_->ReadUint32(), ByteData::kMagicValue);
+  CHECK_EQ(scope_data_->ReadUint32(), ByteData::kMagicValue);
   // The first data item is scope_data_start. Skip over it.
   scope_data_->SetPosition(ByteData::kPlaceholderSize);
+  return true;
 }
 #endif
 
@@ -655,9 +658,7 @@ OnHeapConsumedPreparseData::OnHeapConsumedPreparseData(
     : BaseConsumedPreparseData<PreparseData>(), isolate_(isolate), data_(data) {
   DCHECK_NOT_NULL(isolate);
   DCHECK(data->IsPreparseData());
-#ifdef DEBUG
-  VerifyDataStart();
-#endif
+  DCHECK(VerifyDataStart());
 }
 
 ZonePreparseData::ZonePreparseData(Zone* zone, Vector<uint8_t>* byte_data,
@@ -684,9 +685,7 @@ Handle<PreparseData> ZonePreparseData::Serialize(Isolate* isolate) {
 ZoneConsumedPreparseData::ZoneConsumedPreparseData(Zone* zone,
                                                    ZonePreparseData* data)
     : data_(data), scope_data_wrapper_(data_->byte_data()) {
-#ifdef DEBUG
-  VerifyDataStart();
-#endif
+  DCHECK(VerifyDataStart());
 }
 
 ZoneVectorWrapper ZoneConsumedPreparseData::GetScopeData() {

test/mjsunit/regress/regress-crbug-923705.js

@@ -0,0 +1,15 @@
+// Copyright 2019 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --verify-heap
+
+function __f_5() {
+  function __f_1() {
+    function __f_0() {
+      ({y = eval()}) => assertEquals()();
+    }
+  }
+}
+
+__f_5();