[clang] 8f96139 - [analyzer] StdLibraryFunctionsChecker: match signature based on FunctionDecl
Gabor Marton via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 6 08:35:16 PDT 2020
Author: Gabor Marton
Date: 2020-04-06T17:34:08+02:00
New Revision: 8f961399739f539cb0b3c9ac68ca1b62c2a17a80
URL: https://github.com/llvm/llvm-project/commit/8f961399739f539cb0b3c9ac68ca1b62c2a17a80
DIFF: https://github.com/llvm/llvm-project/commit/8f961399739f539cb0b3c9ac68ca1b62c2a17a80.diff
LOG: [analyzer] StdLibraryFunctionsChecker: match signature based on FunctionDecl
Summary:
Currently we match the summary signature based on the arguments in the CallExpr.
There are a few problems with this approach.
1) Variadic arguments are handled badly. Consider the below code:
int foo(void *stream, const char *format, ...);
void test_arg_constraint_on_variadic_fun() {
foo(0, "%d%d", 1, 2); // CallExpr
}
Here the call expression holds 4 arguments, whereas the function declaration
has only 2 `ParmVarDecl`s. So there is no way to create a summary that
matches the call expression, because the discrepancy in the number of
arguments causes a mismatch.
2) The call expression does not handle the `restrict` type qualifier.
In C99, fwrite's signature is the following:
size_t fwrite(const void *restrict, size_t, size_t, FILE *restrict);
However, in a call expression, like below, the type of the argument does not
have the restrict qualifier.
void test_fread_fwrite(FILE *fp, int *buf) {
size_t x = fwrite(buf, sizeof(int), 10, fp);
}
This can result in an unmatches signature, so the summary is not applied.
The solution is to match the summary against the referened callee
`FunctionDecl` that we can query from the `CallExpr`.
Further patches will continue with additional refactoring where I am going to
do a lookup during the checker initialization and the signature match will
happen there. That way, we will not check the signature during every call,
rather we will compare only two `FunctionDecl` pointers.
Reviewers: NoQ, Szelethus, gamesh411, baloghadamsoftware
Subscribers: whisperity, xazax.hun, kristof.beyls, szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, Charusso, steakhal, danielkiss, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D77410
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/test/Analysis/std-c-library-functions-arg-constraints.c
clang/test/Analysis/std-c-library-functions.c
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 6ca664a28351..5e36938b613d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -268,7 +268,7 @@ class StdLibraryFunctionsChecker
/// Try our best to figure out if the call expression is the call of
/// *the* library function to which this specification applies.
- bool matchesCall(const CallExpr *CE) const;
+ bool matchesCall(const FunctionDecl *FD) const;
};
// The same function (as in, function identifier) may have
diff erent
@@ -316,7 +316,6 @@ class StdLibraryFunctionsChecker
private:
Optional<Summary> findFunctionSummary(const FunctionDecl *FD,
- const CallExpr *CE,
CheckerContext &C) const;
Optional<Summary> findFunctionSummary(const CallEvent &Call,
CheckerContext &C) const;
@@ -532,13 +531,13 @@ bool StdLibraryFunctionsChecker::evalCall(const CallEvent &Call,
}
bool StdLibraryFunctionsChecker::Summary::matchesCall(
- const CallExpr *CE) const {
+ const FunctionDecl *FD) const {
// Check number of arguments:
- if (CE->getNumArgs() != ArgTys.size())
+ if (FD->param_size() != ArgTys.size())
return false;
// Check return type if relevant:
- if (!RetTy.isNull() && RetTy != CE->getType().getCanonicalType())
+ if (!RetTy.isNull() && RetTy != FD->getReturnType().getCanonicalType())
return false;
// Check argument types when relevant:
@@ -550,8 +549,7 @@ bool StdLibraryFunctionsChecker::Summary::matchesCall(
assertTypeSuitableForSummary(FormalT);
- QualType ActualT = StdLibraryFunctionsChecker::getArgType(CE, I);
- assert(ActualT.isCanonical());
+ QualType ActualT = FD->getParamDecl(I)->getType().getCanonicalType();
if (ActualT != FormalT)
return false;
}
@@ -561,12 +559,7 @@ bool StdLibraryFunctionsChecker::Summary::matchesCall(
Optional<StdLibraryFunctionsChecker::Summary>
StdLibraryFunctionsChecker::findFunctionSummary(const FunctionDecl *FD,
- const CallExpr *CE,
CheckerContext &C) const {
- // Note: we cannot always obtain FD from CE
- // (eg. virtual call, or call by pointer).
- assert(CE);
-
if (!FD)
return None;
@@ -590,7 +583,7 @@ StdLibraryFunctionsChecker::findFunctionSummary(const FunctionDecl *FD,
// return values.
const Summaries &SpecVariants = FSMI->second;
for (const Summary &Spec : SpecVariants)
- if (Spec.matchesCall(CE))
+ if (Spec.matchesCall(FD))
return Spec;
return None;
@@ -602,10 +595,7 @@ StdLibraryFunctionsChecker::findFunctionSummary(const CallEvent &Call,
const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
if (!FD)
return None;
- const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
- if (!CE)
- return None;
- return findFunctionSummary(FD, CE, C);
+ return findFunctionSummary(FD, C);
}
void StdLibraryFunctionsChecker::initFunctionSummaries(
@@ -630,9 +620,15 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
const QualType LongTy = ACtx.LongTy;
const QualType LongLongTy = ACtx.LongLongTy;
const QualType SizeTy = ACtx.getSizeType();
- const QualType VoidPtrTy = ACtx.VoidPtrTy; // void *T
+ const QualType VoidPtrTy = ACtx.VoidPtrTy; // void *
+ const QualType VoidPtrRestrictTy =
+ ACtx.getRestrictType(VoidPtrTy); // void *restrict
const QualType ConstVoidPtrTy =
- ACtx.getPointerType(ACtx.VoidTy.withConst()); // const void *T
+ ACtx.getPointerType(ACtx.VoidTy.withConst()); // const void *
+ const QualType ConstCharPtrTy =
+ ACtx.getPointerType(ACtx.CharTy.withConst()); // const char *
+ const QualType ConstVoidPtrRestrictTy =
+ ACtx.getRestrictType(ConstVoidPtrTy); // const void *restrict
const RangeInt IntMax = BVF.getMaxValue(IntTy).getLimitedValue();
const RangeInt LongMax = BVF.getMaxValue(LongTy).getLimitedValue();
@@ -721,7 +717,7 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
ReturnValueCondition(WithinRange, Range(-1, Max))});
};
auto Fread = [&]() {
- return Summary(ArgTypes{VoidPtrTy, Irrelevant, SizeTy, Irrelevant},
+ return Summary(ArgTypes{VoidPtrRestrictTy, Irrelevant, SizeTy, Irrelevant},
RetType{SizeTy}, NoEvalCall)
.Case({
ReturnValueCondition(LessThanOrEq, ArgNo(2)),
@@ -729,8 +725,9 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
.ArgConstraint(NotNull(ArgNo(0)));
};
auto Fwrite = [&]() {
- return Summary(ArgTypes{ConstVoidPtrTy, Irrelevant, SizeTy, Irrelevant},
- RetType{SizeTy}, NoEvalCall)
+ return Summary(
+ ArgTypes{ConstVoidPtrRestrictTy, Irrelevant, SizeTy, Irrelevant},
+ RetType{SizeTy}, NoEvalCall)
.Case({
ReturnValueCondition(LessThanOrEq, ArgNo(2)),
})
@@ -963,7 +960,10 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
{"__defaultparam", Summaries{Summary(ArgTypes{Irrelevant, IntTy},
RetType{IntTy}, EvalCallAsPure)
.ArgConstraint(NotNull(ArgNo(0)))}},
- };
+ {"__variadic", Summaries{Summary(ArgTypes{VoidPtrTy, ConstCharPtrTy},
+ RetType{IntTy}, EvalCallAsPure)
+ .ArgConstraint(NotNull(ArgNo(0)))
+ .ArgConstraint(NotNull(ArgNo(1)))}}};
for (auto &E : TestFunctionSummaryMap) {
auto InsertRes =
FunctionSummaryMap.insert({std::string(E.getKey()), E.getValue()});
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 9753f9eb00cc..62962a398689 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -64,7 +64,7 @@ void test_alnum_symbolic2(int x) {
typedef struct FILE FILE;
typedef typeof(sizeof(int)) size_t;
-size_t fread(void *, size_t, size_t, FILE *);
+size_t fread(void *restrict, size_t, size_t, FILE *);
void test_notnull_concrete(FILE *fp) {
fread(0, sizeof(int), 10, fp); // \
// report-warning{{Function argument constraint is not satisfied}} \
@@ -114,3 +114,11 @@ void test_multiple_constraints_on_same_arg(int x) {
// bugpath-note{{Assuming 'x' is < 1}} \
// bugpath-note{{Left side of '||' is true}}
}
+
+int __variadic(void *stream, const char *format, ...);
+void test_arg_constraint_on_variadic_fun() {
+ __variadic(0, "%d%d", 1, 2); // \
+ // report-warning{{Function argument constraint is not satisfied}} \
+ // bugpath-warning{{Function argument constraint is not satisfied}} \
+ // bugpath-note{{Function argument constraint is not satisfied}}
+}
diff --git a/clang/test/Analysis/std-c-library-functions.c b/clang/test/Analysis/std-c-library-functions.c
index a275ea6720ad..7c32a57c964a 100644
--- a/clang/test/Analysis/std-c-library-functions.c
+++ b/clang/test/Analysis/std-c-library-functions.c
@@ -75,7 +75,7 @@ void test_read_write(int fd, char *buf) {
}
}
-size_t fread(void *, size_t, size_t, FILE *);
+size_t fread(void *restrict, size_t, size_t, FILE *);
size_t fwrite(const void *restrict, size_t, size_t, FILE *restrict);
void test_fread_fwrite(FILE *fp, int *buf) {
More information about the cfe-commits
mailing list