[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

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) {

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}}
 }
 
 #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) };