[cfe-commits] r106480 - in /cfe/trunk: lib/Frontend/TextDiagnosticPrinter.cpp lib/Sema/SemaChecking.cpp test/Sema/format-strings-fixit.c test/Sema/format-strings.c
Tom Care
tcare at apple.com
Mon Jun 21 14:21:01 PDT 2010
Author: tcare
Date: Mon Jun 21 16:21:01 2010
New Revision: 106480
URL: http://llvm.org/viewvc/llvm-project?rev=106480&view=rev
Log:
Bug 7377: printf checking fails to flag some undefined behavior
http://llvm.org/bugs/show_bug.cgi?id=7377
Updated format string highlighting and fixits to take advantage of the new CharSourceRange class.
- Change HighlightRange to allow highlighting whitespace only in a CharSourceRange (for warnings about the ' ' (space) flag)
- Change format specifier range helper function to allow for half-open ranges (+1 to end)
- Enabled previously failing tests (FIXMEs/XFAILs removed)
- Small fixes and additions to format string test cases
M test/Sema/format-strings.c
M test/Sema/format-strings-fixit.c
M lib/Frontend/TextDiagnosticPrinter.cpp
M lib/Sema/SemaChecking.cpp
Modified:
cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/format-strings-fixit.c
cfe/trunk/test/Sema/format-strings.c
Modified: cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp?rev=106480&r1=106479&r2=106480&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp (original)
+++ cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp Mon Jun 21 16:21:01 2010
@@ -123,21 +123,24 @@
assert(StartColNo <= EndColNo && "Invalid range!");
- // Pick the first non-whitespace column.
- while (StartColNo < SourceLine.size() &&
- (SourceLine[StartColNo] == ' ' || SourceLine[StartColNo] == '\t'))
- ++StartColNo;
-
- // Pick the last non-whitespace column.
- if (EndColNo > SourceLine.size())
- EndColNo = SourceLine.size();
- while (EndColNo-1 &&
- (SourceLine[EndColNo-1] == ' ' || SourceLine[EndColNo-1] == '\t'))
- --EndColNo;
-
- // If the start/end passed each other, then we are trying to highlight a range
- // that just exists in whitespace, which must be some sort of other bug.
- assert(StartColNo <= EndColNo && "Trying to highlight whitespace??");
+ // Check that a token range does not highlight only whitespace.
+ if (R.isTokenRange()) {
+ // Pick the first non-whitespace column.
+ while (StartColNo < SourceLine.size() &&
+ (SourceLine[StartColNo] == ' ' || SourceLine[StartColNo] == '\t'))
+ ++StartColNo;
+
+ // Pick the last non-whitespace column.
+ if (EndColNo > SourceLine.size())
+ EndColNo = SourceLine.size();
+ while (EndColNo-1 &&
+ (SourceLine[EndColNo-1] == ' ' || SourceLine[EndColNo-1] == '\t'))
+ --EndColNo;
+
+ // If the start/end passed each other, then we are trying to highlight a range
+ // that just exists in whitespace, which must be some sort of other bug.
+ assert(StartColNo <= EndColNo && "Trying to highlight whitespace??");
+ }
// Fill the range with ~'s.
for (unsigned i = StartColNo; i < EndColNo; ++i)
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=106480&r1=106479&r2=106480&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Jun 21 16:21:01 2010
@@ -1194,8 +1194,8 @@
unsigned specifierLen);
private:
SourceRange getFormatStringRange();
- SourceRange getFormatSpecifierRange(const char *startSpecifier,
- unsigned specifierLen);
+ CharSourceRange getFormatSpecifierRange(const char *startSpecifier,
+ unsigned specifierLen);
SourceLocation getLocationOfByte(const char *x);
bool HandleAmount(const analyze_printf::OptionalAmount &Amt, unsigned k,
@@ -1220,10 +1220,15 @@
return OrigFormatExpr->getSourceRange();
}
-SourceRange CheckPrintfHandler::
+CharSourceRange CheckPrintfHandler::
getFormatSpecifierRange(const char *startSpecifier, unsigned specifierLen) {
- return SourceRange(getLocationOfByte(startSpecifier),
- getLocationOfByte(startSpecifier+specifierLen-1));
+ SourceLocation Start = getLocationOfByte(startSpecifier);
+ SourceLocation End = getLocationOfByte(startSpecifier + specifierLen - 1);
+
+ // Advance the end SourceLocation by one due to half-open ranges.
+ End = End.getFileLocWithOffset(1);
+
+ return CharSourceRange::getCharRange(Start, End);
}
SourceLocation CheckPrintfHandler::getLocationOfByte(const char *x) {
@@ -1451,7 +1456,7 @@
// Check for invalid use of field width
if (!FS.hasValidFieldWidth()) {
- HandleInvalidAmount(FS, FS.getFieldWidth(), /* field width */ 1,
+ HandleInvalidAmount(FS, FS.getFieldWidth(), /* field width */ 0,
startSpecifier, specifierLen);
}
@@ -1466,21 +1471,17 @@
HandleFlag(FS, FS.hasLeadingZeros(), startSpecifier, specifierLen);
if (!FS.hasValidPlusPrefix())
HandleFlag(FS, FS.hasPlusPrefix(), startSpecifier, specifierLen);
- // FIXME: the following lines are disabled due to clang assertions on
- // highlights containing spaces.
- // if (!FS.hasValidSpacePrefix())
- // HandleFlag(FS, FS.hasSpacePrefix(), startSpecifier, specifierLen);
+ if (!FS.hasValidSpacePrefix())
+ HandleFlag(FS, FS.hasSpacePrefix(), startSpecifier, specifierLen);
if (!FS.hasValidAlternativeForm())
HandleFlag(FS, FS.hasAlternativeForm(), startSpecifier, specifierLen);
if (!FS.hasValidLeftJustified())
HandleFlag(FS, FS.isLeftJustified(), startSpecifier, specifierLen);
// Check that flags are not ignored by another flag
- // FIXME: the following lines are disabled due to clang assertions on
- // highlights containing spaces.
- //if (FS.hasSpacePrefix() && FS.hasPlusPrefix()) // ' ' ignored by '+'
- // HandleIgnoredFlag(FS, FS.hasSpacePrefix(), FS.hasPlusPrefix(),
- // startSpecifier, specifierLen);
+ if (FS.hasSpacePrefix() && FS.hasPlusPrefix()) // ' ' ignored by '+'
+ HandleIgnoredFlag(FS, FS.hasSpacePrefix(), FS.hasPlusPrefix(),
+ startSpecifier, specifierLen);
if (FS.hasLeadingZeros() && FS.isLeftJustified()) // '0' ignored by '-'
HandleIgnoredFlag(FS, FS.hasLeadingZeros(), FS.isLeftJustified(),
startSpecifier, specifierLen);
Modified: cfe/trunk/test/Sema/format-strings-fixit.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings-fixit.c?rev=106480&r1=106479&r2=106480&view=diff
==============================================================================
--- cfe/trunk/test/Sema/format-strings-fixit.c (original)
+++ cfe/trunk/test/Sema/format-strings-fixit.c Mon Jun 21 16:21:01 2010
@@ -1,9 +1,6 @@
// RUN: cp %s %t
// RUN: %clang_cc1 -pedantic -Wall -fixit %t || true
// RUN: %clang_cc1 -fsyntax-only -pedantic -Wall -Werror %t
-// XFAIL: *
-// FIXME: Some of these tests currently fail due to a bug in the HighlightRange
-// function in lib/Frontend/TextDiagnosticPrinter.cpp.
/* This is a test of the various code modification hints that are
provided as part of warning or extension diagnostics. All of the
@@ -26,9 +23,10 @@
printf("%1d", (long double) 1.23);
// Flag handling
- printf("%0+s", (unsigned) 31337); // flags should stay
- printf("%0f", "test"); // flag should be removed
+ printf("%0+s", (unsigned) 31337); // 0 flag should stay
printf("%#p", (void *) 0);
+ printf("% +f", 1.23); // + flag should stay
+ printf("%0-f", 1.23); // - flag should stay
// Positional arguments
printf("%1$f:%2$.*3$f:%4$.*3$f\n", 1, 2, 3, 4);
Modified: cfe/trunk/test/Sema/format-strings.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings.c?rev=106480&r1=106479&r2=106480&view=diff
==============================================================================
--- cfe/trunk/test/Sema/format-strings.c (original)
+++ cfe/trunk/test/Sema/format-strings.c Mon Jun 21 16:21:01 2010
@@ -1,7 +1,4 @@
// RUN: %clang_cc1 -fsyntax-only -verify -Wformat-nonliteral %s
-// XFAIL: *
-// FIXME: temporarily marking as expected fail until whitespace highlighting
-// is fixed.
#include <stdarg.h>
typedef __typeof(sizeof(int)) size_t;
@@ -277,7 +274,8 @@
// Bad optional amount use
printf("%.2c", 'a'); // expected-warning{{precision used with 'c' conversion specifier, resulting in undefined behavior}}
- printf("%1n", (void *) 0); // expected-warning{{precision used with 'n' conversion specifier, resulting in undefined behavior}} expected-warning{{use of '%n' in format string discouraged (potentially insecure)}}
+ printf("%1n", (void *) 0); // expected-warning{{field width used with 'n' conversion specifier, resulting in undefined behavior}} expected-warning{{use of '%n' in format string discouraged (potentially insecure)}}
+ printf("%.9n", (void *) 0); // expected-warning{{precision used with 'n' conversion specifier, resulting in undefined behavior}} expected-warning{{use of '%n' in format string discouraged (potentially insecure)}}
// Ignored flags
printf("% +f", 1.23); // expected-warning{{flag ' ' is ignored when flag '+' is present}}
More information about the cfe-commits
mailing list