Skip to content

Commit 12c9964

Browse files
cdumezcdumez
committed
Address Safer C++ static analysis warnings in WebCore bindings code
https://bugs.webkit.org/show_bug.cgi?id=326601 Reviewed by Geoffrey Garen. Fix all Safer C++ warnings in the generated JS bindings and in bindings/js, without adding ref counting churn to hot paths: - Interface conversion results are now Ref<T> / RefPtr<T>. The converter already refs the object, so this only moves the deref after the implementation call. - Trivial accessors are marked NODELETE and pure pointer conversions are marked CLANG_POINTER_CONVERSION. - Values that are passed straight to toJS(), which refs them before anything can run, are produced inside the toJS() functor, so a single suppression in JSDOMConvertBase.h covers them. - Code that runs on the GC thread, and other provably safe hot paths, use suppressions with comments. - Cold paths use protect(). * Source/JavaScriptCore/CMakeLists.txt: * Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj: Export HeapProfiler.h, which HeapSnapshotBuilder's constructor depends on. * Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_header.py: Include FunctionExecutable.h, which the generated init() functions use. * Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-AnotherGuardedInternalBuiltin-Separate.js-result: * Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-ArbitraryConditionalGuard-Separate.js-result: * Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-GuardedBuiltin-Separate.js-result: * Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-GuardedInternalBuiltin-Separate.js-result: * Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-UnguardedBuiltin-Separate.js-result: * Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-xmlCasingTest-Separate.js-result: Rebaseline. * Source/WebCore/bindings/IDLTypes.h: Use Ref<T> / RefPtr<T> as IDLInterface conversion results. Mark the extractValueFromNullable() overloads that return references as pointer conversions. * Source/WebCore/bindings/js/JSDOMConvertInterface.h: Transfer the converter's reference into the conversion result. * Source/WebCore/bindings/js/JSDOMConvertBase.h: Mark getPtrOrRef() as pointer conversions and suppress the warnings for values that toJS() passes straight to the converter. * Source/WebCore/bindings/scripts/CodeGeneratorJS.pm: Pass interface arguments using .get(), produce interface and buffer source results inside the toJS() functor, pass the union setter functor inline so it binds to convert()'s NOESCAPE parameter, protect cold-path arguments, and suppress the warnings for indexed / named property access, security checks and GC-thread code. * Source/WebCore/bindings/scripts/test/JS/JSDOMWindow.cpp: * Source/WebCore/bindings/scripts/test/JS/JSExposedToWorkerAndWindow.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestAsyncIterable.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestCEReactions.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestCallTracer.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestConditionalIncludes.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestConditionallyReadWrite.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestDOMJIT.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestDefaultToJSON.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestDefaultToJSONInherit.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestDefaultToJSONInheritFinal.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestDomainSecurity.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestEnabledForContext.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestEventTarget.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestGenerateAddOpaqueRoot.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestGenerateIsReachable.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestIndexedSetterNoIdentifier.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestIndexedSetterThrowingException.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestIndexedSetterWithIdentifier.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestInterface.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestLegacyNoInterfaceObject.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestLegacyOverrideBuiltIns.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamedAndIndexedSetterNoIdentifier.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamedAndIndexedSetterThrowingException.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamedAndIndexedSetterWithIdentifier.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterNoIdentifier.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterThrowingException.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterWithIdentifier.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterWithIndexedGetter.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamedGetterCallWith.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamedGetterNoIdentifier.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamedGetterWithIdentifier.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterNoIdentifier.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterThrowingException.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithIdentifier.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithIndexedGetter.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithIndexedGetterAndSetter.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithLegacyOverrideBuiltIns.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithLegacyUnforgeableProperties.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithLegacyUnforgeablePropertiesAndLegacyOverrideBuiltIns.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestNamespaceObject.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestOverloadedConstructors.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestReportExtraMemoryCost.cpp: * Source/WebCore/bindings/scripts/test/JS/JSTestTypedefs.cpp: Rebaseline. * Source/WebCore/bindings/js/JSAudioBufferSourceNodeCustom.cpp: * Source/WebCore/bindings/js/JSCSSRuleCustom.h: * Source/WebCore/bindings/js/JSCanvasRenderingContext2DCustom.cpp: * Source/WebCore/bindings/js/JSHTMLCanvasElementCustom.cpp: * Source/WebCore/bindings/js/JSIDBTransactionCustom.cpp: * Source/WebCore/bindings/js/JSIntersectionObserverCustom.cpp: * Source/WebCore/bindings/js/JSMessageChannelCustom.cpp: * Source/WebCore/bindings/js/JSOffscreenCanvasRenderingContext2DCustom.cpp: * Source/WebCore/bindings/js/JSRangeCustom.cpp: * Source/WebCore/bindings/js/JSSVGViewSpecCustom.cpp: * Source/WebCore/bindings/js/JSStaticRangeCustom.cpp: * Source/WebCore/bindings/js/JSStylePropertyMapReadOnlyCustom.cpp: * Source/WebCore/bindings/js/JSTrustedTypePolicyFactoryCustom.cpp: * Source/WebCore/bindings/js/JSWorkerGlobalScopeCustom.cpp: * Source/WebCore/bindings/js/WebCoreTypedArrayController.cpp: Suppress the warnings for code that runs on the GC thread. * Source/WebCore/bindings/js/JSCustomElementInterface.cpp: * Source/WebCore/bindings/js/JSDOMBindingSecurity.cpp: * Source/WebCore/bindings/js/JSDOMGlobalObject.cpp: * Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp: * Source/WebCore/bindings/js/JSDOMPromiseDeferred.h: * Source/WebCore/bindings/js/JSDOMWindowBase.cpp: * Source/WebCore/bindings/js/JSExecState.cpp: * Source/WebCore/bindings/js/JSLazyEventListener.cpp: * Source/WebCore/bindings/js/JSWorkletGlobalScopeBase.cpp: * Source/WebCore/bindings/js/ScheduledAction.cpp: * Source/WebCore/bindings/js/ScriptBufferSourceProvider.h: * Source/WebCore/bindings/js/ScriptController.cpp: * Source/WebCore/bindings/js/ScriptControllerMac.mm: * Source/WebCore/bindings/js/ScriptModuleLoader.cpp: * Source/WebCore/bindings/js/SerializedScriptValue.cpp: * Source/WebCore/bindings/js/WebAssemblyScriptBufferSourceProvider.h: Use protect() / smart pointers. * Source/WebCore/bindings/js/DOMPromiseProxy.h: * Source/WebCore/bindings/js/JSDOMBindingSecurityInlines.h: * Source/WebCore/bindings/js/JSDOMConvertEventListener.h: * Source/WebCore/bindings/js/JSDOMIterator.h: * Source/WebCore/bindings/js/JSEventTargetCustom.h: Suppress the warnings where the object is provably kept alive. * Source/WebCore/bindings/js/GarbageCollectionController.cpp: * Source/WebCore/bindings/js/JSDOMWindowCustom.cpp: * Source/WebCore/bindings/js/JSExecState.h: * Source/WebCore/bindings/js/JSLocationCustom.cpp: Include the headers for forward declared types. * Source/WebCore/bindings/js/JSNodeCustom.cpp: * Source/WebCore/bindings/js/JSPaintRenderingContext2DCustom.cpp: Suppress the warnings for code that runs on the GC thread or where the object is kept alive. Include CustomPaintCanvas.h. * Source/WebCore/dom/Document.h: * Source/WebCore/dom/EmptyScriptExecutionContext.h: * Source/WebCore/dom/ScriptExecutionContext.h: * Source/WebCore/workers/WorkerGlobalScope.h: * Source/WebCore/worklets/WorkletGlobalScope.h: Mark settingsValues(), quirks() and ensureQuirks() as NODELETE. * Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp: * Source/WebCore/html/canvas/CanvasRenderingContext2DBase.h: Take Ref<> for the non-nullable gradient and pattern styles. * Source/WebCore/SaferCPPExpectations/ForwardDeclCheckerExpectations: * Source/WebCore/SaferCPPExpectations/NoUncheckedPtrMemberCheckerExpectations: * Source/WebCore/SaferCPPExpectations/NoUncountedMemberCheckerExpectations: * Source/WebCore/SaferCPPExpectations/UncheckedCallArgsCheckerExpectations: * Source/WebCore/SaferCPPExpectations/UncountedCallArgsCheckerExpectations: * Source/WebCore/SaferCPPExpectations/UnretainedLocalVarsCheckerExpectations: Update expectations. Canonical link: https://commits.webkit.org/323069@main
1 parent 34f64e2 commit 12c9964

114 files changed

Lines changed: 648 additions & 499 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎Source/JavaScriptCore/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,7 @@ set(JavaScriptCore_PRIVATE_FRAMEWORK_HEADERS
859859
heap/HeapInlines.h
860860
heap/HeapIterationScope.h
861861
heap/HeapObserver.h
862+
heap/HeapProfiler.h
862863
heap/HeapSnapshotBuilder.h
863864
heap/IncrementalSweeper.h
864865
heap/IsoCellSet.h

‎Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1631,7 +1631,7 @@
16311631
A53243981856A489002ED692 /* CombinedDomains.json in Headers */ = {isa = PBXBuildFile; fileRef = A53243951856A475002ED692 /* CombinedDomains.json */; settings = {ATTRIBUTES = (Private, ); }; };
16321632
A5339EC61BB399A60054F005 /* InspectorHeapAgent.h in Headers */ = {isa = PBXBuildFile; fileRef = A5339EC51BB399900054F005 /* InspectorHeapAgent.h */; settings = {ATTRIBUTES = (Private, ); }; };
16331633
A5339EC91BB4B4600054F005 /* HeapObserver.h in Headers */ = {isa = PBXBuildFile; fileRef = A5339EC81BB4B4510054F005 /* HeapObserver.h */; settings = {ATTRIBUTES = (Private, ); }; };
1634-
A5398FAB1C750DA40060A963 /* HeapProfiler.h in Headers */ = {isa = PBXBuildFile; fileRef = A5398FAA1C750D950060A963 /* HeapProfiler.h */; };
1634+
A5398FAB1C750DA40060A963 /* HeapProfiler.h in Headers */ = {isa = PBXBuildFile; fileRef = A5398FAA1C750D950060A963 /* HeapProfiler.h */; settings = {ATTRIBUTES = (Private, ); }; };
16351635
A53CE08A18BC21C300BEDF76 /* ConsoleClient.h in Headers */ = {isa = PBXBuildFile; fileRef = A53CE08918BC21C300BEDF76 /* ConsoleClient.h */; settings = {ATTRIBUTES = (Private, ); }; };
16361636
A53F1AC018C90F8F0072EB6D /* framework.sb in Resources */ = {isa = PBXBuildFile; fileRef = A53F1ABE18C90EC70072EB6D /* framework.sb */; };
16371637
A54C2AB11C6544F200A18D78 /* HeapSnapshot.h in Headers */ = {isa = PBXBuildFile; fileRef = A54C2AAF1C6544D100A18D78 /* HeapSnapshot.h */; settings = {ATTRIBUTES = (Private, ); }; };

‎Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-AnotherGuardedInternalBuiltin-Separate.js-result‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include <cstdint>
3636

3737
#include <JavaScriptCore/BuiltinUtils.h>
38+
#include <JavaScriptCore/FunctionExecutable.h>
3839
#include <JavaScriptCore/Identifier.h>
3940
#include <JavaScriptCore/JSFunction.h>
4041
#include <JavaScriptCore/SlotVisitorInlines.h>

‎Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-ArbitraryConditionalGuard-Separate.js-result‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include <cstdint>
3737

3838
#include <JavaScriptCore/BuiltinUtils.h>
39+
#include <JavaScriptCore/FunctionExecutable.h>
3940
#include <JavaScriptCore/Identifier.h>
4041
#include <JavaScriptCore/JSFunction.h>
4142
#include <JavaScriptCore/SlotVisitorInlines.h>

‎Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-GuardedBuiltin-Separate.js-result‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include <cstdint>
3737

3838
#include <JavaScriptCore/BuiltinUtils.h>
39+
#include <JavaScriptCore/FunctionExecutable.h>
3940
#include <JavaScriptCore/Identifier.h>
4041
#include <JavaScriptCore/JSFunction.h>
4142
#include <JavaScriptCore/SlotVisitorInlines.h>

‎Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-GuardedInternalBuiltin-Separate.js-result‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include <cstdint>
3737

3838
#include <JavaScriptCore/BuiltinUtils.h>
39+
#include <JavaScriptCore/FunctionExecutable.h>
3940
#include <JavaScriptCore/Identifier.h>
4041
#include <JavaScriptCore/JSFunction.h>
4142
#include <JavaScriptCore/SlotVisitorInlines.h>

‎Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-UnguardedBuiltin-Separate.js-result‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include <cstdint>
3535

3636
#include <JavaScriptCore/BuiltinUtils.h>
37+
#include <JavaScriptCore/FunctionExecutable.h>
3738
#include <JavaScriptCore/Identifier.h>
3839
#include <JavaScriptCore/JSFunction.h>
3940
#include <JavaScriptCore/SlotVisitorInlines.h>

‎Source/JavaScriptCore/Scripts/tests/builtins/expected/WebCore-xmlCasingTest-Separate.js-result‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include <cstdint>
3737

3838
#include <JavaScriptCore/BuiltinUtils.h>
39+
#include <JavaScriptCore/FunctionExecutable.h>
3940
#include <JavaScriptCore/Identifier.h>
4041
#include <JavaScriptCore/JSFunction.h>
4142
#include <JavaScriptCore/SlotVisitorInlines.h>

‎Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_separate_header.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ def generate_secondary_header_includes(self):
9797
("JavaScriptCore", "builtins/BuiltinUtils.h"),
9898
),
9999

100+
(["WebCore"], ("JavaScriptCore", "runtime/FunctionExecutable.h")),
101+
100102
(["WebCore"],
101103
("JavaScriptCore", "runtime/Identifier.h"),
102104
),

‎Source/WebCore/SaferCPPExpectations/ForwardDeclCheckerExpectations‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,3 @@
1-
CommandLineAPIModuleSourceBuiltins.h
2-
ReadableStreamInternalsBuiltins.h
3-
StreamInternalsBuiltins.h
4-
TransformStreamInternalsBuiltins.h
5-
WritableStreamInternalsBuiltins.h
6-
bindings/js/GarbageCollectionController.cpp
7-
bindings/js/JSLocationCustom.cpp
8-
bindings/js/JSPaintRenderingContext2DCustom.cpp
91
css/values/primitives/CSSPrimitiveData.h
102
dom/ElementInlinesLight.h
113
layout/formattingContexts/inline/InlineFormattingContext.h

0 commit comments

Comments
 (0)