Bug 1613440 - Add new clang plugin to deprecate NS_NewNamedThread r=andi

Creates a NoNewThreadsChecker plugin that looks for the function and checks to see if it's allowed.
-Creates two allowlists - `ThreadAllows.txt` is for thread names, while `ThreadFileAllows.txt` checks for entire files where instances of `NS_NewNamedThread` should be ignored.
-If an instance of `NS_NewNamedThread` is not listed in either list, then the checker throws an error with an additional note containing a more descriptive explanation of the failure.

Differential Revision: https://phabricator.services.mozilla.com/D62635

--HG--
extra : moz-landing-system : lando

build/clang-plugin/Checks.inc

@@ -24,6 +24,7 @@ CHECK(NoAddRefReleaseOnReturnChecker, "no-addref-release-on-return")
 CHECK(NoAutoTypeChecker, "no-auto-type")
 CHECK(NoDuplicateRefCntMemberChecker, "no-duplicate-refcnt-member")
 CHECK(NoExplicitMoveConstructorChecker, "no-explicit-move-constructor")
+CHECK(NoNewThreadsChecker, "no-new-threads")
 CHECK(NonMemMovableMemberChecker, "non-memmovable-member")
 CHECK(NonMemMovableTemplateArgChecker, "non-memmovable-template-arg")
 CHECK(NoUsingNamespaceMozillaJavaChecker, "no-using-namespace-mozilla-java")

build/clang-plugin/ChecksIncludes.inc

@@ -25,6 +25,7 @@
 #include "NoAutoTypeChecker.h"
 #include "NoDuplicateRefCntMemberChecker.h"
 #include "NoExplicitMoveConstructorChecker.h"
+#include "NoNewThreadsChecker.h"
 #include "NonMemMovableMemberChecker.h"
 #include "NonMemMovableTemplateArgChecker.h"
 #include "NonParamInsideFunctionDeclChecker.h"

build/clang-plugin/CustomMatchers.h

@@ -159,6 +159,35 @@ AST_MATCHER(BinaryOperator, isInWhitelistForNaNExpr) {
   return false;
 }
 
+/// This matcher will match a list of files which contain NS_NewNamedThread
+/// code or names of existing threads that we would like to ignore.
+AST_MATCHER(CallExpr, isInAllowlistForThreads) {
+
+  // Get the source location of the call
+  SourceLocation Loc = Node.getRParenLoc();
+  StringRef FileName =
+      getFilename(Finder->getASTContext().getSourceManager(), Loc);
+  for (auto thread_file : allow_thread_files) {
+    if (llvm::sys::path::rbegin(FileName)->equals(thread_file)) {
+      return true;
+    }
+  }
+
+  // Now we get the first arg (the name of the thread) and we check it.
+  const StringLiteral *nameArg =
+      dyn_cast<StringLiteral>(Node.getArg(0)->IgnoreImplicit());
+  if (nameArg) {
+    const StringRef name = nameArg->getString();
+    for (auto thread_name : allow_thread_names) {
+      if (name.equals(thread_name)) {
+        return true;
+      }
+    }
+  }
+
+  return false;
+}
+
 /// This matcher will match all accesses to AddRef or Release methods.
 AST_MATCHER(MemberExpr, isAddRefOrRelease) {
   ValueDecl *Member = Node.getMemberDecl();

build/clang-plugin/NoNewThreadsChecker.cpp

@@ -0,0 +1,36 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "NoNewThreadsChecker.h"
+#include "CustomMatchers.h"
+
+void NoNewThreadsChecker::registerMatchers(MatchFinder *AstMatcher) {
+  // The checker looks for:
+  // -Instances of NS_NewNamedThread that aren't in allowed files
+  // -Instances of NS_NewNamedThread that use names that aren't recognized
+  AstMatcher->addMatcher(
+      callExpr(allOf(isFirstParty(),
+                     callee(functionDecl(hasName("NS_NewNamedThread"))),
+                     unless(isInAllowlistForThreads())))
+          .bind("funcCall"),
+      this);
+}
+
+void NoNewThreadsChecker::check(const MatchFinder::MatchResult &Result) {
+  const CallExpr *FuncCall = Result.Nodes.getNodeAs<CallExpr>("funcCall");
+
+  if (FuncCall) {
+    diag(FuncCall->getBeginLoc(),
+         "Thread name not recognized. Please use the background thread pool.",
+         DiagnosticIDs::Error)
+        << FuncCall->getDirectCallee()->getName();
+    diag(
+        FuncCall->getBeginLoc(),
+        "NS_NewNamedThread has been deprecated in favor of background "
+        "task dispatch via NS_DispatchBackgroundTask and "
+        "NS_CreateBackgroundTaskQueue. If you must create a new ad-hoc thread, "
+        "have your thread name added to ThreadAllows.txt.",
+        DiagnosticIDs::Note);
+  }
+}

build/clang-plugin/NoNewThreadsChecker.h

@@ -0,0 +1,18 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef NoNewThreadsChecker_h__
+#define NoNewThreadsChecker_h__
+
+#include "plugin.h"
+
+class NoNewThreadsChecker : public BaseCheck {
+public:
+  NoNewThreadsChecker(StringRef CheckName, ContextType *Context = nullptr)
+      : BaseCheck(CheckName, Context) {}
+  void registerMatchers(MatchFinder *AstMatcher) override;
+  void check(const MatchFinder::MatchResult &Result) override;
+};
+
+#endif // !defined(NoNewThreadsChecker_h__)

build/clang-plugin/ThreadAllows.py

@@ -0,0 +1,55 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this file,
+# You can obtain one at http://mozilla.org/MPL/2.0/.
+import json
+
+FIRST_LINE = '// This file was generated by generate_thread_allows.py. DO NOT EDIT.'
+
+
+def generate_allows(input_paths):
+    """
+        This script reads in the ThreadAllows.txt and ThreadFileAllows.txt lists
+        and generates a header file containing a two arrays of allowed threads.
+        These can be the following formats:
+            -Files which the checker should ignore
+                These files either contain definitions of NS_NewNamedThread or
+                use args which the plugin can't cast (such as func args).
+            -Thread names which the checker should ignore
+                Specifies which individual thread names to ignore.
+    """
+    file_list = []
+    name_list = []
+    lines = set()
+
+    for path in input_paths:
+        with open(path) as file:
+            lines.update(file.readlines())
+
+    for line in lines:
+        """
+            We are assuming lines ending in .cpp, .h are files. Threads should
+            NOT have names containing filenames. Please don't do that.
+        """
+        line = line.strip()
+        if line.endswith('.cpp') or line.endswith('.h'):
+            file_list.append(line)
+        else:
+            name_list.append(line)
+    file_list_s = ',\n  '.join(json.dumps(elem) for elem in file_list)
+    name_list_s = ',\n  '.join(json.dumps(elem) for elem in name_list)
+    output_string = FIRST_LINE + """
+
+static const char *allow_thread_files[] = {
+  %s
+};
+
+static const char *allow_thread_names[] = {
+  %s
+};
+
+    """ % (file_list_s, name_list_s)
+    return output_string
+
+
+def generate_file(output, *input_paths):
+    output.write(generate_allows(input_paths))

build/clang-plugin/ThreadAllows.txt

@@ -0,0 +1,93 @@
+ApplyUpdates
+AsyncShutdownPr
+AsyncShutdownWt
+Atom Test
+AutoRefCnt Test
+AutoTestThread
+AwaitIdleMixed
+AwaitIdlePaused
+BGReadURLs
+BHMgr Processor
+BgFileSaver
+COM Intcpt Log
+COM MTA
+Cache Deleter
+Cache I/O
+Cameras IPC
+ChainedPipePump
+ChainedPipeRecv
+Checker Test
+Cookie
+CrashRep Inject
+DDMediaLogs
+DOM File
+DOMCacheThread
+DataChannel IO
+DataStorage
+DatabaseLocker
+DecodeToSurface
+Decoder Test
+FileWatcher IO
+Font Loader
+FontEnumThread
+Function Broker
+GMPThread
+Gamepad
+GeckoProfGTest
+GraphRunner
+HTML5 Parser
+IPC Launch
+IPDL Background
+IdentityCrypto
+LS Thread
+LayerScope
+MDCDMThread
+MWQThread
+MediaCache
+MediaTelemetry
+MediaTrackGrph
+mtransport
+NamedPipeSrv
+NetPredictClean
+Netlink Monitor
+OSKeyStore
+OutputDrain
+PaintThread
+PlayEventSound
+ProcessHangMon
+ProfSymbolTable
+ProfilerChild
+ProxyResolution
+RWLockTester
+RacingServMan
+RemVidChild
+RemVidParent
+Sandbox Testing
+SaveScripts
+Socket Thread
+SpeechWorker
+SpinEventLoop
+StressRunner
+SuicideManager
+SuicideThread
+TEQ AwaitIdle
+TelemetryGVIO
+Test Thread
+Test thread
+TestPipe
+TestShortWrites
+TestThreadsMain
+Testing Thread
+Timer Thread
+ToastBgThread
+URL Classifier
+Update Watcher
+VsyncIOThread
+Wifi Monitor
+Worker Launcher
+speechd init
+t1
+t2
+thread
+thread shutdown
+wifi tickler

build/clang-plugin/ThreadFileAllows.txt

@@ -0,0 +1,11 @@
+ActorsParent.cpp
+DecodePool.cpp
+GeckoChildProcessHost.cpp
+LazyIdleThread.cpp
+LazyIdleThread.h
+VRThread.cpp
+mozStorageConnection.cpp
+nr_socket_prsock.cpp
+nsThreadPool.cpp
+nsThreadUtils.cpp
+nsThreadUtils.h

build/clang-plugin/Utils.h

@@ -7,6 +7,7 @@
 
 #include "CustomAttributes.h"
 #include "ThirdPartyPaths.h"
+#include "ThreadAllows.h"
 #include "plugin.h"
 
 inline StringRef getFilename(const SourceManager &SM, SourceLocation Loc) {

build/clang-plugin/import_mozilla_checks.py

@@ -10,6 +10,7 @@ import shutil
 import errno
 
 import ThirdPartyPaths
+import ThreadAllows
 
 
 def copy_dir_contents(src, dest):
@@ -101,6 +102,17 @@ def write_third_party_paths(mozilla_path, module_path):
         ThirdPartyPaths.generate(f, tpp_txt, generated_txt)
 
 
+def generate_thread_allows(mozilla_path, module_path):
+    names = os.path.join(
+        mozilla_path, '../../build/clang-plugin/ThreadAllows.txt'
+    )
+    files = os.path.join(
+        mozilla_path, '../../build/clang-plugin/ThreadFileAllows.txt'
+    )
+    with open(os.path.join(module_path, 'ThreadAllows.h'), 'w') as f:
+        f.write(ThreadAllows.generate_allows({files, names}))
+
+
 def do_import(mozilla_path, clang_tidy_path):
     module = 'mozilla'
     module_path = os.path.join(clang_tidy_path, module)
@@ -112,6 +124,7 @@ def do_import(mozilla_path, clang_tidy_path):
 
     copy_dir_contents(mozilla_path, module_path)
     write_third_party_paths(mozilla_path, module_path)
+    generate_thread_allows(mozilla_path, module_path)
     write_cmake(module_path)
     add_item_to_cmake_section(os.path.join(module_path, '..', 'plugin',
                                            'CMakeLists.txt'),

build/clang-plugin/moz.build

@@ -29,6 +29,7 @@ HOST_SOURCES += [
     'NoAutoTypeChecker.cpp',
     'NoDuplicateRefCntMemberChecker.cpp',
     'NoExplicitMoveConstructorChecker.cpp',
+    'NoNewThreadsChecker.cpp',
     'NonMemMovableMemberChecker.cpp',
     'NonMemMovableTemplateArgChecker.cpp',
     'NonParamInsideFunctionDeclChecker.cpp',
@@ -69,6 +70,12 @@ GeneratedFile('ThirdPartyPaths.cpp', script="ThirdPartyPaths.py",
                   '/tools/rewriting/Generated.txt',
               ])
 
+GeneratedFile('ThreadAllows.h', script="ThreadAllows.py",
+              entry_point="generate_file", inputs=[
+              '/build/clang-plugin/ThreadAllows.txt',
+              '/build/clang-plugin/ThreadFileAllows.txt'
+              ])
+
 HOST_COMPILE_FLAGS['STL'] = []
 HOST_COMPILE_FLAGS['VISIBILITY'] = []
 

build/clang-plugin/tests/TestNoNewThreadsChecker.cpp

@@ -0,0 +1,9 @@
+// Dummy NS_NewNamedThread.
+void NS_NewNamedThread(const char *aName) {}
+
+void func_threads() {
+  // Test to see if the checker recognizes a bad name, and if it recognizes a
+  // name from the ThreadAllows.txt.
+  NS_NewNamedThread("A bad name"); // expected-error {{Thread name not recognized. Please use the background thread pool.}} expected-note {{NS_NewNamedThread has been deprecated in favor of background task dispatch via NS_DispatchBackgroundTask and NS_CreateBackgroundTaskQueue. If you must create a new ad-hoc thread, have your thread name added to ThreadAllows.txt.}}
+  NS_NewNamedThread("Checker Test");
+}

build/clang-plugin/tests/moz.build

@@ -30,6 +30,7 @@ SOURCES += [
     'TestNoAutoType.cpp',
     'TestNoDuplicateRefCntMember.cpp',
     'TestNoExplicitMoveConstructor.cpp',
+    'TestNoNewThreadsChecker.cpp',
     'TestNonHeapClass.cpp',
     'TestNonMemMovable.cpp',
     'TestNonMemMovableStd.cpp',