r337866 - Revert "[analyzer] Extend NoStoreFuncVisitor to insert a note on IVars"
George Karpenkov via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 24 16:23:33 PDT 2018
Author: george.karpenkov
Date: Tue Jul 24 16:23:33 2018
New Revision: 337866
URL: http://llvm.org/viewvc/llvm-project?rev=337866&view=rev
Log:
Revert "[analyzer] Extend NoStoreFuncVisitor to insert a note on IVars"
This reverts commit a9e21bd727112cd69eabc1af648c5da6b773d06e.
Reverted because the dependency has not landed yet.
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.cpp
cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=337866&r1=337865&r2=337866&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Tue Jul 24 16:23:33 2018
@@ -22,7 +22,6 @@
#include "clang/AST/ExprObjC.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/Type.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Analysis/CFG.h"
#include "clang/Analysis/CFGStmtMap.h"
@@ -307,26 +306,15 @@ public:
CallEventRef<> Call =
BRC.getStateManager().getCallEventManager().getCaller(SCtx, State);
+
const PrintingPolicy &PP = BRC.getASTContext().getPrintingPolicy();
const SourceManager &SM = BRC.getSourceManager();
-
- // Region of interest corresponds to an IVar, exiting a method
- // which could have written into that IVar, but did not.
- if (const auto *MC = dyn_cast<ObjCMethodCall>(Call))
- if (const auto *IvarR = dyn_cast<ObjCIvarRegion>(RegionOfInterest))
- if (potentiallyWritesIntoIvar(Call->getRuntimeDefinition().getDecl(),
- IvarR->getDecl()) &&
- !isRegionOfInterestModifiedInFrame(N))
- return notModifiedMemberDiagnostics(
- Ctx, SM, PP, *CallExitLoc, Call,
- MC->getReceiverSVal().getAsRegion());
-
if (const auto *CCall = dyn_cast<CXXConstructorCall>(Call)) {
const MemRegion *ThisRegion = CCall->getCXXThisVal().getAsRegion();
if (RegionOfInterest->isSubRegionOf(ThisRegion)
&& !CCall->getDecl()->isImplicit()
&& !isRegionOfInterestModifiedInFrame(N))
- return notModifiedMemberDiagnostics(Ctx, SM, PP, *CallExitLoc,
+ return notModifiedInConstructorDiagnostics(Ctx, SM, PP, *CallExitLoc,
CCall, ThisRegion);
}
@@ -343,7 +331,7 @@ public:
if (isRegionOfInterestModifiedInFrame(N))
return nullptr;
- return notModifiedParameterDiagnostics(
+ return notModifiedDiagnostics(
Ctx, SM, PP, *CallExitLoc, Call, PVD, R, IndirectionLevel);
}
QualType PT = T->getPointeeType();
@@ -358,22 +346,6 @@ public:
}
private:
-
- /// \return Whether the method declaration \p Parent
- /// syntactically has a binary operation writing into the ivar \p Ivar.
- bool potentiallyWritesIntoIvar(const Decl *Parent,
- const ObjCIvarDecl *Ivar) {
- using namespace ast_matchers;
- if (!Parent)
- return false;
- StatementMatcher WriteIntoIvarM = binaryOperator(
- hasOperatorName("="), hasLHS(ignoringParenImpCasts(objcIvarRefExpr(
- hasDeclaration(equalsNode(Ivar))))));
- StatementMatcher ParentM = stmt(hasDescendant(WriteIntoIvarM));
- auto Matches = match(ParentM, *Parent->getBody(), Parent->getASTContext());
- return !Matches.empty();
- }
-
/// Check and lazily calculate whether the region of interest is
/// modified in the stack frame to which \p N belongs.
/// The calculation is cached in FramesModifyingRegion.
@@ -442,21 +414,19 @@ private:
Ty->getPointeeType().getCanonicalType().isConstQualified();
}
- /// \return Diagnostics piece for the member field not modified
- /// in a given function.
- std::shared_ptr<PathDiagnosticPiece> notModifiedMemberDiagnostics(
+ std::shared_ptr<PathDiagnosticPiece> notModifiedInConstructorDiagnostics(
const LocationContext *Ctx,
const SourceManager &SM,
const PrintingPolicy &PP,
CallExitBegin &CallExitLoc,
- CallEventRef<> Call,
+ const CXXConstructorCall *Call,
const MemRegion *ArgRegion) {
- const char *TopRegionName = isa<ObjCMethodCall>(Call) ? "self" : "this";
SmallString<256> sbuf;
llvm::raw_svector_ostream os(sbuf);
os << DiagnosticsMsg;
- bool out = prettyPrintRegionName(TopRegionName, "->", /*IsReference=*/true,
- /*IndirectionLevel=*/1, ArgRegion, os, PP);
+ bool out = prettyPrintRegionName(
+ "this", "->", /*IsReference=*/true,
+ /*IndirectionLevel=*/1, ArgRegion, os, PP);
// Return nothing if we have failed to pretty-print.
if (!out)
@@ -464,16 +434,14 @@ private:
os << "'";
PathDiagnosticLocation L =
- getPathDiagnosticLocation(CallExitLoc.getReturnStmt(), SM, Ctx, Call);
+ getPathDiagnosticLocation(nullptr, SM, Ctx, Call);
return std::make_shared<PathDiagnosticEventPiece>(L, os.str());
}
- /// \return Diagnostics piece for the parameter \p PVD not modified
- /// in a given function.
/// \p IndirectionLevel How many times \c ArgRegion has to be dereferenced
/// before we get to the super region of \c RegionOfInterest
std::shared_ptr<PathDiagnosticPiece>
- notModifiedParameterDiagnostics(const LocationContext *Ctx,
+ notModifiedDiagnostics(const LocationContext *Ctx,
const SourceManager &SM,
const PrintingPolicy &PP,
CallExitBegin &CallExitLoc,
@@ -513,8 +481,8 @@ private:
/// Pretty-print region \p ArgRegion starting from parent to \p os.
/// \return whether printing has succeeded
- bool prettyPrintRegionName(StringRef TopRegionName,
- StringRef Sep,
+ bool prettyPrintRegionName(const char *TopRegionName,
+ const char *Sep,
bool IsReference,
int IndirectionLevel,
const MemRegion *ArgRegion,
@@ -523,8 +491,7 @@ private:
SmallVector<const MemRegion *, 5> Subregions;
const MemRegion *R = RegionOfInterest;
while (R != ArgRegion) {
- if (!(isa<FieldRegion>(R) || isa<CXXBaseObjectRegion>(R) ||
- isa<ObjCIvarRegion>(R)))
+ if (!(isa<FieldRegion>(R) || isa<CXXBaseObjectRegion>(R)))
return false; // Pattern-matching failed.
Subregions.push_back(R);
R = cast<SubRegion>(R)->getSuperRegion();
@@ -552,10 +519,6 @@ private:
os << Sep;
FR->getDecl()->getDeclName().print(os, PP);
Sep = ".";
- } else if (const auto *IR = dyn_cast<ObjCIvarRegion>(*I)) {
- os << "->";
- IR->getDecl()->getDeclName().print(os, PP);
- Sep = ".";
} else if (isa<CXXBaseObjectRegion>(*I)) {
continue; // Just keep going up to the base region.
} else {
Modified: cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.cpp?rev=337866&r1=337865&r2=337866&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.cpp (original)
+++ cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.cpp Tue Jul 24 16:23:33 2018
@@ -97,14 +97,6 @@ struct C {
int x;
int y;
C(int pX, int pY) : x(pX) {} // expected-note{{Returning without writing to 'this->y'}}
-
- C(int pX, int pY, bool Flag) {
- x = pX;
- if (Flag) // expected-note{{Assuming 'Flag' is not equal to 0}}
- // expected-note at -1{{Taking true branch}}
- return; // expected-note{{Returning without writing to 'this->y'}}
- y = pY;
- }
};
int use_constructor() {
@@ -114,15 +106,6 @@ int use_constructor() {
// expected-warning at -1{{Undefined or garbage value returned to caller}}
}
-int coin();
-
-int use_other_constructor() {
- C c(0, 0, coin()); // expected-note{{Calling constructor for 'C'}}
- // expected-note at -1{{Returning from constructor for 'C'}}
- return c.y; // expected-note{{Undefined or garbage value returned to caller}}
- // expected-warning at -1{{Undefined or garbage value returned to caller}}
-}
-
struct D {
void initialize(int *);
};
@@ -139,6 +122,8 @@ int use_d_initializer(D* d) {
// expected-warning at -1{{Undefined or garbage value returned to caller}}
}
+int coin();
+
struct S2 {
int x;
};
Modified: cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m?rev=337866&r1=337865&r2=337866&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m (original)
+++ cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m Tue Jul 24 16:23:33 2018
@@ -1,10 +1,6 @@
-// RUN: %clang_analyze_cc1 -x objective-c -analyzer-checker=core,nullability -analyzer-output=text -Wno-objc-root-class -fblocks -verify %s
+// RUN: %clang_analyze_cc1 -x objective-c -analyzer-checker=core -analyzer-output=text -Wno-objc-root-class -fblocks -verify %s
-#include "../Inputs/system-header-simulator-for-nullability.h"
-
-extern int coin();
-
- at interface I : NSObject
+ at interface I
- (int)initVar:(int *)var param:(int)param;
@end
@@ -48,30 +44,3 @@ int initFromBlock() {
}();
return z;
}
-
-extern void expectNonNull(NSString * _Nonnull a);
-
- at interface A : NSObject
-- (void) func;
-- (void) initAMaybe;
- at end
-
- at implementation A {
- NSString * a;
-}
-
-- (void) initAMaybe {
- if (coin()) // expected-note{{Assuming the condition is false}}
- // expected-note at -1{{Taking false branch}}
- a = @"string";
-} // expected-note{{Returning without writing to 'self->a'}}
-
-- (void) func {
- a = nil; // expected-note{{nil object reference stored to 'a'}}
- [self initAMaybe]; // expected-note{{Calling 'initAMaybe'}}
- // expected-note at -1{{Returning from 'initAMaybe'}}
- expectNonNull(a); // expected-warning{{nil passed to a callee that requires a non-null 1st parameter}}
- // expected-note at -1{{nil passed to a callee that requires a non-null 1st parameter}}
-}
-
- at end
More information about the cfe-commits
mailing list