Szelethus added a comment. In D140387#4021806 <https://reviews.llvm.org/D140387#4021806>, @Szelethus wrote:
> Would be possible to test the errno specific changes as well? I suppose the tests are done in the followup patch mostly? ================ Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp:310 + const MemRegion *ErrnoR = State->get<ErrnoRegion>(); + if (!ErrnoR) + return State; ---------------- balazske wrote: > Szelethus wrote: > > When can this occur? Maybe we can turn this early return to an assert -- > > when `ModelPOSIX` is true, this mustn't be null (if what I'm saying is > > correct). > I am not totally sure that `errno` is found always (definition of `errno` is > encountered) if a summary for a standard function is added. The standard > function in itself must be there, otherwise no summary is added and this > function should not be called. But definition of `errno` may reside in other > header (errno.h) that is probably not included in the translation unit. If > the errno definition is not found the `ErrnoRegion` is not set (the checker > may still turned be on but does nothing, but these functions are allowed to > be called). Existence of `errno` is not even dependent on `ModelPOSIX`, > `errno` is defined in the (non POSIX) C standard. Very well, I'm sold. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1705-1707 + // FIXME: It should be allowed to have a null buffer if any of + // args 1 or 2 are zero. Remove NotNull check of arg 0, add a check + // for non-null buffer if non-zero size to BufferSizeConstraint? ---------------- balazske wrote: > Szelethus wrote: > > Maybe `StreamChecker` could take over the handling of that case? Seems like > > it also warns about the nullness of the first `fread` parameter. > The current approach was that the NULL argument warning is generated by one > checker only, `StreamChecker` or this checker. The NULL check is already > built into these summaries so this is the better place for the warning. The > NULL warning is removed from `StreamChecker` in D137790. Additionally the > same problem can happen with `BufferSizeConstraint` at other function > summaries, we can say generally (or more often) that if a buffer has size of > 0 it is allowed to be NULL pointer. Aha, okay, so we should extend the existing infrastructure to express this as well. Please do that in another patch though! Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140387/new/ https://reviews.llvm.org/D140387 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
