1. 28 Jul, 2020 1 commit
    • Clemens Backes's avatar
      [wasm] Fix flake about missed breakpoints · dfd86b05
      Clemens Backes authored
      If multiple isolates were involved, we did not always hit the breakpoint
      reliably in all isolates.
      
      This CL fixes this flake this via two changes:
      
      1. Remove breakpoint info when tiering up.
         If we keep the breakpoint information, a second isolate that later
         sets the same breakpoint will see that the breakpoint already exists,
         and will not set it again, even though the code containing the
         breakpoint has been replaced at that point.
         This fixes a flake in the debug/wasm/breakpoints test.
      
      2. Don't overwrite code with breakpoints by default "tiered down" code.
         This is achieved by introducing another state in the {ForDebugging}
         enum which marks that code contains breakpoints. Otherwise it could
         happen that two isolates start tiering down (both recompiling missing
         functions in Liftoff), one isolate finishes and immediately sets a
         breakpoint, then the other isolates finishes and overwrites the code
         with breakpoints by the usual {kForDebugging} code.
         Setting breakpoints is synchronized already, so overwriting
         breakpoint code with other breakpoint code is always safe.
      
      R=thibaudm@chromium.org
      
      Bug: v8:10611, v8:10359
      Change-Id: I171d86b110a54f9eb5e4c3fa35108638904212e8
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2316080
      Commit-Queue: Clemens Backes <clemensb@chromium.org>
      Reviewed-by: 's avatarThibaud Michaud <thibaudm@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#69088}
      dfd86b05
  2. 21 Jul, 2020 2 commits
  3. 29 Jun, 2020 1 commit
  4. 24 Jun, 2020 2 commits
    • Clemens Backes's avatar
      [wasm] Make opcode properties constexpr · 852f43cd
      Clemens Backes authored
      This allows the compiler to eliminate more unneeded branches. Since all
      functions just do a lookup in a static table (either directly, or via
      compiling a switch to such a lookup), they are also good candidates for
      inlining, which is made possible by this change.
      
      One DCHECK is removed instead of pulling in the inl header, which would
      require more refactoring since the check is in a non-inl header.
      
      R=thibaudm@chromium.org
      TBR=jkummerow@chromium.org
      
      Bug: v8:10576
      Change-Id: If0fd25fd62c5f30b896fc67a5458a5ae475a6351
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2259944
      Commit-Queue: Clemens Backes <clemensb@chromium.org>
      Reviewed-by: 's avatarThibaud Michaud <thibaudm@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#68508}
      852f43cd
    • Maya Lekova's avatar
      [gcmole] Enable use-after-free detection · e7606e6b
      Maya Lekova authored
      GCMole now comes with the long forgotten use-after-free detection
      enabled by default. The CL also improves error logging when test
      expectations mismatch with the actual output and updates the hash
      of GCMole to be used with the newly built version with enabled UAF
      detection.
      
      The CL also contains an ignore for isolate.cc due to inability to
      fix a warning there and fixes a couple of UAF warnings.
      
      Bug: v8:9680
      Change-Id: I7a009ffd5f67b1b5437567691ca4235ea873de70
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2257236
      Commit-Queue: Maya Lekova <mslekova@chromium.org>
      Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
      Reviewed-by: 's avatarMichael Achenbach <machenbach@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#68505}
      e7606e6b
  5. 23 Jun, 2020 1 commit
  6. 18 Jun, 2020 1 commit
    • Manos Koukoutos's avatar
      [wasm-gc] Change ValueType representation to account for new types · 52f65296
      Manos Koukoutos authored
      Motivation:
      Changes to the typed function references and gc proposals solidified
      the notion of heap type, clarified nullable vs. non-nullable reference
      types, and introduced rtts, which contain an integer depth field in
      addition to a heap type. This required us to overhaul our ValueType
      representation, which results in extensive changes.
      
      To keep this CL "small", we do not try to implement the binary encoding
      as described in the proposals, but rather devise a simpler one of our
      own (see below). Also, we do not try to implement additional
      functionality for the new types.
      
      Changes:
      - Introduce HeapType. Move heap types from ValueType to HeapType.
      - Introduce Nullability for reference types.
      - Rework ValueType helper methods.
      - Introduce rtts in ValueType with an integer depth field. Include depth
        in the ValueType encoding.
      - Make the constructor of ValueType private, instead expose static
        functions which explicitly state what they create.
      - Change every switch statement on ValueType::Kind. Sometimes, we need
        nested switches.
      - Introduce temporary constants in ValueTypeCode for nullable types,
        use them for decoding.
      - In WasmGlobalObject, split 'flags' into 'raw_type' and 'is_mutable'.
      - Change IsSubtypeOfRef to IsSubtypeOfHeap and implement changes in
        subtyping.
      - kWasmFuncRef initializers are now non-nullable. Initializers are
        only required to be subtypes of the declared global type.
      - Change tests and fuzzers as needed.
      
      Bug: v8:7748
      Change-Id: If41f783bd4128443b07e94188cea7dd53ab0bfa5
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2247657
      Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
      Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
      Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#68408}
      52f65296
  7. 17 Jun, 2020 1 commit
  8. 15 Jun, 2020 1 commit
  9. 09 Jun, 2020 4 commits
  10. 06 Jun, 2020 1 commit
  11. 05 Jun, 2020 2 commits
    • Clemens Backes's avatar
      [wasm][debug] Avoid recompilation for existing breakpoint · c159f097
      Clemens Backes authored
      If multiple workers are sharing the same module, the DevTools frontend
      will set the same breakpoints in all of them, but one after another.
      This CL tries to avoid repeated recompilation of that function in most
      cases. Only if we need special source positions for stack rewriting, we
      need to compile a special version.
      
      R=thibaudm@chromium.org
      
      Bug: v8:10359
      Change-Id: I06114d6feb2030b75dcbde91c62b822f1807ad6e
      Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel
      Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel_ng
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2231339
      Commit-Queue: Clemens Backes <clemensb@chromium.org>
      Reviewed-by: 's avatarThibaud Michaud <thibaudm@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#68213}
      c159f097
    • Clemens Backes's avatar
      [wasm][interpreter] Remove threads support · 47e501e1
      Clemens Backes authored
      The wasm interpreter was always single-threaded, and there are no plans
      to change this. Still, there was a concept of threads, but with the
      hard-coded constraint that there is always exactly one of them.
      
      In order to clean up the code, and as a preparation to remove more
      unneeded functionality before moving the interpreter over to the test
      directory, this CL removes the concept of threads and merges the
      {ThreadImpl} class into {WasmInterpreterInternals}.
      
      Drive-by: Remove the dead {GetFrameCount} method.
      
      R=ahaas@chromium.org
      
      Bug: v8:10389
      Change-Id: If65cdd21b34ce8debf8ba0f24dbeacec15e0a1d7
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2231354Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
      Commit-Queue: Clemens Backes <clemensb@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#68204}
      47e501e1
  12. 03 Jun, 2020 1 commit
  13. 02 Jun, 2020 2 commits
    • Clemens Backes's avatar
      [wasm][interpreter] Remove activations · 38948b8e
      Clemens Backes authored
      Since the interpreter cannot call out to JS any more, there cannot be
      more than one activation at a time. Hence remove the concept of
      activations.
      
      R=ahaas@chromium.org
      
      Bug: v8:10389
      Change-Id: Ifda5624e192464a1aed2943787bc6860d1917719
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2219942Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
      Commit-Queue: Clemens Backes <clemensb@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#68118}
      38948b8e
    • Clemens Backes's avatar
      [wasm][debug] Support multi-threaded breakpoints · 5fcb414a
      Clemens Backes authored
      This adds support for multiple isolates sharing the same module but
      setting different breakpoints. This is simulated by having a debugger
      test that runs in the "--isolates" variant, i.e. two isolates running
      the same test at the same time. Both isolates will set and remove
      breakpoints.
      
      The DebugInfo will keep a separate list of breakpoints per isolate, and
      when recompiling a function for debugging it will respect all
      breakpoints in all isolates.
      In order to ensure consistency if multiple isolates are setting or
      removing breakpoints simultaneously, we go back to a more coarse-grained
      locking scheme, where the DebugInfo lock is held while re-compiling
      Liftoff functions.
      
      While recompilation will install the code in the module-global code
      table and jump table (and hence all isolates will use it for future
      calls), only the stack of the requesting isolate is rewritten to
      immediately use new code. This is OK, because other isolates are not
      interested in the new breakpoint(s) anyway.
      On {SetBreakpoint}, we always need to rewrite the stack of the
      requesting isolate though, even if the breakpoint was set before by
      another isolate.
      
      Drive-by: Some fixes in SharedFunctionInfo in order to support setting
      breakpoints via the Debug mirror.
      
      R=thibaudm@chromium.org
      
      Bug: v8:10359
      Change-Id: If659afb273260fc5e8124b4b617fb4322de473c7
      Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel
      Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel_ng
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2218059Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
      Reviewed-by: 's avatarThibaud Michaud <thibaudm@chromium.org>
      Commit-Queue: Clemens Backes <clemensb@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#68096}
      5fcb414a
  14. 28 May, 2020 1 commit
    • Clemens Backes's avatar
      [wasm][debug] Support multi-threaded stepping · 97434791
      Clemens Backes authored
      Instead of keeping a single {stepping_frame_} per native module, we now
      keep one frame id per isolate. Hence, each isolate can step through a
      different frame, independent of other isolates.
      The on-stack-replacement of the stepping frame already works on a
      per-isolate basis, since we only replace the return address of a single
      frame, part of the isolate that requested stepping.
      
      The new test (which also executes in a variant with two concurrent
      isolates) revealed some more data races to fix.
      
      R=thibaudm@chromium.org
      
      Bug: v8:10359
      Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel
      Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel_ng
      Change-Id: I0bb013737162bd09b9f4be9c08990bca7bf736ac
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2214838Reviewed-by: 's avatarThibaud Michaud <thibaudm@chromium.org>
      Commit-Queue: Clemens Backes <clemensb@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#68045}
      97434791
  15. 26 May, 2020 3 commits
  16. 19 May, 2020 1 commit
  17. 18 May, 2020 2 commits
  18. 13 May, 2020 1 commit
  19. 12 May, 2020 1 commit
    • Clemens Backes's avatar
      [wasm][debug] Only inspect code generated for debugging · cfe1b64b
      Clemens Backes authored
      Liftoff code generated for debugging has an extended function prologue
      which checks the "hook on function entry" flag on the isolate. Because
      of this, code positions between standard Liftoff code and Liftoff code
      for debugging do not match up. When (lazily) generating debug side
      tables, we always generate them for debugging-flavored Liftoff code.
      
      The issue that this CL fixes happened when we tried to inspect non-debug
      Liftoff code, and lazily generated the debug side table for that code.
      As noted above, source positions would not match up in that case, and we
      get DCHECK failures (or crashes in release builds) when inspecting the
      code.
      
      This issue was uncovered as part of the multi-threaded debugging effort,
      but because of the similarity in the stack trace, it might also fix the
      other issues linked below. We will get test coverage as soon as we add
      multi-threaded debugging tests (which are in development, but are still
      hitting other issues).
      
      R=thibaudm@chromium.org
      
      Bug: v8:10359, chromium:1071757, chromium:1079328, chromium:1072839
      Change-Id: Ic0c14e635dc2a0b84ac86ceb6650288202dafedc
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2196349
      Commit-Queue: Clemens Backes <clemensb@chromium.org>
      Reviewed-by: 's avatarThibaud Michaud <thibaudm@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#67756}
      cfe1b64b
  20. 11 May, 2020 1 commit
  21. 07 May, 2020 1 commit
  22. 06 May, 2020 1 commit
  23. 05 May, 2020 1 commit
    • Clemens Backes's avatar
      [wasm] Remove interpreter entry code · 61c2a0f4
      Clemens Backes authored
      This removes the interpreter entry stubs, which are used to redirect
      specific wasm functions to the interpreter. It is only needed when
      mixing JS code with interpreted Wasm code, otherwise the test functions
      just call the interpreter directly.
      Thus a lot of tests that contain such interaction between JS and Wasm
      need to be restricted to execute in Liftoff and TurboFan only.
      
      After this CL, the WASM_INTERPRETER_ENTRY frame type and the
      corresponding WasmInterpreterEntryFrame are dead, and will be removed in
      a follow-up CL.
      
      R=thibaudm@chromium.org
      
      Bug: v8:10389
      Change-Id: I8e50d350dbc2afcc1cddaeb98baf23711117af2d
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2172962
      Commit-Queue: Clemens Backes <clemensb@chromium.org>
      Reviewed-by: 's avatarThibaud Michaud <thibaudm@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#67559}
      61c2a0f4
  24. 28 Apr, 2020 3 commits
  25. 24 Apr, 2020 1 commit
  26. 20 Apr, 2020 1 commit
  27. 17 Apr, 2020 2 commits
    • Clemens Backes's avatar
      [wasm][debug] Don't publish code compiled for stepping · fb403653
      Clemens Backes authored
      This adds another enum value in the {ForDebugging} enum for stepping
      code.
      By not adding the code to the code table and jump table, we will never
      execute this code via a wasm function call. The code will only be used
      for the one frame where we want to step through.
      This speeds up stepping over recursive calls enormously, since the
      recursive calls don't run into the flooded breakpoints any more.
      It also fixes issues with non-local control flow, i.e. catching a trap
      and reentering the same wasm function.
      
      R=thibaudm@chromium.org
      
      Bug: v8:10235
      Change-Id: Idb304dd465418f842016a20c21d68989bb78cf1d
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2153205
      Commit-Queue: Clemens Backes <clemensb@chromium.org>
      Reviewed-by: 's avatarThibaud Michaud <thibaudm@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#67205}
      fb403653
    • Clemens Backes's avatar
      [wasm][debug] Only patch a single frame for stepping · c66cd826
      Clemens Backes authored
      Stepping only happens in one frame at a time, so we don't need to
      rewrite the whole stack. This allows us to remove the
      {flooded_function_index_}, since no function is globally flooded any
      more.
      A follow-up CL will ensure that the code will also not be installed in
      the code table and jump table any more, to fix issues with non-local
      control flow (i.e. catching a trap and reentering wasm), where we
      could currently accidentally execute flooded code. It will also speed
      up stepping over recursive calls enormously, since the recursive calls
      don't run into the flooded breakpoints any more.
      
      R=thibaudm@chromium.org
      
      Bug: v8:10235
      Change-Id: Ifae5e35c3242c95e1fe1a89a169ce874b818a288
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2152646Reviewed-by: 's avatarThibaud Michaud <thibaudm@chromium.org>
      Commit-Queue: Clemens Backes <clemensb@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#67202}
      c66cd826