martong created this revision.
martong added reviewers: NoQ, Szelethus, gamesh411, baloghadamsoftware.
Herald added subscribers: cfe-commits, ASDenysPetrov, danielkiss, steakhal,
Charusso, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet,
kristof.beyls, xazax.hun, whisperity.
Herald added a project: clang.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D77410
Files:
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
clang/test/Analysis/std-c-library-functions-arg-constraints.c
clang/test/Analysis/std-c-library-functions.c
Index: clang/test/Analysis/std-c-library-functions.c
===================================================================
--- clang/test/Analysis/std-c-library-functions.c
+++ clang/test/Analysis/std-c-library-functions.c
@@ -75,7 +75,7 @@
}
}
-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) {
Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===================================================================
--- clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -64,7 +64,7 @@
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 @@
// 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}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -268,7 +268,7 @@
/// 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 different
@@ -316,7 +316,6 @@
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::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,7 +549,7 @@
assertTypeSuitableForSummary(FormalT);
- QualType ActualT = StdLibraryFunctionsChecker::getArgType(CE, I);
+ QualType ActualT = FD->getParamDecl(I)->getType().getCanonicalType();
assert(ActualT.isCanonical());
if (ActualT != FormalT)
return false;
@@ -561,12 +560,7 @@
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 +584,7 @@
// 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 +596,7 @@
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 +621,15 @@
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 +718,7 @@
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 +726,9 @@
.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)),
})
@@ -960,7 +958,10 @@
ArgumentCondition(0U, OutOfRange, SingleValue(1)))
.ArgConstraint(
ArgumentCondition(0U, OutOfRange, SingleValue(2)))}},
- };
+ {"__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()});
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
