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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to