NoQ added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:201-207
+void CERTStrChecker::evalGets(const CallEvent &Call, const CallContext &CallC,
+ CheckerContext &C) const {
+ unsigned DestPos = *CallC.DestinationPos;
+ const Expr *DestArg = Call.getArgExpr(DestPos)->IgnoreImpCasts();
+ SVal DestV = Call.getArgSVal(DestPos);
+
+ auto Report = getReport(*BT, Call, CallC, C);
----------------
NoQ wrote:
> All right, so basically what you're saying is that literally every invocation
> of `gets()` deserves a warning. This means that for all practical purposes
> your checker //is// an AST-based checker, just implemented with
> path-sensitive callbacks. A path-sensitive checker emits warnings based on
> multiple events that happen sequentially along the path (use-after-free:
> "memory deallocated - that same memory used", division by zero: "value
> constrained to zero - something is being divided by that same value", etc.)
> but your checker emits the warning by looking at only one statement:
> "`gets()` is invoked".
>
> Do i understand correctly that your plan is to use path-sensitive analysis
> for fixits only? But you can't emit fixits for any truly path-sensitive
> warning anyway. Fixits must work correctly on all execution paths, so you
> cannot emit a correct fixit by looking at only one execution path. In order
> to emit fixits, you need to either resort to a pure AST check anyway ("this
> expression refers to an array of fixed size"), or maybe implement auxiliary
> data-flow analysis for a certain must-problem (eg., "the buffer argument may
> have exactly one possible value across all paths that reach `gets()`"). But
> in both cases the path-sensitive engine does literally nothing to help you;
> all the data that you'll need for your fixit will be available from the AST.
Like, i think this was an interesting investigation and i was genuinely curious
about how this turns out to be, but for now it seems that the problem you're
trying to solve cannot be solved this way. Path sensitive analysis is
fundamentally applicable to only 50% of the problems (to "may-problems" but not
to "must-problems"), and the problem you're trying to solve is in the latter
category. I believe you'll have to fall back to the relatively boring task of
adding fixits to `security.insecureAPI.gets`; but then, again, if you manage to
employ use-def chains for this problem, that might be quite an inspiring start.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69813/new/
https://reviews.llvm.org/D69813
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
