[PATCH] D77178: [Analyzer][WebKit] NoUncountedMembersChecker

2020-06-02 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Looks similar to https://bugs.llvm.org/show_bug.cgi?id=46142. It sounds pretty 
important to address quickly because it crashes on a fairly large portion of 
C++ code with default settings.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77178/new/

https://reviews.llvm.org/D77178



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


[PATCH] D77178: [Analyzer][WebKit] NoUncountedMembersChecker

2020-06-01 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

Hi, @jkorous

Thanks for working on this analyzer check.

Just give you some feedback on this check, it seems to trigger the assertion 
quite often (we noticed that internally there were a lot of crashes).

you can easily reproduce it by running it a LLVM file, e.g. `./bin/clang-tidy 
-checks="-*,clang-analyzer-webkit*" 
../clang-tools-extra/clangd/ClangdLSPServer.cpp`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77178/new/

https://reviews.llvm.org/D77178



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


[PATCH] D77178: [Analyzer][WebKit] NoUncountedMembersChecker

2020-05-27 Thread Jan Korous via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG660cda572d6e: [Analyzer][WebKit] NoUncountedMembersChecker 
(authored by jkorous).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77178/new/

https://reviews.llvm.org/D77178

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/WebKit/DiagOutputUtils.h
  clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
  clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp

Index: clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/WebKit/uncounted-members.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.WebKitNoUncountedMemberChecker -verify %s
+
+#include "mock-types.h"
+
+namespace members {
+  struct Foo {
+  private:
+RefCountable* a = nullptr;
+// expected-warning@-1{{Member variable 'a' in 'members::Foo' is a raw pointer to ref-countable type 'RefCountable'}}
+
+  protected:
+RefPtr b;
+
+  public:
+RefCountable silenceWarningAboutInit;
+RefCountable& c = silenceWarningAboutInit;
+// expected-warning@-1{{Member variable 'c' in 'members::Foo' is a reference to ref-countable type 'RefCountable'}}
+Ref d;
+  };
+
+  template
+  struct FooTmpl {
+T* a;
+// expected-warning@-1{{Member variable 'a' in 'members::FooTmpl' is a raw pointer to ref-countable type 'RefCountable'}}
+  };
+
+  void forceTmplToInstantiate(FooTmpl) {}
+}
+
+namespace ignore_unions {
+  union Foo {
+RefCountable* a;
+RefPtr b;
+Ref c;
+  };
+
+  template
+  union RefPtr {
+T* a;
+  };
+
+  void forceTmplToInstantiate(RefPtr) {}
+}
Index: clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
@@ -0,0 +1,150 @@
+//===- NoUncountedMembersChecker.cpp -*- C++ -*-==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ASTUtils.h"
+#include "DiagOutputUtils.h"
+#include "PtrTypesSemantics.h"
+#include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/Support/Casting.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class NoUncountedMemberChecker
+: public Checker> {
+private:
+  BugType Bug;
+  mutable BugReporter *BR;
+
+public:
+  NoUncountedMemberChecker()
+  : Bug(this,
+"Member variable is a raw-poiner/reference to reference-countable "
+"type",
+"WebKit coding guidelines") {}
+
+  void checkASTDecl(const TranslationUnitDecl *TUD, AnalysisManager &MGR,
+BugReporter &BRArg) const {
+BR = &BRArg;
+
+// The calls to checkAST* from AnalysisConsumer don't
+// visit template instantiations or lambda classes. We
+// want to visit those, so we make our own RecursiveASTVisitor.
+struct LocalVisitor : public RecursiveASTVisitor {
+  const NoUncountedMemberChecker *Checker;
+  explicit LocalVisitor(const NoUncountedMemberChecker *Checker)
+  : Checker(Checker) {
+assert(Checker);
+  }
+
+  bool shouldVisitTemplateInstantiations() const { return true; }
+  bool shouldVisitImplicitCode() const { return false; }
+
+  bool VisitRecordDecl(const RecordDecl *RD) {
+Checker->visitRecordDecl(RD);
+return true;
+  }
+};
+
+LocalVisitor visitor(this);
+visitor.TraverseDecl(const_cast(TUD));
+  }
+
+  void visitRecordDecl(const RecordDecl *RD) const {
+if (shouldSkipDecl(RD))
+  return;
+
+for (auto Member : RD->fields()) {
+  const Type *MemberType = Member->getType().getTypePtrOrNull();
+  if (!MemberType)
+continue;
+
+  if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl()) {
+if (isRefCountable(MemberCXXRD))
+  reportBug(Member, MemberType, MemberCXXRD, RD);
+  }
+}
+  }
+
+  bool shouldSkipDecl(const RecordDecl *RD) const {
+if (!RD->isThisDeclarationADefinition())