[clang] e271049 - [clang][analyzer] StdLibraryFunctionsChecker: Allow NULL buffer in `fread` and `fwrite` if size is zero.
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 19 00:58:46 PDT 2023
Author: Balázs Kéri
Date: 2023-07-19T09:58:14+02:00
New Revision: e271049bc6a1408aa4e53771321117b3da6440ab
URL: https://github.com/llvm/llvm-project/commit/e271049bc6a1408aa4e53771321117b3da6440ab
DIFF: https://github.com/llvm/llvm-project/commit/e271049bc6a1408aa4e53771321117b3da6440ab.diff
LOG: [clang][analyzer] StdLibraryFunctionsChecker: Allow NULL buffer in `fread` and `fwrite` if size is zero.
Reviewed By: donat.nagy
Differential Revision: https://reviews.llvm.org/D154509
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/test/Analysis/std-c-library-functions-arg-constraints.c
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 683b0369504dc8..d18e6f63df4484 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -403,6 +403,53 @@ class StdLibraryFunctionsChecker
}
};
+ /// Check null or non-null-ness of an argument that is of pointer type.
+ /// The argument is meant to be a buffer that has a size constraint, and it
+ /// is allowed to have a NULL value if the size is 0. The size can depend on
+ /// 1 or 2 additional arguments, if one of these is 0 the buffer is allowed to
+ /// be NULL. This is useful for functions like `fread` which have this special
+ /// property.
+ class NotNullBufferConstraint : public ValueConstraint {
+ using ValueConstraint::ValueConstraint;
+ ArgNo SizeArg1N;
+ std::optional<ArgNo> SizeArg2N;
+ // This variable has a role when we negate the constraint.
+ bool CannotBeNull = true;
+
+ public:
+ NotNullBufferConstraint(ArgNo ArgN, ArgNo SizeArg1N,
+ std::optional<ArgNo> SizeArg2N,
+ bool CannotBeNull = true)
+ : ValueConstraint(ArgN), SizeArg1N(SizeArg1N), SizeArg2N(SizeArg2N),
+ CannotBeNull(CannotBeNull) {}
+
+ ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
+ const Summary &Summary,
+ CheckerContext &C) const override;
+
+ void describe(DescriptionKind DK, const CallEvent &Call,
+ ProgramStateRef State, const Summary &Summary,
+ llvm::raw_ostream &Out) const override;
+
+ bool describeArgumentValue(const CallEvent &Call, ProgramStateRef State,
+ const Summary &Summary,
+ llvm::raw_ostream &Out) const override;
+
+ ValueConstraintPtr negate() const override {
+ NotNullBufferConstraint Tmp(*this);
+ Tmp.CannotBeNull = !this->CannotBeNull;
+ return std::make_shared<NotNullBufferConstraint>(Tmp);
+ }
+
+ protected:
+ bool checkSpecificValidity(const FunctionDecl *FD) const override {
+ const bool ValidArg = getArgType(FD, ArgN)->isPointerType();
+ assert(ValidArg &&
+ "This constraint should be applied only on a pointer type");
+ return ValidArg;
+ }
+ };
+
// Represents a buffer argument with an additional size constraint. The
// constraint may be a concrete value, or a symbolic value in an argument.
// Example 1. Concrete value as the minimum buffer size.
@@ -1140,6 +1187,54 @@ bool StdLibraryFunctionsChecker::NotNullConstraint::describeArgumentValue(
return true;
}
+ProgramStateRef StdLibraryFunctionsChecker::NotNullBufferConstraint::apply(
+ ProgramStateRef State, const CallEvent &Call, const Summary &Summary,
+ CheckerContext &C) const {
+ SVal V = getArgSVal(Call, getArgNo());
+ if (V.isUndef())
+ return State;
+ DefinedOrUnknownSVal L = V.castAs<DefinedOrUnknownSVal>();
+ if (!isa<Loc>(L))
+ return State;
+
+ std::optional<DefinedOrUnknownSVal> SizeArg1 =
+ getArgSVal(Call, SizeArg1N).getAs<DefinedOrUnknownSVal>();
+ std::optional<DefinedOrUnknownSVal> SizeArg2;
+ if (SizeArg2N)
+ SizeArg2 = getArgSVal(Call, *SizeArg2N).getAs<DefinedOrUnknownSVal>();
+
+ auto IsArgZero = [State](std::optional<DefinedOrUnknownSVal> Val) {
+ if (!Val)
+ return false;
+ auto [IsNonNull, IsNull] = State->assume(*Val);
+ return IsNull && !IsNonNull;
+ };
+
+ if (IsArgZero(SizeArg1) || IsArgZero(SizeArg2))
+ return State;
+
+ return State->assume(L, CannotBeNull);
+}
+
+void StdLibraryFunctionsChecker::NotNullBufferConstraint::describe(
+ DescriptionKind DK, const CallEvent &Call, ProgramStateRef State,
+ const Summary &Summary, llvm::raw_ostream &Out) const {
+ assert(CannotBeNull &&
+ "Describe should not be used when the value must be NULL");
+ if (DK == Violation)
+ Out << "should not be NULL";
+ else
+ Out << "is not NULL";
+}
+
+bool StdLibraryFunctionsChecker::NotNullBufferConstraint::describeArgumentValue(
+ const CallEvent &Call, ProgramStateRef State, const Summary &Summary,
+ llvm::raw_ostream &Out) const {
+ assert(!CannotBeNull && "This function is used when the value is NULL");
+ Out << "is NULL";
+ return true;
+}
+
ProgramStateRef StdLibraryFunctionsChecker::BufferSizeConstraint::apply(
ProgramStateRef State, const CallEvent &Call, const Summary &Summary,
CheckerContext &C) const {
@@ -1681,6 +1776,10 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
auto IsNull = [&](ArgNo ArgN) {
return std::make_shared<NotNullConstraint>(ArgN, false);
};
+ auto NotNullBuffer = [&](ArgNo ArgN, ArgNo SizeArg1N, ArgNo SizeArg2N) {
+ return std::make_shared<NotNullBufferConstraint>(ArgN, SizeArg1N,
+ SizeArg2N);
+ };
std::optional<QualType> FileTy = lookupTy("FILE");
std::optional<QualType> FilePtrTy = getPointerTy(FileTy);
@@ -1935,11 +2034,8 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
.Case({ArgumentCondition(1U, WithinRange, SingleValue(0)),
ReturnValueCondition(WithinRange, SingleValue(0))},
ErrnoMustNotBeChecked, GenericSuccessMsg)
- .ArgConstraint(NotNull(ArgNo(0)))
+ .ArgConstraint(NotNullBuffer(ArgNo(0), ArgNo(1), ArgNo(2)))
.ArgConstraint(NotNull(ArgNo(3)))
- // FIXME: It should be allowed to have a null buffer if any of
- // args 1 or 2 are zero. Remove NotNull check of arg 0, add a check
- // for non-null buffer if non-zero size to BufferSizeConstraint?
.ArgConstraint(BufferSize(/*Buffer=*/ArgNo(0), /*BufSize=*/ArgNo(1),
/*BufSizeMultiplier=*/ArgNo(2)));
@@ -3452,6 +3548,12 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
"__not_null", Signature(ArgTypes{IntPtrTy}, RetType{IntTy}),
Summary(EvalCallAsPure).ArgConstraint(NotNull(ArgNo(0))));
+ addToFunctionSummaryMap(
+ "__not_null_buffer",
+ Signature(ArgTypes{VoidPtrTy, IntTy, IntTy}, RetType{IntTy}),
+ Summary(EvalCallAsPure)
+ .ArgConstraint(NotNullBuffer(ArgNo(0), ArgNo(1), ArgNo(2))));
+
// Test inside range constraints.
addToFunctionSummaryMap(
"__single_val_0", Signature(ArgTypes{IntTy}, RetType{IntTy}),
diff --git a/clang/test/Analysis/std-c-library-functions-arg-constraints.c b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
index 57698224fa2764..062faccfb63cdb 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -184,6 +184,44 @@ void test_notnull_symbolic2(FILE *fp, int *buf) {
// bugpath-warning{{The 1st argument to 'fread' is NULL but should not be NULL}} \
// bugpath-note{{The 1st argument to 'fread' is NULL but should not be NULL}}
}
+
+int __not_null_buffer(void *, int, int);
+
+void test_notnull_buffer_1(void *buf) {
+ __not_null_buffer(buf, 0, 1);
+ clang_analyzer_eval(buf != 0); // \
+ // report-warning{{TRUE}} \
+ // bugpath-warning{{TRUE}} \
+ // report-warning{{FALSE}} \
+ // bugpath-warning{{FALSE}} \
+ // bugpath-note{{TRUE}} \
+ // bugpath-note{{FALSE}} \
+ // bugpath-note{{Assuming 'buf' is equal to null}} \
+ // bugpath-note{{Assuming 'buf' is not equal to null}}
+}
+
+void test_notnull_buffer_2(void *buf) {
+ __not_null_buffer(buf, 1, 0);
+ clang_analyzer_eval(buf != 0); // \
+ // report-warning{{TRUE}} \
+ // bugpath-warning{{TRUE}} \
+ // report-warning{{FALSE}} \
+ // bugpath-warning{{FALSE}} \
+ // bugpath-note{{TRUE}} \
+ // bugpath-note{{FALSE}} \
+ // bugpath-note{{Assuming 'buf' is equal to null}} \
+ // bugpath-note{{Assuming 'buf' is not equal to null}}
+}
+
+void test_notnull_buffer_3(void *buf) {
+ __not_null_buffer(buf, 1, 1);
+ clang_analyzer_eval(buf != 0); // \
+ // report-warning{{TRUE}} \
+ // bugpath-warning{{TRUE}} \
+ // bugpath-note{{TRUE}} \
+ // bugpath-note{{'buf' is not equal to null}}
+}
+
void test_no_node_after_bug(FILE *fp, size_t size, size_t n, void *buf) {
if (fp) // \
// bugpath-note{{Assuming 'fp' is null}} \
More information about the cfe-commits
mailing list