NoQ added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:409
- Optional<Summary> FoundSummary = findFunctionSummary(FD, CE, C);
+ for (const ValueConstraintPtr& VC : Summary.ArgConstraints) {
+ ProgramStateRef SuccessSt = VC->apply(State, Call, Summary);
----------------
martong wrote:
> NoQ wrote:
> > martong wrote:
> > > NoQ wrote:
> > > > Maybe we should add an assertion that the same argument isn't specified
> > > > multiple times.
> > > I think there could be cases when we want to have e.g. a not-null
> > > constraint on the 1st argument, but also we want to express that the 1st
> > > argument's size is described by the 2nd argument. I am planning to
> > > implement such a constraints in the future. In that case we would have
> > > two constraints on the 1st argument and the assert would fire.
> > Wait, i misunderstood the code. It's even worse than that: you're adding
> > transitions in a loop, so it'll cause state splits for every constraint.
> > Because you do not intend to create multiple branches here, there needs to
> > be exactly one `addTransition` performed every time `checkPreCall` is
> > called. I.e., for now this code is breaking everything whenever there's
> > more than one constraint, regardless of whether it's on the same argument.
> Yeah, that's a very good catch, thanks! I am going to prepare a patch to fix
> this soon. My idea is to store the `SuccessSt` and apply the next argument
> constraint on that. And once the loop is finished I'll have call the
> `addTransition()`.
Yup, that's the common thing to do in such cases.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73898/new/
https://reviews.llvm.org/D73898
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
