[Lex] Fix handling numerical literals ending with ' and signed exponent.

[vsapsai]

Summary

For input 0'e+1 lexer tokenized as numeric constant only 0'e. Later
NumericLiteralParser skipped 0 and ' as digits and parsed e+1 as valid
exponent going past the end of the token. Because it didn't mark numeric
literal as having an error, it continued parsing and tried to expandUCNs
with StringRef of length -2.

The fix is not to parse exponent when we reached the end of token.

Discovered by OSS-Fuzz:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=4588

rdar://problem/36076719

[vsapsai]

This fixes the OSS-Fuzz bug but I don't know if it is sufficient. Should I also make Lexer::LexNumericConstant to include +1 part as tok::numeric_constant?

[rsmith]

The lexer is doing the right thing; per the C++ lexical rules, the +1 is not part of the token in this case.

I don't think this fix is in the right place; we will still examine characters after the end of the literal, even with this applied, and that doesn't seem right to me (even though the literal parser is constructed in such a way that it is valid to do so, as long as it doesn't read past a nul byte). It looks like the problem is that NumericLiteralParser::ParseDecimalOrOctalCommon will examine *s in cases where it might point past the end of the literal; changing

if (*s == '+' || *s == '-')  s++; // sign

to

if (s != ThisTokEnd && (*s == '+' || *s == '-'))  s++; // sign

would seem appropriate. But I think I'd be most in favor of that change plus your change plus a change to suppress the "no digits in suffix" error if we've already had an error. Seem reasonable?

[vsapsai]

Yep, the plan sounds reasonable. I also noticed that we have

if (*s == '+' || *s == '-')  s++; // sign

code in NumericLiteralParser::ParseNumberStartingWithZero too. I plan to make the same change for hexadecimal numbers and check the behaviour in debugger.

[vsapsai]

• Don't parse exponent past the end of token, add same test+fix for hexadecimal numbers.

[vsapsai]

I've addressed all known issues, please take another look.

[vsapsai]

Checked that suggested change also fixes another OSS-Fuzz bug https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=4664

[vsapsai]

Ping.

[rsmith]

LGTM, thanks!

[vsapsai]

Thanks for the review.

clang/lib/Lex/LiteralSupport.cpp

if (*s == '.') {
     s = SkipDigits(s); // Skip suffix.
   }
   if (*s == 'e' || *s == 'E') { // exponent
     checkSeparator(TokLoc, s, CSK_AfterDigits);
     const char *Exponent = s;
     s++;
     radix = 10;
     saw_exponent = true;
-    if (*s == '+' || *s == '-')  s++; // sign
+    if (s != ThisTokEnd && (*s == '+' || *s == '-'))  s++; // sign
     const char *first_non_digit = SkipDigits(s);
     if (containsDigits(s, first_non_digit)) {
       checkSeparator(TokLoc, s, CSK_BeforeDigits);
       s = first_non_digit;
     } else {
+      if (!hadError) {
         PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, Exponent-ThisTokBegin),
                 diag::err_exponent_has_no_digits);
         hadError = true;
+      }
       return;
     }
   }
 }
 
 /// Determine whether a suffix is a valid ud-suffix. We avoid treating reserved
 /// suffixes as ud-suffixes, because the diagnostic experience is better if we
 /// treat it as an invalid suffix.
void NumericLiteralParser::checkSeparator(SourceLocation TokLoc,
                                           CheckSeparatorKind IsAfterDigits) {
   if (IsAfterDigits == CSK_AfterDigits) {
     if (Pos == ThisTokBegin)
       return;
     --Pos;
   } else if (Pos == ThisTokEnd)
     return;
 
-  if (isDigitSeparator(*Pos))
+  if (isDigitSeparator(*Pos)) {
     PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, Pos - ThisTokBegin),
             diag::err_digit_separator_not_between_digits)
       << IsAfterDigits;
+    hadError = true;
+  }
 }
 
 /// ParseNumberStartingWithZero - This method is called when the first character
 /// of the number is found to be a zero.  This means it is either an octal
 /// number (like '04') or a hex number ('0x123a') a binary number ('0b1010') or
 /// a floating point number (01239.123e4).  Eat the prefix, determining the
 /// radix etc.
 void NumericLiteralParser::ParseNumberStartingWithZero(SourceLocation TokLoc) {
if ((c1 == 'x' || c1 == 'X') && (isHexDigit(s[1]) || s[1] == '.')) {
 
     // A binary exponent can appear with or with a '.'. If dotted, the
     // binary exponent is required.
     if (*s == 'p' || *s == 'P') {
       checkSeparator(TokLoc, s, CSK_AfterDigits);
       const char *Exponent = s;
       s++;
       saw_exponent = true;
-      if (*s == '+' || *s == '-')  s++; // sign
+      if (s != ThisTokEnd && (*s == '+' || *s == '-'))  s++; // sign
       const char *first_non_digit = SkipDigits(s);
       if (!containsDigits(s, first_non_digit)) {
+        if (!hadError) {
           PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, Exponent-ThisTokBegin),
                   diag::err_exponent_has_no_digits);
           hadError = true;
+        }
         return;
       }
       checkSeparator(TokLoc, s, CSK_BeforeDigits);
       s = first_non_digit;
 
       if (!PP.getLangOpts().HexFloats)
         PP.Diag(TokLoc, PP.getLangOpts().CPlusPlus
                             ? diag::ext_hex_literal_invalid

clang/test/Lexer/cxx1y_digit_separators.cpp

namespace floating {
   float p = 0'e1; // expected-error {{digit separator cannot appear at end of digit sequence}}
   float q = 0'0e1;
   float r = 0.'0e1; // expected-error {{digit separator cannot appear at start of digit sequence}}
   float s = 0.0'e1; // expected-error {{digit separator cannot appear at end of digit sequence}}
   float t = 0.0e'1; // expected-error {{digit separator cannot appear at start of digit sequence}}
   float u = 0x.'p1f; // expected-error {{hexadecimal floating literal requires a significand}}
   float v = 0e'f; // expected-error {{exponent has no digits}}
   float w = 0x0p'f; // expected-error {{exponent has no digits}}
+  float x = 0'e+1; // expected-error {{digit separator cannot appear at end of digit sequence}}
+  float y = 0x0'p+1; // expected-error {{digit separator cannot appear at end of digit sequence}}
 }
 
 #line 123'456
 static_assert(__LINE__ == 123456, "");
 
 // x has value 0 in C++11 and 34 in C++1y.
 #define M(x, ...) __VA_ARGS__
 constexpr int x = { M(1'2,3'4) };