NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
When a class forgets to initialize a field in the constructor, and then gets
copied around, a warning is emitted that the value assigned to a specific field
is undefined. When the copy/move constructor is implicit (not written out in
the code) but not trivial (is not a trivial memory copy, eg. because members
have an explicit copy constructor), the body of such constructor is
auto-generated in the AST. In this case the checker's warning message is
squeezed at the top of the class declaration, and it gets hard to guess which
field is at fault.
Fix the warning message to include the name of the field.
Such warnings have become fairly popular in our temporary constructor
evaluation.
This warning should probably be extended to the trivial copy case, but that's a
separate story.
Repository:
rC Clang
https://reviews.llvm.org/D43798
Files:
lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp
test/Analysis/implicit-ctor-undef-value.cpp
Index: test/Analysis/implicit-ctor-undef-value.cpp
===================================================================
--- /dev/null
+++ test/Analysis/implicit-ctor-undef-value.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-output=text -verify %s
+
+namespace implicit_constructor {
+struct S {
+public:
+ S() {}
+ S(const S &) {}
+};
+
+// Warning is in a weird position because the body of the constructor is
+// missing. Specify which field is being assigned.
+class C { // expected-warning{{Value assigned to field 'y' is garbage or undefined}}
+ // expected-note@-1{{Value assigned to field 'y' is garbage or undefined}}
+ int x, y;
+ S s;
+
+public:
+ C(): x(0) {}
+};
+
+void test() {
+ C c1;
+ C c2(c1); // expected-note{{Calling implicit copy constructor for 'C'}}
+}
+} // end namespace implicit_constructor
+
+
+namespace explicit_constructor {
+class C {
+ int x, y;
+
+public:
+ C(): x(0) {}
+ // It is not necessary to specify which field is being assigned to.
+ C(const C &c):
+ x(c.x),
+ y(c.y) // expected-warning{{Assigned value is garbage or undefined}}
+ // expected-note@-1{{Assigned value is garbage or undefined}}
+ {}
+};
+
+void test() {
+ C c1;
+ C c2(c1); // expected-note{{Calling copy constructor for 'C'}}
+}
+} // end namespace explicit_constructor
Index: lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp
@@ -51,17 +51,20 @@
if (!N)
return;
- const char *str = "Assigned value is garbage or undefined";
-
+ static const char *const DefaultMsg =
+ "Assigned value is garbage or undefined";
if (!BT)
- BT.reset(new BuiltinBug(this, str));
+ BT.reset(new BuiltinBug(this, DefaultMsg));
// Generate a report for this bug.
+ std::string Str;
+ llvm::raw_string_ostream OS(Str);
+
const Expr *ex = nullptr;
while (StoreE) {
if (const UnaryOperator *U = dyn_cast<UnaryOperator>(StoreE)) {
- str = "The expression is an uninitialized value. "
+ OS << "The expression is an uninitialized value. "
"The computed value will also be garbage";
ex = U->getSubExpr();
@@ -71,7 +74,7 @@
if (const BinaryOperator *B = dyn_cast<BinaryOperator>(StoreE)) {
if (B->isCompoundAssignmentOp()) {
if (C.getSVal(B->getLHS()).isUndef()) {
- str = "The left expression of the compound assignment is an "
+ OS << "The left expression of the compound assignment is an "
"uninitialized value. The computed value will also be garbage";
ex = B->getLHS();
break;
@@ -87,10 +90,26 @@
ex = VD->getInit();
}
+ if (const auto *CD =
+ dyn_cast<CXXConstructorDecl>(C.getStackFrame()->getDecl())) {
+ if (CD->isImplicit()) {
+ for (auto I : CD->inits()) {
+ if (I->getInit()->IgnoreImpCasts() == StoreE) {
+ OS << "Value assigned to field '" << I->getMember()->getName()
+ << "' is garbage or undefined";
+ break;
+ }
+ }
+ }
+ }
+
break;
}
- auto R = llvm::make_unique<BugReport>(*BT, str, N);
+ if (OS.str().empty())
+ OS << DefaultMsg;
+
+ auto R = llvm::make_unique<BugReport>(*BT, OS.str(), N);
if (ex) {
R->addRange(ex->getSourceRange());
bugreporter::trackNullOrUndefValue(N, ex, *R);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
