[PATCH] D69948: [Checkers] Added support for freopen to StreamChecker.

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 19 15:46:24 PST 2019


NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:201
+    std::tie(StateRetNotNull, StateRetNull) =
+        CM.assumeDual(StateStreamNull, RetVal);
+    if (StateRetNull) {
----------------
Mmm, wait, this doesn't look right to me. You cannot deduce from the presence of `freopen()` in the code that the argument may be null, so the split over the argument is not well-justified.

The right thing to do here will be to produce a "Schrödinger file descriptor" that's both `Opened` and `OpenFailed` until we observe the return value to be constrained to null or non-null later during analysis (cf. D32449), otherwise conservatively assume that the open succeeded when queried for the first time (because that's how non-paranoid code under analysis will behave).

Or you could drop the failed path immediately if the argument isn't known to be zero. That'll drop the coverage slightly but won't cause anything bad to happen.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69948





More information about the cfe-commits mailing list