RESOLVED FIXED299242
[JSC] Math.random JIT path uses arithmetic instead of logical right shifts
https://bugs.webkit.org/show_bug.cgi?id=299242
Summary [JSC] Math.random JIT path uses arithmetic instead of logical right shifts
oze41998
Reported 2025-09-20 09:25:51 PDT
This report concerns Math.random output when taking the JIT path. Currently, state advancement in emitRandomThunkImpl (Source/JavaScriptCore/jit/AssemblyHelpers.cpp, around line 847) uses arithmetic right shifts (rshift64), whereas all other advancement paths use logical right shifts. Reference implementation (WeakRandom): WeakRandom::nextState uses the >> operator on a uint64_t, which performs a logical shift. Source: Source/WTF/wtf/WeakRandom.h, line 109. FTL lowering path: compileArithRandom uses lShr (logical shift right). Source: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp, line 3501. JIT path (problem): emitRandomThunkImpl calls jit.rshift64, which emits SAR (arithmetic shift). Source: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h, line 5311. By contrast, urshift64 emits SHR (logical shift). Source: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h, line 5350. This matches the intended semantics. Impact: This discrepancy means that Math.random produces different output depending on whether the JIT path is taken. The bug is not immediately obvious because both arithmetic and logical right shifts still generate values within the expected [0, 1) range, and the distribution may look reasonable at a glance. However, the underlying state advancement diverges from the intended algorithm, which leads to inconsistent and incorrect behavior compared to other code paths. Proposed fix: Replace calls to rshift64 with urshift64 inside emitRandomThunkImpl. A pull request for this fix will follow.
Attachments
oze41998
Comment 1 2025-09-20 20:03:44 PDT
Radar WebKit Bug Importer
Comment 2 2025-09-27 09:26:11 PDT
Radar WebKit Bug Importer
Comment 3 2025-09-27 09:26:11 PDT
EWS
Comment 4 2025-09-30 16:28:39 PDT
Committed 300785@main (cf0e17e68442): <https://commits.webkit.org/300785@main> Reviewed commits have been landed. Closing PR #51077 and removing active labels.
Note You need to log in before you can comment on or make changes to this bug.