[PATCH] D104046: [analyzer] Simplify the process of producing notes for stores
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG16f7a952ec3e: [analyzer] Simplify the process of producing
notes for stores (authored by vsavchenko).
Changed prior to commit:
https://reviews.llvm.org/D104046?vs=351515=352071#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104046/new/
https://reviews.llvm.org/D104046
Files:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1214,136 +1214,146 @@
return FrameSpace->getStackFrame() == LCtx->getStackFrame();
}
+static bool isObjCPointer(const MemRegion *R) {
+ if (R->isBoundable())
+if (const auto *TR = dyn_cast(R))
+ return TR->getValueType()->isObjCObjectPointerType();
+
+ return false;
+}
+
+static bool isObjCPointer(const ValueDecl *D) {
+ return D->getType()->isObjCObjectPointerType();
+}
+
/// Show diagnostics for initializing or declaring a region \p R with a bad value.
-static void showBRDiagnostics(const char *action, llvm::raw_svector_ostream ,
- const MemRegion *NewR, SVal V,
- const MemRegion *OldR, const DeclStmt *DS) {
- if (NewR->canPrintPretty()) {
-NewR->printPretty(os);
-os << " ";
- }
-
- if (V.getAs()) {
-bool b = false;
-if (NewR->isBoundable()) {
- if (const auto *TR = dyn_cast(NewR)) {
-if (TR->getValueType()->isObjCObjectPointerType()) {
- os << action << "nil";
- b = true;
-}
- }
-}
-if (!b)
- os << action << "a null pointer value";
-
- } else if (auto CVal = V.getAs()) {
-os << action << CVal->getValue();
- } else if (OldR && OldR->canPrintPretty()) {
-os << action << "the value of ";
-OldR->printPretty(os);
- } else if (DS) {
-if (V.isUndef()) {
- if (isa(NewR)) {
+static void showBRDiagnostics(llvm::raw_svector_ostream , StoreInfo SI) {
+ const bool HasPrefix = SI.Dest->canPrintPretty();
+
+ if (HasPrefix) {
+SI.Dest->printPretty(OS);
+OS << " ";
+ }
+
+ const char *Action = nullptr;
+
+ switch (SI.StoreKind) {
+ case StoreInfo::Initialization:
+Action = HasPrefix ? "initialized to " : "Initializing to ";
+break;
+ case StoreInfo::BlockCapture:
+Action = HasPrefix ? "captured by block as " : "Captured by block as ";
+break;
+ default:
+llvm_unreachable("Unexpected store kind");
+ }
+
+ if (SI.Value.getAs()) {
+OS << Action << (isObjCPointer(SI.Dest) ? "nil" : "a null pointer value");
+
+ } else if (auto CVal = SI.Value.getAs()) {
+OS << Action << CVal->getValue();
+
+ } else if (SI.Origin && SI.Origin->canPrintPretty()) {
+OS << Action << "the value of ";
+SI.Origin->printPretty(OS);
+
+ } else if (SI.StoreKind == StoreInfo::Initialization) {
+// We don't need to check here, all these conditions were
+// checked by StoreSiteFinder, when it figured out that it is
+// initialization.
+const auto *DS =
+cast(SI.StoreSite->getLocationAs()->getStmt());
+
+if (SI.Value.isUndef()) {
+ if (isa(SI.Dest)) {
const auto *VD = cast(DS->getSingleDecl());
+
if (VD->getInit()) {
- os << (NewR->canPrintPretty() ? "initialized" : "Initializing")
+ OS << (HasPrefix ? "initialized" : "Initializing")
<< " to a garbage value";
} else {
- os << (NewR->canPrintPretty() ? "declared" : "Declaring")
+ OS << (HasPrefix ? "declared" : "Declaring")
<< " without an initial value";
}
}
} else {
- os << (NewR->canPrintPretty() ? "initialized" : "Initialized") << " here";
+ OS << (HasPrefix ? "initialized" : "Initialized") << " here";
}
}
}
/// Display diagnostics for passing bad region as a parameter.
-static void showBRParamDiagnostics(llvm::raw_svector_ostream ,
- const VarRegion *VR, SVal V,
- const MemRegion *ValueR) {
+static void showBRParamDiagnostics(llvm::raw_svector_ostream ,
+ StoreInfo SI) {
+ const auto *VR = cast(SI.Dest);
const auto *Param = cast(VR->getDecl());
- os << "Passing ";
+ OS << "Passing ";
+
+ if (SI.Value.getAs()) {
+OS << (isObjCPointer(Param) ? "nil object reference"
+: "null pointer value");
+
+ } else if (SI.Value.isUndef()) {
+OS << "uninitialized value";
+
+ } else if (auto CI = SI.Value.getAs()) {
+OS << "the value " << CI->getValue();
+
+ } else if (SI.Origin && SI.Origin->canPrintPretty()) {
+
[PATCH] D104046: [analyzer] Simplify the process of producing notes for stores
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Looks great, thanks!
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1299
+ } else if (SI.Value.isUndef()) {
+OS << "uninitialized value";
+
This isn't new but oof this note is terrible.
I don't think we have any tests where it's actually emitted. I suspect that
this shouldn't happen in practice unless `core` checkers are disabled.
Now that I think of it, our actual warning for undef passed into function says
"function call argument is an uninitialized value" which is almost as bad as
this.
Just sayin' :)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104046/new/
https://reviews.llvm.org/D104046
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D104046: [analyzer] Simplify the process of producing notes for stores
vsavchenko updated this revision to Diff 351515.
vsavchenko added a comment.
Rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104046/new/
https://reviews.llvm.org/D104046
Files:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1214,136 +1214,144 @@
return FrameSpace->getStackFrame() == LCtx->getStackFrame();
}
+static bool isObjCPointer(const MemRegion *R) {
+ if (R->isBoundable())
+if (const auto *TR = dyn_cast(R))
+ return TR->getValueType()->isObjCObjectPointerType();
+
+ return false;
+}
+
+static bool isObjCPointer(const ValueDecl *D) {
+ return D->getType()->isObjCObjectPointerType();
+}
+
/// Show diagnostics for initializing or declaring a region \p R with a bad value.
-static void showBRDiagnostics(const char *action, llvm::raw_svector_ostream ,
- const MemRegion *NewR, SVal V,
- const MemRegion *OldR, const DeclStmt *DS) {
- if (NewR->canPrintPretty()) {
-NewR->printPretty(os);
-os << " ";
- }
-
- if (V.getAs()) {
-bool b = false;
-if (NewR->isBoundable()) {
- if (const auto *TR = dyn_cast(NewR)) {
-if (TR->getValueType()->isObjCObjectPointerType()) {
- os << action << "nil";
- b = true;
-}
- }
-}
-if (!b)
- os << action << "a null pointer value";
-
- } else if (auto CVal = V.getAs()) {
-os << action << CVal->getValue();
- } else if (OldR && OldR->canPrintPretty()) {
-os << action << "the value of ";
-OldR->printPretty(os);
- } else if (DS) {
-if (V.isUndef()) {
- if (isa(NewR)) {
+static void showBRDiagnostics(llvm::raw_svector_ostream , StoreInfo SI) {
+ const bool HasPrefix = SI.Dest->canPrintPretty();
+
+ if (HasPrefix) {
+SI.Dest->printPretty(OS);
+OS << " ";
+ }
+
+ const char *Action = nullptr;
+
+ switch (SI.StoreKind) {
+ case StoreInfo::Initialization:
+Action = HasPrefix ? "initialized to " : "Initializing to ";
+break;
+ case StoreInfo::BlockCapture:
+Action = HasPrefix ? "captured by block as " : "Captured by block as ";
+break;
+ default:
+llvm_unreachable("Unexpected store kind");
+ }
+
+ if (SI.Value.getAs()) {
+OS << Action << (isObjCPointer(SI.Dest) ? "nil" : "a null pointer value");
+
+ } else if (auto CVal = SI.Value.getAs()) {
+OS << Action << CVal->getValue();
+
+ } else if (SI.Origin && SI.Origin->canPrintPretty()) {
+OS << Action << "the value of ";
+SI.Origin->printPretty(OS);
+
+ } else if (SI.StoreKind == StoreInfo::Initialization) {
+// We don't need to check here, all these conditions were
+// checked by StoreSiteFinder, when it figured out that it is
+// initialization.
+const auto *DS =
+cast(SI.StoreSite->getLocationAs()->getStmt());
+
+if (SI.Value.isUndef()) {
+ if (isa(SI.Dest)) {
const auto *VD = cast(DS->getSingleDecl());
+
if (VD->getInit()) {
- os << (NewR->canPrintPretty() ? "initialized" : "Initializing")
+ OS << (HasPrefix ? "initialized" : "Initializing")
<< " to a garbage value";
} else {
- os << (NewR->canPrintPretty() ? "declared" : "Declaring")
+ OS << (HasPrefix ? "declared" : "Declaring")
<< " without an initial value";
}
}
} else {
- os << (NewR->canPrintPretty() ? "initialized" : "Initialized") << " here";
+ OS << (HasPrefix ? "initialized" : "Initialized") << " here";
}
}
}
/// Display diagnostics for passing bad region as a parameter.
-static void showBRParamDiagnostics(llvm::raw_svector_ostream ,
- const VarRegion *VR, SVal V,
- const MemRegion *ValueR) {
+static void showBRParamDiagnostics(llvm::raw_svector_ostream ,
+ StoreInfo SI) {
+ const auto *VR = cast(SI.Dest);
const auto *Param = cast(VR->getDecl());
- os << "Passing ";
+ OS << "Passing ";
+
+ if (SI.Value.getAs()) {
+OS << (isObjCPointer(Param) ? "nil object reference"
+: "null pointer value");
+
+ } else if (SI.Value.isUndef()) {
+OS << "uninitialized value";
+
+ } else if (auto CI = SI.Value.getAs()) {
+OS << "the value " << CI->getValue();
+
+ } else if (SI.Origin && SI.Origin->canPrintPretty()) {
+SI.Origin->printPretty(OS);
- if (V.getAs()) {
-if (Param->getType()->isObjCObjectPointerType())
- os << "nil object reference";
-else
- os << "null pointer value";
- } else if (V.isUndef()) {
-os << "uninitialized value";
-
[PATCH] D104046: [analyzer] Simplify the process of producing notes for stores
vsavchenko created this revision.
vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus,
manas, RedDocMD.
Herald added subscribers: ASDenysPetrov, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, rnkovacs, szepet, baloghadamsoftware.
vsavchenko requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D104046
Files:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1214,136 +1214,144 @@
return FrameSpace->getStackFrame() == LCtx->getStackFrame();
}
+static bool isObjCPointer(const MemRegion *R) {
+ if (R->isBoundable())
+if (const auto *TR = dyn_cast(R))
+ return TR->getValueType()->isObjCObjectPointerType();
+
+ return false;
+}
+
+static bool isObjCPointer(const ValueDecl *D) {
+ return D->getType()->isObjCObjectPointerType();
+}
+
/// Show diagnostics for initializing or declaring a region \p R with a bad value.
-static void showBRDiagnostics(const char *action, llvm::raw_svector_ostream ,
- const MemRegion *NewR, SVal V,
- const MemRegion *OldR, const DeclStmt *DS) {
- if (NewR->canPrintPretty()) {
-NewR->printPretty(os);
-os << " ";
- }
-
- if (V.getAs()) {
-bool b = false;
-if (NewR->isBoundable()) {
- if (const auto *TR = dyn_cast(NewR)) {
-if (TR->getValueType()->isObjCObjectPointerType()) {
- os << action << "nil";
- b = true;
-}
- }
-}
-if (!b)
- os << action << "a null pointer value";
-
- } else if (auto CVal = V.getAs()) {
-os << action << CVal->getValue();
- } else if (OldR && OldR->canPrintPretty()) {
-os << action << "the value of ";
-OldR->printPretty(os);
- } else if (DS) {
-if (V.isUndef()) {
- if (isa(NewR)) {
+static void showBRDiagnostics(llvm::raw_svector_ostream , StoreInfo SI) {
+ const bool HasPrefix = SI.Dest->canPrintPretty();
+
+ if (HasPrefix) {
+SI.Dest->printPretty(OS);
+OS << " ";
+ }
+
+ const char *Action = nullptr;
+
+ switch (SI.StoreKind) {
+ case StoreInfo::Initialization:
+Action = HasPrefix ? "initialized to " : "Initializing to ";
+break;
+ case StoreInfo::BlockCapture:
+Action = HasPrefix ? "captured by block as " : "Captured by block as ";
+break;
+ default:
+llvm_unreachable("Unexpected store kind");
+ }
+
+ if (SI.Value.getAs()) {
+OS << Action << (isObjCPointer(SI.Dest) ? "nil" : "a null pointer value");
+
+ } else if (auto CVal = SI.Value.getAs()) {
+OS << Action << CVal->getValue();
+
+ } else if (SI.Origin && SI.Origin->canPrintPretty()) {
+OS << Action << "the value of ";
+SI.Origin->printPretty(OS);
+
+ } else if (SI.StoreKind == StoreInfo::Initialization) {
+// We don't need to check here, all these conditions were
+// checked by StoreSiteFinder, when it figured out that it is
+// initialization.
+const auto *DS =
+cast(SI.StoreSite->getLocationAs()->getStmt());
+
+if (SI.Value.isUndef()) {
+ if (isa(SI.Dest)) {
const auto *VD = cast(DS->getSingleDecl());
+
if (VD->getInit()) {
- os << (NewR->canPrintPretty() ? "initialized" : "Initializing")
+ OS << (HasPrefix ? "initialized" : "Initializing")
<< " to a garbage value";
} else {
- os << (NewR->canPrintPretty() ? "declared" : "Declaring")
+ OS << (HasPrefix ? "declared" : "Declaring")
<< " without an initial value";
}
}
} else {
- os << (NewR->canPrintPretty() ? "initialized" : "Initialized") << " here";
+ OS << (HasPrefix ? "initialized" : "Initialized") << " here";
}
}
}
/// Display diagnostics for passing bad region as a parameter.
-static void showBRParamDiagnostics(llvm::raw_svector_ostream ,
- const VarRegion *VR, SVal V,
- const MemRegion *ValueR) {
+static void showBRParamDiagnostics(llvm::raw_svector_ostream ,
+ StoreInfo SI) {
+ const auto *VR = cast(SI.Dest);
const auto *Param = cast(VR->getDecl());
- os << "Passing ";
+ OS << "Passing ";
+
+ if (SI.Value.getAs()) {
+OS << (isObjCPointer(Param) ? "nil object reference"
+: "null pointer value");
+
+ } else if (SI.Value.isUndef()) {
+OS << "uninitialized value";
+
+ } else if (auto CI = SI.Value.getAs()) {
+OS << "the value " << CI->getValue();
+
+ } else if (SI.Origin && SI.Origin->canPrintPretty()) {
+SI.Origin->printPretty(OS);
-
