From 98a709fb1b1bdfe5c7d6cbe75a3f53ea441b4705 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Thu, 15 Aug 2024 15:01:36 -0700 Subject: [PATCH] Further clang-analyzer (#13324) --- .github/workflows/run-lint-cpp.yml | 2 +- CMakeLists.txt | 8 ++++---- package.json | 2 +- scripts/download-webkit.ps1 | 2 +- scripts/download-zig.ps1 | 2 +- src/bun.js/bindings/Serialization.cpp | 2 +- src/bun.js/bindings/webcore/BroadcastChannel.cpp | 2 ++ src/bun.js/bindings/webcore/SerializedScriptValue.cpp | 4 ++++ .../bindings/webcrypto/CryptoAlgorithmAesKeyParams.h | 2 +- src/bun.js/bindings/webcrypto/CryptoAlgorithmIdentifier.h | 3 ++- src/bun.js/bindings/webcrypto/CryptoAlgorithmParameters.h | 3 ++- src/bun.js/bindings/webcrypto/SubtleCrypto.cpp | 3 +++ src/bun.js/bindings/webcrypto/SubtleCrypto.h | 2 +- src/codegen/generate-classes.ts | 8 ++++---- src/codegen/helpers.ts | 5 +++-- 15 files changed, 31 insertions(+), 19 deletions(-) diff --git a/.github/workflows/run-lint-cpp.yml b/.github/workflows/run-lint-cpp.yml index 3c394a46a3..fc8b42dd49 100644 --- a/.github/workflows/run-lint-cpp.yml +++ b/.github/workflows/run-lint-cpp.yml @@ -26,7 +26,7 @@ jobs: - name: Setup Bun uses: ./.github/actions/setup-bun with: - bun-version: latest + bun-version: 1.1.23 - name: Install Dependencies env: HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK: 1 diff --git a/CMakeLists.txt b/CMakeLists.txt index f1560ddbcf..4b21b29ec5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1632,14 +1632,14 @@ endif() if(BUN_TIDY_ONLY) find_program(CLANG_TIDY_EXE NAMES "clang-tidy") - set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "-checks=-*,clang-analyzer-*,-clang-analyzer-webkit.UncountedLambdaCapturesChecker,-clang-analyzer-optin.core.EnumCastOutOfRange" "--fix" "--fix-errors" "--format-style=webkit" "--warnings-as-errors=*") + + # webkit ones are disabled disabled because it's noisy, e.g. for JavaScriptCore/Lookup.h + set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "-checks=-*,clang-analyzer-*,-clang-analyzer-webkit.UncountedLambdaCapturesChecker,-clang-analyzer-optin.core.EnumCastOutOfRange,-clang-analyzer-webkit.RefCntblBaseVirtualDtor" "--fix" "--fix-errors" "--format-style=webkit" "--warnings-as-errors=*") set_target_properties(${bun} PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND}") endif() if(BUN_TIDY_ONLY_EXTRA) find_program(CLANG_TIDY_EXE NAMES "clang-tidy") - - # -clang-analyzer-optin.core.EnumCastOutOfRange is disabled because it's too noisy, e.g. for JavaScriptCore/Lookup.h - set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "-checks=-*,clang-analyzer-*,performance-*,-clang-analyzer-webkit.UncountedLambdaCapturesChecker,-clang-analyzer-optin.core.EnumCastOutOfRange" "--fix" "--fix-errors" "--format-style=webkit" "--warnings-as-errors=*") + set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "-checks=-*,clang-analyzer-*,performance-*,-clang-analyzer-webkit.UncountedLambdaCapturesChecker,-clang-analyzer-optin.core.EnumCastOutOfRange,-clang-analyzer-webkit.RefCntblBaseVirtualDtor" "--fix" "--fix-errors" "--format-style=webkit" "--warnings-as-errors=*") set_target_properties(${bun} PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND}") endif() diff --git a/package.json b/package.json index 071b538813..7ad1387664 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "bump": "bun ./scripts/bump.ts", "build": "if [ ! -e build ]; then bun setup; fi && ninja -C build", "build:valgrind": "cmake . -DZIG_OPTIMIZE=Debug -DUSE_DEBUG_JSC=ON -DCMAKE_BUILD_TYPE=Debug -GNinja -Bbuild-valgrind && ninja -Cbuild-valgrind", - "build:tidy": "BUN_SILENT=1 cmake --log-level=WARNING . -DZIG_OPTIMIZE=Debug -DUSE_DEBUG_JSC=ON -DBUN_TIDY_ONLY=ON -DCMAKE_BUILD_TYPE=Debug -GNinja -Bbuild-tidy >> ${GITHUB_STEP_SUMMARY:-/dev/stdout} && BUN_SILENT=1 ninja -Cbuild-tidy >> ${GITHUB_STEP_SUMMARY:-/dev/stdout}", + "build:tidy": "bash ./scripts/env.sh && BUN_SILENT=1 cmake --log-level=WARNING . ${CMAKE_FLAGS[@]} -DZIG_OPTIMIZE=Debug -DUSE_DEBUG_JSC=ON -DBUN_TIDY_ONLY=ON -DCMAKE_BUILD_TYPE=Debug -GNinja -Bbuild-tidy && BUN_SILENT=1 ninja -Cbuild-tidy", "build:tidy-extra": "cmake . -DZIG_OPTIMIZE=Debug -DUSE_DEBUG_JSC=ON -DBUN_TIDY_ONLY_EXTRA=ON -DCMAKE_BUILD_TYPE=Debug -GNinja -Bbuild-tidy && ninja -Cbuild-tidy", "build:release": "cmake . -DCMAKE_BUILD_TYPE=Release -GNinja -Bbuild-release && ninja -Cbuild-release", "build:release:local": "cmake . -DCMAKE_BUILD_TYPE=Release -DWEBKIT_DIR=$(pwd)/src/bun.js/WebKit/WebKitBuild/Release -GNinja -Bbuild-release-local && ninja -Cbuild-release-local", diff --git a/scripts/download-webkit.ps1 b/scripts/download-webkit.ps1 index b2b504cd49..4e4e6ec5d0 100755 --- a/scripts/download-webkit.ps1 +++ b/scripts/download-webkit.ps1 @@ -24,7 +24,7 @@ try { Write-Host "-- Downloading WebKit" if (!(Test-Path $TarPath)) { try { - Invoke-WebRequest $Url -OutFile $TarPath + Invoke-WebRequest $Url -OutFile $TarPath -MaximumRetryCount 3 -RetryIntervalSec 1 } catch { Write-Error "Failed to fetch WebKit from: $Url" throw $_ diff --git a/scripts/download-zig.ps1 b/scripts/download-zig.ps1 index d45939fa9a..1ec37a3f04 100755 --- a/scripts/download-zig.ps1 +++ b/scripts/download-zig.ps1 @@ -23,7 +23,7 @@ try { if (!(Test-Path $TarPath)) { try { Write-Host "-- Downloading Zig" - Invoke-RestMethod $Url -OutFile $TarPath + Invoke-RestMethod $Url -OutFile $TarPath -MaximumRetryCount 3 -RetryIntervalSec 1 } catch { Write-Error "Failed to fetch Zig from: $Url" throw $_ diff --git a/src/bun.js/bindings/Serialization.cpp b/src/bun.js/bindings/Serialization.cpp index ad9e254da7..fafb08b373 100644 --- a/src/bun.js/bindings/Serialization.cpp +++ b/src/bun.js/bindings/Serialization.cpp @@ -12,7 +12,7 @@ using namespace WebCore; struct SerializedValueSlice { const uint8_t* bytes; size_t size; - WebCore::SerializedScriptValue* value; + WebCore::SerializedScriptValue* value; // NOLINT }; /// Returns a "slice" that also contains a pointer to the SerializedScriptValue. Must be freed by the caller diff --git a/src/bun.js/bindings/webcore/BroadcastChannel.cpp b/src/bun.js/bindings/webcore/BroadcastChannel.cpp index 69b66ac42f..dbd4df5ff4 100644 --- a/src/bun.js/bindings/webcore/BroadcastChannel.cpp +++ b/src/bun.js/bindings/webcore/BroadcastChannel.cpp @@ -88,6 +88,8 @@ public: String name() const { return m_name.isolatedCopy(); } ScriptExecutionContextIdentifier contextId() const { return m_contextId; } + virtual ~MainThreadBridge() = default; + private: MainThreadBridge(BroadcastChannel&, const String& name, ScriptExecutionContext&); diff --git a/src/bun.js/bindings/webcore/SerializedScriptValue.cpp b/src/bun.js/bindings/webcore/SerializedScriptValue.cpp index 56aea5cfff..c3b936459e 100644 --- a/src/bun.js/bindings/webcore/SerializedScriptValue.cpp +++ b/src/bun.js/bindings/webcore/SerializedScriptValue.cpp @@ -2263,6 +2263,10 @@ private: case CryptoAlgorithmIdentifier::Ed25519: write(CryptoAlgorithmIdentifierTag::ED25519); break; + case CryptoAlgorithmIdentifier::None: { + RELEASE_ASSERT_NOT_REACHED(); + break; + } } } diff --git a/src/bun.js/bindings/webcrypto/CryptoAlgorithmAesKeyParams.h b/src/bun.js/bindings/webcrypto/CryptoAlgorithmAesKeyParams.h index c86fdcc7b0..4905955951 100644 --- a/src/bun.js/bindings/webcrypto/CryptoAlgorithmAesKeyParams.h +++ b/src/bun.js/bindings/webcrypto/CryptoAlgorithmAesKeyParams.h @@ -33,7 +33,7 @@ namespace WebCore { class CryptoAlgorithmAesKeyParams final : public CryptoAlgorithmParameters { public: - unsigned short length; + unsigned short length = 0; Class parametersClass() const final { return Class::AesKeyParams; } }; diff --git a/src/bun.js/bindings/webcrypto/CryptoAlgorithmIdentifier.h b/src/bun.js/bindings/webcrypto/CryptoAlgorithmIdentifier.h index a0c07a78a0..d6a112ecb8 100644 --- a/src/bun.js/bindings/webcrypto/CryptoAlgorithmIdentifier.h +++ b/src/bun.js/bindings/webcrypto/CryptoAlgorithmIdentifier.h @@ -29,7 +29,8 @@ namespace WebCore { -enum class CryptoAlgorithmIdentifier { +enum class CryptoAlgorithmIdentifier : uint8_t { + None = 0, RSAES_PKCS1_v1_5 = 1, RSASSA_PKCS1_v1_5, RSA_PSS, diff --git a/src/bun.js/bindings/webcrypto/CryptoAlgorithmParameters.h b/src/bun.js/bindings/webcrypto/CryptoAlgorithmParameters.h index 8398404b8e..ae47350ec9 100644 --- a/src/bun.js/bindings/webcrypto/CryptoAlgorithmParameters.h +++ b/src/bun.js/bindings/webcrypto/CryptoAlgorithmParameters.h @@ -35,6 +35,7 @@ namespace WebCore { class CryptoAlgorithmParameters { WTF_MAKE_FAST_ALLOCATED; + public: enum class Class { None, @@ -57,7 +58,7 @@ public: // FIXME: Consider merging name and identifier. String name; - CryptoAlgorithmIdentifier identifier; + CryptoAlgorithmIdentifier identifier { CryptoAlgorithmIdentifier::None }; virtual ~CryptoAlgorithmParameters() = default; diff --git a/src/bun.js/bindings/webcrypto/SubtleCrypto.cpp b/src/bun.js/bindings/webcrypto/SubtleCrypto.cpp index f41895cf5a..961b714f65 100644 --- a/src/bun.js/bindings/webcrypto/SubtleCrypto.cpp +++ b/src/bun.js/bindings/webcrypto/SubtleCrypto.cpp @@ -357,7 +357,10 @@ static ExceptionOr> normalizeCryptoAl case CryptoAlgorithmIdentifier::SHA_384: case CryptoAlgorithmIdentifier::SHA_512: return Exception { NotSupportedError }; + case CryptoAlgorithmIdentifier::None: + return Exception { NotSupportedError }; } + break; case Operations::WrapKey: case Operations::UnwrapKey: diff --git a/src/bun.js/bindings/webcrypto/SubtleCrypto.h b/src/bun.js/bindings/webcrypto/SubtleCrypto.h index e9ef23e6a5..dbfee4649b 100644 --- a/src/bun.js/bindings/webcrypto/SubtleCrypto.h +++ b/src/bun.js/bindings/webcrypto/SubtleCrypto.h @@ -50,7 +50,7 @@ class BufferSource; class CryptoKey; class DeferredPromise; -enum class CryptoAlgorithmIdentifier; +enum class CryptoAlgorithmIdentifier : uint8_t; enum class CryptoKeyUsage; class SubtleCrypto : public ContextDestructionObserver, public RefCounted, public CanMakeWeakPtr { diff --git a/src/codegen/generate-classes.ts b/src/codegen/generate-classes.ts index d93b26a62b..a6365c3b7e 100644 --- a/src/codegen/generate-classes.ts +++ b/src/codegen/generate-classes.ts @@ -147,7 +147,7 @@ JSC_DEFINE_JIT_OPERATION(${DOMJITName( CallFrame* callFrame = DECLARE_CALL_FRAME(vm); IGNORE_WARNINGS_END JSC::JITOperationPrologueCallFrameTracer tracer(vm, callFrame); -#ifdef BUN_DEBUG +#if BUN_DEBUG ${jsClassName}* wrapper = reinterpret_cast<${jsClassName}*>(thisValue); JSC::EncodedJSValue result = ${DOMJITName(symName)}(wrapper->wrapped(), lexicalGlobalObject${retArgs}); JSValue decoded = JSValue::decode(result); @@ -1124,7 +1124,7 @@ JSC_DEFINE_HOST_FUNCTION(${symbolName(typeName, name)}Callback, (JSGlobalObject JSC::EnsureStillAliveScope thisArg = JSC::EnsureStillAliveScope(thisObject); -#if defined(BUN_DEBUG) && BUN_DEBUG +#if BUN_DEBUG /** View the file name of the JS file that called this function * from a debugger */ SourceOrigin sourceOrigin = callFrame->callerSourceOrigin(vm); @@ -1134,7 +1134,7 @@ JSC_DEFINE_HOST_FUNCTION(${symbolName(typeName, name)}Callback, (JSGlobalObject lastFileName = fileName; } - JSC::EncodedJSValue result = ${symbolName(typeName, fn)}(thisObject->wrapped(), lexicalGlobalObject, callFrame, callFrame${proto[name].passThis ? ", JSValue::encode(thisObject)" : ""}); + JSC::EncodedJSValue result = ${symbolName(typeName, fn)}(thisObject->wrapped(), lexicalGlobalObject, callFrame${proto[name].passThis ? ", JSValue::encode(thisObject)" : ""}); ASSERT_WITH_MESSAGE(!JSValue::decode(result).isEmpty() or DECLARE_CATCH_SCOPE(vm).exception() != 0, \"${typeName}.${proto[name].fn} returned an empty value without an exception\"); @@ -1318,7 +1318,7 @@ function generateClassHeader(typeName, obj: ClassDefinition) { } function domJITTypeCheckFields(proto, klass) { - var output = "#ifdef BUN_DEBUG\n"; + var output = "#if BUN_DEBUG\n"; for (const name in proto) { const { DOMJIT, fn } = proto[name]; if (!DOMJIT) continue; diff --git a/src/codegen/helpers.ts b/src/codegen/helpers.ts index a97007b767..aa9ce40ce9 100644 --- a/src/codegen/helpers.ts +++ b/src/codegen/helpers.ts @@ -75,13 +75,14 @@ export function checkAscii(str: string) { export function writeIfNotChanged(file: string, contents: string) { if (Array.isArray(contents)) contents = contents.join(""); + contents = contents.replaceAll("\r\n", "\n").trim() + "\n"; - if (fs.existsSync(file)) { + try { const oldContents = fs.readFileSync(file, "utf8"); if (oldContents === contents) { return; } - } + } catch (e) {} try { fs.writeFileSync(file, contents);