From: Artem Dergachev Date: Mon, 5 Jun 2017 12:40:03 +0000 (+0000) Subject: [analyzer] Nullability: fix notes around synthesized ObjC property accessors. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d6f929a22be467bf8cf89eafd5e022a081a45488;p=clang [analyzer] Nullability: fix notes around synthesized ObjC property accessors. Nullable-to-nonnull checks used to crash when the custom bug visitor was trying to add its notes to autosynthesized accessors of Objective-C properties. Now we avoid this, mostly automatically outside of checker control, by moving the diagnostic to the parent stack frame where the accessor has been called. Differential revision: https://reviews.llvm.org/D32437 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@304710 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h b/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h index dc6e54a332..a07cd88950 100644 --- a/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h +++ b/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h @@ -550,13 +550,15 @@ public: class PathDiagnosticCallPiece : public PathDiagnosticPiece { PathDiagnosticCallPiece(const Decl *callerD, const PathDiagnosticLocation &callReturnPos) - : PathDiagnosticPiece(Call), Caller(callerD), Callee(nullptr), - NoExit(false), callReturn(callReturnPos) {} + : PathDiagnosticPiece(Call), Caller(callerD), Callee(nullptr), + NoExit(false), IsCalleeAnAutosynthesizedPropertyAccessor(false), + callReturn(callReturnPos) {} PathDiagnosticCallPiece(PathPieces &oldPath, const Decl *caller) - : PathDiagnosticPiece(Call), Caller(caller), Callee(nullptr), - NoExit(true), path(oldPath) {} - + : PathDiagnosticPiece(Call), Caller(caller), Callee(nullptr), + NoExit(true), IsCalleeAnAutosynthesizedPropertyAccessor(false), + path(oldPath) {} + const Decl *Caller; const Decl *Callee; @@ -564,6 +566,10 @@ class PathDiagnosticCallPiece : public PathDiagnosticPiece { // call exit. bool NoExit; + // Flag signifying that the callee function is an Objective-C autosynthesized + // property getter or setter. + bool IsCalleeAnAutosynthesizedPropertyAccessor; + // The custom string, which should appear after the call Return Diagnostic. // TODO: Should we allow multiple diagnostics? std::string CallStackMessage; diff --git a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp index 41999d2527..fa9a317683 100644 --- a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp @@ -326,7 +326,7 @@ NullabilityChecker::NullabilityBugVisitor::VisitNode(const ExplodedNode *N, // Retrieve the associated statement. const Stmt *S = TrackedNullab->getNullabilitySource(); - if (!S) { + if (!S || S->getLocStart().isInvalid()) { S = PathDiagnosticLocation::getStmt(N); } diff --git a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp index 7c5ee3b259..6aa6da560e 100644 --- a/lib/StaticAnalyzer/Core/PathDiagnostic.cpp +++ b/lib/StaticAnalyzer/Core/PathDiagnostic.cpp @@ -694,7 +694,30 @@ PathDiagnosticLocation::create(const ProgramPoint& P, return PathDiagnosticLocation(S, SMng, P.getLocationContext()); } +static const LocationContext * +findTopAutosynthesizedParentContext(const LocationContext *LC) { + assert(LC->getAnalysisDeclContext()->isBodyAutosynthesized()); + const LocationContext *ParentLC = LC->getParent(); + assert(ParentLC && "We don't start analysis from autosynthesized code"); + while (ParentLC->getAnalysisDeclContext()->isBodyAutosynthesized()) { + LC = ParentLC; + ParentLC = LC->getParent(); + assert(ParentLC && "We don't start analysis from autosynthesized code"); + } + return LC; +} + const Stmt *PathDiagnosticLocation::getStmt(const ExplodedNode *N) { + // We cannot place diagnostics on autosynthesized code. + // Put them onto the call site through which we jumped into autosynthesized + // code for the first time. + const LocationContext *LC = N->getLocationContext(); + if (LC->getAnalysisDeclContext()->isBodyAutosynthesized()) { + // It must be a stack frame because we only autosynthesize functions. + return cast(findTopAutosynthesizedParentContext(LC)) + ->getCallSite(); + } + // Otherwise, see if the node's program point directly points to a statement. ProgramPoint P = N->getLocation(); if (Optional SP = P.getAs()) return SP->getStmt(); @@ -912,6 +935,17 @@ void PathDiagnosticCallPiece::setCallee(const CallEnter &CE, callEnterWithin = PathDiagnosticLocation::createBegin(Callee, SM); callEnter = getLocationForCaller(CalleeCtx, CE.getLocationContext(), SM); + + // Autosynthesized property accessors are special because we'd never + // pop back up to non-autosynthesized code until we leave them. + // This is not generally true for autosynthesized callees, which may call + // non-autosynthesized callbacks. + // Unless set here, the IsCalleeAnAutosynthesizedPropertyAccessor flag + // defaults to false. + if (const ObjCMethodDecl *MD = dyn_cast(Callee)) + IsCalleeAnAutosynthesizedPropertyAccessor = ( + MD->isPropertyAccessor() && + CalleeCtx->getAnalysisDeclContext()->isBodyAutosynthesized()); } static inline void describeClass(raw_ostream &Out, const CXXRecordDecl *D, @@ -986,7 +1020,11 @@ static bool describeCodeDecl(raw_ostream &Out, const Decl *D, std::shared_ptr PathDiagnosticCallPiece::getCallEnterEvent() const { - if (!Callee) + // We do not produce call enters and call exits for autosynthesized property + // accessors. We do generally produce them for other functions coming from + // the body farm because they may call callbacks that bring us back into + // visible code. + if (!Callee || IsCalleeAnAutosynthesizedPropertyAccessor) return nullptr; SmallString<256> buf; @@ -1020,7 +1058,11 @@ PathDiagnosticCallPiece::getCallEnterWithinCallerEvent() const { std::shared_ptr PathDiagnosticCallPiece::getCallExitEvent() const { - if (NoExit) + // We do not produce call enters and call exits for autosynthesized property + // accessors. We do generally produce them for other functions coming from + // the body farm because they may call callbacks that bring us back into + // visible code. + if (NoExit || IsCalleeAnAutosynthesizedPropertyAccessor) return nullptr; SmallString<256> buf; diff --git a/test/Analysis/nullability-notes.m b/test/Analysis/nullability-notes.m new file mode 100644 index 0000000000..a863ce2c5e --- /dev/null +++ b/test/Analysis/nullability-notes.m @@ -0,0 +1,18 @@ +// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull,nullability.NullablePassedToNonnull,nullability.NullableReturnedFromNonnull,nullability.NullableDereferenced -analyzer-output=text -verify %s + +#include "Inputs/system-header-simulator-for-nullability.h" + +void takesNonnull(NSObject *_Nonnull y); + +@interface ClassWithProperties: NSObject +@property(copy, nullable) NSObject *x; +-(void) method; +@end; +@implementation ClassWithProperties +-(void) method { + // no-crash + NSObject *x = self.x; // expected-note{{Nullability 'nullable' is inferred}} + takesNonnull(x); // expected-warning{{Nullable pointer is passed to a callee that requires a non-null 1st parameter}} + // expected-note@-1{{Nullable pointer is passed to a callee that requires a non-null 1st parameter}} +} +@end