<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 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>