[clang] 536456a - [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax
Gabor Marton via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 13 04:56:04 PST 2020
Author: Gabor Marton
Date: 2020-02-13T13:51:51+01:00
New Revision: 536456a7e93d73b9ff4e92f3e51d1aa1c72628fe
URL: https://github.com/llvm/llvm-project/commit/536456a7e93d73b9ff4e92f3e51d1aa1c72628fe
DIFF: https://github.com/llvm/llvm-project/commit/536456a7e93d73b9ff4e92f3e51d1aa1c72628fe.diff
LOG: [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax
Summary:
Both EOF and the max value of unsigned char is platform dependent. In this
patch we try our best to deduce the value of EOF from the Preprocessor,
if we can't we fall back to -1.
Reviewers: Szelethus, NoQ
Subscribers: whisperity, xazax.hun, kristof.beyls, baloghadamsoftware, szepet, rnkovacs, a.sidorin, mikhail.ramalh
Tags: #clang
Differential Revision: https://reviews.llvm.org/D74473
Added:
clang/test/Analysis/std-c-library-functions-eof.c
Modified:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
index b53c042a1ca1..f253c14cc487 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
+++ b/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 Expr;
class VarDecl;
class QualType;
class AttributedType;
+class Preprocessor;
namespace ento {
@@ -62,8 +64,13 @@ enum class Nullability : char {
/// 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
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index e8668775ab85..608015781c49 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/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 @@ class StdLibraryFunctionsChecker : public Checker<check::PostCall, eval::Call> {
const CallExpr *CE,
CheckerContext &C) const;
- void initFunctionSummaries(BasicValueFactory &BVF) const;
+ void initFunctionSummaries(CheckerContext &C) const;
};
} // end of anonymous namespace
@@ -286,6 +287,12 @@ ProgramStateRef StdLibraryFunctionsChecker::ValueRange::applyAsWithinRange(
// "WithinRange R" is treated as "outside [T_MIN, T_MAX] \ R".
// We cut off [T_MIN, min(R) - 1] and [max(R) + 1, T_MAX] if necessary,
// and then cut away all holes in R one by one.
+ //
+ // E.g. consider a range list R as [A, B] and [C, D]
+ // -------+--------+------------------+------------+----------->
+ // A B C D
+ // Then we assume that the value is not in [-inf, A - 1],
+ // then not in [D + 1, +inf], then not in [B + 1, C - 1]
if (auto N = V.getAs<NonLoc>()) {
const IntRangeVector &R = getRanges();
size_t E = R.size();
@@ -312,10 +319,11 @@ ProgramStateRef StdLibraryFunctionsChecker::ValueRange::applyAsWithinRange(
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 +457,7 @@ StdLibraryFunctionsChecker::findFunctionSummary(const FunctionDecl *FD,
if (!FD)
return None;
- SValBuilder &SVB = C.getSValBuilder();
- BasicValueFactory &BVF = SVB.getBasicValueFactory();
- initFunctionSummaries(BVF);
+ initFunctionSummaries(C);
IdentifierInfo *II = FD->getIdentifier();
if (!II)
@@ -478,11 +484,13 @@ StdLibraryFunctionsChecker::findFunctionSummary(const FunctionDecl *FD,
}
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 +499,28 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
// 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 +573,8 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
// 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 +609,12 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
// 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 +625,11 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
{{'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 +692,9 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
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 +728,10 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
{{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 +742,10 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
.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 +765,10 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
// 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
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp b/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
index 11693132de68..e43b172d018d 100644
--- a/clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ b/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 @@ Nullability getNullabilityAnnotation(QualType Type) {
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
diff --git a/clang/test/Analysis/std-c-library-functions-eof.c b/clang/test/Analysis/std-c-library-functions-eof.c
new file mode 100644
index 000000000000..0b09a1c63d97
--- /dev/null
+++ b/clang/test/Analysis/std-c-library-functions-eof.c
@@ -0,0 +1,26 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -triple i686-unknown-linux -analyzer-checker=core,apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux -analyzer-checker=core,apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -triple armv7-a15-linux -analyzer-checker=core,apiModeling.StdCLibraryFunctions,debug.ExprInspection -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -triple thumbv7-a15-linux -analyzer-checker=core,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}}
+ }
+}
More information about the cfe-commits
mailing list