Fix the bit massaging code in CompleteParserRecorder::WriteNumber.

The original code, added by
https://codereview.chromium.org/3384003/diff/7001/src/parser.cc 3.5 years ago,
failed to write numbers which contain a chunk of 7 zeroes in the middle. The
smallest such number is 2^14, so this is a problem if the source file to
preparse contains 16384 or more symbols (which happens in the wild).

This bug went unnoticed because the symbol data was not used by Parser (see
https://codereview.chromium.org/172753002/ for starting to use it again) and
there were no tests.

R=ulan@chromium.org
BUG=346221
LOG=y

Review URL: https://codereview.chromium.org/179433004

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@19538 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/preparse-data.cc

@@ -167,16 +167,26 @@ Vector<unsigned> CompleteParserRecorder::ExtractData() {
 
 
 void CompleteParserRecorder::WriteNumber(int number) {
+  // Split the number into chunks of 7 bits. Write them one after another (the
+  // most significant first). Use the MSB of each byte for signalling that the
+  // number continues. See ScriptDataImpl::ReadNumber for the reading side.
   ASSERT(number >= 0);
 
   int mask = (1 << 28) - 1;
-  for (int i = 28; i > 0; i -= 7) {
-    if (number > mask) {
-      symbol_store_.Add(static_cast<byte>(number >> i) | 0x80u);
-      number &= mask;
-    }
+  int i = 28;
+  // 26 million symbols ought to be enough for anybody.
+  ASSERT(number <= mask);
+  while (number < mask) {
     mask >>= 7;
+    i -= 7;
   }
+  while (i > 0) {
+    symbol_store_.Add(static_cast<byte>(number >> i) | 0x80u);
+    number &= mask;
+    mask >>= 7;
+    i -= 7;
+  }
+  ASSERT(number < (1 << 7));
   symbol_store_.Add(static_cast<byte>(number));
 }
 

src/preparse-data.h

@@ -183,6 +183,10 @@ class CompleteParserRecorder: public FunctionLoggingParserRecorder {
   virtual int symbol_ids() { return symbol_id_; }
 
  private:
+  // For testing. Defined in test-parsing.cc.
+  friend void FakeWritingSymbolIdInPreParseData(CompleteParserRecorder* log,
+                                                int number);
+
   struct Key {
     bool is_ascii;
     Vector<const byte> literal_bytes;

test/cctest/test-parsing.cc

@@ -360,6 +360,50 @@ TEST(PreparsingObjectLiterals) {
   }
 }
 
+namespace v8 {
+namespace internal {
+
+void FakeWritingSymbolIdInPreParseData(i::CompleteParserRecorder* log,
+                                       int number) {
+  log->WriteNumber(number);
+  if (log->symbol_id_ < number + 1) {
+    log->symbol_id_ = number + 1;
+  }
+}
+
+}
+}
+
+
+TEST(StoringNumbersInPreParseData) {
+  // Symbol IDs are split into chunks of 7 bits for storing. This is a
+  // regression test for a bug where a symbol id was incorrectly stored if some
+  // of the chunks in the middle were all zeros.
+  i::CompleteParserRecorder log;
+  for (int i = 0; i < 18; ++i) {
+    FakeWritingSymbolIdInPreParseData(&log, 1 << i);
+  }
+  for (int i = 1; i < 18; ++i) {
+    FakeWritingSymbolIdInPreParseData(&log, (1 << i) + 1);
+  }
+  for (int i = 6; i < 18; ++i) {
+    FakeWritingSymbolIdInPreParseData(&log, (3 << i) + (5 << (i - 6)));
+  }
+  i::Vector<unsigned> store = log.ExtractData();
+  i::ScriptDataImpl script_data(store);
+  script_data.Initialize();
+  // Check that we get the same symbols back.
+  for (int i = 0; i < 18; ++i) {
+    CHECK_EQ(1 << i, script_data.GetSymbolIdentifier());
+  }
+  for (int i = 1; i < 18; ++i) {
+    CHECK_EQ((1 << i) + 1, script_data.GetSymbolIdentifier());
+  }
+  for (int i = 6; i < 18; ++i) {
+    CHECK_EQ((3 << i) + (5 << (i - 6)), script_data.GetSymbolIdentifier());
+  }
+}
+
 
 TEST(RegressChromium62639) {
   v8::V8::Initialize();