[PATCH] D51866: [analyzer][UninitializedObjectChecker][WIP] New flag to ignore guarded uninitialized fields

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 168592.
Szelethus added a comment.

Reimplemented with AST matchers.
@george.karpenkov I know you wanted to test this feature for a while, but sadly 
I've been busy with the macro expansion related projects, I hope it's still 
relevant.


https://reviews.llvm.org/D51866

Files:
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
  test/Analysis/cxx-uninitialized-object-unguarded-access.cpp

Index: test/Analysis/cxx-uninitialized-object-unguarded-access.cpp
===
--- /dev/null
+++ test/Analysis/cxx-uninitialized-object-unguarded-access.cpp
@@ -0,0 +1,257 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \
+// RUN:   -analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true -DPEDANTIC \
+// RUN:   -analyzer-config alpha.cplusplus.UninitializedObject:IgnoreGuardedFields=true \
+// RUN:   -std=c++11 -verify  %s
+
+//===--===//
+// Helper functions for tests.
+//===--===//
+void halt() __attribute__((__noreturn__));
+void assert(int b) {
+  if (!b)
+halt();
+}
+
+int rand();
+
+//===--===//
+// Tests for fields properly guarded by asserts.
+//===--===//
+
+class NoUnguardedFieldsTest {
+public:
+  enum Kind {
+V,
+A
+  };
+
+private:
+  int Volume, Area;
+  Kind K;
+
+public:
+  NoUnguardedFieldsTest(Kind K) : K(K) {
+switch (K) {
+case V:
+  Volume = 0;
+  break;
+case A:
+  Area = 0;
+  break;
+}
+  }
+
+  void operator-() {
+assert(K == Kind::A);
+(void)Area;
+  }
+
+  void operator+() {
+assert(K == Kind::V);
+(void)Volume;
+  }
+};
+
+void fNoUnguardedFieldsTest() {
+  NoUnguardedFieldsTest T1(NoUnguardedFieldsTest::Kind::A);
+  NoUnguardedFieldsTest T2(NoUnguardedFieldsTest::Kind::V);
+}
+
+class NoUnguardedFieldsWithUndefMethodTest {
+public:
+  enum Kind {
+V,
+A
+  };
+
+private:
+  int Volume, Area;
+  Kind K;
+
+public:
+  NoUnguardedFieldsWithUndefMethodTest(Kind K) : K(K) {
+switch (K) {
+case V:
+  Volume = 0;
+  break;
+case A:
+  Area = 0;
+  break;
+}
+  }
+
+  void operator-() {
+assert(K == Kind::A);
+(void)Area;
+  }
+
+  void operator+() {
+assert(K == Kind::V);
+(void)Volume;
+  }
+
+  void methodWithoutDefinition();
+};
+
+void fNoUnguardedFieldsWithUndefMethodTest() {
+  NoUnguardedFieldsWithUndefMethodTest
+  T1(NoUnguardedFieldsWithUndefMethodTest::Kind::A);
+  NoUnguardedFieldsWithUndefMethodTest
+  T2(NoUnguardedFieldsWithUndefMethodTest::Kind::V);
+}
+
+class UnguardedFieldThroughMethodTest {
+public:
+  enum Kind {
+V,
+A
+  };
+
+private:
+  int Volume, Area; // expected-note {{uninitialized field 'this->Volume'}}
+  Kind K;
+
+public:
+  UnguardedFieldThroughMethodTest(Kind K) : K(K) {
+switch (K) {
+case V:
+  Volume = 0;
+  break;
+case A:
+  Area = 0; // expected-warning {{1 uninitialized field}}
+  break;
+}
+  }
+
+  void operator-() {
+assert(K == Kind::A);
+(void)Area;
+  }
+
+  void operator+() {
+(void)Volume;
+  }
+};
+
+void fUnguardedFieldThroughMethodTest() {
+  UnguardedFieldThroughMethodTest T1(UnguardedFieldThroughMethodTest::Kind::A);
+}
+
+class UnguardedPublicFieldsTest {
+public:
+  enum Kind {
+V,
+A
+  };
+
+  int Volume, Area; // expected-note {{uninitialized field 'this->Volume'}}
+  Kind K;
+
+public:
+  UnguardedPublicFieldsTest(Kind K) : K(K) {
+switch (K) {
+case V:
+  Volume = 0;
+  break;
+case A:
+  Area = 0; // expected-warning {{1 uninitialized field}}
+  break;
+}
+  }
+
+  void operator-() {
+assert(K == Kind::A);
+(void)Area;
+  }
+
+  void operator+() {
+assert(K == Kind::V);
+(void)Volume;
+  }
+};
+
+void fUnguardedPublicFieldsTest() {
+  UnguardedPublicFieldsTest T1(UnguardedPublicFieldsTest::Kind::A);
+}
+
+//===--===//
+// Highlights of some false negatives due to syntactic checking.
+//===--===//
+
+class UnguardedFalseNegativeTest1 {
+public:
+  enum Kind {
+V,
+A
+  };
+
+private:
+  int Volume, Area;
+  Kind K;
+
+public:
+  UnguardedFalseNegativeTest1(Kind K) : K(K) {
+switch (K) {
+case V:
+  Volume = 0;
+  break;
+case A:
+  Area = 0;
+  break;
+}
+  }
+
+  void operator-() {
+if (rand())
+  assert(K == Kind::A);
+(void)Area;
+  }
+
+  void operator+() {
+if (rand())
+  assert(K == Kind::V);
+

[PATCH] D51866: [analyzer][UninitializedObjectChecker][WIP] New flag to ignore guarded uninitialized fields

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

You can retrieve any node from the `ASTMatcher` by `.bind()`ing sub-matchers to 
string keys and later retrieving them from the `MatchResult` dictionary by 
those keys. It's like a regex capturing groups (and, well, `ASTMatchers` also 
support backreferences to such groups, i.e. `equalsBoundNode()`).


Repository:
  rC Clang

https://reviews.llvm.org/D51866



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51866: [analyzer][UninitializedObjectChecker][WIP] New flag to ignore guarded uninitialized fields

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

I played around with it, and this seemed a bit cleaner. Since I would only 
match against the definition of the record (and the definition of its methods), 
the matcher would need to look like this: `stmt(hasDescendant(blahBlah(...)))`, 
and I'd only be able to retrieve (with my limited knowledge) the entire method 
definition, not the actual `FieldDecl` in the case of a match.
I didn'd do a whole lot of AST based analysis, so feel free to correct me on 
this one ^-^


Repository:
  rC Clang

https://reviews.llvm.org/D51866



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51866: [analyzer][UninitializedObjectChecker][WIP] New flag to ignore guarded uninitialized fields

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

Thanks! The usual question: would it be easier to implement using AST matchers?


Repository:
  rC Clang

https://reviews.llvm.org/D51866



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51866: [analyzer][UninitializedObjectChecker][WIP] New flag to ignore guarded uninitialized fields

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

@george.karpenkov @xazax.hun, is this how you imagined checking whether an 
access is guarded?


Repository:
  rC Clang

https://reviews.llvm.org/D51866



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51866: [analyzer][UninitializedObjectChecker][WIP] New flag to ignore guarded uninitialized fields

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: george.karpenkov, NoQ, xazax.hun, rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet, 
whisperity.

This patch is an implementation of the ideas discussed on the mailing list 
. It didn't 
cause any crashes on the already existing test (I checked it by adding the flag 
to them and run lit), but then again, I wasn't able to check it on larger code 
bases just yet, and the LLVM codebase tends to have a couple tricks up it's 
sleeve, so be beware of that.


Repository:
  rC Clang

https://reviews.llvm.org/D51866

Files:
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
  test/Analysis/cxx-uninitialized-object-unguarded-access.cpp

Index: test/Analysis/cxx-uninitialized-object-unguarded-access.cpp
===
--- /dev/null
+++ test/Analysis/cxx-uninitialized-object-unguarded-access.cpp
@@ -0,0 +1,257 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \
+// RUN:   -analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true -DPEDANTIC \
+// RUN:   -analyzer-config alpha.cplusplus.UninitializedObject:IgnoreGuardedFields=true \
+// RUN:   -std=c++11 -verify  %s
+
+//===--===//
+// Helper functions for tests.
+//===--===//
+void halt() __attribute__((__noreturn__));
+void assert(int b) {
+  if (!b)
+halt();
+}
+
+int rand();
+
+//===--===//
+// Tests for fields properly guarded by asserts.
+//===--===//
+
+class NoUnguardedFieldsTest {
+public:
+  enum Kind {
+V,
+A
+  };
+
+private:
+  int Volume, Area;
+  Kind K;
+
+public:
+  NoUnguardedFieldsTest(Kind K) : K(K) {
+switch (K) {
+case V:
+  Volume = 0;
+  break;
+case A:
+  Area = 0;
+  break;
+}
+  }
+
+  void operator-() {
+assert(K == Kind::A);
+(void)Area;
+  }
+
+  void operator+() {
+assert(K == Kind::V);
+(void)Volume;
+  }
+};
+
+void fNoUnguardedFieldsTest() {
+  NoUnguardedFieldsTest T1(NoUnguardedFieldsTest::Kind::A);
+  NoUnguardedFieldsTest T2(NoUnguardedFieldsTest::Kind::V);
+}
+
+class NoUnguardedFieldsWithUndefMethodTest {
+public:
+  enum Kind {
+V,
+A
+  };
+
+private:
+  int Volume, Area;
+  Kind K;
+
+public:
+  NoUnguardedFieldsWithUndefMethodTest(Kind K) : K(K) {
+switch (K) {
+case V:
+  Volume = 0;
+  break;
+case A:
+  Area = 0;
+  break;
+}
+  }
+
+  void operator-() {
+assert(K == Kind::A);
+(void)Area;
+  }
+
+  void operator+() {
+assert(K == Kind::V);
+(void)Volume;
+  }
+
+  void methodWithoutDefinition();
+};
+
+void fNoUnguardedFieldsWithUndefMethodTest() {
+  NoUnguardedFieldsWithUndefMethodTest
+  T1(NoUnguardedFieldsWithUndefMethodTest::Kind::A);
+  NoUnguardedFieldsWithUndefMethodTest
+  T2(NoUnguardedFieldsWithUndefMethodTest::Kind::V);
+}
+
+class UnguardedFieldThroughMethodTest {
+public:
+  enum Kind {
+V,
+A
+  };
+
+public:
+  int Volume, Area; // expected-note {{uninitialized field 'this->Volume'}}
+  Kind K;
+
+public:
+  UnguardedFieldThroughMethodTest(Kind K) : K(K) {
+switch (K) {
+case V:
+  Volume = 0;
+  break;
+case A:
+  Area = 0; // expected-warning {{1 uninitialized field}}
+  break;
+}
+  }
+
+  void operator-() {
+assert(K == Kind::A);
+(void)Area;
+  }
+
+  void operator+() {
+(void)Volume;
+  }
+};
+
+void fUnguardedFieldThroughMethodTest() {
+  UnguardedFieldThroughMethodTest T1(UnguardedFieldThroughMethodTest::Kind::A);
+}
+
+class UnguardedPublicFieldsTest {
+public:
+  enum Kind {
+V,
+A
+  };
+
+  int Volume, Area; // expected-note {{uninitialized field 'this->Volume'}}
+  Kind K;
+
+public:
+  UnguardedPublicFieldsTest(Kind K) : K(K) {
+switch (K) {
+case V:
+  Volume = 0;
+  break;
+case A:
+  Area = 0; // expected-warning {{1 uninitialized field}}
+  break;
+}
+  }
+
+  void operator-() {
+assert(K == Kind::A);
+(void)Area;
+  }
+
+  void operator+() {
+assert(K == Kind::V);
+(void)Volume;
+  }
+};
+
+void fUnguardedPublicFieldsTest() {
+  UnguardedPublicFieldsTest T1(UnguardedPublicFieldsTest::Kind::A);
+}
+
+//===--===//
+// Highlights of some false negatives due to syntactic checking.
+//===--===//
+
+class UnguardedFalseNegativeTest1 {
+public:
+  enum Kind {
+V,
+A
+  };
+
+private:
+  int Volume,