<div dir="ltr"><div>Ping?<br><br></div><div>I have found that SourceColMap may trigger assertion failure when the input source code contains some non-ascii characters.  This is due to the fact that some column is consisted of multiple bytes and the code is mixing the column index and byte index.  This patch should fix the problem.  Please have a look.  Thanks!<br><br>
<a href="http://reviews.llvm.org/D6746" target="_blank">http://reviews.llvm.org/D6746</a><br><br></div><div>Logan<br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Dec 30, 2014 at 8:11 AM, Logan Chien <span dir="ltr"><<a href="mailto:tzuhsiang.chien@gmail.com" target="_blank">tzuhsiang.chien@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Ping?<div><div class="h5"><br><div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Dec 25, 2014 at 12:54 AM, Logan Chien <span dir="ltr"><<a href="mailto:tzuhsiang.chien@gmail.com" target="_blank">tzuhsiang.chien@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Hi Richard,<br><br></div><div>I found that some assertion failure will be raised from the lib/Frontend/TextDiagnostic.cpp since the assertion incorrectly compares the byte index with the number of columns, which is a problem when the input source code contains non-ascii characters (multiple bytes per column.)  The patch should fix the problem, please have a look.  Thanks.<br><br>
<a href="http://reviews.llvm.org/D6746" target="_blank">http://reviews.llvm.org/D6746</a><br><br>Sincerely,<br></div><div>Logan<br></div><div><br></div></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Dec 20, 2014 at 5:17 PM, Logan Chien <span dir="ltr"><<a href="mailto:tzuhsiang.chien@gmail.com" target="_blank">tzuhsiang.chien@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi rsmith,<br>
<br>
<a href="http://reviews.llvm.org/D6746" target="_blank">http://reviews.llvm.org/D6746</a><br>
<br>
Files:<br>
  lib/Frontend/TextDiagnostic.cpp<br>
  test/Frontend/source-col-map.c<br>
<br>
Index: lib/Frontend/TextDiagnostic.cpp<br>
===================================================================<br>
--- lib/Frontend/TextDiagnostic.cpp<br>
+++ lib/Frontend/TextDiagnostic.cpp<br>
@@ -293,14 +293,14 @@<br>
<br>
   /// \brief Map from a byte index to the next byte which starts a column.<br>
   int startOfNextColumn(int N) const {<br>
-    assert(0 <= N && N < static_cast<int>(m_columnToByte.size() - 1));<br>
+    assert(0 <= N && N < static_cast<int>(m_byteToColumn.size() - 1));<br>
     while (byteToColumn(++N) == -1) {}<br>
     return N;<br>
   }<br>
<br>
   /// \brief Map from a byte index to the previous byte which starts a column.<br>
   int startOfPreviousColumn(int N) const {<br>
-    assert(0 < N && N < static_cast<int>(m_columnToByte.size()));<br>
+    assert(0 < N && N < static_cast<int>(m_byteToColumn.size()));<br>
     while (byteToColumn(--N) == -1) {}<br>
     return N;<br>
   }<br>
@@ -323,9 +323,10 @@<br>
                                           std::string &FixItInsertionLine,<br>
                                           unsigned Columns,<br>
                                           const SourceColumnMap &map) {<br>
-  unsigned MaxColumns = std::max<unsigned>(map.columns(),<br>
-                                           std::max(CaretLine.size(),<br>
-                                                    FixItInsertionLine.size()));<br>
+  unsigned CaretColumns = CaretLine.size();<br>
+  unsigned FixItColumns = llvm::sys::locale::columnWidth(FixItInsertionLine);<br>
+  unsigned MaxColumns = std::max(static_cast<unsigned>(map.columns()),<br>
+                                 std::max(CaretColumns, FixItColumns));<br>
   // if the number of columns is less than the desired number we're done<br>
   if (MaxColumns <= Columns)<br>
     return;<br>
@@ -1110,12 +1111,13 @@<br>
   // Copy the line of code into an std::string for ease of manipulation.<br>
   std::string SourceLine(LineStart, LineEnd);<br>
<br>
-  // Create a line for the caret that is filled with spaces that is the same<br>
-  // length as the line of source code.<br>
-  std::string CaretLine(LineEnd-LineStart, ' ');<br>
-<br>
+  // Build the byte to column map.<br>
   const SourceColumnMap sourceColMap(SourceLine, DiagOpts->TabStop);<br>
<br>
+  // Create a line for the caret that is filled with spaces that is the same<br>
+  // number of columns as the line of source code.<br>
+  std::string CaretLine(sourceColMap.columns(), ' ');<br>
+<br>
   // Highlight all of the characters covered by Ranges with ~ characters.<br>
   for (SmallVectorImpl<CharSourceRange>::iterator I = Ranges.begin(),<br>
                                                   E = Ranges.end();<br>
Index: test/Frontend/source-col-map.c<br>
===================================================================<br>
--- /dev/null<br>
+++ test/Frontend/source-col-map.c<br>
@@ -0,0 +1,41 @@<br>
+// RUN: not %clang_cc1 %s -fsyntax-only -fmessage-length 75 -o /dev/null 2>&1 | FileCheck %s -strict-whitespace<br>
+<br>
+// Test case for the text diagnostics source column conversion crash.<br>
+<br>
+// There are two problems to be tested:<br>
+//<br>
+// 1. The assertion in startOfNextColumn() and startOfPreviousColumn()<br>
+//    should not be raised when the byte index is greater than the column index<br>
+//    since the non-ascii characters may use more than one bytes to store a<br>
+//    character in a column.<br>
+//<br>
+// 2. The length of the caret line should be equal to the number of columns of<br>
+//    source line, instead of the length of the source line.  Otherwise, the<br>
+//    assertion in selectInterestingSourceRegion will be raised because the<br>
+//    removed columns plus the kept columns are not greater than the max column,<br>
+//    which means that we should not remove any column at all.<br>
+<br>
+// NOTE: This file is encoded in UTF-8 and intentionally contains some<br>
+// non-ASCII characters.<br>
+<br>
+__attribute__((format(printf, 1, 2)))<br>
+extern int printf(const char *fmt, ...);<br>
+<br>
+void test1(Unknown* b);  // αααα αααα αααα αααα αααα αααα αααα αααα αααα αααα αααα<br>
+// CHECK: unknown type name 'Unknown'<br>
+// CHECK-NEXT: void test1(Unknown* b);  // αααα αααα αααα αααα αααα αααα αααα ααα...<br>
+// CHECK-NEXT: {{^           \^$}}<br>
+<br>
+void test2(Unknown* b);  // αααα αααα αααα αααα αααα αααα αααα αααα αααα<br>
+<br>
+// CHECK: unknown type name 'Unknown'<br>
+// CHECK-NEXT: void test2(Unknown* b);  // αααα αααα αααα αααα αααα αααα αααα αααα αααα<br>
+// CHECK-NEXT: {{^           \^$}}<br>
+<br>
+void test3() {<br>
+   /* αααα αααα αααα αααα αααα αααα αααα αααα αααα αααα */ printf("%d", "s");<br>
+}<br>
+// CHECK:       format specifies type 'int' but the argument has type 'char *'<br>
+// CHECK-NEXT:   ...αααα αααα αααα αααα αααα αααα αααα αααα αααα */ printf("%d", "s");<br>
+// CHECK-NEXT: {{^                                                             ~~   \^~~$}}<br>
+// CHECK-NEXT: {{^                                                             %s$}}<br>
<br>
EMAIL PREFERENCES<br>
  <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div></div></div></div></div>
</blockquote></div><br></div>