Skip to content

V
V8
Switch branch/tag
  1. 02 Mar, 2021 5 commits
    • Benedikt Meurer's avatar
      [debug] Don't put a source position on internal `Return`s. · 06359f74
      Benedikt Meurer authored 
      Be explicit about source positions for `Return`s in the
BytecodeGenerator, and only do self-healing explicitly in the
`ReturnStatement` translation, where an end position of
`kNoSourcePosition` is turned into the return position of the
function literal.

This allows us to reason more easily about which `Return`s actually
receive a meaningful source position, and in particular it allows us
to construct the internal `Return`s for `yield` and `yield*` with no
source position attached to them. Previously they'd get the source
position for the implicit (final) return attached to it, which confused
the debugger and led to breakpoints being set in the completely wrong
spot.

Considering the simplified example

```
function* foo(){
  var a = 1;
}
```

this would previously generate the following bytecode

```
        0 : SwitchOnGeneratorState r0, [0], [1] { 0: @20 }
        4 : Mov <closure>, r2
        7 : Mov <this>, r3
 13 E> 10 : InvokeIntrinsic [_CreateJSGeneratorObject], r2-r3
       14 : Star0
 13 E> 15 : SuspendGenerator r0, r0-r1, [0]
       20 : ResumeGenerator r0, r0-r1
       24 : Star2
       25 : InvokeIntrinsic [_GeneratorGetResumeMode], r0-r0
       29 : SwitchOnSmiNoFeedback [1], [2], [0] { 0: @39, 1: @36 }
       33 : Ldar r2
 13 E> 35 : Throw
       36 : Ldar r2
 30 S> 38 : Return    <=========================== internal Return
 27 S> 39 : LdaSmi [1]
       41 : Star1
       42 : LdaUndefined
 30 S> 43 : Return
```

where everything between offset 4 and 42 corresponds to the implicit
yield at the beginning of every generator function, in particular the
code between 20 and 42 corresponds to that initial yields resumption
logic. Notice how the internal Return at offset 38 gets assigned the
source position of the function literal (the same as the implicit
return at the end). This confuses the debugger quite a bit when trying
to set a breakpoint on the closing brace, since it's going in bytecode
order and will thus discover the `Return` at offset 38 first (matching
the source position 30 it's currently looking for) and setting the
breakpoint there. This `Return` bytecode however is only executed when
the generator is resumed via `GeneratorPrototype.return()`, and it'll
not hit when the developer uses the generator normally, which is not
the desired behavior and extremely confusing (especially since stepping
on the other hand works as expected).

With this patch, we no longer slap a source position (and in particular
not the function literal's return position) onto these internal
`Return`s as you can see from the generated bytecode below:

```
       0 : SwitchOnGeneratorState r0, [0], [1] { 0: @20 }
       4 : Mov <closure>, r2
       7 : Mov <this>, r3
13 E> 10 : InvokeIntrinsic [_CreateJSGeneratorObject], r2-r3
      14 : Star0
13 E> 15 : SuspendGenerator r0, r0-r1, [0]
      20 : ResumeGenerator r0, r0-r1
      24 : Star2
      25 : InvokeIntrinsic [_GeneratorGetResumeMode], r0-r0
      29 : SwitchOnSmiNoFeedback [1], [2], [0] { 0: @39, 1: @36 }
      33 : Ldar r2
13 E> 35 : Throw
      36 : Ldar r2
      38 : Return
27 S> 39 : LdaSmi [1]
      41 : Star1
      42 : LdaUndefined
30 S> 43 : Return
```

This also allows us to remove the break position finding hack that was
kept in BreakIterator::BreakIndexFromPosition() for generators and
modules.

Fixed: chromium:901819
Change-Id: If19a6b26e2622d49b6b5e54bf7a162747543f970
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2727820Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73119}
      06359f74
    • Dan Elphick's avatar
      [build] Split out EmbeddedFileWriterInterface · be5ccde2
      Dan Elphick authored 
      To reduce gn check errors, this moves EmbeddedFileWriterInterface into
its own header file that can be included directly by isolate.cc since
embedded-file-writer.h is in the mksnapshot target.

Bug: v8:7330
Change-Id: I3bfb1f48c646680b71189e8229b8df54ead9eea0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2727817
Auto-Submit: Dan Elphick <delphick@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73118}
      be5ccde2
    • Dan Elphick's avatar
      [build] Move wasm constants files into v8_shared_internal_headers · 179ee5f2
      Dan Elphick authored 
      Moves src/wasm/wasm-constants.h and src/wasm/wasm-limits.h into
v8_shared_internal_headers so v8_flags can access them.

Bug: v8:7330
Change-Id: I322ac483d26a03fd79e9961678462227a89e594b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2727498
Auto-Submit: Dan Elphick <delphick@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: 's avatarMichael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73117}
      179ee5f2
    • v8-ci-autoroll-builder's avatar
      Update V8 DEPS. · 547c37f5
      v8-ci-autoroll-builder authored 
      Rolling v8/build: https://chromium.googlesource.com/chromium/src/build/+log/06d89c7..a1f3c3d

Rolling v8/buildtools/third_party/libunwind/trunk: https://chromium.googlesource.com/external/github.com/llvm/llvm-project/libunwind/+log/8d5fb6f..201ff85

Rolling v8/third_party/aemu-linux-x64: 12utHdzpAJMv14HvzgxQA19OLkI0UrGervsCs46ESbIC..gUyZgCT7R0DTHyu0Lq1Q7yrbbA9cage7ltjq_oN3mXkC

Rolling v8/third_party/catapult: https://chromium.googlesource.com/catapult/+log/df7850d..cae643a

Rolling v8/third_party/depot_tools: https://chromium.googlesource.com/chromium/tools/depot_tools/+log/e952fae..6ac4a15

TBR=v8-waterfall-sheriff@grotations.appspotmail.com

Change-Id: Ic0e83640067bc37ed7b5ec5d35c6118e5e649e72
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2728641Reviewed-by: 's avatarv8-ci-autoroll-builder <v8-ci-autoroll-builder@chops-service-accounts.iam.gserviceaccount.com>
Commit-Queue: v8-ci-autoroll-builder <v8-ci-autoroll-builder@chops-service-accounts.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#73116}
      547c37f5
    • Frank Tang's avatar
      Revert "[test] Disable tests failing with new msan roll" · 13ac253a
      Frank Tang authored 
      This reverts commit 57ae048b.

Reason for revert: bug fixed in https://chromium-review.googlesource.com/c/v8/v8/+/2706353

Original change's description:
> [test] Disable tests failing with new msan roll
>
> Temporarily disable these tests failing on msan builds after latest
> roll:
> - test262/intl402/DateTimeFormat/timezone-invalid
> - intl/regress-364374
> - mjsunit/regress/regress-crbug-627935
>
> No-Try: true
> No-Tree-Checks: true
> Bug: v8:11438
> Change-Id: I4a7755f9f65b2e9a12463c9e12fbbe39d3f5efb2
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2692188
> Reviewed-by: Clemens Backes <clemensb@chromium.org>
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Auto-Submit: Nico Hartmann <nicohartmann@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#72691}

Bug: v8:11438
Change-Id: Iaca0a401a2c6d89e1bc8292ad41ae0086943c635
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2724862Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Reviewed-by: 's avatarNico Hartmann <nicohartmann@chromium.org>
Commit-Queue: Frank Tang <ftang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73115}
      13ac253a
  3. 01 Mar, 2021 28 commits
  5. 28 Feb, 2021 1 commit
  7. 27 Feb, 2021 2 commits
  9. 26 Feb, 2021 4 commits