diff --git a/.github/workflows/lagom-template.yml b/.github/workflows/lagom-template.yml index 0f737d77f76..d14fff55849 100644 --- a/.github/workflows/lagom-template.yml +++ b/.github/workflows/lagom-template.yml @@ -227,12 +227,3 @@ jobs: echo "$log_output" echo "Sanitizer errors happened while running tests; see the Test step above." fi - - - name: Lints - if: ${{ inputs.os_name == 'Linux' && inputs.build_preset == 'Sanitizer' }} - working-directory: ${{ github.workspace }} - run: | - git ls-files '*.ipc' | xargs ./Build/bin/IPCMagicLinter - env: - ASAN_OPTIONS: 'strict_string_checks=1:check_initialization_order=1:strict_init_order=1:detect_stack_use_after_return=1:allocator_may_return_null=1' - UBSAN_OPTIONS: 'print_stacktrace=1:print_summary=1:halt_on_error=1' diff --git a/Meta/Lagom/Tools/CMakeLists.txt b/Meta/Lagom/Tools/CMakeLists.txt index 3cefd9a5952..db36c13b4f0 100644 --- a/Meta/Lagom/Tools/CMakeLists.txt +++ b/Meta/Lagom/Tools/CMakeLists.txt @@ -17,4 +17,3 @@ function(lagom_tool tool) endfunction() add_subdirectory(CodeGenerators) -add_subdirectory(IPCMagicLinter) diff --git a/Meta/Lagom/Tools/IPCMagicLinter/CMakeLists.txt b/Meta/Lagom/Tools/IPCMagicLinter/CMakeLists.txt deleted file mode 100644 index 4a221c77e51..00000000000 --- a/Meta/Lagom/Tools/IPCMagicLinter/CMakeLists.txt +++ /dev/null @@ -1,5 +0,0 @@ -set(SOURCES - main.cpp -) - -lagom_tool(IPCMagicLinter LIBS LibMain) diff --git a/Meta/Lagom/Tools/IPCMagicLinter/main.cpp b/Meta/Lagom/Tools/IPCMagicLinter/main.cpp deleted file mode 100644 index 233f3b7b764..00000000000 --- a/Meta/Lagom/Tools/IPCMagicLinter/main.cpp +++ /dev/null @@ -1,111 +0,0 @@ -/* - * Copyright (c) 2021, Ben Wiederhake - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#include -#include -#include -#include -#include -#include - -// Exit code is bitwise-or of these values: -static constexpr auto EXIT_COLLISION = 0x1; -static constexpr auto EXIT_ERROR = 0x2; - -ErrorOr ladybird_main(Main::Arguments arguments) -{ - if (arguments.argc < 3) { - warnln("Usage: {} path/to/some.ipc path/to/other.ipc [more ipc files ...]", arguments.strings[0]); - return EXIT_ERROR; - } - - // Read files, compute their hashes, ignore collisions for now. - HashMap, IdentityHashTraits> inverse_hashes; - bool had_errors = false; - for (auto filename : arguments.strings.slice(1)) { - - auto const open_file = [](StringView filename) -> ErrorOr> { - auto file = TRY(Core::File::open(filename, Core::File::OpenMode::Read)); - return Core::InputBufferedFile::create(move(file)); - }; - - auto file_or_error = open_file(filename); - - if (file_or_error.is_error()) { - warnln("Error: Cannot open '{}': {}", filename, file_or_error.error()); - had_errors = true; - continue; // next file - } - - auto file = file_or_error.release_value(); - - ByteString endpoint_name; - - auto const read_lines = [&]() -> ErrorOr { - while (TRY(file->can_read_line())) { - Array buffer; - auto line = TRY(file->read_line(buffer)); - - if (!line.starts_with("endpoint "sv)) - continue; - auto line_endpoint_name = line.substring_view("endpoint "sv.length()); - if (!endpoint_name.is_empty()) { - // Note: If there are three or more endpoints defined in a file, these errors will look a bit wonky. - // However, that's fine, because it shouldn't happen in the first place. - warnln("Error: Multiple endpoints in file '{}': Found {} and {}", filename, endpoint_name, line_endpoint_name); - had_errors = true; - continue; // next line - } - endpoint_name = line_endpoint_name; - } - - return {}; - }; - - auto maybe_error = read_lines(); - - if (maybe_error.is_error()) { - warnln("Error: Failed to read '{}': {}", filename, maybe_error.release_error()); - had_errors = true; - continue; // next file - } - if (endpoint_name.is_empty()) { - // If this happens, this file probably needs to parse the endpoint name more carefully. - warnln("Error: Could not detect endpoint name in file '{}'", filename); - had_errors = true; - continue; // next file - } - u32 hash = endpoint_name.hash(); - auto& files_with_hash = inverse_hashes.ensure(hash); - files_with_hash.append(filename); - } - - // Report any collisions - bool had_collisions = false; - for (auto const& specific_collisions : inverse_hashes) { - if (specific_collisions.value.size() <= 1) - continue; - outln("Collision: Multiple endpoints use the magic number {}:", specific_collisions.key); - for (auto const& colliding_file : specific_collisions.value) { - outln("- {}", colliding_file); - } - had_collisions = true; - } - - outln("Checked {} files, saw {} distinct magic numbers.", arguments.argc - 1, inverse_hashes.size()); - if (had_collisions) - outln("Consider giving your new service a different name."); - - if (had_errors) - warnln("Some errors were encountered. There may be endpoints with colliding magic numbers."); - - int exit_code = 0; - if (had_collisions) - exit_code |= EXIT_COLLISION; - if (had_errors) - exit_code |= EXIT_ERROR; - return exit_code; -} diff --git a/Meta/Linters/lint_ipc.py b/Meta/Linters/lint_ipc.py new file mode 100755 index 00000000000..82077402b74 --- /dev/null +++ b/Meta/Linters/lint_ipc.py @@ -0,0 +1,82 @@ +#!/usr/bin/env python3 + +# Copyright (c) 2021, Ben Wiederhake +# Copyright (c) 2026-present, the Ladybird developers. +# +# SPDX-License-Identifier: BSD-2-Clause + +import argparse +import sys + +from collections import defaultdict +from pathlib import Path +from typing import Dict +from typing import List + +sys.path.append(str(Path(__file__).resolve().parent.parent.parent)) + +from Meta.Generators.string_hash import string_hash + +ENDPOINT_PREFIX = "endpoint " + + +def main() -> int: + parser = argparse.ArgumentParser(description="Check IPC endpoint files for magic-number collisions.") + parser.add_argument("ipc_files", nargs="+", help="IPC endpoint definition files") + args = parser.parse_args() + + files_by_magic: Dict[int, List[str]] = defaultdict(list) + error_count = 0 + + def report_error(message: str) -> None: + nonlocal error_count + print(f"Error: {message}", file=sys.stderr) + error_count += 1 + + for path in args.ipc_files: + endpoint_name = "" + + try: + with open(path, "r", encoding="utf-8") as ipc_file: + for line in ipc_file: + line = line.strip() + if not line.startswith(ENDPOINT_PREFIX): + continue + + remaining = line[len(ENDPOINT_PREFIX) :] + if endpoint_name: + report_error(f"Multiple endpoints in file '{path}': Found {endpoint_name} and {remaining}") + continue + + endpoint_name = remaining + except OSError as error: + report_error(f"Cannot open '{path}': {error}") + continue + + if not endpoint_name: + report_error(f"Could not detect endpoint name in file '{path}'") + continue + + files_by_magic[string_hash(endpoint_name)].append(path) + + for magic, files in files_by_magic.items(): + if len(files) <= 1: + continue + + report_error(f"Collision: Multiple endpoints use the magic number {magic}:") + for colliding_file in files: + print(f" - {colliding_file}") + + print(f"Checked {len(args.ipc_files)} files, saw {len(files_by_magic)} distinct magic numbers.") + + if error_count: + print( + "Some errors were encountered. There may be endpoints with colliding magic numbers.", + file=sys.stderr, + ) + + return error_count + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/Meta/lint-ci.sh b/Meta/lint-ci.sh index 57484ac0d4c..fa4cd4d37f9 100755 --- a/Meta/lint-ci.sh +++ b/Meta/lint-ci.sh @@ -33,15 +33,11 @@ for cmd in \ fi done -if [ -x ./Build/lagom/bin/IPCMagicLinter ]; then - if { git ls-files '*.ipc' | xargs ./Build/lagom/bin/IPCMagicLinter; }; then - echo -e "[${GREEN}OK${NC}]: IPCMagicLinter (in Meta/lint-ci.sh)" - else - echo -e "[${BOLD_RED}FAIL${NC}]: IPCMagicLinter (in Meta/lint-ci.sh)" - ((FAILURES+=1)) - fi +if { git ls-files '*.ipc' | xargs Meta/Linters/lint_ipc.py; }; then + echo -e "[${GREEN}OK${NC}]: Meta/Linters/lint_ipc.py" else - echo -e "[${GREEN}SKIP${NC}]: IPCMagicLinter (in Meta/lint-ci.sh)" + echo -e "[${BOLD_RED}FAIL${NC}]: Meta/Linters/lint_ipc.py" + ((FAILURES+=1)) fi if Meta/Linters/lint_clang_format.py --overwrite-inplace "$@" && git diff --exit-code -- ':*.cpp' ':*.h' ':*.mm'; then