r304710 - [analyzer] Nullability: fix notes around synthesized ObjC property accessors.

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 5 05:40:03 PDT 2017


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);
+
+ at interface ClassWithProperties: NSObject
+ at property(copy, nullable) NSObject *x;
+-(void) method;
+ at end;
+ at 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 at -1{{Nullable pointer is passed to a callee that requires a non-null 1st parameter}}
+}
+ at end




More information about the cfe-commits mailing list