[clang-format][PR41964] Fix crash with SIGFPE when TabWidth is set to 0 and line starts with tab

[MyDeveloperDay]

Summary

clang-format 8.0 crashes with SIGFPE (floating point exception) when formatting following file:
app.cpp:
void a() {
//line starts with '\t'
}

$ clang-format -style='{TabWidth: 0}' app.cpp

[klimek]

clang/lib/Format/FormatTokenLexer.cpp

660 ↗

(On Diff #220535)

Shouldn't that be (Style.TabWidth ? Column % Style.TabWidth)?
Otherwise can't that still crash when Column != 0 and TabWidth is 0?

[MyDeveloperDay]

clang/lib/Format/FormatTokenLexer.cpp

660 ↗

(On Diff #220535)

let me add some tests...

[MyDeveloperDay]

I should have known it was more involved.

v % 0 and v / 0 are both undefined behavior

[klimek]

LG. That makes lots of sense now :) Thanks!

cfe/trunk/lib/Format/Encoding.h

inline unsigned columnWidthWithTabs(StringRef Text, unsigned StartColumn,
                                     unsigned TabWidth, Encoding Encoding) {
   unsigned TotalWidth = 0;
   StringRef Tail = Text;
   for (;;) {
     StringRef::size_type TabPos = Tail.find('\t');
     if (TabPos == StringRef::npos)
       return TotalWidth + columnWidth(Tail, Encoding);
     TotalWidth += columnWidth(Tail.substr(0, TabPos), Encoding);
+    if (TabWidth)
       TotalWidth += TabWidth - (TotalWidth + StartColumn) % TabWidth;
     Tail = Tail.substr(TabPos + 1);
   }
 }
 
 /// Gets the number of bytes in a sequence representing a single
 /// codepoint and starting with FirstChar in the specified Encoding.
 inline unsigned getCodePointNumBytes(char FirstChar, Encoding Encoding) {
   switch (Encoding) {

cfe/trunk/lib/Format/FormatTokenLexer.cpp

for (int i = 0, e = Text.size(); i != e; ++i) {
       case '\f':
       case '\v':
         Column = 0;
         break;
       case ' ':
         ++Column;
         break;
       case '\t':
-        Column += Style.TabWidth - Column % Style.TabWidth;
+        Column +=
+            Style.TabWidth - (Style.TabWidth ? Column % Style.TabWidth : 0);
         break;
       case '\\':
         if (i + 1 == e || (Text[i + 1] != '\r' && Text[i + 1] != '\n'))
           FormatTok->Type = TT_ImplicitStringLiteral;
         break;
       default:
         FormatTok->Type = TT_ImplicitStringLiteral;
         break;

cfe/trunk/lib/Format/WhitespaceManager.cpp

 void WhitespaceManager::appendIndentText(std::string &Text,
                                          unsigned IndentLevel, unsigned Spaces,
                                          unsigned WhitespaceStartColumn) {
   switch (Style.UseTab) {
   case FormatStyle::UT_Never:
     Text.append(Spaces, ' ');
     break;
   case FormatStyle::UT_Always: {
+    if (Style.TabWidth) {
       unsigned FirstTabWidth =
           Style.TabWidth - WhitespaceStartColumn % Style.TabWidth;
+
       // Insert only spaces when we want to end up before the next tab.
       if (Spaces < FirstTabWidth || Spaces == 1) {
         Text.append(Spaces, ' ');
         break;
       }
       // Align to the next tab.
       Spaces -= FirstTabWidth;
       Text.append("\t");
 
       Text.append(Spaces / Style.TabWidth, '\t');
       Text.append(Spaces % Style.TabWidth, ' ');
+    } else if (Spaces == 1) {
+      Text.append(Spaces, ' ');
+    }
     break;
   }
   case FormatStyle::UT_ForIndentation:
     if (WhitespaceStartColumn == 0) {
       unsigned Indentation = IndentLevel * Style.IndentWidth;
       // This happens, e.g. when a line in a block comment is indented less than
       // the first one.
       if (Indentation > Spaces)
         Indentation = Spaces;
+      if (Style.TabWidth) {
         unsigned Tabs = Indentation / Style.TabWidth;
         Text.append(Tabs, '\t');
         Spaces -= Tabs * Style.TabWidth;
       }
+    }
     Text.append(Spaces, ' ');
     break;
   case FormatStyle::UT_ForContinuationAndIndentation:
-    if (WhitespaceStartColumn == 0) {
+    if (WhitespaceStartColumn == 0 && Style.TabWidth) {
       unsigned Tabs = Spaces / Style.TabWidth;
       Text.append(Tabs, '\t');
       Spaces -= Tabs * Style.TabWidth;
     }
     Text.append(Spaces, ' ');
     break;
   }
 }
 
 } // namespace format
 } // namespace clang

cfe/trunk/unittests/Format/FormatTest.cpp

verifyFormat("class Assign {\n"
                "\t\tint         random = 4;\n"
                "\t\tstd::string alphabet =\n"
                "\t\t\t\"abcdefghijklmnopqrstuvwxyz\";\n"
                "\t}\n"
                "};",
                Tab);
 }
 
+TEST_F(FormatTest, ZeroTabWidth) {
+  FormatStyle Tab = getLLVMStyleWithColumns(42);
+  Tab.IndentWidth = 8;
+  Tab.UseTab = FormatStyle::UT_Never;
+  Tab.TabWidth = 0;
+  EXPECT_EQ("void a(){\n"
+            "    // line starts with '\t'\n"
+            "};",
+            format("void a(){\n"
+                   "\t// line starts with '\t'\n"
+                   "};",
+                   Tab));
+
+  EXPECT_EQ("void a(){\n"
+            "    // line starts with '\t'\n"
+            "};",
+            format("void a(){\n"
+                   "\t\t// line starts with '\t'\n"
+                   "};",
+                   Tab));
+
+  Tab.UseTab = FormatStyle::UT_ForIndentation;
+  EXPECT_EQ("void a(){\n"
+            "    // line starts with '\t'\n"
+            "};",
+            format("void a(){\n"
+                   "\t// line starts with '\t'\n"
+                   "};",
+                   Tab));
+
+  EXPECT_EQ("void a(){\n"
+            "    // line starts with '\t'\n"
+            "};",
+            format("void a(){\n"
+                   "\t\t// line starts with '\t'\n"
+                   "};",
+                   Tab));
+
+  Tab.UseTab = FormatStyle::UT_ForContinuationAndIndentation;
+  EXPECT_EQ("void a(){\n"
+            "    // line starts with '\t'\n"
+            "};",
+            format("void a(){\n"
+                   "\t// line starts with '\t'\n"
+                   "};",
+                   Tab));
+
+  EXPECT_EQ("void a(){\n"
+            "    // line starts with '\t'\n"
+            "};",
+            format("void a(){\n"
+                   "\t\t// line starts with '\t'\n"
+                   "};",
+                   Tab));
+
+  Tab.UseTab = FormatStyle::UT_Always;
+  EXPECT_EQ("void a(){\n"
+            "// line starts with '\t'\n"
+            "};",
+            format("void a(){\n"
+                   "\t// line starts with '\t'\n"
+                   "};",
+                   Tab));
+
+  EXPECT_EQ("void a(){\n"
+            "// line starts with '\t'\n"
+            "};",
+            format("void a(){\n"
+                   "\t\t// line starts with '\t'\n"
+                   "};",
+                   Tab));
+}
+
 TEST_F(FormatTest, CalculatesOriginalColumn) {
   EXPECT_EQ("\"qqqqqqqqqqqqqqqqqqqqqqqqqq\\\n"
             "q\"; /* some\n"
             "       comment */",
             format("  \"qqqqqqqqqqqqqqqqqqqqqqqqqq\\\n"
                    "q\"; /* some\n"
                    "       comment */",
                    getLLVMStyle()));