Skip to content

Commit

Permalink
[JSC] Wrong B3 range analysis on 64-bit values
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=262224
rdar://115897433

Reviewed by Mark Lam.

This patch fixes B3's range analysis. When using 64bit value, we should use INT64_MIN / INT64_MAX instead of INT_MIN / INT_MAX.
We use std::numeric_limits to make it work. We also adjust `+ 1` check to avoid potential UB.

* Source/JavaScriptCore/b3/B3ReduceStrength.cpp:
* Source/JavaScriptCore/b3/testb3.h:
* Source/JavaScriptCore/b3/testb3_1.cpp:
(run):
* Source/JavaScriptCore/b3/testb3_5.cpp:
(testCheckAdd64Range):

Originally-landed-as: 267815.118@safari-7617-branch (3e7f362). rdar://119594339
Canonical link: https://commits.webkit.org/272223@main
  • Loading branch information
Constellation authored and robert-jenner committed Dec 18, 2023
1 parent a0e2451 commit 7736ce7
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 3 deletions.
8 changes: 5 additions & 3 deletions Source/JavaScriptCore/b3/B3ReduceStrength.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,12 @@ class IntRange {
template<typename T>
static IntRange rangeForMask(T mask)
{
if (!(mask + 1))
if (mask == static_cast<T>(-1))
return top<T>();
if (mask < 0)
return IntRange(INT_MIN & mask, mask & INT_MAX);
if constexpr (std::is_signed_v<T>) {
if (mask < 0)
return IntRange(std::numeric_limits<T>::min() & mask, mask & std::numeric_limits<T>::max());
}
return IntRange(0, mask);
}

Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/b3/testb3.h
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,7 @@ void testCheckAddImmCommute();
void testCheckAddImmSomeRegister();
void testCheckAdd();
void testCheckAdd64();
void testCheckAdd64Range();
void testCheckAddFold(int, int);
void testCheckAddFoldFail(int, int);
void test42();
Expand Down
1 change: 1 addition & 0 deletions Source/JavaScriptCore/b3/testb3_1.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,7 @@ void run(const TestConfig* config)
RUN(testCheckAddImmSomeRegister());
RUN(testCheckAdd());
RUN(testCheckAdd64());
RUN(testCheckAdd64Range());
RUN(testCheckAddFold(100, 200));
RUN(testCheckAddFoldFail(2147483647, 100));
RUN(testCheckAddArgumentAliasing64());
Expand Down
26 changes: 26 additions & 0 deletions Source/JavaScriptCore/b3/testb3_5.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -745,6 +745,32 @@ void testCheckAdd64()
CHECK(invoke<double>(*code, 9223372036854775807ll, 42ll) == static_cast<double>(9223372036854775807ll) + 42.0);
}

void testCheckAdd64Range()
{
Procedure proc;
BasicBlock* root = proc.addBlock();

Value* x0 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0);
Value* b1 = root->appendNew<Value>(proc, JSC::B3::BitAnd, Origin(), x0, root->appendNew<Const64Value>(proc, Origin(), 0xffffffff00000000LL));
Value* b2 = root->appendNew<Value>(proc, JSC::B3::Sub, Origin(), b1, root->appendNew<Const64Value>(proc, Origin(), 0xffffffff00000000LL));
Value* b3 = root->appendNew<Value>(proc, JSC::B3::ZShr, Origin(), b2, root->appendNew<Const32Value>(proc, Origin(), 28));
Value* b4 = root->appendNew<Const64Value>(proc, Origin(), 0x7fffffffffffff00LL);

CheckValue* checkAdd = root->appendNew<CheckValue>(proc, CheckAdd, Origin(), b3, b4);
checkAdd->setGenerator(
[&] (CCallHelpers& jit, const StackmapGenerationParams&) {
AllowMacroScratchRegisterUsage allowScratch(jit);
jit.move(CCallHelpers::TrustedImm32(42), GPRInfo::returnValueGPR);
jit.emitFunctionEpilogue();
jit.ret();
});
root->appendNewControlValue(proc, Return, Origin(), checkAdd);

auto code = compileProc(proc);

CHECK(invoke<int64_t>(*code, 0x8ffffffe00000000LL) == 42.0);
}

void testCheckAddFold(int a, int b)
{
Procedure proc;
Expand Down

0 comments on commit 7736ce7

Please sign in to comment.