This is a reland of commit a664aef0ca.
The test is made ~25x faster by using integer parameters instead of
floating point.
Original change's description:
> [asm] Reject import calls with too many parameters
>
> The asm parser was missing a check for too many parameters for calls to
> imported functions. For regular functions this check implicitly existed
> because the limit was checked at the function declaration, and the call
> site needs to match the declared parameter count.
>
> R=mslekova@chromium.org
>
> Bug: chromium:1302596
> Change-Id: I0d35e70a66d682ee8fdecf5c8ea4d2b1419ce684
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3509393
> Reviewed-by: Maya Lekova <mslekova@chromium.org>
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#79415}
Bug: chromium:1302596
Change-Id: I138561742b38939a1c2c9a69a6fa508d4f3a028d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3513613
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79424}
This reverts commit a664aef0ca.
Reason for revert: Times out on CFI: https://cr-buildbucket.appspot.com/build/8820170241901030897
Original change's description:
> [asm] Reject import calls with too many parameters
>
> The asm parser was missing a check for too many parameters for calls to
> imported functions. For regular functions this check implicitly existed
> because the limit was checked at the function declaration, and the call
> site needs to match the declared parameter count.
>
> R=mslekova@chromium.org
>
> Bug: chromium:1302596
> Change-Id: I0d35e70a66d682ee8fdecf5c8ea4d2b1419ce684
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3509393
> Reviewed-by: Maya Lekova <mslekova@chromium.org>
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#79415}
Bug: chromium:1302596
Change-Id: I743647f739e0cc93b2e99145086dbbb7d2660c79
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3512853
Auto-Submit: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#79417}
The asm parser was missing a check for too many parameters for calls to
imported functions. For regular functions this check implicitly existed
because the limit was checked at the function declaration, and the call
site needs to match the declared parameter count.
R=mslekova@chromium.org
Bug: chromium:1302596
Change-Id: I0d35e70a66d682ee8fdecf5c8ea4d2b1419ce684
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3509393
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79415}
According to the spec, the three parameters (stdlib, foreign, and heap)
must be mutually distinct. We did not check this yet, which led to
observable differences between asm validation and standard JavaScript
semantics.
R=thibaudm@chromium.org
Bug: chromium:1068355
Change-Id: I451f63d10ea50474aeb6e8a547918b5af769343b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3244408
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77554}
This makes sure that the order of exports as they appear in asm.js
modules is maintained globally (not just per function) while being
translated to a WASM module.
R=clemensh@chromium.org
TEST=mjsunit/asm/asm-validation
BUG=chromium:720586
Change-Id: I8b26d717ae2f88467d41670bced901f196c7b3fc
Reviewed-on: https://chromium-review.googlesource.com/503708
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45277}
This makes sure that the observable property order of the module export
maintains insertion order. Now that properties are configurable, we no
longer need to reverse the export processing.
R=clemensh@chromium.org
TEST=mjsunit/asm/asm-validation
BUG=chromium:715420
Change-Id: Ib2024254c07bdad7fee1cf2fa0bd3e847721f5b5
Reviewed-on: https://chromium-review.googlesource.com/488022
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44891}
Fix a few items broken during review of scanner + parser:
* Make the scanner retain stale newline state on a rewind (as otherwise it must be able to correctly rewind that too, though it doesn't need it). (Probably should revisit).
* Change StashCode in the builder skip to the zero case, as it crashes for some reason (added TODO).
Also fix:
* Drop test based on constant expression evaluation in main parser
* Support constant defined based on existing constant.
* Type constants as signed.
* Added a check that all used functions are defined eventually.
* Zone allocate strings for simplicity (TODOs to refactor better).
BUG=v8:6090
BUG=v8:4203
R=mstarzinger@chromium.org,marja@chromium.org,vogelheim@chromium.org
Review-Url: https://codereview.chromium.org/2771183002
Cr-Original-Commit-Position: refs/heads/master@{#44200}
Committed: be0dbdd679
Review-Url: https://codereview.chromium.org/2771183002
Cr-Commit-Position: refs/heads/master@{#44203}
Reason for revert:
Fails on gc-stress.
Original issue's description:
> [wasm][asm.js] Fix and enable several asm.js tests with the new parser.
>
> Fix a few items broken during review of scanner + parser:
> * Make the scanner retain stale newline state on a rewind (as otherwise it must be able to correctly rewind that too, though it doesn't need it). (Probably should revisit).
> * Change StashCode in the builder skip to the zero case, as it crashes for some reason (added TODO).
>
> Also fix:
> * Drop test based on constant expression evaluation in main parser
> * Support constant defined based on existing constant.
> * Type constants as signed.
> * Added a check that all used functions are defined eventually.
> * Zone allocate strings for simplicity (TODOs to refactor better).
>
> BUG=v8:6090
> BUG=v8:4203
> R=mstarzinger@chromium.org,marja@chromium.org,vogelheim@chromium.org
>
> Review-Url: https://codereview.chromium.org/2771183002
> Cr-Commit-Position: refs/heads/master@{#44200}
> Committed: be0dbdd679TBR=clemensh@chromium.org,bradnelson@google.com,marja@chromium.org,mstarzinger@chromium.org,vogelheim@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:6090
Review-Url: https://codereview.chromium.org/2782613002
Cr-Commit-Position: refs/heads/master@{#44201}
Fix a few items broken during review of scanner + parser:
* Make the scanner retain stale newline state on a rewind (as otherwise it must be able to correctly rewind that too, though it doesn't need it). (Probably should revisit).
* Change StashCode in the builder skip to the zero case, as it crashes for some reason (added TODO).
Also fix:
* Drop test based on constant expression evaluation in main parser
* Support constant defined based on existing constant.
* Type constants as signed.
* Added a check that all used functions are defined eventually.
* Zone allocate strings for simplicity (TODOs to refactor better).
BUG=v8:6090
BUG=v8:4203
R=mstarzinger@chromium.org,marja@chromium.org,vogelheim@chromium.org
Review-Url: https://codereview.chromium.org/2771183002
Cr-Commit-Position: refs/heads/master@{#44200}
In practice, Emscripten seems to emit cond?+a:+b type return
expressions. This is not allowed by the spec or errata, but we need
to support it for compatibility.
Similar patterns with ints / signed, do not seem to be supported.
BUG=v8:5891
R=mtrofin@chromium.org,aseemgarg@chromium.org
Review-Url: https://codereview.chromium.org/2648353010
Cr-Commit-Position: refs/heads/master@{#42677}
Deferred function call validation is required to support out of order
asm.js function declaration. Unfortunately, since we've started interleaving
validation and asm-wasm building, we don't check names are resolved until
the end.
Fortunately, undefined names can be detected from their CallType.
Check this at asm-typer time.
BUG=676797
R=aseemgarg@chromium.org,titzer@chromium.org
Review-Url: https://codereview.chromium.org/2615443003
Cr-Commit-Position: refs/heads/master@{#42158}
Because the parser optimizes !123 -> false,
we allow booleans in expressions (but not parameter annotations).
Allow this in asm-wasm-builder.
Turn on an early out case in asm-typer that is fine.
BUG=672784
R=titzer@chromium.org
Review-Url: https://codereview.chromium.org/2561193003
Cr-Commit-Position: refs/heads/master@{#41622}
By now the predicate in question is an exact negation of %IsAsmWasmCode
as the name intuitively implies. The need for two separate test methods
no longer exists and one of the two can be removed.
R=bradnelson@chromium.org
Review-Url: https://codereview.chromium.org/2562003002
Cr-Commit-Position: refs/heads/master@{#41616}
By now the compiler pipeline will not produce optimized code for asm.js
functions unless validation failed (even when --always-opt is enabled).
The related workaround in the testing predicate can be removed.
R=rmcilroy@chromium.org
Review-Url: https://codereview.chromium.org/2549463002
Cr-Commit-Position: refs/heads/master@{#41614}
We have been assuming in several places that ContainsDot or ToInt32 is
sufficient to check a value is a valid double or int.
Refactoring all the checks to one place and making them cope with booleans
or other unexpected types being present.
BUG=672044
R=titzer@chromium.org
Review-Url: https://codereview.chromium.org/2555323003
Cr-Commit-Position: refs/heads/master@{#41595}
We recently allowed global constants in asm.js validated code.
When used in a return statement, these need to be of an allowed type.
BUG=660813
R=jpp@chromium.org,aseemgarg@chromium.org
Review-Url: https://codereview.chromium.org/2481103002
Cr-Commit-Position: refs/heads/master@{#40850}
Make use of %IsAsmWasmCode in place of Wasm.instantiateModuleFromAsm,
in order to reduce the surface area of the Wasm object,
and to focus on testing asm.js coming in via the parser.
Ignore extra CONST_LEGACY assignment introduced by the parser
when modules have the form:
(function Foo(a, b, c) {..});
This requires both a validator and AsmWasmBuilder change.
Move stdlib use collection to import time,
to reject modules that import a function, even if not used.
BUG= https://bugs.chromium.org/p/v8/issues/detail?id=4203
LOG=N
R=jpp@chromium.org,titzer@chromium.org
Review-Url: https://codereview.chromium.org/2264913002
Cr-Commit-Position: refs/heads/master@{#38806}
This fixes the case where a module is instantiated twice via the same
closure when in the meantime another closure has destroyed and removed
the WASM data attached to the SharedFunctionInfo.
R=bradnelson@chromium.org
TEST=mjsunit/asm/asm-validation
BUG=v8:4203
Review-Url: https://codereview.chromium.org/2253613004
Cr-Commit-Position: refs/heads/master@{#38696}
Our previous per-arch instantiation thunks for asm.js
didn't support modules that had or were called with anything other
than 3 arguments. Adding support for this.
Addding a runtime test method to check if asm validation succeeded.
Adding a test of validation with different argument count combinations.
R=mstarzinger@chromium.org
TEST=mjsunit/asm/asm-validator.js
BUG= https://bugs.chromium.org/p/v8/issues/detail?id=4203
Review-Url: https://codereview.chromium.org/2229723002
Cr-Commit-Position: refs/heads/master@{#38688}
