steakhal added a comment. Looks great. I'm loving it! BTW what is the semantics of `[p retain]` in ObjC? Can `p` be null in that context? Or does it count as a dereferences, hence it constraints the pointer?
Why did you touch the `AnalysisOrderChecker`, should we separate those changes? Additionally, why do you think it needs to be in the `alpha.core` package instead of the `core`? What sort of false-positives have you seen in the wild justifying that classification? ================ Comment at: clang/docs/analyzer/checkers.rst:1703 +alpha.core.ReverseNull (C) +""""""""""""""""""""""""" +Checks whether a dereferenced (and as such, assumed to be non-null) pointer is ---------------- ================ Comment at: clang/lib/StaticAnalyzer/Checkers/NullPtrInterferenceChecker.cpp:196-198 + const MemRegion *MR = Cond->getAsRegion(); + if (!MR) + return; ---------------- I think additionally to this, you should also check for the type of the `Condition` expression. It's gotta be a pointer. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/NullPtrInterferenceChecker.cpp:211-216 + // We want to be sure that the constraint and the condition are in the + // same stackframe. Caller and callee functions' pre/post conditions may + // not apply to the caller stackframe. A similar issue is discussed here: + // https://discourse.llvm.org/t/static-analyzer-query-why-is-suppress-null-return-paths-enabled-by-default/ + if (NBeforeConstraint->getLocationContext() != Ctx.getLocationContext()) + return; ---------------- IMO we should catch these as well: ```lang=C++ void store(int *q, int value) { *q = value; } void top(int *p) { store(p, 5); if (!p) return; } ``` In this case, the stack-frame in which the pointer gets constrained is not the same as where the redundant check resides - rather a child frame of that. ================ Comment at: clang/test/Analysis/null-pointer-interference.c:18 + // expected-note@-1{{Pointer assumed non-null here}} + // expected-note@-2{{Consider moving the condition here}} + if (p) ---------------- "before this expression"? The term `here` is not well defined IMO. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120992/new/ https://reviews.llvm.org/D120992 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
