[PATCH] D77148: [analyzer] ApiModeling: Add buffer size arg constraint with multiplier involved

Gábor Horváth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 26 09:44:44 PDT 2020


xazax.hun added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1013
+    addToFunctionSummaryMap(
+        "__buf_size_arg_constraint_mul",
+        Summary(ArgTypes{ConstVoidPtrTy, SizeTy, SizeTy}, RetType{IntTy},
----------------
martong wrote:
> xazax.hun wrote:
> > Why do we need these test functions? Above I saw `fread` as an example that requires this capability. Wouldn't it be better to make its summary utilize the new feature and use `fread` in tests? Do I miss something?
> Yeah, we could test that with `fread`. However, in `fread` there are multiple argument constraints for the different args. I wanted a test function which has this arg constraint in an isolation. In my opinion it is good to have small isolated unit tests that test only one functionality. Also if we decide to remove or modify `fread`s summary for any reason, then we should modify this test too, on the other hand, with this test function it is not a problem.
I see. The plan in the future is to split this checker up a bit. In this case I'd like to have these test functions in a separate test checker. Until then I'm fine with a FIXME/TODO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77148





More information about the cfe-commits mailing list