Skip to content

[contributors.json] Relocation #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from

Conversation

JonWBedard
Copy link
Member

@JonWBedard JonWBedard commented Aug 30, 2021

b4d627c

[contributors.json] Relocation (Part 6)
https://bugs.webkit.org/show_bug.cgi?id=229690
<rdar://problem/82552403>

Reviewed by NOBODY (OOPS!).

* WebKitBot/src/Commit.mjs: Load contributors from metadata/contributors.json.
* WebKitBot/src/Contributors.mjs: Load contributors from metadata/contributors.json, parse as list instead of dictionary.
* WebKitBot/tests/Commit.test.mjs: Use name instead of fullName, stop relying on nicks.

89526ad

[contributors.json] Relocation (Part 5)
https://bugs.webkit.org/show_bug.cgi?id=229690
<rdar://problem/82552403>

Reviewed by NOBODY (OOPS!).

* CISupport/ews-build/steps.py:
(ValidateCommiterAndReviewer): Pull from metadata/contributors.json.
(ValidateCommiterAndReviewer.load_contributors_from_disk): Ditto.
(ValidateCommiterAndReviewer.load_contributors): Parse contributors.json as list instead of dictionary.
* CISupport/ews-build/steps_unittest.py:

f59a8ee

[contributors.json] Relocation (Part 4)
https://bugs.webkit.org/show_bug.cgi?id=229690
<rdar://problem/82552403>

Reviewed by NOBODY (OOPS!).

* committers-autocomplete.js: Use metadata/contributors.json.
(parseCommittersPy): Parse contributors.json as list instead of dictionary.
(isMatch): Strip irc references.
(updateMenu): Ditto.

36b2c4b

[contributors.json] Relocation (Part 3)
https://bugs.webkit.org/show_bug.cgi?id=229690
<rdar://problem/82552403>

Reviewed by NOBODY (OOPS!).

* commit-review.md: Link to metadata/contributors.json.
* wp-content/themes/webkit/team.php: Load from metadata/contributors.json,
load list instead of dictionary.

08a02d4

[contributors.json] Relocation (Part 2)
https://bugs.webkit.org/show_bug.cgi?id=229690
<rdar://problem/82552403>

Reviewed by NOBODY (OOPS!).

* Scripts/webkitpy/common/config/committers.py:
(Contributor.as_dict): Include name in dictionary.
(CommitterList.load_json): Use list instead of dictionary.
(CommitterList.as_json): Encode as list instead of dictionary.
* metadata/contributors.json: Convert from dictionary to list.
(CommitterList._contributor_list_to_dict): Deleted.

2748b23

[contributors.json] Relocation (Part 1)
https://bugs.webkit.org/show_bug.cgi?id=229690
<rdar://problem/82552403>

Reviewed by Aakash Jain.

* Scripts/webkitpy/common/config/committers.py:
(CommitterList.load_json): Read from metadata/contributors.json.
(CommitterList.reformat_in_place): Ditto.
* metadata/contributors.json: Copied from Tools/Scripts/webkitpy/common/config/contributors.json.

https://bugs.webkit.org/show_bug.cgi?id=229690
<rdar://problem/82552403>

Reviewed by Aakash Jain.

* Scripts/webkitpy/common/config/committers.py:
(CommitterList.load_json): Read from metadata/contributors.json.
(CommitterList.reformat_in_place): Ditto.
* metadata/contributors.json: Copied from Tools/Scripts/webkitpy/common/config/contributors.json.
@JonWBedard JonWBedard force-pushed the eng/relocate-contributors branch from 74d274b to f9453b7 Compare August 31, 2021 23:37
@JonWBedard JonWBedard changed the title [git-webkit] Relocate contributors.json (Part 1) [contributors.json] Relocation Aug 31, 2021
https://bugs.webkit.org/show_bug.cgi?id=229690
<rdar://problem/82552403>

Reviewed by NOBODY (OOPS!).

* Scripts/webkitpy/common/config/committers.py:
(Contributor.as_dict): Include name in dictionary.
(CommitterList.load_json): Use list instead of dictionary.
(CommitterList.as_json): Encode as list instead of dictionary.
* metadata/contributors.json: Convert from dictionary to list.
(CommitterList._contributor_list_to_dict): Deleted.
https://bugs.webkit.org/show_bug.cgi?id=229690
<rdar://problem/82552403>

Reviewed by NOBODY (OOPS!).

* commit-review.md: Link to metadata/contributors.json.
* wp-content/themes/webkit/team.php: Load from metadata/contributors.json,
load list instead of dictionary.
https://bugs.webkit.org/show_bug.cgi?id=229690
<rdar://problem/82552403>

Reviewed by NOBODY (OOPS!).

* committers-autocomplete.js: Use metadata/contributors.json.
(parseCommittersPy): Parse contributors.json as list instead of dictionary.
(isMatch): Strip irc references.
(updateMenu): Ditto.
@JonWBedard JonWBedard force-pushed the eng/relocate-contributors branch from f9453b7 to f59a8ee Compare August 31, 2021 23:50
https://bugs.webkit.org/show_bug.cgi?id=229690
<rdar://problem/82552403>

Reviewed by NOBODY (OOPS!).

* CISupport/ews-build/steps.py:
(ValidateCommiterAndReviewer): Pull from metadata/contributors.json.
(ValidateCommiterAndReviewer.load_contributors_from_disk): Ditto.
(ValidateCommiterAndReviewer.load_contributors): Parse contributors.json as list instead of dictionary.
* CISupport/ews-build/steps_unittest.py:
https://bugs.webkit.org/show_bug.cgi?id=229690
<rdar://problem/82552403>

Reviewed by NOBODY (OOPS!).

* WebKitBot/src/Commit.mjs: Load contributors from metadata/contributors.json.
* WebKitBot/src/Contributors.mjs: Load contributors from metadata/contributors.json, parse as list instead of dictionary.
* WebKitBot/tests/Commit.test.mjs: Use name instead of fullName, stop relying on nicks.
@JonWBedard
Copy link
Member Author

Finished landing this in https://commits.webkit.org/242672@main (28e9d03)

@JonWBedard JonWBedard closed this Oct 9, 2021
@JonWBedard JonWBedard deleted the eng/relocate-contributors branch October 9, 2021 00:48
webkit-commit-queue pushed a commit to Constellation/WebKit that referenced this pull request Jan 24, 2023
https://bugs.webkit.org/show_bug.cgi?id=251063
rdar://104585575

Reviewed by Mark Lam and Justin Michaud.

This patch enhances CallFrame::dump to support wasm frames in btjs stacktrace.
The example is as follows.

    frame #0: 0x00000001035fca78 JavaScriptCore`JSC::functionBreakpoint(globalObject=0x000000012f410068, callFrame=0x000000016fdfa9d0) at JSDollarVM.cpp:2273:9 [opt]
    frame WebKit#1: 0x000000010ec44204 0x10eccc5dc
    frame WebKit#2: 0x000000010eccc5dc callback#Dwaxn6 [Baseline bc#50](Undefined)
    frame WebKit#3: 0x000000010ec4ca84 wasm-stub [WasmToJS](Wasm::Instance: 0x10d29da40)
    frame WebKit#4: 0x000000010ed0c060 <?>.wasm-function[1] [OMG](Wasm::Instance: 0x10d29da40)
    frame WebKit#5: 0x000000010ed100d0 jsToWasm#CWTx6k [FTL bc#22](Cell[JSModuleEnvironment]: 0x12f524540, Cell[WebAssemblyFunction]: 0x10d06a3a8, 1, 2, 3)
    frame WebKit#6: 0x000000010ec881b0 #D5ymZE [Baseline bc#733](Undefined, Cell[Generator]: 0x12f55c180, 1, Cell[Object]: 0x12f69dfc0, 0, Cell[JSLexicalEnvironment]: 0x12f52cee0)
    frame WebKit#7: 0x000000010ec3c008 asyncFunctionResume#A4ayYg [LLInt bc#49](Undefined, Cell[Generator]: 0x12f55c180, Cell[Object]: 0x12f69dfc0, 0)
    frame WebKit#8: 0x000000010ec3c008 promiseReactionJobWithoutPromise#D0yDF1 [LLInt bc#25](Undefined, Cell[Function]: 0x12f44f3c0, Cell[Object]: 0x12f69dfc0, Cell[Generator]: 0x12f55c180)
    frame WebKit#9: 0x000000010ec80ec0 promiseReactionJob#EdShZz [Baseline bc#74](Undefined, Undefined, Cell[Function]: 0x12f44f3c0, Cell[Object]: 0x12f69dfc0, Cell[Generator]: 0x12f55c180)
    frame WebKit#10: 0x000000010ec3c728
    frame WebKit#11: 0x0000000103137560 JavaScriptCore`JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) [inlined] JSC::JITCode::execute(this=<unavailable>, vm=<unavailable>, protoCallFrame=<unavailable>) at JITCodeInlines.h:42:38 [opt]
    frame WebKit#12: 0x0000000103137524 JavaScriptCore`JSC::Interpreter::executeCall(this=<unavailable>, lexicalGlobalObject=<unavailable>, function=<unavailable>, callData=<unavailable>, thisValue=<unavailable>, args=<unavailable>) at Interpreter.cpp:1093:27 [opt]
    frame WebKit#13: 0x000000010349d6d0 JavaScriptCore`JSC::runJSMicrotask(globalObject=0x000000012f410068, identifier=(m_identifier = 81), job=JSValue @ x22, argument0=JSValue @ x26, argument1=JSValue @ x25, argument2=<unavailable>, argument3=<unavailable>) at JSMicrotask.cpp:98:9 [opt]
    frame WebKit#14: 0x00000001039dfc54 JavaScriptCore`JSC::VM::drainMicrotasks() (.cold.1) at VM.cpp:0:9 [opt]
    frame WebKit#15: 0x00000001035e58a4 JavaScriptCore`JSC::VM::drainMicrotasks() [inlined] JSC::MicrotaskQueue::dequeue(this=<unavailable>) at VM.cpp:0:9 [opt]
    frame WebKit#16: 0x00000001035e5894 JavaScriptCore`JSC::VM::drainMicrotasks(this=0x000000012f000000) at VM.cpp:1255:46 [opt]
    ...

* Source/JavaScriptCore/interpreter/CallFrame.cpp:
(JSC::CallFrame::dump const):

Canonical link: https://commits.webkit.org/259262@main
webkit-commit-queue pushed a commit to robert-jenner/WebKit that referenced this pull request Dec 18, 2024
https://bugs.webkit.org/show_bug.cgi?id=281902
rdar://136486349

Reviewed by Mike Wyrzykowski.

Metal: Ensure potentially infinite loops have defined behavior

The MSL compiler would omit infinite loops and assume number domains
based on the omission logic. This would induce incorrect number domains
in case the infinite loops would be invokable. Infinite loops are
undefined in C++ and thus in MSL. It is the job of the programmer
to ensure undefined behavior cannot happen.

Consider GLSL loop like:
uniform float i;
...
    if (i != 0.5) for(;;) { }
    gl_FragColor = vec4(i);

Historically this would emit MSL loop in spirit of:
    if (i != 0.5) {
      bool c = true;
      while (c) { }
    }
    ANGLE_fragmentOut.gl_FragColor = metal::float4(i, i, i, i);

Since This could cause the MSL compiler to optimize the function to
equivalent of:
    ANGLE_fragmentOut.gl_FragColor = metal::float4(0.5, 0.5, 0.5, 0.5);

Presumably this loop omission would happen at the clang frontend part.

Before, was worked around by emitting asm statements to the MSL:
    bool c = true;
    while (c) {
        __asm__("");
    }

The asm injection would would work for this particular source pattern,
presumably because injecting the asm would avoid the loop omission at
the clang frontend part.

The MSL/C++ code is still UB, though. The asm statement does not cause
anything that C++ would consider as "forward progress" of the loop. The
success was just due to how the backend worked. The bitcode produced
would be similar to:

4:
    tail call void asm sideeffect "", ""() WebKit#6, !srcloc !28
    br label %4, !llvm.loop !29

Here, the compiler can be seen to simply fail to detect a loop that
does not make forward progress.

Considering GLSL of form:
uniform int f;
...
    for (;;) { if (f <= 1) break; }

With asm injection to the loop, this would produce:

5:
    tail call void asm sideeffect "", ""() WebKit#8, !srcloc !29
    %6 = load i32, i32 addrspace(2)* %4, align 4, !tbaa !30
    %7 = icmp slt i32 %6, 2
    br i1 %7, label %8, label %5
8:

This code is still assumed to make progress. The backend optimizer is
free to assume that the condition holds, since the load to break the
loop is from constant address space. I.e. uniform f does not change its
value during the loop.

Instead of injecting asm, inject a read of unused volatile variable.
The volatile variable access is defined in C++ as forward progress.
This means infinite loop containing such read is considered defined.
To simplify the implementation and to avoid volatile writes, the read
is to a dummy variable instead of the loop condition bool.

The tests here do not pass completely for MSL backend. In case the
compiler would omit the infinite loop (unpatched code), they would fail
with demonstration of how the values behave. After fixing, the loops
cause timeout but Metal backend does not have implementation to report
context loss. Also, the ReadPixels is just for demostration purposes of the
unpatched code.

* Source/ThirdParty/ANGLE/src/compiler/translator/msl/EmitMetal.cpp:
(GenMetalTraverser::GenMetalTraverser):
(GenMetalTraverser::emitLoopBody):
(GenMetalTraverser::emitForwardProgressStore):
(GenMetalTraverser::emitForwardProgressSignal):
(GenMetalTraverser::visitForLoop):
(GenMetalTraverser::visitWhileLoop):
(GenMetalTraverser::visitDoWhileLoop):
* Source/ThirdParty/ANGLE/src/tests/angle_end2end_tests.gni:
* Source/ThirdParty/ANGLE/src/tests/gl_tests/TimeoutDrawTest.cpp: Added.
(angle::TimeoutDrawTest::TimeoutDrawTest):
(angle::TEST_P):

Originally-landed-as: 283286.350@safari-7620-branch (b82d94e). rdar://141318430
Canonical link: https://commits.webkit.org/288020@main
philn added a commit to philn/WebKit that referenced this pull request Feb 6, 2025
Reviewed by NOBODY (OOPS!).

This was spotted by ASan, the real and imaginary AudioFloatArrays end-up using aligned_alloc() for
their storage, which expects a power-of-two size. Using fftSize / 2 + 1 makes the size
non-power-of-two, the full fftSize being power-of-two (we now have an ASSERT for this).

==1733723==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fc75c438bca bp 0x7ffed612c860 sp 0x7ffed612c028 T0)
==1733723==The signal is caused by a WRITE memory access.
==1733723==Hint: address points to the zero page.
    #0 0x7fc75c438bca in __memset_avx2_unaligned_erms (/lib64/libc.so.6+0x16dbca) (BuildId: 77c77fee058b19c6f001cf2cb0371ce3b8341211)
    WebKit#1 0x2fc4ce in __asan_memset (/var/home/phil/WebKit/local-build-gtk/WebKitBuild/GTK/Release/bin/WebKitWebProcess+0x2fc4ce) (BuildId: e2b0aff0c8fcab48026d63cf711cd70d0aeceb79)
    WebKit#2 0x7fc77047599e in WebCore::FFTFrame::FFTFrame(unsigned int) UnifiedSource-3c72abbe-20.cpp
    WebKit#3 0x7fc76d41c08b in WebCore::PeriodicWave::createBandLimitedTables(float const*, float const*, unsigned int, WebCore::ShouldDisableNormalization) UnifiedSource-f8afad56-52.cpp
    WebKit#4 0x7fc76d41f371 in WebCore::PeriodicWave::generateBasicWaveform(WebCore::PeriodicWave::Type) UnifiedSource-f8afad56-52.cpp
    WebKit#5 0x7fc76d41e633 in WebCore::PeriodicWave::createSine(float) UnifiedSource-f8afad56-52.cpp
    WebKit#6 0x7fc76d3c2352 in WebCore::BaseAudioContext::periodicWave(WebCore::OscillatorType) UnifiedSource-f8afad56-49.cpp
    WebKit#7 0x7fc76d410dbf in WebCore::OscillatorNode::setTypeForBindings(WebCore::OscillatorType) UnifiedSource-f8afad56-52.cpp
    WebKit#8 0x7fc76d4102f4 in WebCore::OscillatorNode::create(WebCore::BaseAudioContext&, WebCore::OscillatorOptions const&) UnifiedSource-f8afad56-52.cpp
    WebKit#9 0x7fc76d3bc210 in WebCore::BaseAudioContext::createOscillator() UnifiedSource-f8afad56-49.cpp
    WebKit#10 0x7fc76acbc743 in WebCore::jsBaseAudioContextPrototypeFunction_createOscillator(JSC::JSGlobalObject*, JSC::CallFrame*) UnifiedSource-3a52ce78-11.cpp
    WebKit#11 0x7fc705c10037  (<unknown module>)

* Source/WebCore/platform/audio/gstreamer/FFTFrameGStreamer.cpp:
(WebCore::FFTFrame::FFTFrame):
SunnyM00n pushed a commit to SunnyM00n/WebKit that referenced this pull request Apr 17, 2025
https://bugs.webkit.org/show_bug.cgi?id=281902
rdar://136486349

Reviewed by Mike Wyrzykowski.

Metal: Ensure potentially infinite loops have defined behavior

The MSL compiler would omit infinite loops and assume number domains
based on the omission logic. This would induce incorrect number domains
in case the infinite loops would be invokable. Infinite loops are
undefined in C++ and thus in MSL. It is the job of the programmer
to ensure undefined behavior cannot happen.

Consider GLSL loop like:
uniform float i;
...
    if (i != 0.5) for(;;) { }
    gl_FragColor = vec4(i);

Historically this would emit MSL loop in spirit of:
    if (i != 0.5) {
      bool c = true;
      while (c) { }
    }
    ANGLE_fragmentOut.gl_FragColor = metal::float4(i, i, i, i);

Since This could cause the MSL compiler to optimize the function to
equivalent of:
    ANGLE_fragmentOut.gl_FragColor = metal::float4(0.5, 0.5, 0.5, 0.5);

Presumably this loop omission would happen at the clang frontend part.

Before, was worked around by emitting asm statements to the MSL:
    bool c = true;
    while (c) {
        __asm__("");
    }

The asm injection would would work for this particular source pattern,
presumably because injecting the asm would avoid the loop omission at
the clang frontend part.

The MSL/C++ code is still UB, though. The asm statement does not cause
anything that C++ would consider as "forward progress" of the loop. The
success was just due to how the backend worked. The bitcode produced
would be similar to:

4:
    tail call void asm sideeffect "", ""() WebKit#6, !srcloc !28
    br label %4, !llvm.loop !29

Here, the compiler can be seen to simply fail to detect a loop that
does not make forward progress.

Considering GLSL of form:
uniform int f;
...
    for (;;) { if (f <= 1) break; }

With asm injection to the loop, this would produce:

5:
    tail call void asm sideeffect "", ""() WebKit#8, !srcloc !29
    %6 = load i32, i32 addrspace(2)* %4, align 4, !tbaa !30
    %7 = icmp slt i32 %6, 2
    br i1 %7, label %8, label %5
8:

This code is still assumed to make progress. The backend optimizer is
free to assume that the condition holds, since the load to break the
loop is from constant address space. I.e. uniform f does not change its
value during the loop.

Instead of injecting asm, inject a read of unused volatile variable.
The volatile variable access is defined in C++ as forward progress.
This means infinite loop containing such read is considered defined.
To simplify the implementation and to avoid volatile writes, the read
is to a dummy variable instead of the loop condition bool.

The tests here do not pass completely for MSL backend. In case the
compiler would omit the infinite loop (unpatched code), they would fail
with demonstration of how the values behave. After fixing, the loops
cause timeout but Metal backend does not have implementation to report
context loss. Also, the ReadPixels is just for demostration purposes of the
unpatched code.

* Source/ThirdParty/ANGLE/src/compiler/translator/msl/EmitMetal.cpp:
(GenMetalTraverser::GenMetalTraverser):
(GenMetalTraverser::emitLoopBody):
(GenMetalTraverser::emitForwardProgressStore):
(GenMetalTraverser::emitForwardProgressSignal):
(GenMetalTraverser::visitForLoop):
(GenMetalTraverser::visitWhileLoop):
(GenMetalTraverser::visitDoWhileLoop):
* Source/ThirdParty/ANGLE/src/tests/angle_end2end_tests.gni:
* Source/ThirdParty/ANGLE/src/tests/gl_tests/TimeoutDrawTest.cpp: Added.
(angle::TimeoutDrawTest::TimeoutDrawTest):
(angle::TEST_P):

Canonical link: https://commits.webkit.org/283286.350@safari-7620-branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant