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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
