Author: dergachev Date: Mon Jun 5 07:40:03 2017 New Revision: 304710 URL: http://llvm.org/viewvc/llvm-project?rev=304710&view=rev Log: [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 Added: cfe/trunk/test/Analysis/nullability-notes.m Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h?rev=304710&r1=304709&r2=304710&view=diff ============================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h Mon Jun 5 07:40:03 2017 @@ -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 P // 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; Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp?rev=304710&r1=304709&r2=304710&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp Mon Jun 5 07:40:03 2017 @@ -326,7 +326,7 @@ NullabilityChecker::NullabilityBugVisito // Retrieve the associated statement. const Stmt *S = TrackedNullab->getNullabilitySource(); - if (!S) { + if (!S || S->getLocStart().isInvalid()) { S = PathDiagnosticLocation::getStmt(N); } Modified: cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp?rev=304710&r1=304709&r2=304710&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp Mon Jun 5 07:40:03 2017 @@ -694,7 +694,30 @@ PathDiagnosticLocation::create(const Pro 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<StackFrameContext>(findTopAutosynthesizedParentContext(LC)) + ->getCallSite(); + } + // Otherwise, see if the node's program point directly points to a statement. ProgramPoint P = N->getLocation(); if (Optional<StmtPoint> SP = P.getAs<StmtPoint>()) return SP->getStmt(); @@ -912,6 +935,17 @@ void PathDiagnosticCallPiece::setCallee( 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<ObjCMethodDecl>(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 std::shared_ptr<PathDiagnosticEventPiece> 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::getCallEnterWit std::shared_ptr<PathDiagnosticEventPiece> 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; Added: cfe/trunk/test/Analysis/nullability-notes.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/nullability-notes.m?rev=304710&view=auto ============================================================================== --- cfe/trunk/test/Analysis/nullability-notes.m (added) +++ cfe/trunk/test/Analysis/nullability-notes.m Mon Jun 5 07:40:03 2017 @@ -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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits