Skip to content

fix: prevent DCE from removing throwing bigint unary minus#1934

Open
Kenaz123 wants to merge 1 commit intofacebook:static_hfrom
Kenaz123:fix-bint
Open

fix: prevent DCE from removing throwing bigint unary minus#1934
Kenaz123 wants to merge 1 commit intofacebook:static_hfrom
Kenaz123:fix-bint

Conversation

@Kenaz123
Copy link

Summary

Fix DCE incorrectly removing throwing unary minus on BigInt values.

Before this change, UnaryOperatorInst::getSideEffectImpl() treated any unary operation on a primitive operand as idempotent:

SideEffect UnaryOperatorInst::getSideEffectImpl() const {
  if (getSingleOperand()->getType().isPrimitive()) {
    return SideEffect{}.setIdempotent();
  }
  ...
}

That is wrong for UnaryMinusInst when the operand can be a BigInt. Negating a negative max-size BigInt may need one extra digit in the result, which throws RangeError: Maximum BigInt size exceeded. Because the instruction was marked
idempotent, DCE could delete an unused -bigint and silently suppress the exception under -O.

This change fixes that by marking unary minus as throwing when its operand canBeBigInt():

if (getKind() == ValueKind::UnaryMinusInstKind &&
    getSingleOperand()->getType().canBeBigInt()) {
  return SideEffect{}.setThrow();
}

poc.js

Original PoC reproduction:

./build-static_h/bin/hermes -O ./poc.js
./build-static_h/bin/hermes -O0 ./poc.js

Original output before the fix:

$ ./build-static_h/bin/hermes -O poc.js
Bug: RangeError was suppressed by DCE

$ ./build-static_h/bin/hermes -O0 poc.js
Uncaught RangeError: Maximum BigInt size exceeded

Test Plan

cmake -S ./hermes -B ./hermes/build-static_h -G Ninja -DCMAKE_BUILD_TYPE=Debug
cmake --build ./hermes/build-static_h --target hermes hermesc shermes -j4

./hermes/build-static_h/bin/hermes -O0 \
  ./hermes/test/shermes/regress-bigint-unary-minus-dce.js

./hermes/build-static_h/bin/hermes -O \
  ./hermes/test/shermes/regress-bigint-unary-minus-dce.js

./hermes/build-static_h/bin/hermesc -dump-ir -O \
  ./hermes/test/Optimizer/regress-bigint-unary-minus-dce.js

Observed results:

$ hermes -O0 test/shermes/regress-bigint-unary-minus-dce.js
RangeError: Maximum BigInt size exceeded

$ hermes -O test/shermes/regress-bigint-unary-minus-dce.js
RangeError: Maximum BigInt size exceeded

$ hermesc -dump-ir -O test/Optimizer/regress-bigint-unary-minus-dce.js
...
%2 = UnaryMinusInst (:bigint) 1: bigint
%4 = UnaryMinusInst (:number|bigint) %3: any
%5 = UnaryMinusInst (:number|bigint) %4: number|bigint
...

Thanks for looking into this and I appreciate any feedback!

@meta-cla meta-cla bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Do not delete this pull request or issue due to inactivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant