[clang-format] Fix incorrect isspace input (NFC)

[kevcadieux]

Summary

This change fixes a clang-format unit test failure introduced by D124748. The countLeadingWhitespace function was calling isspace with values that could fall outside the valid input range. The valid input range for isspace is unsigned 0-255. Values outside this range produce undefined behavior, which on Windows manifests as an assertion being raised in the debug runtime libraries. countLeadingWhitespace was calling isspace with a signed char that could produce a negative value if the underlying byte's value was 128 or above, which can happen for non-ASCII encodings. The fix is to use StringRef's bytes_begin and bytes_end iterators to read the values as unsigned chars instead.

This bug can be reproduced by building the check-clang-unit target with a DEBUG configuration under Windows. This change is already covered by existing unit tests.

[kevcadieux]

FYI: failure looks like the following:

[ RUN      ] FormatTest.IncorrectUnbalancedBracesInMacrosWithUnicode
Exception Code: 0x80000003
 #0 0x00007ffe4253fc44 (C:\WINDOWS\SYSTEM32\ucrtbased.dll+0x7fc44)
 #1 0x00007ffe4254a379 (C:\WINDOWS\SYSTEM32\ucrtbased.dll+0x8a379)
 #2 0x00007ffe4254a245 (C:\WINDOWS\SYSTEM32\ucrtbased.dll+0x8a245)
 #3 0x00007ffe4254a896 (C:\WINDOWS\SYSTEM32\ucrtbased.dll+0x8a896)
 #4 0x00007ff649e04088 clang::format::countLeadingWhitespace D:\github\llvm-project\clang\lib\Format\FormatTokenLexer.cpp:869:0
 #5 0x00007ff649e02184 clang::format::FormatTokenLexer::getNextToken(void) D:\github\llvm-project\clang\lib\Format\FormatTokenLexer.cpp:912:0
 #6 0x00007ff649dfdd79 clang::format::FormatTokenLexer::lex(void) D:\github\llvm-project\clang\lib\Format\FormatTokenLexer.cpp:80:0
 #7 0x00007ff649e97a3b clang::format::TokenAnalyzer::process(void) D:\github\llvm-project\clang\lib\Format\TokenAnalyzer.cpp:109:0
 #8 0x00007ff649db28d6 <lambda_371ccad48ec3793236c652f6dc6807bc>::operator() D:\github\llvm-project\clang\lib\Format\Format.cpp:3277:0
 #9 0x00007ff649dd00d8 std::invoke<<lambda_371ccad48ec3793236c652f6dc6807bc> &,clang::format::Environment const &> C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\type_traits:1534:0
#10 0x00007ff649dbf2e8 std::_Invoker_ret<std::pair<clang::tooling::Replacements,unsigned int>,0>::_Call<<lambda_371ccad48ec3793236c652f6dc6807bc> &,clang::format::Environment const &> C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\functional:660:0
#11 0x00007ff649db40b7 std::_Func_impl_no_alloc<<lambda_371ccad48ec3793236c652f6dc6807bc>,std::pair<clang::tooling::Replacements,unsigned int>,clang::format::Environment const &>::_Do_call C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\functional:822:0
#12 0x00007ff649de48a5 std::_Func_class<struct std::pair<class clang::tooling::Replacements, unsigned int>, class clang::format::Environment const &>::operator()(class clang::format::Environment const &) const C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\functional:869:0
#13 0x00007ff649da8575 clang::format::internal::reformat(struct clang::format::FormatStyle const &, class llvm::StringRef, class llvm::ArrayRef<class clang::tooling::Range>, unsigned int, unsigned int, unsigned int, class llvm::StringRef, struct clang::format::FormattingAttemptStatus *) D:\github\llvm-project\clang\lib\Format\Format.cpp:3321:0
#14 0x00007ff649da5659 clang::format::reformat(struct clang::format::FormatStyle const &, class llvm::StringRef, class llvm::ArrayRef<class clang::tooling::Range>, class llvm::StringRef, struct clang::format::FormattingAttemptStatus *) D:\github\llvm-project\clang\lib\Format\Format.cpp:3346:0
#15 0x00007ff6497bc403 clang::format::`anonymous namespace'::FormatTest::format D:\github\llvm-project\clang\unittests\Format\FormatTest.cpp:43:0
#16 0x00007ff6497bd3a5 clang::format::`anonymous namespace'::FormatTest::verifyNoCrash D:\github\llvm-project\clang\unittests\Format\FormatTest.cpp:107:0
#17 0x00007ff64983abc3 clang::format::`anonymous namespace'::FormatTest_IncorrectUnbalancedBracesInMacrosWithUnicode_Test::TestBody D:\github\llvm-project\clang\unittests\Format\FormatTest.cpp:12178:0
#18 0x00007ff649ce991d testing::internal::HandleSehExceptionsInMethodIfSupported<class testing::Test, void>(class testing::Test *, void (__cdecl testing::Test::*)(void), char const *) D:\github\llvm-project\llvm\utils\unittest\googletest\src\gtest.cc:2418:0
#19 0x00007ff649ce977c testing::internal::HandleExceptionsInMethodIfSupported<class testing::Test, void>(class testing::Test *, void (__cdecl testing::Test::*)(void), char const *) D:\github\llvm-project\llvm\utils\unittest\googletest\src\gtest.cc:2488:0
#20 0x00007ff649cc61eb testing::Test::Run(void) D:\github\llvm-project\llvm\utils\unittest\googletest\src\gtest.cc:2515:0
#21 0x00007ff649cc6c56 testing::TestInfo::Run(void) D:\github\llvm-project\llvm\utils\unittest\googletest\src\gtest.cc:2687:0
#22 0x00007ff649cc72ee testing::TestSuite::Run(void) D:\github\llvm-project\llvm\utils\unittest\googletest\src\gtest.cc:2817:0
#23 0x00007ff649ccd668 testing::internal::UnitTestImpl::RunAllTests(void) D:\github\llvm-project\llvm\utils\unittest\googletest\src\gtest.cc:5339:0
#24 0x00007ff649ce9b0d testing::internal::HandleSehExceptionsInMethodIfSupported<class testing::internal::UnitTestImpl, bool>(class testing::internal::UnitTestImpl *, bool (__cdecl testing::internal::UnitTestImpl::*)(void), char const *) D:\github\llvm-project\llvm\utils\unittest\googletest\src\gtest.cc:2418:0
#25 0x00007ff649ce98cc testing::internal::HandleExceptionsInMethodIfSupported<class testing::internal::UnitTestImpl, bool>(class testing::internal::UnitTestImpl *, bool (__cdecl testing::internal::UnitTestImpl::*)(void), char const *) D:\github\llvm-project\llvm\utils\unittest\googletest\src\gtest.cc:2488:0
#26 0x00007ff649cc7940 testing::UnitTest::Run(void) D:\github\llvm-project\llvm\utils\unittest\googletest\src\gtest.cc:4925:0
#27 0x00007ff64a061043 RUN_ALL_TESTS(void) D:\github\llvm-project\llvm\utils\unittest\googletest\include\gtest\gtest.h:2473:0
#28 0x00007ff64a060fbe main D:\github\llvm-project\llvm\utils\unittest\UnitTestMain\TestMain.cpp:56:0
#29 0x00007ff64a044639 invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:79:0
#30 0x00007ff64a0444de __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288:0
#31 0x00007ff64a04439e __scrt_common_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:331:0
#32 0x00007ff64a0446ce mainCRTStartup D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_main.cpp:17:0
#33 0x00007ffe94b07034 (C:\WINDOWS\System32\KERNEL32.DLL+0x17034)
#34 0x00007ffe96222651 (C:\WINDOWS\SYSTEM32\ntdll.dll+0x52651)

--
exit: 2147483651
--
shard JSON output does not exist: D:\github\llvm-project\build\tools\clang\unittests\Format\.\FormatTests.exe-Clang-Unit-15488-15-30.json
********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
********************
Failed Tests (1):
  Clang-Unit :: Format/./FormatTests.exe/15/30

[MyDeveloperDay]

LGTM

clang/lib/Format/FormatTokenLexer.cpp

 
 /// Count the length of leading whitespace in a token.
 static size_t countLeadingWhitespace(StringRef Text) {
   // Basically counting the length matched by this regex.
   // "^([\n\r\f\v \t]|(\\\\|\\?\\?/)[\n\r])+"
   // Directly using the regex turned out to be slow. With the regex
   // version formatting all files in this directory took about 1.25
   // seconds. This version took about 0.5 seconds.
-  const char *Cur = Text.begin();
-  while (Cur < Text.end()) {
+  const unsigned char *const Begin = Text.bytes_begin();
+  const unsigned char *const End = Text.bytes_end();
+  const unsigned char *Cur = Begin;
+  while (Cur < End) {
     if (isspace(Cur[0])) {
       ++Cur;
     } else if (Cur[0] == '\\' && (Cur[1] == '\n' || Cur[1] == '\r')) {
       // A '\' followed by a newline always escapes the newline, regardless
       // of whether there is another '\' before it.
       // The source has a null byte at the end. So the end of the entire input
       // isn't reached yet. Also the lexer doesn't break apart an escaped
       // newline.
-      assert(Text.end() - Cur >= 2);
+      assert(End - Cur >= 2);
       Cur += 2;
     } else if (Cur[0] == '?' && Cur[1] == '?' && Cur[2] == '/' &&
                (Cur[3] == '\n' || Cur[3] == '\r')) {
       // Newlines can also be escaped by a '?' '?' '/' trigraph. By the way, the
       // characters are quoted individually in this comment because if we write
       // them together some compilers warn that we have a trigraph in the code.
-      assert(Text.end() - Cur >= 4);
+      assert(End - Cur >= 4);
       Cur += 4;
     } else {
       break;
     }
   }
-  return Cur - Text.begin();
+  return Cur - Begin;
 }
 
 FormatToken *FormatTokenLexer::getNextToken() {
   if (StateStack.top() == LexerState::TOKEN_STASHED) {
     StateStack.pop();
     return getStashedToken();
   }