Skip to content

Core: Fix missing second frame in QUnit.stack() in Safari#1776

Merged
Krinkle merged 1 commit intoqunitjs:mainfrom
Krinkle:fix-stack-safari-tco
Jul 5, 2024
Merged

Core: Fix missing second frame in QUnit.stack() in Safari#1776
Krinkle merged 1 commit intoqunitjs:mainfrom
Krinkle:fix-stack-safari-tco

Conversation

@Krinkle
Copy link
Copy Markdown
Member

@Krinkle Krinkle commented Jul 5, 2024

Safari implements ES6 Tail-Call Optimization, which is when:

  • the file is in strict mode,
  • and, in a regular function (not async or generator),
  • and, the return statement ends in a simple function call.

Then, the current function is removed from the stack before the child function begins. TCO applies even for calls that are not recursive.

The result is that, given:

makeFakeFailure -> exampleMain -> exampleParent -> exampleCurrent ->
QUnit.stack -> sourceFromStacktrace -> new Error.

In Firefox and Chrome, e.stack is:

[0] sourceFromStacktrace (SLICED)
[1] QUnit.stack          (SLICED)
[2] exampleCurrent
[3] exampleParent
[4] exampleMain
[5] makeFakeFailure

But, in Safari, the second frame gets lost because our tiny QUnit.stack() function is a candidate for Tail-Call Optimization.

[0] sourceFromStacktrace (SLICED)
[1] exampleCurrent       (SLICED)
[2] exampleParent
[3] exampleMain
[4] makeFakeFailure

This, combined with the fact that we strip the first two frames as a way to hide internal offsets, meant that in Safari we ended up attributing failed assertions and test definitions to the parent of the caller rather than the actual caller, e.g. exampleParent() instead of exampleCurrent.

Ref https://bugs.webkit.org/show_bug.cgi?id=276187.

Safari implements ES6 Tail-Call Optimization, which is when:
- the file is in strict mode,
- and, in a regular function (not async or generator),
- and, the return statement ends in a simple function call.

Then, the current function is removed from the stack before the
child function begins. TCO applies even for calls that are not
recursive.

The result is that, given:

> makeFakeFailure -> exampleMain -> exampleParent -> exampleCurrent ->
> QUnit.stack -> sourceFromStacktrace -> new Error.

In Firefox and Chrome, `e.stack` is:

```
[0] sourceFromStacktrace (SLICED)
[1] QUnit.stack          (SLICED)
[2] exampleCurrent
[3] exampleParent
[4] exampleMain
[5] makeFakeFailure
```

But, in Safari, the second frame gets lost because our tiny
`QUnit.stack()` function is a candidate for Tail-Call Optimization.

```
[0] sourceFromStacktrace (SLICED)
[1] exampleCurrent       (SLICED)
[2] exampleParent
[3] exampleMain
[4] makeFakeFailure
```

This, combined with the fact that we strip the first two frames as
a way to hide internal offsets, meant that in Safari we ended up
attributing failed assertions and test definitions to the parent of
the caller rather than the actual caller, e.g. exampleParent() instead
of exampleCurrent.

Ref https://bugs.webkit.org/show_bug.cgi?id=276187.
@Krinkle Krinkle merged commit 52f37b8 into qunitjs:main Jul 5, 2024
@Krinkle Krinkle deleted the fix-stack-safari-tco branch July 5, 2024 04:40
Krinkle added a commit that referenced this pull request Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant