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



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

Reply via email to