martong created this revision. martong added reviewers: NoQ, Szelethus, baloghadamsoftware. Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun, whisperity. Herald added a project: clang. martong added a comment.
I was thinking about the below test, but then I reverted back because I don't want to add "fake" summaries for testing purposes. Perhaps adding a new checker option could enable these "fake" summaries, @Szelethus what's your opinion? diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp index 64a412bb4db..c1022492429 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -736,6 +736,14 @@ void StdLibraryFunctionsChecker::initFunctionSummaries( }; FunctionSummaryMap = { + { + "__test", + Summaries{ + Summary(ArgTypes{IntTy, IntTy}, RetType{IntTy}, EvalCallAsPure) + .ArgConstraint(ArgumentCondition(0U, WithinRange, SingleValue(1))) + .ArgConstraint(ArgumentCondition(1U, WithinRange, SingleValue(1))) + } + }, // The isascii() family of functions. // The behavior is undefined if the value of the argument is not // representable as unsigned char or is not equal to EOF. See e.g. C99 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 a20b90ad1cc..01109e28b99 100644 --- a/clang/test/Analysis/std-c-library-functions-arg-constraints.c +++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.c @@ -85,3 +85,18 @@ void test_notnull_symbolic2(FILE *fp, int *buf) { // bugpath-warning{{Function argument constraint is not satisfied}} \ // bugpath-note{{Function argument constraint is not satisfied}} } + +int __test(int, int); + +void test_multiple_constraints(int x, int y) { + __test(x, y); + clang_analyzer_eval(x == 1); // \ + // report-warning{{TRUE}} \ + // bugpath-warning{{TRUE}} \ + // bugpath-note{{TRUE}} + clang_analyzer_eval(y == 1); // \ + // report-warning{{TRUE}} \ + // bugpath-warning{{TRUE}} \ + // bugpath-note{{TRUE}} +} + Previously we induced a state split if there were multiple argument constraints given for a function. This was because we called `addTransition` inside the for loop. The fix is to is to store the state and apply the next argument constraint on that. And once the loop is finished we call `addTransition`. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D76790 Files: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -452,23 +452,26 @@ const Summary &Summary = *FoundSummary; ProgramStateRef State = C.getState(); + ProgramStateRef NewState = State; for (const ValueConstraintPtr& VC : Summary.ArgConstraints) { - ProgramStateRef SuccessSt = VC->apply(State, Call, Summary); - ProgramStateRef FailureSt = VC->negate()->apply(State, Call, Summary); + ProgramStateRef SuccessSt = VC->apply(NewState, Call, Summary); + ProgramStateRef FailureSt = VC->negate()->apply(NewState, Call, Summary); // The argument constraint is not satisfied. if (FailureSt && !SuccessSt) { - if (ExplodedNode *N = C.generateErrorNode(State)) + if (ExplodedNode *N = C.generateErrorNode(NewState)) reportBug(Call, N, C); break; } else { - // Apply the constraint even if we cannot reason about the argument. This - // means both SuccessSt and FailureSt can be true. If we weren't applying - // the constraint that would mean that symbolic execution continues on a - // code whose behaviour is undefined. + // We will apply the constraint even if we cannot reason about the + // argument. This means both SuccessSt and FailureSt can be true. If we + // weren't applying the constraint that would mean that symbolic + // execution continues on a code whose behaviour is undefined. assert(SuccessSt); - C.addTransition(SuccessSt); + NewState = SuccessSt; } } + if (NewState && NewState != State) + C.addTransition(NewState); } void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -452,23 +452,26 @@ const Summary &Summary = *FoundSummary; ProgramStateRef State = C.getState(); + ProgramStateRef NewState = State; for (const ValueConstraintPtr& VC : Summary.ArgConstraints) { - ProgramStateRef SuccessSt = VC->apply(State, Call, Summary); - ProgramStateRef FailureSt = VC->negate()->apply(State, Call, Summary); + ProgramStateRef SuccessSt = VC->apply(NewState, Call, Summary); + ProgramStateRef FailureSt = VC->negate()->apply(NewState, Call, Summary); // The argument constraint is not satisfied. if (FailureSt && !SuccessSt) { - if (ExplodedNode *N = C.generateErrorNode(State)) + if (ExplodedNode *N = C.generateErrorNode(NewState)) reportBug(Call, N, C); break; } else { - // Apply the constraint even if we cannot reason about the argument. This - // means both SuccessSt and FailureSt can be true. If we weren't applying - // the constraint that would mean that symbolic execution continues on a - // code whose behaviour is undefined. + // We will apply the constraint even if we cannot reason about the + // argument. This means both SuccessSt and FailureSt can be true. If we + // weren't applying the constraint that would mean that symbolic + // execution continues on a code whose behaviour is undefined. assert(SuccessSt); - C.addTransition(SuccessSt); + NewState = SuccessSt; } } + if (NewState && NewState != State) + C.addTransition(NewState); } void StdLibraryFunctionsChecker::checkPostCall(const CallEvent &Call,
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits