Revision: 19538
Author: [email protected]
Date: Tue Feb 25 11:51:02 2014 UTC
Log: 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.
[email protected]
BUG=346221
LOG=y
Review URL: https://codereview.chromium.org/179433004
http://code.google.com/p/v8/source/detail?r=19538
Modified:
/branches/bleeding_edge/src/preparse-data.cc
/branches/bleeding_edge/src/preparse-data.h
/branches/bleeding_edge/test/cctest/test-parsing.cc
=======================================
--- /branches/bleeding_edge/src/preparse-data.cc Fri Jul 5 09:52:11 2013
UTC
+++ /branches/bleeding_edge/src/preparse-data.cc Tue Feb 25 11:51:02 2014
UTC
@@ -167,16 +167,26 @@
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));
}
=======================================
--- /branches/bleeding_edge/src/preparse-data.h Thu Feb 28 17:03:34 2013 UTC
+++ /branches/bleeding_edge/src/preparse-data.h Tue Feb 25 11:51:02 2014 UTC
@@ -183,6 +183,10 @@
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;
=======================================
--- /branches/bleeding_edge/test/cctest/test-parsing.cc Thu Feb 20 11:35:37
2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-parsing.cc Tue Feb 25 11:51:02
2014 UTC
@@ -359,6 +359,50 @@
CHECK_EQ("foo", *utf8);
}
}
+
+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-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
