NoQ updated this revision to Diff 76031.
NoQ marked 2 inline comments as done.
NoQ added a comment.
In https://reviews.llvm.org/D25940#579227, @dcoughlin wrote:
> Are the parameter types actually needed? I think in general the rest of the
> analyzer uses arity alone.
Arity checks are to avoid crashes `getArg...()`. I'm also avoiding assertion
failures on `APSInt` manipulations, which must be of the same type. And even if
`APSInt`s didn't assert-fail and were promoted correctly automatically in all
operations, we would still want to avoid mismatching types, because results may
be completely unexpected.
All right, yep, i'm getting the point about overrides. It's true that we need
to find the correct functions better, rather than match more functions. Trying
that in the new revision.
https://reviews.llvm.org/D25940
Files:
lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
test/Analysis/std-c-library-functions.c
Index: test/Analysis/std-c-library-functions.c
===================================================================
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -1,4 +1,8 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=unix.StdCLibraryFunctions,debug.ExprInspection -verify %s
+// RUN: %clang_cc1 -triple i686-unknown-linux -analyze -analyzer-checker=unix.StdCLibraryFunctions,debug.ExprInspection -verify %s
// RUN: %clang_cc1 -triple x86_64-unknown-linux -analyze -analyzer-checker=unix.StdCLibraryFunctions,debug.ExprInspection -verify %s
+// RUN: %clang_cc1 -triple armv7-a15-linux -analyze -analyzer-checker=unix.StdCLibraryFunctions,debug.ExprInspection -verify %s
+// RUN: %clang_cc1 -triple thumbv7-a15-linux -analyze -analyzer-checker=unix.StdCLibraryFunctions,debug.ExprInspection -verify %s
void clang_analyzer_eval(int);
@@ -22,7 +26,6 @@
clang_analyzer_eval(fgetc(fp) >= 0); // expected-warning{{UNKNOWN}}
}
-
typedef unsigned long size_t;
typedef signed long ssize_t;
ssize_t read(int, void *, size_t);
Index: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -52,13 +52,15 @@
//===----------------------------------------------------------------------===//
#include "ClangSACheckers.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
using namespace clang;
using namespace clang::ento;
+using namespace ast_matchers;
namespace {
class StdLibraryFunctionsChecker : public Checker<check::PostCall, eval::Call> {
@@ -79,11 +81,15 @@
/// impose a constraint that involves other argument or return value symbols.
enum ValueRangeKindTy { OutOfRange, WithinRange, ComparesToArgument };
+ // The universal integral type to use in value range descriptions.
+ // Unsigned to make sure overflows are well-defined.
+ typedef uint64_t RangeIntTy;
+
/// Normally, describes a single range constraint, eg. {{0, 1}, {3, 4}} is
/// a non-negative integer, which less than 5 and not equal to 2. For
/// `ComparesToArgument', holds information about how exactly to compare to
/// the argument.
- typedef std::vector<std::pair<uint64_t, uint64_t>> IntRangeVectorTy;
+ typedef std::vector<std::pair<RangeIntTy, RangeIntTy>> IntRangeVectorTy;
/// A reference to an argument or return value by its number.
/// ArgNo in CallExpr and CallEvent is defined as Unsigned, but
@@ -212,6 +218,14 @@
return ArgNo == Ret ? Call.getReturnValue() : Call.getArgSVal(ArgNo);
}
+ // Methods for defining default types such as size_t or ssize_t
+ // accurately. We need them in the summary.
+ static QualType getTypedefByName(StringRef Name, ASTContext &ACtx,
+ QualType DefaultT);
+
+ static QualType toggleSignedness(QualType OriginalT, ASTContext &ACtx,
+ QualType DefaultT);
+
public:
void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
bool evalCall(const CallExpr *CE, CheckerContext &C) const;
@@ -449,6 +463,65 @@
return Spec;
}
+QualType StdLibraryFunctionsChecker::toggleSignedness(QualType OriginalT,
+ ASTContext &ACtx,
+ QualType DefaultT) {
+ if (const BuiltinType *BT = OriginalT->getAs<BuiltinType>()) {
+ switch (BT->getKind()) {
+ // We cannot toggle signedness of a plain "char" or "wchar_t",
+ // because there's no corresponding type of different signedness.
+ // However, we can do well with "signed char" vs. "unsigned char".
+ case BuiltinType::SChar:
+ return ACtx.UnsignedCharTy;
+ case BuiltinType::Short:
+ return ACtx.UnsignedShortTy;
+ case BuiltinType::Int:
+ return ACtx.UnsignedIntTy;
+ case BuiltinType::Long:
+ return ACtx.UnsignedLongTy;
+ case BuiltinType::LongLong:
+ return ACtx.UnsignedLongLongTy;
+ case BuiltinType::Int128:
+ return ACtx.UnsignedInt128Ty;
+ case BuiltinType::UChar:
+ return ACtx.SignedCharTy;
+ case BuiltinType::UShort:
+ return ACtx.ShortTy;
+ case BuiltinType::UInt:
+ return ACtx.IntTy;
+ case BuiltinType::ULong:
+ return ACtx.LongTy;
+ case BuiltinType::ULongLong:
+ return ACtx.LongLongTy;
+ case BuiltinType::UInt128:
+ return ACtx.Int128Ty;
+ default:
+ break;
+ }
+ }
+ return DefaultT;
+}
+
+QualType StdLibraryFunctionsChecker::getTypedefByName(StringRef Name,
+ ASTContext &ACtx,
+ QualType DefaultT) {
+ class Callback : public MatchFinder::MatchCallback {
+ QualType FoundT;
+
+ public:
+ void run(const MatchFinder::MatchResult &Result) {
+ FoundT = *Result.Nodes.getNodeAs<QualType>("t");
+ }
+ const QualType &get() const { return FoundT; }
+ };
+
+ MatchFinder F;
+ Callback CB;
+ F.addMatcher(typedefDecl(hasName(Name), hasType(qualType().bind("t"))), &CB);
+ F.matchAST(ACtx);
+ return (!CB.get().isNull()) ? CB.get().getCanonicalType() : DefaultT;
+}
+
void StdLibraryFunctionsChecker::initFunctionSummaries(
BasicValueFactory &BVF) const {
if (!FunctionSummaryMap.empty())
@@ -460,14 +533,23 @@
// New specifications should probably introduce more types.
QualType Irrelevant; // A placeholder, whenever we do not care about the type.
QualType IntTy = ACtx.IntTy;
- QualType SizeTy = ACtx.getSizeType();
- QualType SSizeTy = ACtx.getIntTypeForBitwidth(ACtx.getTypeSize(SizeTy), true);
-
- // Don't worry about truncation here, it'd be cast back to SIZE_MAX when used.
- LLVM_ATTRIBUTE_UNUSED int64_t SizeMax =
+ QualType SizeTy = getTypedefByName("::size_t", ACtx,
+ /*DefaultT=*/ACtx.getSizeType());
+ // Try the following to obtain ssize_t as accurately as possible:
+ // 1. Find an actual typedef with this name.
+ // 2. Take the same type as size_t, just unsigned.
+ // 3. Take any type with the same size as size_t just signed.
+ QualType SSizeTy = getTypedefByName("::ssize_t", ACtx,
+ /*DefaultT=*/toggleSignedness(SizeTy, ACtx,
+ /*DefaultT=*/ACtx.getIntTypeForBitwidth(
+ ACtx.getTypeSize(SizeTy), true)));
+
+ LLVM_ATTRIBUTE_UNUSED RangeIntTy SizeMax =
BVF.getMaxValue(SizeTy).getLimitedValue();
- int64_t SSizeMax =
- BVF.getMaxValue(SSizeTy).getLimitedValue();
+ // Don't worry about truncation here, it'd be cast back
+ // to SSIZE_MAX when used.
+ RangeIntTy SSizeMax =
+ BVF.getMaxValue(SSizeTy).getLimitedValue();
// We are finally ready to define specifications for all supported functions.
//
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
