[clang][Lex] Fix a crash on malformed string literals

[tahonermann]

I think this looks right, but it would be good for @cor3ntin to take a look.

[cor3ntin]

Yes, this looks reasonable. The additional warning is somewhat unfortunate, and we could avoid it, but at the same time i don't expect it would bother anyone given how unlikely the scenario is to begin with.

Thanks for fixing that!

[hokein]

Thanks! I believe it fixes a long tail crash.

clang/unittests/Lex/LexerTest.cpp

665

nit: I'd probably use a clang lit test

auto nocrash = L"\N";

but up to you.

[cor3ntin]

clang/unittests/Lex/LexerTest.cpp

665

Nah, this test is good. It could only crash at at the end of a file, which would be flacky at best with lit.

[kadircet]

this is causing some crashes in buildbots, and i can't repro. reverting until i figure this out.

[kadircet]

i'll reland the fix without the test case, as it's clearly fixing one of the codepaths that'll lead to a crash. it's only the test case that's crashing, because i don't think there are certain test cases that exposed literal parser to invalid/incomplete input and i am not able to reproduce any of the crashes i've seen in buildbots locally (even under asan/msan) and because the bots don't have stack traces for whatever reason, it's impossible to chase from my side. i've pinged buildbot owners in https://discourse.llvm.org/t/buildbots-missing-stack-traces/65753 to see the issue fixed.

[cor3ntin]

In D135161#3839510, @kadircet wrote:

i'll reland the fix without the test case, as it's clearly fixing one of the codepaths that'll lead to a crash. it's only the test case that's crashing, because i don't think there are certain test cases that exposed literal parser to invalid/incomplete input and i am not able to reproduce any of the crashes i've seen in buildbots locally (even under asan/msan) and because the bots don't have stack traces for whatever reason, it's impossible to chase from my side. i've pinged buildbot owners in https://discourse.llvm.org/t/buildbots-missing-stack-traces/65753 to see the issue fixed.

Makes sense. The fix is correct and if it wasn't it would affect all bots equally (I highly doubt that code path is exercised anywhere but the lex test)

clang/lib/Lex/LiteralSupport.cpp

static bool ProcessNamedUCNEscape(const char *ThisTokBegin,
   assert(UcnBegin[0] == '\\' && UcnBegin[1] == 'N');
   ThisTokBuf += 2;
   if (ThisTokBuf == ThisTokEnd || *ThisTokBuf != '{') {
     if (Diags) {
       Diag(Diags, Features, Loc, ThisTokBegin, UcnBegin, ThisTokBuf,
            diag::err_delimited_escape_missing_brace)
           << StringRef(&ThisTokBuf[-1], 1);
     }
-    ThisTokBuf++;
     return false;
   }
   ThisTokBuf++;
   const char *ClosingBrace =
       std::find_if_not(ThisTokBuf, ThisTokEnd, [](char C) {
         return llvm::isAlnum(C) || llvm::isSpace(C) || C == '_' || C == '-';
       });
   bool Incomplete = ClosingBrace == ThisTokEnd || *ClosingBrace != '}';

clang/test/Lexer/char-escapes-delimited.c

void named(void) {
 
   unsigned f = L'\N{GREEK CAPITAL LETTER DELTA}'; // ext-warning {{extension}} cxx2b-warning {{C++2b}}
 
   unsigned g = u'\N{LOTUS}'; // expected-error {{character too large for enclosing character literal type}} \
                              // ext-warning {{extension}} cxx2b-warning {{C++2b}}
 
   unsigned h = U'\N{LOTUS}';                      // ext-warning {{extension}} cxx2b-warning {{C++2b}}
   unsigned i = u'\N{GREEK CAPITAL LETTER DELTA}'; // ext-warning {{extension}} cxx2b-warning {{C++2b}}
-  char j = '\NN';                                 // expected-error {{expected '{' after '\N' escape sequence}}
+  char j = '\NN';                                 // expected-error {{expected '{' after '\N' escape sequence}} expected-warning {{multi-character character constant}}
   unsigned k = u'\N{LOTUS';                       // expected-error {{incomplete universal character name}}
 }
 
 void separators(void) {
   (void)"\x{12'3}"; // expected-error {{invalid digit ''' in escape sequence}}
   (void)"\u{12'3}"; // expected-error {{invalid digit ''' in escape sequence}}
   (void)"\o{12'3}"; // expected-error {{invalid digit ''' in escape sequence}}
 

clang/unittests/Lex/LexerTest.cpp

 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/TargetOptions.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/HeaderSearchOptions.h"
+#include "clang/Lex/LiteralSupport.h"
 #include "clang/Lex/MacroArgs.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/ModuleLoader.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "gmock/gmock.h"
TEST_F(LexerTest, RawAndNormalLexSameForLineComments) {
   EXPECT_FALSE(ToksView.empty());
   while (!L.LexFromRawLexer(T)) {
     ASSERT_TRUE(!ToksView.empty());
     EXPECT_EQ(T.getKind(), ToksView.front().getKind());
     ToksView = ToksView.drop_front();
   }
   EXPECT_TRUE(ToksView.empty());
 }
+
+TEST_F(LexerTest, BrokenStringLiteral) {
+  const llvm::StringLiteral Source = R"cpp("\N")cpp";
+  // Make sure this isn't crashing.
+  StringLiteralParser P(Lex(Source), *PP);
+  EXPECT_TRUE(P.hadError);
+}
 } // anonymous namespace