[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

I like it.

How about an implementation like this:

  void CStringChecker::evalSprintfCommon(CheckerContext &C, const CallExpr *CE,
                                         bool IsBounded) const {
    ProgramStateRef State = C.getState();
    DestinationArgExpr Dest = {CE->getArg(0), 0};
  
    const auto NumParams = CE->getCalleeDecl()->getAsFunction()->getNumParams();
    assert(CE->getNumArgs() >= NumParams);
  
    auto AllArguments =
        llvm::make_range(CE->getArgs(), CE->getArgs() + CE->getNumArgs());
    auto VariadicArguments = drop_begin(enumerate(AllArguments), NumParams);
  
    for (auto [ArgIdx, ArgExpr] : VariadicArguments) {
      // We consider only string buffers
      if (QualType PointeeTy = ArgExpr->getType()->getPointeeType();
          PointeeTy.isNull() || !PointeeTy->isAnyCharacterType())
        continue;
      SourceArgExpr Source = {ArgExpr, unsigned(ArgIdx)};
  
      // Ensure the buffers do not overlap.
      SizeArgExpr SrcExprAsSizeDummy = {Source.Expression, Source.ArgumentIndex};
      State = CheckOverlap(
          C, State,
          (IsBounded ? SizeArgExpr{CE->getArg(1), 1} : SrcExprAsSizeDummy), Dest,
          Source);
      if (!State)
        return;
    }
  
    C.addTransition(State);
  }



- Using ranges to iterate over the variadic arguments. This way it will just work with both builtins and whatnot.
- Adding a transition to preserve the gained knowledge about the relation of the source and destination buffers. In this case, it won't be really effective, but at least we obey this convention.

Maybe add a few test cases where the warning should not be present.
And it is also valuable to have at least one test case asserting the complete message.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:179-180
       {{CDF_MaybeBuiltin, {"explicit_bzero"}, 2}, &CStringChecker::evalBzero},
+      {{CDF_MaybeBuiltin, {"sprintf"}}, &CStringChecker::evalSprintf},
+      {{CDF_MaybeBuiltin, {"snprintf"}}, &CStringChecker::evalSnprintf},
   };
----------------
Let's express that we expect at least 2 arguments at the callsites.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150430/new/

https://reviews.llvm.org/D150430