1. 05 Oct, 2017 1 commit
    • Marja Hölttä's avatar
      [parser] Skipping inner funcs: Fix hoisting. · f6f5bafe
      Marja Hölttä authored
      The catch variable is a special VAR-mode variable which is not in a declaration
      scope. Normally creating such a variable is not possible with DeclareVariable,
      but Parser bypasses it by calling DeclareLocal directly (which doesn't have the
      hoisting check).
      
      PreParser used to cut corners and declare the catch variable as a LET-mode
      variable to prevent hoisting.
      
      But since LET and VAR variables behave differently when deciding whether they
      block sloppy block function hoisting, that approach doesn't fly.
      
      BUG=v8:5516,chromium:771474
      
      Change-Id: Ic6f5f4996416c9fa59132725c8b0b6b570c72f48
      Reviewed-on: https://chromium-review.googlesource.com/700634
      Commit-Queue: Marja Hölttä <marja@chromium.org>
      Reviewed-by: 's avatarAdam Klein <adamk@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#48308}
      f6f5bafe
  2. 02 Oct, 2017 1 commit
    • Mathias Bynens's avatar
      [parser] Add use counter for U+2028 & U+2029 · d3c98121
      Mathias Bynens authored
      The context is the following proposal to make JSON a subset of
      JavaScript: https://github.com/tc39/proposal-json-superset
      
      There’s interest in performing a side investigation to answer the
      question of what would happen if we stopped treating U+2028 and U+2029
      as `LineTerminator`s *entirely*. (Note that this is separate from the
      proposal, which just changes how these characters are handled in
      ECMAScript strings.) This is technically a breaking change, and IMHO it
      would be wonderful if we could get away with it, but no one really has
      any data on whether or not we could. Adding this use counter lets us get
      that data.
      
      BUG=v8:6827
      
      Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
      Change-Id: Ia22e8db1634df4d3f965bec8e1cfa11cc7b5e9aa
      Reviewed-on: https://chromium-review.googlesource.com/693155
      Commit-Queue: Mathias Bynens <mathias@chromium.org>
      Reviewed-by: 's avatarMarja Hölttä <marja@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#48260}
      d3c98121
  3. 29 Sep, 2017 1 commit
    • Marja Hölttä's avatar
      [parser] Skipping inner funcs: Make the data on heap smaller. · a0258063
      Marja Hölttä authored
      We were unnecessarily storing everything as uint32_t, even though many items in
      the preparsed scope data can be stored as uint8_t. This CL also adds an
      (internal) API which abstracts away the actual data storing, so the backing
      store can be made even more efficient (e.g., use only 1-3 bytes for some
      uint32_t values, if they fit) without affecting other parts of the code.
      
      BUG=v8:5516,chromium:762492
      
      Change-Id: I7cd4d91dc11f87f8aec9c7584044a6f2a59b73ba
      Reviewed-on: https://chromium-review.googlesource.com/684182
      Commit-Queue: Marja Hölttä <marja@chromium.org>
      Reviewed-by: 's avatarCamillo Bruni <cbruni@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#48231}
      a0258063
  4. 21 Sep, 2017 1 commit
    • Marja Hölttä's avatar
      [unicode] Return (the correct) errors for overlong / surrogate sequences. · 6389b7e6
      Marja Hölttä authored
      This fix is two-fold:
      
      1) Incremental UTF-8 decoding: Unify incorrect UTF-8 handling between V8 and
      Blink.
      
      Incremental UTF-8 decoding used to allow some overlong sequences / invalid code
      points which Blink treated as errors. This caused the decoder and the Blink
      UTF-8 decoder to produce a different number of bytes, resulting in random
      failures when scripts were streamed (especially, this was detected by the
      skipping inner functions feature which adds CHECKs against expected function
      positions).
      
      2) Non-incremental UTF-8 decoding: return the correct amount of invalid characters.
      
      According to the encoding spec ( https://encoding.spec.whatwg.org/#utf-8-decoder
      ), the first byte of an overlong sequence / invalid code point generates an
      invalid character, and the rest of the bytes are not processed (i.e., pushed
      back to the byte stream). When they're handled, they will look like lonely
      continuation bytes, and will generate an invalid character each.
      
      As a result, an overlong 4-byte sequence should generate 4 invalid characters
      (not 1).
      
      This is a potentially breaking change, since the (non-incremental) UTF-8
      decoding is exposed via the API (String::NewFromUtf8). The behavioral difference
      happens when the client is passing in invalid UTF-8 (containing overlong /
      surrogate sequences).
      
      However, afaict, this doesn't change the semantics of any JavaScript program:
      according to the ECMAScript spec, the program is a sequence of Unicode code
      points, and there's no way to invoke the UTF-8 decoding functionalities from
      inside JavaScript. Though, this changes the behavior of d8 when decoding source
      files which are invalid UTF-8.
      
      This doesn't change anything related to URI decoding (it already throws
      exceptions for overlong sequences / invalid code points).
      
      BUG: chromium:765608, chromium:758236, v8:5516
      Bug: 
      Change-Id: Ib029f6a8e87186794b092e4e8af32d01cee3ada0
      Reviewed-on: https://chromium-review.googlesource.com/671020
      Commit-Queue: Marja Hölttä <marja@chromium.org>
      Reviewed-by: 's avatarFranziska Hinkelmann <franzih@chromium.org>
      Reviewed-by: 's avatarCamillo Bruni <cbruni@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#48105}
      6389b7e6
  5. 18 Sep, 2017 1 commit
  6. 08 Sep, 2017 1 commit
  7. 31 Aug, 2017 1 commit
  8. 24 Aug, 2017 1 commit
    • Marja Hölttä's avatar
      [script streaming] Fix U+feff handling. · 4e453429
      Marja Hölttä authored
      U+feff is the UTF BOM but if it occurs inside the text, it's a "zero-width
      no-break space". However, the UTF-8 decoder in script streaming still thought
      it's a BOM and skipped it. The correct way to handle it would be to create a
      U+feff code point instead - the Scanner will then handle it as whitespace.
      
      This is a discrepancy between the Blink UTF-8 decoder and the V8 UTF-8 decoder,
      and caused the source positions be off by one. This bug went unnoticed, since
      normally off-by-one in this situation doesn't make the code to break.
      
      BUG=chromium:758508,chromium:758236
      
      Change-Id: Ib92a3ee65c402e21b77e42537db2a021cff55379
      Reviewed-on: https://chromium-review.googlesource.com/632096Reviewed-by: 's avatarAdam Klein <adamk@chromium.org>
      Commit-Queue: Marja Hölttä <marja@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#47583}
      4e453429
  9. 16 Aug, 2017 1 commit
  10. 14 Aug, 2017 1 commit
  11. 12 Aug, 2017 2 commits
  12. 09 Aug, 2017 1 commit
  13. 04 Aug, 2017 1 commit
    • Marja Hölttä's avatar
      [parser] Skipping inner funcs: implement a bailout. · e7a46253
      Marja Hölttä authored
      In some cases, PreParser cannot replicate the Scope structure created by
      Parser. It happens esp. with arrow function parameters, since the relevant
      information is already lost by the time we figure out it's an arrow function.
      
      In these cases, PreParser should bail out of trying to create data for skipping
      inner functions.
      
      Implementation notes:
      
      - The arrow function case is more fundamental; the non-arrow case could be
        hacked together somehow if we implemented tracking is_simple for each param
        separately; but now that it's possible to bail out consistently from both
        cases, I don't think the is_simple complication is worth it.
      
      - The added mjsunit test cases are based on the test262 test cases which exposed
        the problem.
      
      - cctest/preparser/PreParserScopeAnalysis was exercising similar cases, but the
        problem didn't show up because the function parameters didn't contain
        skippable functions. Those test cases have been repurposed for testing the
        bailout.
      
      - Extra precaution: the bailout tests are in a separate file, to guard from the
        bug that a bailout case results in bailing out of *all* data creation, which
        would make all skipping tests in the same file useless.
      
      BUG=v8:5516
      
      Change-Id: I4324749a5ec602fa5d7dc27647ade0284a6842fe
      Reviewed-on: https://chromium-review.googlesource.com/599849Reviewed-by: 's avatarAdam Klein <adamk@chromium.org>
      Commit-Queue: Marja Hölttä <marja@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#47170}
      e7a46253
  14. 30 Jun, 2017 1 commit
  15. 23 Jun, 2017 1 commit
  16. 22 Jun, 2017 2 commits
    • Marja Hölttä's avatar
      [parser] Skipping inner funcs: Fix function name declarations · 1fce2d2d
      Marja Hölttä authored
      let f = function g() { ... } declares "g" inside the function. This
      CL makes the preparser declare it too, and saves + restores the scope data for
      it.
      
      BUG=v8:5516
      
      Change-Id: Id4c64f446d30f5252038cfb0f0f473b85ba24a9b
      Reviewed-on: https://chromium-review.googlesource.com/544816
      Commit-Queue: Marja Hölttä <marja@chromium.org>
      Reviewed-by: 's avatarDaniel Vogelheim <vogelheim@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#46133}
      1fce2d2d
    • Marja Hölttä's avatar
      [parser] Skipping inner funcs: fix the test setup. · 2b730f63
      Marja Hölttä authored
      The test setup was as follows:
      - Preparse function test() { ... }, get scope allocation data.
      - Apply the scope allocation data to (function test() { ... })();
      - Compare against normal scope allocation for (function test() { ... })();
      
      But the IIFE is unnecessary - we already disable lazy parsing.
      
      Cleaning this up is needed because in the next CL, I want to fix the Scopes
      produced by PreParser in this case:
      
      let f = function g() {
        // Here we should declare g!
      }
      
      And that fix will make the variables in
      function test() {
        // Here we don't declare test
      }
      and
      (function test() {
        // Here we do declare test
      })();
      not match any more, so it doesn't make sense to compare them against each other.
      
      BUG=v8:5516
      
      Change-Id: I93d154c6977bb3cbe405b6ca193cf6283df297bc
      Reviewed-on: https://chromium-review.googlesource.com/543341Reviewed-by: 's avatarDaniel Vogelheim <vogelheim@chromium.org>
      Commit-Queue: Marja Hölttä <marja@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#46128}
      2b730f63
  17. 19 Jun, 2017 1 commit
  18. 09 Jun, 2017 1 commit
  19. 06 Jun, 2017 1 commit
  20. 02 Jun, 2017 1 commit
  21. 31 May, 2017 1 commit
    • Marja Hölttä's avatar
      [parser] Skipping inner funcs: make more functions skippable. · 06f05ec2
      Marja Hölttä authored
      - Enable aggressive lazy inner funcs (make non-declaration funcs lazy, ie let f =
        function() { ... } when --experimental-preparser-scope-analysis is on.
      - Turn on variable tracking for lazy top level functions: this makes their inner
        functions skippable.
      - Test fix for an testing bug uncovered by this work: when restoring the data
        for the relevant scope, don't assume it's the outermost scope for which we
        have data.
      - Fix: if we abort lazy parsing a function, we shouldn't produce any data for
        it.
      
      BUG=v8:5516
      
      Change-Id: I0606fbabb5886dc57dbb53ab5f3fb894ff5d032e
      Reviewed-on: https://chromium-review.googlesource.com/518165Reviewed-by: 's avatarDaniel Vogelheim <vogelheim@chromium.org>
      Commit-Queue: Marja Hölttä <marja@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#45615}
      06f05ec2
  22. 18 May, 2017 1 commit
    • Adam Klein's avatar
      Revert "[parser] Refactor streaming scanner streams." · 9397c1b7
      Adam Klein authored
      This reverts commit ce538f70.
      
      Reason for revert: breaks BOM handling (thus breaking Outlook web apps).
      
      Original change's description:
      > [parser] Refactor streaming scanner streams.
      > 
      > Unify, simplify logic, reduce UTF8 specific handling.
      > 
      > Intend of this is also to have stream views.
      > Stream views can be used concurrently by multiple threads, but
      > only one thread may fetch new data from the underlying source.
      > This together with unified stream view creation is intended to be
      > used for parse tasks.
      > 
      > BUG=v8:6093
      > 
      > Change-Id: Ied8e93090c506d4735080298f0fdaeed32043915
      > Reviewed-on: https://chromium-review.googlesource.com/501789
      > Commit-Queue: Wiktor Garbacz <wiktorg@google.com>
      > Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
      > Reviewed-by: Marja Hölttä <marja@chromium.org>
      > Cr-Commit-Position: refs/heads/master@{#45336}
      
      TBR=marja@chromium.org,vogelheim@chromium.org,jochen@chromium.org,wiktorg@google.com
      BUG=v8:6093, chromium:724166
      
      Change-Id: I022a23b8052d20d83a640c07b7864c622548bf90
      Reviewed-on: https://chromium-review.googlesource.com/508888Reviewed-by: 's avatarAdam Klein <adamk@chromium.org>
      Commit-Queue: Adam Klein <adamk@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#45404}
      9397c1b7
  23. 17 May, 2017 1 commit
  24. 16 May, 2017 2 commits
  25. 15 May, 2017 1 commit
    • Marja Hölttä's avatar
      [parser] Skipping inner funcs: Fix related to classes. · 3e12ed1f
      Marja Hölttä authored
      - Default constructor scopes won't need the scope data for deciding the scope
      allocation of variables inside them. Also, PreParser doesn't construct them. So
      they should be just skipped when applying the scope data.
      
      - PreParser needs to declare the class name + have a proper end position for
      the class scope.
      
      - This makes all mjsunit tests pass with --experimental-preparser-scope-analysis.
      
      - Also added several DCHECKs which were useful for debugging.
      
      BUG=v8:5516
      
      Change-Id: I5b3e6c60ed79efe25f33576a3547d707c700c6dd
      Reviewed-on: https://chromium-review.googlesource.com/503208
      Commit-Queue: Marja Hölttä <marja@chromium.org>
      Reviewed-by: 's avatarDaniel Vogelheim <vogelheim@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#45303}
      3e12ed1f
  26. 09 May, 2017 1 commit
  27. 04 May, 2017 1 commit
  28. 19 Apr, 2017 1 commit
  29. 18 Apr, 2017 1 commit
  30. 28 Mar, 2017 3 commits
  31. 17 Mar, 2017 1 commit
  32. 07 Mar, 2017 1 commit
    • Marja Hölttä's avatar
      [parser] Skipping inner funcs: collect data needed for allocation, not the allocation result. · f489f7ab
      Marja Hölttä authored
      This pretty much rewrites the preparsed scope data collection. We used to store
      the allocation result, but it's faster to just store the raw data which is
      needed for deciding it later. (This way we don't need to run the allocation
      algorithm for just getting this data.)
      
      For each variable: is_used, maybe_assigned,
      has_forced_context_allocation, and for each scope:
      inner_scope_calls_eval_.
      
      In addition, this CL moves data handling out of Scope and into
      PreParsedScopeData where it belongs and simplifies the API for
      PreParsedScopeData.
      
      BUG=v8:5516
      R=vogelheim@chromium.org
      
      Change-Id: Ia5a4fa52f585cd4f483ce9a92f2dd7d9754f34ed
      Reviewed-on: https://chromium-review.googlesource.com/451273
      Commit-Queue: Marja Hölttä <marja@chromium.org>
      Reviewed-by: 's avatarDaniel Vogelheim <vogelheim@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#43641}
      f489f7ab
  33. 03 Mar, 2017 1 commit
  34. 01 Mar, 2017 1 commit
  35. 24 Feb, 2017 1 commit