]> granicus.if.org Git - clang/commitdiff
Fix crash in getStringSplit.
authorAlexander Kornienko <alexfh@google.com>
Tue, 26 Nov 2013 10:38:53 +0000 (10:38 +0000)
committerAlexander Kornienko <alexfh@google.com>
Tue, 26 Nov 2013 10:38:53 +0000 (10:38 +0000)
Summary:
getStringSplit used to crash, when trying to split a long string
literal containing both printable and unprintable multi-byte UTF-8 characters.

Reviewers: djasper, klimek

Reviewed By: djasper

CC: cfe-commits, klimek
Differential Revision: http://llvm-reviews.chandlerc.com/D2268

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@195728 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Format/BreakableToken.cpp
lib/Format/Encoding.h
unittests/Format/FormatTest.cpp

index d720ce990b52801e59d1af248f92b40ef7eb03e0..a08102a3b787cf70268742e3ff1acb7bf553730d 100644 (file)
@@ -92,9 +92,7 @@ static BreakableToken::Split getStringSplit(StringRef Text,
     return BreakableToken::Split(StringRef::npos, 0);
   if (ColumnLimit <= UsedColumns)
     return BreakableToken::Split(StringRef::npos, 0);
-  unsigned MaxSplit = std::min<unsigned>(
-      ColumnLimit - UsedColumns,
-      encoding::columnWidthWithTabs(Text, UsedColumns, TabWidth, Encoding) - 1);
+  unsigned MaxSplit = ColumnLimit - UsedColumns;
   StringRef::size_type SpaceOffset = 0;
   StringRef::size_type SlashOffset = 0;
   StringRef::size_type WordStartOffset = 0;
@@ -110,7 +108,7 @@ static BreakableToken::Split getStringSplit(StringRef Text,
           Text.substr(0, Advance), UsedColumns + Chars, TabWidth, Encoding);
     }
 
-    if (Chars > MaxSplit)
+    if (Chars > MaxSplit || Text.size() == Advance)
       break;
 
     if (IsBlank(Text[0]))
index 356334d5376f1480dae36ea9d4de46b8dc8b63e7..dba5174b97b40b6b3ebdf76a14db5b9ad6566ae5 100644 (file)
@@ -64,6 +64,10 @@ inline unsigned getCodePointCount(StringRef Text, Encoding Encoding) {
 inline unsigned columnWidth(StringRef Text, Encoding Encoding) {
   if (Encoding == Encoding_UTF8) {
     int ContentWidth = llvm::sys::unicode::columnWidthUTF8(Text);
+    // FIXME: Figure out the correct way to handle this in the presence of both
+    // printable and unprintable multi-byte UTF-8 characters. Falling back to
+    // returning the number of bytes may cause problems, as columnWidth suddenly
+    // becomes non-additive.
     if (ContentWidth >= 0)
       return ContentWidth;
   }
@@ -81,9 +85,7 @@ inline unsigned columnWidthWithTabs(StringRef Text, unsigned StartColumn,
     StringRef::size_type TabPos = Tail.find('\t');
     if (TabPos == StringRef::npos)
       return TotalWidth + columnWidth(Tail, Encoding);
-    int Width = columnWidth(Tail.substr(0, TabPos), Encoding);
-    assert(Width >= 0);
-    TotalWidth += Width;
+    TotalWidth += columnWidth(Tail.substr(0, TabPos), Encoding);
     TotalWidth += TabWidth - (TotalWidth + StartColumn) % TabWidth;
     Tail = Tail.substr(TabPos + 1);
   }
index fc0e935037bff77d6b748f0b38e352bfd19e87b2..a7dce3b86f5765872caefab9f871c5e7d23464fc 100644 (file)
@@ -6991,6 +6991,16 @@ TEST_F(FormatTest, CountsUTF8CharactersProperly) {
 }
 
 TEST_F(FormatTest, SplitsUTF8Strings) {
+  // Non-printable characters' width is currently considered to be the length in
+  // bytes in UTF8. The characters can be displayed in very different manner
+  // (zero-width, single width with a substitution glyph, expanded to their code
+  // (e.g. "<8d>"), so there's no single correct way to handle them.
+  EXPECT_EQ("\"aaaaÄ\"\n"
+            "\"\8d\";",
+            format("\"aaaaÄ\8d\";", getLLVMStyleWithColumns(10)));
+  EXPECT_EQ("\"aaaaaaaÄ\"\n"
+            "\"\8d\";",
+            format("\"aaaaaaaÄ\8d\";", getLLVMStyleWithColumns(10)));
   EXPECT_EQ(
       "\"Однажды, в \"\n"
       "\"студёную \"\n"
@@ -7024,6 +7034,8 @@ TEST_F(FormatTest, HandlesDoubleWidthCharsInMultiLineStrings) {
 }
 
 TEST_F(FormatTest, SplitsUTF8LineComments) {
+  EXPECT_EQ("// aaaaÄ\8d",
+            format("// aaaaÄ\8d", getLLVMStyleWithColumns(10)));
   EXPECT_EQ("// Я из лесу\n"
             "// вышел; был\n"
             "// сильный\n"