Skip to content

Conversation

@RoundofThree
Copy link
Member

When the PCC is sealed in purecap ABI, the LSB bit is set, leading to an off-by-one error in the computation of the offset from the instruction start to the current PCC address. This can cause errors such as in OptimizedFrame::LookupExceptionHandlerInTable, where the computed offset is used to look up an exception handler table.

Without this patch, executing the following JS code using ./d8 --allow-natives-syntax ~/v8-work/turbofan-exception-missing-code.js:

function a() { try { aaa } catch (e) {} }
a();
%PrepareFunctionForOptimization(a);
a();
%OptimizeFunctionOnNextCall(a);
a();
%GetOptimizationStatus(a);

leads to an exception after the function a() is optimised by Turbofan because OptimizedFrame::LookupExceptionHandlerInTable could not find the exception handler:

/home/zyj20/v8-work/turbofan-exception-missing-code.js:1: ReferenceError: aaa is not defined
function a() { try { aaa } catch (e) {} }
                     ^
ReferenceError: aaa is not defined
    at a (/home/zyj20/v8-work/turbofan-exception-missing-code.js:1:22)
    at /home/zyj20/v8-work/turbofan-exception-missing-code.js:6:1

With the patch, the script doesn't raise an exception as the ReferenceError is caught.

@RoundofThree RoundofThree requested a review from dstolfa July 22, 2025 00:10
@RoundofThree RoundofThree force-pushed the fix-optimised-exception-handler branch from bfa8f2c to 0fa711c Compare July 27, 2025 20:57
@RoundofThree RoundofThree requested a review from dstolfa July 27, 2025 20:59
@dstolfa
Copy link

dstolfa commented Jul 30, 2025

It appears that there is some discrepancy with test262 in particular with this patch. I haven't dug deeply into it, but here are the numbers:

Without the patch:

===
=== 1107 tests failed
===
>>> 57187 base tests produced 93466 (163%) non-filtered tests
>>> 93466 tests ran

With the patch:

===
=== 1754 tests failed
===
>>> 57187 base tests produced 93466 (163%) non-filtered tests
>>> 93466 tests ran

An example of a failing test with the patch can be seen:

${OUT}/d8 test/test262/data/test/built-ins/AsyncGeneratorPrototype/return/return-suspendedStart-broken-promise.js

test/test262/data/test/built-ins/AsyncGeneratorPrototype/return/return-suspendedStart-broken-promise.js:41: Error: broken promise
    throw new Error('broken promise');
    ^
Error: broken promise
    at Promise.get (test/test262/data/test/built-ins/AsyncGeneratorPrototype/return/return-suspendedStart-broken-promise.js:41:11)
    at g.return (<anonymous>)
    at test/test262/data/test/built-ins/AsyncGeneratorPrototype/return/return-suspendedStart-broken-promise.js:45:10

and without the patch:

${OUT}/d8 test/test262/data/test/built-ins/AsyncGeneratorPrototype/return/return-suspendedStart-broken-promise.js

test/test262/data/test/built-ins/AsyncGeneratorPrototype/return/return-suspendedStart-broken-promise.js:54: ReferenceError: $DONE is not defined
  .then($DONE, $DONE);
        ^
ReferenceError: $DONE is not defined
    at test/test262/data/test/built-ins/AsyncGeneratorPrototype/return/return-suspendedStart-broken-promise.js:54:9

test/test262/data/test/built-ins/AsyncGeneratorPrototype/return/return-suspendedStart-broken-promise.js:51: ReferenceError: assert is not defined
      assert.sameValue(err.message, 'broken promise');
      ^
ReferenceError: assert is not defined
    at test/test262/data/test/built-ins/AsyncGeneratorPrototype/return/return-suspendedStart-broken-promise.js:51:7

1 pending unhandled Promise rejection(s) detected.

It may be that without this patch, the test worked accidentally. However, the output of this particular test in the latest commit in upstream V8 as of today seems to match, so I'm inclined to believe that something else is going on:

test/test262/data/test/built-ins/AsyncGeneratorPrototype/return/return-suspendedStart-broken-promise.js:54: ReferenceError: $DONE is not defined
  .then($DONE, $DONE);
        ^
ReferenceError: $DONE is not defined
    at test/test262/data/test/built-ins/AsyncGeneratorPrototype/return/return-suspendedStart-broken-promise.js:54:9

test/test262/data/test/built-ins/AsyncGeneratorPrototype/return/return-suspendedStart-broken-promise.js:51: ReferenceError: assert is not defined
      assert.sameValue(err.message, 'broken promise');
      ^
ReferenceError: assert is not defined
    at test/test262/data/test/built-ins/AsyncGeneratorPrototype/return/return-suspendedStart-broken-promise.js:51:7

1 pending unhandled Promise rejection(s) detected.

…fsetFromInstructionStart

When the PCC is sealed in purecap ABI, the LSB bit is set, leading
to an off-by-one error in the computation of the offset from the
instruction start to the current PCC address. This can cause errors
such as in OptimizedFrame::LookupExceptionHandlerInTable, where the
computed offset is used to look up an exception handler table.
@RoundofThree RoundofThree force-pushed the fix-optimised-exception-handler branch from 0fa711c to e04b2bb Compare August 2, 2025 13:31
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.

3 participants