martong updated this revision to Diff 244173.
martong marked 6 inline comments as done.
martong added a comment.
- Move include of Preprocessor.h to CheckerHelpers.cpp
- Try -> try
- Use PP.getIdentifierInfo
- Handle parens in tryExpandAsInteger
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74473/new/
https://reviews.llvm.org/D74473
Files:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
clang/test/Analysis/std-c-library-functions-eof.c
Index: clang/test/Analysis/std-c-library-functions-eof.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-eof.c
@@ -0,0 +1,26 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -triple i686-unknown-linux -analyzer-checker=apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux -analyzer-checker=apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -triple armv7-a15-linux -analyzer-checker=apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -triple thumbv7-a15-linux -analyzer-checker=apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+
+void clang_analyzer_eval(int);
+
+typedef struct FILE FILE;
+// Unorthodox EOF value.
+#define EOF (-2)
+
+int getc(FILE *);
+void test_getc(FILE *fp) {
+
+ int x;
+ while ((x = getc(fp)) != EOF) {
+ clang_analyzer_eval(x > 255); // expected-warning{{FALSE}}
+ clang_analyzer_eval(x >= 0); // expected-warning{{TRUE}}
+ }
+
+ int y = getc(fp);
+ if (y < 0) {
+ clang_analyzer_eval(y == -2); // expected-warning{{TRUE}}
+ }
+}
Index: clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
@@ -13,6 +13,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
+#include "clang/Lex/Preprocessor.h"
namespace clang {
@@ -109,6 +110,45 @@
return Nullability::Unspecified;
}
+llvm::Optional<int> tryExpandAsInteger(StringRef Macro,
+ const Preprocessor &PP) {
+ const auto *MacroII = PP.getIdentifierInfo(Macro);
+ if (!MacroII)
+ return llvm::None;
+ const MacroInfo *MI = PP.getMacroInfo(MacroII);
+ if (!MI)
+ return llvm::None;
+
+ // Filter out parens.
+ std::vector<Token> FilteredTokens;
+ FilteredTokens.reserve(MI->tokens().size());
+ for (auto &T : MI->tokens())
+ if (!T.isOneOf(tok::l_paren, tok::r_paren))
+ FilteredTokens.push_back(T);
+
+ if (FilteredTokens.size() > 2)
+ return llvm::None;
+
+ // Parse an integer at the end of the macro definition.
+ const Token &T = FilteredTokens.back();
+ if (!T.isLiteral())
+ return llvm::None;
+ StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength());
+ llvm::APInt IntValue;
+ constexpr unsigned AutoSenseRadix = 0;
+ if (ValueStr.getAsInteger(AutoSenseRadix, IntValue))
+ return llvm::None;
+
+ // Parse an optional minus sign.
+ if (FilteredTokens.size() == 2) {
+ if (FilteredTokens.front().is(tok::minus))
+ IntValue = -IntValue;
+ else
+ return llvm::None;
+ }
+
+ return IntValue.getSExtValue();
+}
-} // end namespace ento
-} // end namespace clang
+} // namespace ento
+} // namespace clang
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -55,6 +55,7 @@
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
using namespace clang;
using namespace clang::ento;
@@ -241,7 +242,7 @@
const CallExpr *CE,
CheckerContext &C) const;
- void initFunctionSummaries(BasicValueFactory &BVF) const;
+ void initFunctionSummaries(CheckerContext &C) const;
};
} // end of anonymous namespace
@@ -312,10 +313,11 @@
for (size_t I = 1; I != E; ++I) {
const llvm::APSInt &Min = BVF.getValue(R[I - 1].second + 1ULL, T);
const llvm::APSInt &Max = BVF.getValue(R[I].first - 1ULL, T);
- assert(Min <= Max);
- State = CM.assumeInclusiveRange(State, *N, Min, Max, false);
- if (!State)
- return nullptr;
+ if (Min <= Max) {
+ State = CM.assumeInclusiveRange(State, *N, Min, Max, false);
+ if (!State)
+ return nullptr;
+ }
}
}
@@ -449,9 +451,7 @@
if (!FD)
return None;
- SValBuilder &SVB = C.getSValBuilder();
- BasicValueFactory &BVF = SVB.getBasicValueFactory();
- initFunctionSummaries(BVF);
+ initFunctionSummaries(C);
IdentifierInfo *II = FD->getIdentifier();
if (!II)
@@ -478,11 +478,13 @@
}
void StdLibraryFunctionsChecker::initFunctionSummaries(
- BasicValueFactory &BVF) const {
+ CheckerContext &C) const {
if (!FunctionSummaryMap.empty())
return;
- ASTContext &ACtx = BVF.getContext();
+ SValBuilder &SVB = C.getSValBuilder();
+ BasicValueFactory &BVF = SVB.getBasicValueFactory();
+ const ASTContext &ACtx = BVF.getContext();
// These types are useful for writing specifications quickly,
// New specifications should probably introduce more types.
@@ -491,15 +493,28 @@
// of function summary for common cases (eg. ssize_t could be int or long
// or long long, so three summary variants would be enough).
// Of course, function variants are also useful for C++ overloads.
- QualType Irrelevant; // A placeholder, whenever we do not care about the type.
- QualType IntTy = ACtx.IntTy;
- QualType LongTy = ACtx.LongTy;
- QualType LongLongTy = ACtx.LongLongTy;
- QualType SizeTy = ACtx.getSizeType();
-
- RangeInt IntMax = BVF.getMaxValue(IntTy).getLimitedValue();
- RangeInt LongMax = BVF.getMaxValue(LongTy).getLimitedValue();
- RangeInt LongLongMax = BVF.getMaxValue(LongLongTy).getLimitedValue();
+ const QualType
+ Irrelevant; // A placeholder, whenever we do not care about the type.
+ const QualType IntTy = ACtx.IntTy;
+ const QualType LongTy = ACtx.LongTy;
+ const QualType LongLongTy = ACtx.LongLongTy;
+ const QualType SizeTy = ACtx.getSizeType();
+
+ const RangeInt IntMax = BVF.getMaxValue(IntTy).getLimitedValue();
+ const RangeInt LongMax = BVF.getMaxValue(LongTy).getLimitedValue();
+ const RangeInt LongLongMax = BVF.getMaxValue(LongLongTy).getLimitedValue();
+
+ const RangeInt UCharMax =
+ BVF.getMaxValue(ACtx.UnsignedCharTy).getLimitedValue();
+
+ // The platform dependent value of EOF.
+ // Try our best to parse this from the Preprocessor, otherwise fallback to -1.
+ const auto EOFv = [&C]() -> RangeInt {
+ if (const llvm::Optional<int> OptInt =
+ tryExpandAsInteger("EOF", C.getPreprocessor()))
+ return *OptInt;
+ return -1;
+ }();
// We are finally ready to define specifications for all supported functions.
//
@@ -552,7 +567,8 @@
// Templates for summaries that are reused by many functions.
auto Getc = [&]() {
return Summary(ArgTypes{Irrelevant}, RetType{IntTy}, NoEvalCall)
- .Case({ReturnValueCondition(WithinRange, Range(-1, 255))});
+ .Case(
+ {ReturnValueCondition(WithinRange, {{EOFv, EOFv}, {0, UCharMax}})});
};
auto Read = [&](RetType R, RangeInt Max) {
return Summary(ArgTypes{Irrelevant, Irrelevant, SizeTy}, RetType{R},
@@ -587,10 +603,12 @@
// The locale-specific range.
// No post-condition. We are completely unaware of
// locale-specific return values.
- .Case({ArgumentCondition(0U, WithinRange, {{128, 255}})})
- .Case({ArgumentCondition(
- 0U, OutOfRange,
- {{'0', '9'}, {'A', 'Z'}, {'a', 'z'}, {128, 255}}),
+ .Case({ArgumentCondition(0U, WithinRange, {{128, UCharMax}})})
+ .Case({ArgumentCondition(0U, OutOfRange,
+ {{'0', '9'},
+ {'A', 'Z'},
+ {'a', 'z'},
+ {128, UCharMax}}),
ReturnValueCondition(WithinRange, SingleValue(0))})},
},
{
@@ -601,11 +619,11 @@
{{'A', 'Z'}, {'a', 'z'}}),
ReturnValueCondition(OutOfRange, SingleValue(0))})
// The locale-specific range.
- .Case({ArgumentCondition(0U, WithinRange, {{128, 255}})})
- .Case(
- {ArgumentCondition(0U, OutOfRange,
- {{'A', 'Z'}, {'a', 'z'}, {128, 255}}),
- ReturnValueCondition(WithinRange, SingleValue(0))})},
+ .Case({ArgumentCondition(0U, WithinRange, {{128, UCharMax}})})
+ .Case({ArgumentCondition(
+ 0U, OutOfRange,
+ {{'A', 'Z'}, {'a', 'z'}, {128, UCharMax}}),
+ ReturnValueCondition(WithinRange, SingleValue(0))})},
},
{
"isascii",
@@ -668,9 +686,9 @@
ArgumentCondition(0U, OutOfRange, Range('a', 'z')),
ReturnValueCondition(WithinRange, SingleValue(0))})
// The locale-specific range.
- .Case({ArgumentCondition(0U, WithinRange, {{128, 255}})})
+ .Case({ArgumentCondition(0U, WithinRange, {{128, UCharMax}})})
// Is not an unsigned char.
- .Case({ArgumentCondition(0U, OutOfRange, Range(0, 255)),
+ .Case({ArgumentCondition(0U, OutOfRange, Range(0, UCharMax)),
ReturnValueCondition(WithinRange, SingleValue(0))})},
},
{
@@ -704,9 +722,10 @@
{{9, 13}, {' ', ' '}}),
ReturnValueCondition(OutOfRange, SingleValue(0))})
// The locale-specific range.
- .Case({ArgumentCondition(0U, WithinRange, {{128, 255}})})
- .Case({ArgumentCondition(0U, OutOfRange,
- {{9, 13}, {' ', ' '}, {128, 255}}),
+ .Case({ArgumentCondition(0U, WithinRange, {{128, UCharMax}})})
+ .Case({ArgumentCondition(
+ 0U, OutOfRange,
+ {{9, 13}, {' ', ' '}, {128, UCharMax}}),
ReturnValueCondition(WithinRange, SingleValue(0))})},
},
{
@@ -717,10 +736,10 @@
.Case({ArgumentCondition(0U, WithinRange, Range('A', 'Z')),
ReturnValueCondition(OutOfRange, SingleValue(0))})
// The locale-specific range.
- .Case({ArgumentCondition(0U, WithinRange, {{128, 255}})})
+ .Case({ArgumentCondition(0U, WithinRange, {{128, UCharMax}})})
// Other.
.Case({ArgumentCondition(0U, OutOfRange,
- {{'A', 'Z'}, {128, 255}}),
+ {{'A', 'Z'}, {128, UCharMax}}),
ReturnValueCondition(WithinRange, SingleValue(0))})},
},
{
@@ -740,9 +759,10 @@
// The getc() family of functions that returns either a char or an EOF.
{"getc", Summaries{Getc()}},
{"fgetc", Summaries{Getc()}},
- {"getchar", Summaries{Summary(ArgTypes{}, RetType{IntTy}, NoEvalCall)
- .Case({ReturnValueCondition(WithinRange,
- Range(-1, 255))})}},
+ {"getchar",
+ Summaries{Summary(ArgTypes{}, RetType{IntTy}, NoEvalCall)
+ .Case({ReturnValueCondition(
+ WithinRange, {{EOFv, EOFv}, {0, UCharMax}})})}},
// read()-like functions that never return more than buffer size.
// We are not sure how ssize_t is defined on every platform, so we
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
@@ -14,6 +14,7 @@
#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CHECKERHELPERS_H
#include "clang/AST/Stmt.h"
+#include "llvm/ADT/Optional.h"
#include <tuple>
namespace clang {
@@ -22,6 +23,7 @@
class VarDecl;
class QualType;
class AttributedType;
+class Preprocessor;
namespace ento {
@@ -62,8 +64,13 @@
/// Get nullability annotation for a given type.
Nullability getNullabilityAnnotation(QualType Type);
-} // end GR namespace
+/// Try to parse the value of a defined preprocessor macro. We can only parse
+/// simple expressions that consist of an optional minus sign token and then a
+/// token for an integer. If we cannot parse the value then None is returned.
+llvm::Optional<int> tryExpandAsInteger(StringRef Macro, const Preprocessor &PP);
-} // end clang namespace
+} // namespace ento
+
+} // namespace clang
#endif
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
