1. 26 Mar, 2019 1 commit
  2. 20 Mar, 2019 1 commit
    • Leszek Swirski's avatar
      Revert "V8 x64 backend doesn't emit ABI compliant stack frames" · 9f6ddb48
      Leszek Swirski authored
      This reverts commit 3cda21de.
      
      Reason for revert: Breaks the roll on Windows (see https://cr-buildbucket.appspot.com/build/8918477701097622400)
      
      Original change's description:
      > V8 x64 backend doesn't emit ABI compliant stack frames
      > 
      > On 64 bit Windows, the OS stack walking does not work because the V8 x64
      > backend doesn't emit unwinding info and also because it doesn't emit ABI
      > compliant stack frames. See
      > https://docs.google.com/document/d/1-wf50jFlii0c_Pr52lm2ZU-49m220nhYMrHDi3vXnh0/edit
      > for more details.
      > 
      > This problem can be fixed by observing that V8 frames usually all have the same
      > prolog and epilog:
      > 
      > push rbp,
      > mov rbp, rsp
      > ...
      > pop rbp
      > ret N
      > 
      > and that it is possible to define XDATA (UNWIND_CODEs) that specify how Windows
      > should walk through V8 frames. Furthermore, since V8 Code objects are all
      > allocated in the same code-range for an Isolate, it is possible to register a
      > single PDATA/XDATA entry to cover stack walking for all the code generated
      > inside that code-range.
      > 
      > This PR contains changes required to enable stack walking on Win64:
      > 
      > EmbeddedFileWriter now adds assembler directives to the builtins
      > snapshot source file (embedded.cc) to emit additional entries in the .pdata and
      > in the .xdata section of the V8 executable. This takes care of stack walking
      > for embedded builtins. (The case of non-embedded builtins is not supported).
      > The x64 Assembler has been modified to collect the information required to emit
      > this unwind info for builtins.
      > 
      > Stack walking for jitted code is handled is Isolate.cpp, by registering
      > dynamically PDATA/XDATA for the whole code-range address space every time a new
      > Isolate is initialized, and by unregistering them when the Isolate is
      > destroyed.
      > 
      > Stack walking for WASM jitted code is handled is the same way in
      > wasm::NativeModule (wasm/wasm-code-manager.cpp).
      > 
      > It is important to note that Crashpad and Breakpad are already registering
      > PDATA/XDATA to manage and report unhandled exceptions (but not for embedded
      > builtins). Since it is not possible to register multiple PDATA entries for the
      > same address range, a new function is added to the V8 API:
      > SetUnhandledExceptionCallback() can be used by an embedder to register its own
      > unhandled exception handler for exceptions that arise in v8-generated code.
      > V8 embedders should be modified accordingly (code for this is in a separate PR
      > in the Chromium repository:
      > https://chromium-review.googlesource.com/c/chromium/src/+/1474703).
      > 
      > All these changes are experimental, behind:
      > 
      > the 'v8_win64_unwinding_info' build flag, and
      > the '--win64-unwinding-info' runtime flag.
      > 
      > Bug: v8:3598
      > Change-Id: Iea455ab6d0e2bf1c556aa1cf870841d44ab6e4b1
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1469329
      > Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
      > Reviewed-by: Jakob Gruber <jgruber@chromium.org>
      > Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
      > Commit-Queue: Paolo Severini <paolosev@microsoft.com>
      > Cr-Commit-Position: refs/heads/master@{#60330}
      
      TBR=bbudge@chromium.org,ulan@chromium.org,mvstanton@chromium.org,mstarzinger@chromium.org,gdeepti@chromium.org,jgruber@chromium.org,paolosev@microsoft.com
      
      Change-Id: If8470da94c58df8c800cbe8887f9f86236e43353
      No-Presubmit: true
      No-Tree-Checks: true
      No-Try: true
      Bug: v8:3598
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1532321Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
      Commit-Queue: Leszek Swirski <leszeks@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#60372}
      9f6ddb48
  3. 19 Mar, 2019 3 commits
    • Paolo Severini's avatar
      V8 x64 backend doesn't emit ABI compliant stack frames · 3cda21de
      Paolo Severini authored
      On 64 bit Windows, the OS stack walking does not work because the V8 x64
      backend doesn't emit unwinding info and also because it doesn't emit ABI
      compliant stack frames. See
      https://docs.google.com/document/d/1-wf50jFlii0c_Pr52lm2ZU-49m220nhYMrHDi3vXnh0/edit
      for more details.
      
      This problem can be fixed by observing that V8 frames usually all have the same
      prolog and epilog:
      
      push rbp,
      mov rbp, rsp
      ...
      pop rbp
      ret N
      
      and that it is possible to define XDATA (UNWIND_CODEs) that specify how Windows
      should walk through V8 frames. Furthermore, since V8 Code objects are all
      allocated in the same code-range for an Isolate, it is possible to register a
      single PDATA/XDATA entry to cover stack walking for all the code generated
      inside that code-range.
      
      This PR contains changes required to enable stack walking on Win64:
      
      EmbeddedFileWriter now adds assembler directives to the builtins
      snapshot source file (embedded.cc) to emit additional entries in the .pdata and
      in the .xdata section of the V8 executable. This takes care of stack walking
      for embedded builtins. (The case of non-embedded builtins is not supported).
      The x64 Assembler has been modified to collect the information required to emit
      this unwind info for builtins.
      
      Stack walking for jitted code is handled is Isolate.cpp, by registering
      dynamically PDATA/XDATA for the whole code-range address space every time a new
      Isolate is initialized, and by unregistering them when the Isolate is
      destroyed.
      
      Stack walking for WASM jitted code is handled is the same way in
      wasm::NativeModule (wasm/wasm-code-manager.cpp).
      
      It is important to note that Crashpad and Breakpad are already registering
      PDATA/XDATA to manage and report unhandled exceptions (but not for embedded
      builtins). Since it is not possible to register multiple PDATA entries for the
      same address range, a new function is added to the V8 API:
      SetUnhandledExceptionCallback() can be used by an embedder to register its own
      unhandled exception handler for exceptions that arise in v8-generated code.
      V8 embedders should be modified accordingly (code for this is in a separate PR
      in the Chromium repository:
      https://chromium-review.googlesource.com/c/chromium/src/+/1474703).
      
      All these changes are experimental, behind:
      
      the 'v8_win64_unwinding_info' build flag, and
      the '--win64-unwinding-info' runtime flag.
      
      Bug: v8:3598
      Change-Id: Iea455ab6d0e2bf1c556aa1cf870841d44ab6e4b1
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1469329Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
      Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
      Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
      Commit-Queue: Paolo Severini <paolosev@microsoft.com>
      Cr-Commit-Position: refs/heads/master@{#60330}
      3cda21de
    • Igor Sheludko's avatar
      [ptr-compr][ubsan] Use [Read/Write]UnalignedValue for unaligned fields · 0188634e
      Igor Sheludko authored
      When pointer compression is enabled the [u]intptr_t and double fields are
      only kTaggedSize aligned so in order to avoid undefined behavior in C++ code
      we have to access these values in an unaligned pointer friendly way although
      both x64 and arm64 architectures (where pointer compression is supported)
      allow unaligned access.
      
      These changes will be removed once v8:8875 is fixed and all the
      kSystemPointerSize fields are properly aligned.
      
      Bug: v8:7703
      Change-Id: I4df477cbdeab806303bb4f675d52b61c06342c8e
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1528996
      Commit-Queue: Igor Sheludko <ishell@chromium.org>
      Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
      Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
      Reviewed-by: 's avatarClemens Hammacher <clemensh@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#60321}
      0188634e
    • Benedikt Meurer's avatar
      [turbofan] Significantly improve ConsString creation performance. · d6a60a0e
      Benedikt Meurer authored
      This change significantly improves the performance of string
      concatenation in optimized code for the case where the resulting string
      is represented as a ConsString. On the relevant test cases we go from
      
        serializeNaive: 10762 ms.
        serializeClever: 7813 ms.
        serializeConcat: 10271 ms.
      
      to
      
        serializeNaive: 10278 ms.
        serializeClever: 5533 ms.
        serializeConcat: 10310 ms.
      
      which represents a 30% improvement on the "clever" benchmark, which
      tests specifically the ConsString creation performance.
      
      This was accomplished via a couple of different steps, which are briefly
      outlined here:
      
        1. The empty_string gets its own map, so that we can easily recognize
           and handle it appropriately in the TurboFan type system. This
           allows us to express (and assert) that the inputs to NewConsString
           are non-empty strings, making sure that TurboFan no longer creates
           "crippled ConsStrings" with empty left or right hand sides.
        2. Further split the existing String types in TurboFan to be able to
           distinguish between OneByte and TwoByte strings on the type system
           level. This allows us to avoid having to dynamically lookup the
           resulting ConsString map in case of ConsString creation (i.e. when
           we know that both input strings are OneByte strings or at least
           one of the input strings is TwoByte).
        3. We also introduced more finegrained feedback for the Add bytecode
           in the interpreter, having it collect feedback about ConsStrings,
           specifically ConsOneByteString and ConsTwoByteString. This feedback
           can be used by TurboFan to only inline the relevant code for what
           was seen so far. This allows us to remove the Octane/Splay specific
           magic in JSTypedLowering to detect ConsString creation, and instead
           purely rely on the feedback of what was seen so far (also making it
           possible to change the semantics of NewConsString to be a low-level
           operator, which is only introduced in SimplifiedLowering by looking
           at the input types of StringConcat).
        4. On top of the before mentioned type and interpreter changes we added
           new operators CheckNonEmptyString, CheckNonEmptyOneByteString, and
           CheckNonEmptyTwoByteString, which perform the appropriate (dynamic)
           checks.
      
      There are several more improvements that are possible based on this, but
      since the change was already quite big, we decided not to put everything
      into the first change, but do some follow up tweaks to the type system,
      and builtin optimizations later.
      
      Tbr: mstarzinger@chromium.org
      Bug: v8:8834, v8:8931, v8:8939, v8:8951
      Change-Id: Ia24e17c6048bf2b04df966d3cd441f0edda05c93
      Cq-Include-Trybots: luci.chromium.try:linux-blink-rel
      Doc: https://bit.ly/fast-string-concatenation-in-javascript
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1499497
      Commit-Queue: Michael Achenbach <machenbach@chromium.org>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Reviewed-by: 's avatarJaroslav Sevcik <jarin@chromium.org>
      Reviewed-by: 's avatarMythri Alle <mythria@chromium.org>
      Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#60318}
      d6a60a0e
  4. 18 Mar, 2019 3 commits
  5. 14 Mar, 2019 1 commit
  6. 13 Mar, 2019 2 commits
  7. 12 Mar, 2019 2 commits
  8. 09 Mar, 2019 1 commit
  9. 07 Mar, 2019 4 commits
  10. 06 Mar, 2019 2 commits
  11. 04 Mar, 2019 3 commits
    • Igor Sheludko's avatar
      [ptr-compr] Prepare for changing kTaggedSize, pt.3 · 17448030
      Igor Sheludko authored
      This CL also gives up trying to maintain double and system word
      fields at aligned addresses because currently it's not always
      maintained (v8:8875) and Torque object definitions do not support
      padding fields (v8:8863).
      
      Given that both platforms where pointer compression is going to be
      enabled (x64 and arm64) support loading of doubles and full words
      from 4-byte aligned addresses we are fine.
      
      Bug: v8:7703
      Change-Id: I99fc6da5a0927f4db9b8fb24c7cc0bfc416523bc
      Reviewed-on: https://chromium-review.googlesource.com/c/1496974
      Auto-Submit: Igor Sheludko <ishell@chromium.org>
      Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
      Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
      Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#60013}
      17448030
    • Benedikt Meurer's avatar
      [cleanup] Remove obsolete "one byte data hint" for strings. · 683cf6f4
      Benedikt Meurer authored
      In the early days of Chrome when we used WebKit there was no support for
      ASCII strings on the C++ side, so we put a hint onto these two-byte
      strings that said "string only contains one byte data", such that
      internally in V8 when these were involved in string operations, we could
      instead create the *cheaper* one byte strings.
      
      Nowadays Blink properly supports one-byte string representations and
      this additional hint only comes with overhead, since we check it in
      quite a few places (i.e. on the hot path for string concatenation), plus
      we end up consuming more memory due to the additional string maps.
      Removing the hint also frees one bit in the InstanceType zoo for
      strings.
      
      This alone improves performance on the `bench-dom-serialize.js` test case
      by around **3%**.
      
      Tbr: mstarzinger@chromium.org
      Bug: v8:6622, v8:8834, v8:8939
      Cq-Include-Trybots: luci.chromium.try:linux-blink-rel
      Change-Id: I0753f2859cee7b5a37b6f0da64d8ec39fcb044ff
      Doc: https://bit.ly/fast-string-concatenation-in-javascript
      Reviewed-on: https://chromium-review.googlesource.com/c/1498478
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#60006}
      683cf6f4
    • Dan Elphick's avatar
      [api] Add new configuration change methods · 980e0d32
      Dan Elphick authored
      This adds a new method Isolate::LocaleConfigurationChangeNotification
      that clears the cached Locale allowing new Locales to be picked up in
      later Locale operations.
      
      It moves Date::DateTimeConfigurationChangeNotification to Isolate
      (deprecating the old one) so that the configuration change methods are
      found together.
      
      Change-Id: Iffc15e326933c5bc5baf2f0eafdd5c148b8279a8
      Reviewed-on: https://chromium-review.googlesource.com/c/1491608Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
      Reviewed-by: 's avatarSathya Gunasekaran <gsathya@chromium.org>
      Commit-Queue: Dan Elphick <delphick@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#60003}
      980e0d32
  12. 01 Mar, 2019 1 commit
  13. 28 Feb, 2019 1 commit
  14. 25 Feb, 2019 1 commit
  15. 21 Feb, 2019 1 commit
    • Peter Marshall's avatar
      [tracing] Fix races in TracingController implementation · 51e80efd
      Peter Marshall authored
      The default TracingController (used by d8 and Node) has some concurrency
      issues. The new test flushes these out, when a second thread logs trace
      events while the main thread calls StopTracing().
      
      - Use an acquire load in UpdateCategoryGroupEnabledFlags() because this
        was racing with GetCategoryGroupEnabled() where a new category is
        added in the slow path. g_category_groups is append-only, but
        reads/writes to g_category_index need to be correctly ordered so that
        new categories are added and only then is the change to the index
        visible. The relaxed load ignored this and caused unsynchronized
        read/write.
      - Use a relaxed load in ~ScopedTracer() to access category_group_enabled
        as this previously used a non-atomic operation which caused a race
        with UpdateCategoryGroupEnabledFlag() which does a relaxed store.
      - Replace TracingController::mode_ with an atomic bool as read/writes to
        mode_ were not synchronized and caused TSAN errors. It only has two
        states and it doesn't seem like we will extend this so just convert it
        to bool.
      - Take the lock around calling trace_object->Initialize in
        AddTraceEvent(), and around trace_buffer_->Flush() in StopTracing().
        These two raced previously as the underlying TraceBufferRingBuffer
        passes out pointers to TraceObjects in a synchronized way, but the
        caller (AddTraceEvent) then writes into the object without
        synchronization. This leads to races when Flush() is called, at which
        time TraceBufferRingBuffer assumes that all the pointers it handed out
        are to valid, initialized TraceObjects - which is not true because
        AddTraceEvent may still be calling Initialize on them. This could be
        the cause of issues in Node.js where the last line of tracing/logging
        sometimes gets cut off. This is kind of a band-aid solution - access
        to the TraceObjects handed out by the ring buffer really needs proper
        synchronization which at this point would require redesign. It's quite
        likely we will replace this with Perfetto in the near future so not
        much point investing in this code right now.
      - Enable TracingCpuProfiler test which was flaky due to these bugs.
      
      Bug: v8:8821
      Change-Id: I141296800c6906ac0e7f3f21dd16d861b07dae62
      Reviewed-on: https://chromium-review.googlesource.com/c/1477283
      Commit-Queue: Peter Marshall <petermarshall@chromium.org>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Reviewed-by: 's avatarAli Ijaz Sheikh <ofrobots@google.com>
      Cr-Commit-Position: refs/heads/master@{#59752}
      51e80efd
  16. 20 Feb, 2019 1 commit
  17. 19 Feb, 2019 3 commits
  18. 15 Feb, 2019 1 commit
  19. 14 Feb, 2019 1 commit
    • Hannu Trey's avatar
      Re-detect the host time zone if requested by an embedder · f781f522
      Hannu Trey authored
      Add an enum argument to DateTimeConfigurationChangeNotification to
      control whether or not to redetect the host time zone. The default value
      kSkip doesn't cause redetecting so that callers do not need to change if
      they want the current behavior (e.g. Chromium).
      
      Note that the host time zone detection does not work when v8 is run
      inside a sandbox as in Chromium so that Chromium detects the host time
      zone outside the sandbox before calling
      DateTimeConfigurationChangeNotification. OTOH, other v8 embedders may
      find it more convenient for v8 to do the host time zone detection on
      their behalf. In that case, they can call the function with the new
      argument set to value kRedetect.
      
      Test:
      With PHP+V8Js on linux, execute:
      php -r '
        putenv("TZ=Europe/Helsinki");
        $v8 = new V8Js();
        $v8->executeString("print((new Date(0)).toString()+\"\\n\");");
        putenv("TZ=America/New_York");
        $v8->executeString("print((new Date(0)).toString()+\"\\n\");");'
      
      Result before modification:
      Thu Jan 01 1970 02:00:00 GMT+0200 (Eastern European Standard Time)
      Thu Jan 01 1970 02:00:00 GMT+0200 (Eastern European Standard Time)
      
      Result after modification:
      Thu Jan 01 1970 02:00:00 GMT+0200 (Eastern European Standard Time)
      Thu Jan 01 1970 02:00:00 GMT+0200 (Eastern European Standard Time)
      
      Result after V8JS is modified to use value kRedetect when calling
      
      Thu Jan 01 1970 02:00:00 GMT+0200 (Eastern European Standard Time)
      Wed Dec 31 1969 19:00:00 GMT-0500 (Eastern Standard Time)
      
      DateTimeConfigurationChangeNotification: 
      Change-Id: I005192dd42669a94f606a49baa9eafad3475b9fd
      Reviewed-on: https://chromium-review.googlesource.com/c/1449637Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
      Reviewed-by: 's avatarJungshik Shin <jshin@chromium.org>
      Commit-Queue: Jungshik Shin <jshin@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#59613}
      f781f522
  20. 13 Feb, 2019 1 commit
  21. 12 Feb, 2019 1 commit
    • tzik's avatar
      Move MicrotasksPolicy management to MicrotaskQueue · df95cff9
      tzik authored
      This CL moves MicrotasksPolicy from Isolate's HandleScopeImplementer
      to MicrotaskQueue for better non-default MicrotaskQueue support.
      
      After this:
       * MicrotaskPolicy is per-MicrotaskQueue rather than single global one.
       * ENTER_V8 runs MicrotaskQueue associated to the current Context, rather
         than the default_microtask_queue().
       * SuppressMicrotaskExecutionScope and MicrotasksScope are ready to
         take MicrotaskQueue parameter, rather than using the default one.
      
      Note that there's no way to use a non-default microtask queue until we
      expose it as a V8 API.
      
      Bug: v8:8124
      Change-Id: I79cbc53d26d9f3f4cfb7c64d303b12e395b76815
      Reviewed-on: https://chromium-review.googlesource.com/c/1429720Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
      Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#59517}
      df95cff9
  22. 11 Feb, 2019 1 commit
  23. 08 Feb, 2019 4 commits