[PATCH] D77066: [analyzer] ApiModeling: Add buffer size arg constraint

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:249-250
+      // cannot apply the constraint. Actually, other checkers like
+      // CallAndMessage should catch this situation earlier, because we call a
+      // function with an uninitialized argument.
+      return nullptr;
----------------
Szelethus wrote:
> Would an unreachable be appropriate here then?
Yes, good point, just added that. CallAndMessage is already a dependency, so this must not fire.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:993
+                                    RetType{IntTy}, EvalCallAsPure)
+                                .ArgConstraint(BufferSize(0, 1)));
   }
----------------
Szelethus wrote:
> In most places, where we refer to an argument number, we use `ArgNo`. Is there a reason we don't do that here? Can we enforce greater type safety?
Yeah, good point, I am going with this:
```
                BufferSize(/*Buffer=*/ArgNo(0), /*BufSize=*/ArgNo(1))));
```

About type safety: I was thinking about a strong typedef, but we don't actually have a convenient template for that in LLVM. And most of the time here in LLVM people just apply the /*Arg=*/ pythonish form.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77066