[parser] implement error reporting for Scanner errors

src/parsing/scanner.cc

 // Copyright 2011 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.
 
 // Features shared by parsing and pre-parsing scanners.
 
 #include "src/parsing/scanner.h"
 
 #include <stdint.h>
 
@@ skipping 42 matching lines @@
   // Need to capture identifiers in order to recognize "get" and "set"
   // in object literals.
   Init();
   // Skip initial whitespace allowing HTML comment ends just like
   // after a newline and scan first token.
   has_line_terminator_before_next_ = true;
   SkipWhiteSpace();
   Scan();
 }
 
-
-template <bool capture_raw>
+template <bool capture_raw, bool unicode>
 uc32 Scanner::ScanHexNumber(int expected_length) {
   DCHECK(expected_length <= 4);  // prevent overflow
 
+  int begin = source_pos() - 2;
   uc32 x = 0;
   for (int i = 0; i < expected_length; i++) {
     int d = HexValue(c0_);
     if (d < 0) {
+      ReportScannerError(unicode
+                             ? MessageTemplate::kInvalidUnicodeEscapeSequence
+                             : MessageTemplate::kInvalidHexEscapeSequence,
+                         begin, begin + expected_length + 2);
       return -1;
     }
     x = x * 16 + d;
     Advance<capture_raw>();
   }
 
   return x;
 }
 
-
 template <bool capture_raw>
-uc32 Scanner::ScanUnlimitedLengthHexNumber(int max_value) {
+uc32 Scanner::ScanUnlimitedLengthHexNumber(int max_value, bool& bad_codepoint) {

[adamk, 2016/03/18 18:21:55]

The right type here is bool* in V8

[caitp (gmail), 2016/03/18 19:00:36]

dont you think reference ls are better for preventing accidents here? anyway,
will do

[adamk, 2016/03/18 19:09:57]

I think bool args are dangerous in C++ no matter what you do :)

As for reference vs pointer, V8 follows Google style and uses pointers for
outparams (Blink inherited WebKit style, which is the reason it uses
references).

[caitp (gmail), 2016/03/21 16:15:03]

Since the early return was put back, there isn't much value in the parameter
anymore anyways, so it's gone

   uc32 x = 0;
   int d = HexValue(c0_);
-  if (d < 0) {
-    return -1;

[adamk, 2016/03/18 18:21:55]

Losing this and the below early return makes me a little nervous. Why don't you
just want to return here and below and handle the error-reporting in the caller,
since you already have error-reporting there? Is it so that your reported
location is wider? That seems odd to me, given that the problem happens as soon
as you hit a non-hex character. Scanning after that is pretty meaningless.

[caitp (gmail), 2016/03/18 19:00:36]

the idea is, if you get hex characters until the }, you can say that its an
invalid codepoint, and if you fail to get a hex digit or closing brace, you can
report the unclosed error. I'd like to get the chromium devtools using the
location data to clarify the problem, and in this case it will point right to
the invalid codepoint.

its not worth much without location data being used, though. i can remove it
until after that change is made, I guess.

[adamk, 2016/03/18 19:09:57]

Yeah, I'd just prefer a minimal change if the location data isn't being used;
the improved message seems like the main point.

[caitp (gmail), 2016/03/21 16:15:03]

Done.

-  }
+
   while (d >= 0) {
     x = x * 16 + d;
-    if (x > max_value) return -1;
+    if (x > max_value) {
+      bad_codepoint = true;
+    }
     Advance<capture_raw>();
     d = HexValue(c0_);
   }
+
+  if (d < 0 && (c0_ != '}')) {
+    ReportScannerError(x != 0 ? MessageTemplate::kUnclosedUnicodeEscapeSequence
+                              : MessageTemplate::kInvalidUnicodeEscapeSequence,
+                       source_pos());
+    return -1;
+  }
+
   return x;
 }
 
 
 // Ensure that tokens can be stored in a byte.
 STATIC_ASSERT(Token::NUM_TOKENS <= 0x100);
 
 // Table of one-character tokens, by character (0x00..0x7f only).
 static const byte one_char_tokens[] = {
   Token::ILLEGAL,
@@ skipping 735 matching lines @@
     if (c == '\\') break;
     Advance<false, false>();
     AddLiteralChar(c);
   }
 
   while (c0_ != quote && c0_ >= 0
          && !unicode_cache_->IsLineTerminator(c0_)) {
     uc32 c = c0_;
     Advance();
     if (c == '\\') {
-      if (c0_ < 0 || !ScanEscape<false, false>()) return Token::ILLEGAL;
+      if (c0_ < 0 || !ScanEscape<false, false>()) {
+        return Token::ILLEGAL;
+      }
     } else {
       AddLiteralChar(c);
     }
   }
   if (c0_ != quote) return Token::ILLEGAL;
   literal.Complete();
 
   Advance();  // consume quote
   return Token::STRING;
 }
@@ skipping 11 matching lines @@
   //
   // A TEMPLATE_SPAN should always be followed by an Expression, while a
   // TEMPLATE_TAIL terminates a TemplateLiteral and does not need to be
   // followed by an Expression.
 
   Token::Value result = Token::TEMPLATE_SPAN;
   LiteralScope literal(this);
   StartRawLiteral();
   const bool capture_raw = true;
   const bool in_template_literal = true;
-
   while (true) {
     uc32 c = c0_;
     Advance<capture_raw>();
     if (c == '`') {
       result = Token::TEMPLATE_TAIL;
       ReduceRawLiteralLength(1);
       break;
     } else if (c == '$' && c0_ == '{') {
       Advance<capture_raw>();  // Consume '{'
       ReduceRawLiteralLength(2);
@@ skipping 199 matching lines @@
   Advance();
   return ScanUnicodeEscape<false>();
 }
 
 
 template <bool capture_raw>
 uc32 Scanner::ScanUnicodeEscape() {
   // Accept both \uxxxx and \u{xxxxxx}. In the latter case, the number of
   // hex digits between { } is arbitrary. \ and u have already been read.
   if (c0_ == '{') {
+    int begin = source_pos() - 2;
+    bool bad_codepoint = false;
     Advance<capture_raw>();
-    uc32 cp = ScanUnlimitedLengthHexNumber<capture_raw>(0x10ffff);
-    if (cp < 0) {
-      return -1;
-    }
-    if (c0_ != '}') {
+    uc32 cp =
+        ScanUnlimitedLengthHexNumber<capture_raw>(0x10ffff, bad_codepoint);
+    if (cp < 0 || bad_codepoint) {
+      ReportScannerError(MessageTemplate::kUndefinedUnicodeCodePoint, begin,
+                         source_pos() + 1);
       return -1;
     }
     Advance<capture_raw>();
     return cp;
   }
-  return ScanHexNumber<capture_raw>(4);
+  const bool unicode = true;
+  return ScanHexNumber<capture_raw, unicode>(4);
 }
 
 
 // ----------------------------------------------------------------------------
 // Keyword Matcher
 
 #define KEYWORDS(KEYWORD_GROUP, KEYWORD)                    \
   KEYWORD_GROUP('b')                                        \
   KEYWORD("break", Token::BREAK)                            \
   KEYWORD_GROUP('c')                                        \
@@ skipping 546 matching lines @@
     backing_store_.Add(static_cast<uint8_t>((one_byte_length >> 7) | 0x80u));
   }
   backing_store_.Add(static_cast<uint8_t>(one_byte_length & 0x7f));
 
   backing_store_.AddBlock(bytes);
   return backing_store_.EndSequence().start();
 }
 
 }  // namespace internal
 }  // namespace v8