NoQ added a comment.
We're doing much more here than just fixing the CFError behavior; we're
actually making the whole analyzer respect these annotations in top frame.
Let's add checker-inspecific tests. We have a test checker
`debug.ExprInspection` just for that:
// RUN: %clang_analyze_cc1 ... -analyzer-checker=debug.ExprInspection ...
void clang_analyzer_eval(bool);
void foo(void *x) __attribute__((nonnull)) {
clang_analyzer_eval(x != nullptr); // expected-warning{{TRUE}}
}
================
Comment at: clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp:218
+/// We want to trust developer annotations and consider all 'nonnul' parameters
+/// as non-null indeed. Each marked parameter will get a corresponding
----------------
Typo :p
================
Comment at: clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp:234
+/// \endcode
+void NonNullParamChecker::checkBeginFunction(CheckerContext &Context) const {
+ const LocationContext *LocContext = Context.getLocationContext();
----------------
As far as i understand, you should only do all this for //top-level//
functions, i.e. the ones from which we've started the analysis. You can skip
inlined functions here because a similar assumption is already made for their
parameters in `checkPreCall`.
You can figure out if you're in top frame via `Context.inTopFrame()`.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp:238-239
+ const Decl *FD = LocContext->getDecl();
+ // AnyCall helps us here to avoid checking for FunctionDecl and
ObjCMethodDecl
+ // separately and aggregates interfaces of these classes.
+ auto AbstractCall = AnyCall::forDecl(FD);
----------------
Nice! Should we add new tests for ObjC method calls as well then?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp:253
+ Loc ParameterLoc = State->getLValue(Parameter, LocContext);
+ auto StoredVal = State->getSVal(ParameterLoc).getAs<loc::MemRegionVal>();
+ if (!StoredVal)
----------------
Once you restrict yourself to top frame, you can save two lines of code and one
asterisk by doing `.castAs<DefinedOrUnknownSVal>()` here instead. Because we'll
never assume that we're being fed an undefined value in an out-of-context
top-level function. A much stronger assertion can probably be made.
================
Comment at: clang/test/Analysis/nonnull.cpp:28
+int globalVar = 15;
+void moreParamsThanArgs [[gnu::nonnull(2, 4)]] (int a, int *p, int b = 42, int
*q = &globalVar);
+
----------------
C-style variadic functions are the real problem. Variadic templates are easy;
they're just duplicated in the AST as many times as necessary and all the
parameter declarations are in place. Default arguments are also easy; the
argument expression is still present in the AST even if it's not explicitly
written down at the call site. C-style variadic functions are hard because they
actually have more arguments than they have parameters.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77806/new/
https://reviews.llvm.org/D77806
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
