diff --git a/build/clang-plugin/CustomMatchers.h b/build/clang-plugin/CustomMatchers.h index ece717e79918..5a74c155707b 100644 --- a/build/clang-plugin/CustomMatchers.h +++ b/build/clang-plugin/CustomMatchers.h @@ -194,34 +194,17 @@ AST_MATCHER(CallExpr, isInWhiteListForPrincipalGetUri) { /// code or names of existing threads that we would like to ignore. AST_MATCHER(CallExpr, isInAllowlistForThreads) { - // Get the source location of the call. + // Get the source location of the call SourceLocation Loc = Node.getRParenLoc(); StringRef FileName = getFilename(Finder->getASTContext().getSourceManager(), Loc); - - const auto rbegin = [](StringRef s) { return llvm::sys::path::rbegin(s); }; - const auto rend = [](StringRef s) { return llvm::sys::path::rend(s); }; - - // Files in the allowlist are (definitionally) explicitly permitted to create - // new threads. for (auto thread_file : allow_thread_files) { - // All the provided path-elements must match. - const bool match = [&] { - auto it1 = rbegin(FileName), it2 = rbegin(thread_file), - end1 = rend(FileName), end2 = rend(thread_file); - for (; it2 != end2; ++it1, ++it2) { - if (it1 == end1 || !it1->equals(*it2)) { - return false; - } - } - return true; - }(); - if (match) { + if (llvm::sys::path::rbegin(FileName)->equals(thread_file)) { return true; } } - // Check the first arg (the name of the thread). + // Now we get the first arg (the name of the thread) and we check it. const StringLiteral *nameArg = dyn_cast(Node.getArg(0)->IgnoreImplicit()); if (nameArg) { diff --git a/build/clang-plugin/ThreadAllows.py b/build/clang-plugin/ThreadAllows.py index 8275d427067e..e45f62925466 100644 --- a/build/clang-plugin/ThreadAllows.py +++ b/build/clang-plugin/ThreadAllows.py @@ -2,78 +2,58 @@ # 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 -import os -import posixpath -from os import PathLike -from typing import Iterable, Literal, Set - -FIRST_LINE = "// This file was generated by {}. DO NOT EDIT.".format( - # `posixpath` for forward slashes, for presentation purposes - posixpath.relpath(__file__, os.getenv("TOPSRCDIR", "/")) -) +FIRST_LINE = "// This file was generated by generate_thread_allows.py. DO NOT EDIT." -def generate_allowed_items( - which: Literal["files", "names"], paths: Iterable[PathLike] -) -> str: - def remove_trailing_comment(s: str) -> str: - return s[0 : s.find("#")] +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() - def read_items_from_path(path: PathLike) -> Set[str]: - out = set() + for path in input_paths: with open(path) as file: - for line in file.readlines(): - line = remove_trailing_comment(line).strip() - if not line: - continue # comment or empty line; discard - out.add(line) - return out + lines.update(file.readlines()) - allowed = set().union(*(read_items_from_path(path) for path in paths)) - # BUG: `json.dumps` may not correctly handle use of the quote character in - # thread names - allowed_list_s = ",\n ".join(json.dumps(elem) for elem in sorted(allowed)) - - return f"""\ -static const char *allow_thread_{which}[] = {{ - {allowed_list_s} -}};""" - - -def generate_allows( - *, allowed_names: Iterable[PathLike], allowed_files: Iterable[PathLike] -) -> str: - """ - This function reads in the specified sets of files -- ordinarily, - ["ThreadAllows.txt"] and ["ThreadFileAllows.txt"] -- and generates the text - of a header file containing two arrays with their contents, for inclusion by - the thread-name checker. - - The checker will reject the creation of any thread via NS_NewNamedThread - unless either: - - the thread's name is a literal string which is found in the set of - allowed thread names; or - - the thread's creation occurs within a file which is found in the set of - unchecked files. - - The latter condition exists mostly for the definition of NS_NewNamedThread, - but there also exist a few cases where the thread name is dynamically - computed (and so can't be checked). - """ + for line in sorted(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 - + "\n\n" - + generate_allowed_items("files", allowed_files) - + "\n\n" - + generate_allowed_items("names", allowed_names) - + "\n" + + """ + +static const char *allow_thread_files[] = { + %s +}; + +static const char *allow_thread_names[] = { + %s +}; + + """ + % (file_list_s, name_list_s) ) return output_string -# Entry point used by build/clang-plugin/moz.build (q.v.). -def generate_file(output, allowed_names, allowed_files): - output.write( - generate_allows(allowed_names=[allowed_names], allowed_files=[allowed_files]) - ) +def generate_file(output, *input_paths): + output.write(generate_allows(input_paths)) diff --git a/build/clang-plugin/ThreadAllows.txt b/build/clang-plugin/ThreadAllows.txt index 777919e98fc2..e05d669842cb 100644 --- a/build/clang-plugin/ThreadAllows.txt +++ b/build/clang-plugin/ThreadAllows.txt @@ -1,41 +1,11 @@ -# This file is ingested by `ThreadAllows.py` to produce a list of thread names -# which our clang plugin will allow to be used with `NS_NewNamedThread`. -# -# Permitted thread names are a maximum of 15 characters in length, and must be -# string literals at their point-of-use in the code -- i.e., in the invocation -# of `NS_NewNamedThread`. -# -# Within this file, each permitted thread name is on a separate line. Comments -# begin with `#`, as seen here. Leading and trailing whitespace, trailing -# comments, and blank lines are ignored, and may be used freely. -# -# Please explain the addition of any new thread names in comments, preferably -# with a pointer to a relevant bug. (Do not add thread names only used in tests -# to this file; instead, add the test file to `ThreadFileAllows.txt`.) - -###### -# Gecko/Firefox thread names -# -# (None documented yet -- but see "Unsorted thread names", below.) - -###### -# Thunderbird-only thread names -IMAP - -###### -# Other -Checker Test # used only as part of tests for the thread-checker itself - -###### -# Unsorted thread names -# -# Thread names below this point are grandfathered in. Please do not add new -# thread names to this list -- and please remove any that you can, whether by -# documenting and moving them or by confirming that they are no longer required. -# -# In particular, if a thread name is only used for testing, please consider -# moving its declarator to `ThreadFileAllows.txt`. - +ApplyUpdates +AsyncShutdownPr +AsyncShutdownWt +Atom Test +AutoRefCnt Test +AutoTestThread +AwaitIdleMixed +AwaitIdlePaused BGReadURLs BHMgr Monitor BHMgr Processor @@ -44,6 +14,9 @@ COM MTA Cache I/O Cameras IPC CanvasRenderer +ChainedPipePump +ChainedPipeRecv +Checker Test Compositor Cookie CrashRep Inject @@ -51,21 +24,29 @@ DDMediaLogs DOMCacheThread DataChannel IO DataStorage +DatabaseLocker +DecodeToSurface +Decoder Test FileWatcher IO Font Loader FontEnumThread Function Broker GMPThread Gamepad +GeckoProfGTest GraphRunner HTML5 Parser ICS parser +IMAP IPC Launch +IPCFuzzLoop IPDL Background +IPDL UnitTest IdentityCrypto ImageBridgeChld LS Thread MDCDMThread +MWQThread MediaCache MediaTelemetry MediaTrackGrph @@ -83,16 +64,28 @@ ProfSymbolTable ProfilerChild ProxyResolution RemoteLzyStream +RWLockTester +RacingServMan RemVidChild Renderer ResetCleanup +Sandbox Testing SaveScripts Socket Thread SpeechWorker +SpinEventLoop StressRunner SuicideManager +SuicideThread System Proxy +TEQ AwaitIdle TelemetryModule +Test Thread +Test thread +TestPipe +TestShortWrites +TestThreadsMain +Testing Thread Timer ToastBgThread TRR Background @@ -103,6 +96,8 @@ VsyncIOThread Wifi Monitor Worker Launcher speechd init +t1 +t2 thread thread shutdown wifi tickler diff --git a/build/clang-plugin/ThreadFileAllows.txt b/build/clang-plugin/ThreadFileAllows.txt index d211bea2e1e2..7fe559ce70b7 100644 --- a/build/clang-plugin/ThreadFileAllows.txt +++ b/build/clang-plugin/ThreadFileAllows.txt @@ -1,40 +1,4 @@ -# This file is ingested by `ThreadAllows.py` to produce a list of files which -# our clang plugin will allow to use `NS_NewNamedThread`. -# -# Files may be specified with any number of slash-separated path-elements; all -# provided path-elements must match. (Because we often move and/or symlink -# header files, this means headers will usually have no path-elements.) -# -# Note that this file contains a list of _files_, not _paths_. The clang plugin -# has no notion of $TOPSRCDIR. - -###### -# Release files - -# declaration and definition of `NS_NewNamedThread` -nsThreadUtils.h -xpcom/threads/nsThreadUtils.cpp - -# Thread-pools are permitted to make dynamically many threads, using dynamic -# thread names with explicit numbering. -xpcom/threads/nsThreadPool.cpp - -###### -# Test files - -# Tests for XPCOM threads themselves. -xpcom/tests/gtest/TestThreadManager.cpp -xpcom/tests/gtest/TestThreads.cpp -xpcom/tests/gtest/TestThreadUtils.cpp - -###### -# Unsorted release files -# -# Files below this point are grandfathered in. Please do not add new files to -# this list -- and please remove any that you can, whether by documenting and -# moving them or by confirming that they are no longer required. -dom/indexedDB/ActorsParent.cpp -dom/quota/ActorsParent.cpp +ActorsParent.cpp DecodePool.cpp GeckoChildProcessHost.cpp LazyIdleThread.cpp @@ -42,31 +6,6 @@ LazyIdleThread.h VRThread.cpp mozStorageConnection.cpp nr_socket_prsock.cpp - -###### -# Unsorted test files -# -# Files below this point are quasi-grandfathered in: these are test files which -# create new threads whose names were formerly in ThreadAllows.txt (without -# justification), and have been moved here (without justification). -dom/media/doctor/test/gtest/TestMultiWriterQueue.cpp -image/test/fuzzing/TestDecoders.cpp -image/test/gtest/TestDecodeToSurface.cpp -ipc/ipdl/test/gtest/IPDLUnitTest.cpp -security/sandbox/common/test/SandboxTestingThread.h -storage/test/gtest/test_interruptSynchronousConnection.cpp -storage/test/gtest/test_unlock_notify.cpp -toolkit/components/telemetry/geckoview/gtest/TestGeckoViewStreaming.cpp -toolkit/components/telemetry/tests/gtest/TestScalars.cpp -toolkit/components/url-classifier/tests/gtest/Common.cpp -tools/fuzzing/ipc/IPCFuzzController.cpp -tools/profiler/tests/gtest/GeckoProfiler.cpp -xpcom/tests/gtest/TestAtoms.cpp -xpcom/tests/gtest/TestAutoRefCnt.cpp -xpcom/tests/gtest/TestDelayedRunnable.cpp -xpcom/tests/gtest/TestLogging.cpp -xpcom/tests/gtest/TestPipes.cpp -xpcom/tests/gtest/TestRacingServiceManager.cpp -xpcom/tests/gtest/TestRWLock.cpp -xpcom/tests/gtest/TestThrottledEventQueue.cpp -xpcom/tests/gtest/TestTimers.cpp +nsThreadPool.cpp +nsThreadUtils.cpp +nsThreadUtils.h diff --git a/build/clang-plugin/import_mozilla_checks.py b/build/clang-plugin/import_mozilla_checks.py index f18abfae647f..70585c88594b 100755 --- a/build/clang-plugin/import_mozilla_checks.py +++ b/build/clang-plugin/import_mozilla_checks.py @@ -103,9 +103,7 @@ 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(allowed_names=[names], allowed_files=[files]) - ) + f.write(ThreadAllows.generate_allows({files, names})) def do_import(mozilla_path, clang_tidy_path, import_options):