Alternate format string checking: issue a warning for invalid conversion specifiers.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@94792 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/Analysis/Analyses/PrintfFormatString.h b/include/clang/Analysis/Analyses/PrintfFormatString.h
index 488d208..3c4a971 100644
--- a/include/clang/Analysis/Analyses/PrintfFormatString.h
+++ b/include/clang/Analysis/Analyses/PrintfFormatString.h
@@ -200,7 +200,9 @@
virtual void HandleIncompletePrecision(const char *periodChar) {}
- virtual void HandleInvalidConversionSpecifier(const char *conversionChar) {}
+ virtual void HandleInvalidConversionSpecifier(const FormatSpecifier &FS,
+ const char *startSpecifier,
+ unsigned specifierLen) {}
virtual bool HandleFormatSpecifier(const FormatSpecifier &FS,
const char *startSpecifier,
diff --git a/lib/Analysis/PrintfFormatString.cpp b/lib/Analysis/PrintfFormatString.cpp
index 3045fdd..3f03420 100644
--- a/lib/Analysis/PrintfFormatString.cpp
+++ b/lib/Analysis/PrintfFormatString.cpp
@@ -21,17 +21,17 @@
class FormatSpecifierResult {
FormatSpecifier FS;
const char *Start;
- bool HasError;
+ bool Stop;
public:
- FormatSpecifierResult(bool err = false)
- : Start(0), HasError(err) {}
+ FormatSpecifierResult(bool stop = false)
+ : Start(0), Stop(stop) {}
FormatSpecifierResult(const char *start,
const FormatSpecifier &fs)
- : FS(fs), Start(start), HasError(false) {}
+ : FS(fs), Start(start), Stop(false) {}
const char *getStart() const { return Start; }
- bool hasError() const { return HasError; }
+ bool shouldStop() const { return Stop; }
bool hasValue() const { return Start != 0; }
const FormatSpecifier &getValue() const {
assert(hasValue());
@@ -194,11 +194,10 @@
// Finally, look for the conversion specifier.
const char *conversionPosition = I++;
- ConversionSpecifier::Kind k;
+ ConversionSpecifier::Kind k = ConversionSpecifier::InvalidSpecifier;
switch (*conversionPosition) {
default:
- H.HandleInvalidConversionSpecifier(conversionPosition);
- return true;
+ break;
// C99: 7.19.6.1 (section 8).
case 'd': k = ConversionSpecifier::dArg; break;
case 'i': k = ConversionSpecifier::iArg; break;
@@ -223,6 +222,11 @@
case '@': k = ConversionSpecifier::ObjCObjArg; break;
}
FS.setConversionSpecifier(ConversionSpecifier(conversionPosition, k));
+
+ if (k == ConversionSpecifier::InvalidSpecifier) {
+ H.HandleInvalidConversionSpecifier(FS, Beg, I - Beg);
+ return false; // Keep processing format specifiers.
+ }
return FormatSpecifierResult(Start, FS);
}
@@ -232,13 +236,14 @@
// Keep looking for a format specifier until we have exhausted the string.
while (I != E) {
const FormatSpecifierResult &FSR = ParseFormatSpecifier(H, I, E);
- // Did an error of any kind occur when parsing the specifier? If so,
- // don't do any more processing.
- if (FSR.hasError())
+ // Did a fail-stop error of any kind occur when parsing the specifier?
+ // If so, don't do any more processing.
+ if (FSR.shouldStop())
return true;;
- // Done processing the string?
+ // Did we exhaust the string or encounter an error that
+ // we can recover from?
if (!FSR.hasValue())
- break;
+ continue;
// We have a format specifier. Pass it to the callback.
if (!H.HandleFormatSpecifier(FSR.getValue(), FSR.getStart(),
I - FSR.getStart()))
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 3cd45a2..a6d5097 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -1306,7 +1306,16 @@
TheCall(theCall), FormatIdx(formatIdx) {}
void DoneProcessing();
-
+
+// void HandleIncompleteFormatSpecifier(const char *startSpecifier,
+// const char *endSpecifier);
+
+// void HandleIncompletePrecision(const char *periodChar);
+
+ void HandleInvalidConversionSpecifier(const analyze_printf::FormatSpecifier &FS,
+ const char *startSpecifier,
+ unsigned specifierLen);
+
void HandleNullChar(const char *nullCharacter);
bool HandleFormatSpecifier(const analyze_printf::FormatSpecifier &FS,
@@ -1331,6 +1340,20 @@
return S.getLocationOfStringLiteralByte(FExpr, x - Beg);
}
+void CheckPrintfHandler::
+HandleInvalidConversionSpecifier(const analyze_printf::FormatSpecifier &FS,
+ const char *startSpecifier,
+ unsigned specifierLen) {
+
+ ++NumConversions;
+
+ SourceLocation Loc =
+ getLocationOfByte(FS.getConversionSpecifier().getStart());
+ S.Diag(Loc, diag::warn_printf_invalid_conversion)
+ << llvm::StringRef(startSpecifier, specifierLen)
+ << getFormatRange();
+}
+
void CheckPrintfHandler::HandleNullChar(const char *nullCharacter) {
// The presence of a null character is likely an error.
S.Diag(getLocationOfByte(nullCharacter),
@@ -1373,7 +1396,6 @@
}
return true;
}
-
bool
CheckPrintfHandler::HandleFormatSpecifier(const analyze_printf::FormatSpecifier &FS,
@@ -1397,20 +1419,17 @@
return false;
}
- ++NumConversions;
-
// Check for using an Objective-C specific conversion specifier
// in a non-ObjC literal.
if (!IsObjCLiteral && CS.isObjCArg()) {
- SourceLocation Loc = getLocationOfByte(CS.getStart());
- S.Diag(Loc, diag::warn_printf_invalid_conversion)
- << llvm::StringRef(startSpecifier, specifierLen)
- << getFormatRange();
+ HandleInvalidConversionSpecifier(FS, startSpecifier, specifierLen);
// Continue checking the other format specifiers.
return true;
}
+ ++NumConversions;
+
// Are we using '%n'? Issue a warning about this being
// a possible security issue.
if (CS.getKind() == ConversionSpecifier::OutIntPtrArg) {