Page MenuHomePhabricator

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

Authored by kevcadieux on Jun 28 2022, 11:42 PM.

Details

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.

Diff Detail

Event Timeline

kevcadieux created this revision.Jun 28 2022, 11:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 28 2022, 11:42 PM
kevcadieux requested review of this revision.Jun 28 2022, 11:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 28 2022, 11:42 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

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
This revision is now accepted and ready to land.Jun 29 2022, 8:00 AM
This revision was automatically updated to reflect the committed changes.
HazardyKnusperkeks added a project: Restricted Project.Jul 4 2022, 3:34 AM