r326258 - [analyzer] UndefinedAssignmentChecker: Better warning message in implicit ctors.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Tue Feb 27 14:05:55 2018
New Revision: 326258

URL: http://llvm.org/viewvc/llvm-project?rev=326258&view=rev
Log:
[analyzer] UndefinedAssignmentChecker: Better warning message in implicit ctors.

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.

Differential Revision: https://reviews.llvm.org/D43798

Added:
    cfe/trunk/test/Analysis/implicit-ctor-undef-value.cpp
Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp?rev=326258&r1=326257&r2=326258&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp Tue Feb 27 14:05:55 2018
@@ -51,17 +51,20 @@ void UndefinedAssignmentChecker::checkBi
   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.
+  llvm::SmallString<128> Str;
+  llvm::raw_svector_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 @@ void UndefinedAssignmentChecker::checkBi
     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 @@ void UndefinedAssignmentChecker::checkBi
       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()
+               << "' in implicit constructor 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);

Added: cfe/trunk/test/Analysis/implicit-ctor-undef-value.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/implicit-ctor-undef-value.cpp?rev=326258&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/implicit-ctor-undef-value.cpp (added)
+++ cfe/trunk/test/Analysis/implicit-ctor-undef-value.cpp Tue Feb 27 14:05:55 2018
@@ -0,0 +1,75 @@
+// 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' in implicit constructor is garbage or undefined}}
+          // expected-note at -1{{Value assigned to field 'y' in implicit constructor 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 at -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
+
+
+namespace base_class_constructor {
+struct S {
+public:
+  S() {}
+  S(const S &) {}
+};
+
+class C { // expected-warning{{Value assigned to field 'y' in implicit constructor is garbage or undefined}}
+          // expected-note at -1{{Value assigned to field 'y' in implicit constructor is garbage or undefined}}
+  int x, y;
+  S s;
+
+public:
+  C(): x(0) {}
+};
+
+class D: public C {
+public:
+  D(): C() {}
+};
+
+void test() {
+  D d1;
+  D d2(d1); // expected-note   {{Calling implicit copy constructor for 'D'}}
+            // expected-note at -1{{Calling implicit copy constructor for 'C'}}
+}
+} // end namespace base_class_constructor