| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
|
|
|
|
|
| |
When using nominal types, func.ref of two functions with identical signatures
but different HeapTypes will yield different types. To preserve these semantics,
Functions need to track their HeapTypes, not just their Signatures.
This PR replaces the Signature field in Function with a HeapType field and adds
new utility methods to make it almost as simple to update and query the function
HeapType as it was to update and query the Function Signature.
|
| |
|
|
|
|
|
|
|
|
| |
That traversal did not mention BrOn, which led to it doing incorrect work in
SimplifyLocals.
Also add assertions at the end, that aim to prevent future issues.
The rest of the fix is to make SimplifyLocals not assume that things
are a Switch if they are not an If/Block/etc., so that we don't crash
on a BrOn.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
The code for printing and emitting the experimental nominal type constructors
added in #3933 assumes that supertypes were only returned from `getSuperType`
when nominal typing was enabled. `getSuperType` in turn was assuming that the
supertype field would only be set if nominal typing was enabled, but this was
not the case. This bug caused use-after-free errors because equirecursive
canonicalization left the supertype field pointing to a temporary HeapTypeInfo
that would be freed at the end of parsing but then accessed during module
writing.
To fix the issue, only set `supertype` if nominal typing is enabled, as
originally intended.
|
| | |
|
| |
|
|
|
|
|
|
|
| |
PostEmscripten will turn an invoke of a constant function
pointer index into a direct call. However, due to UB it is possible to
have invalid function pointers, and we should not crash on that
(and do nothing to optimize, of course).
Mostly whitespace; to avoid deep nesting, I added more
early returns.
|
| |
|
|
|
|
|
|
|
|
|
| |
This changes the encoding of the `attribute` field, which currently only
contains the value `0` denoting this tag is for an exception, from
`varuint32` to `uint8`. This field is effectively unused at the moment
and reserved for future use, and it is not likely to need `varuint32`
even in future.
See https://github.com/WebAssembly/exception-handling/pull/162.
This does not change any encoded binaries because `0` is encoded in the
same way both in `varuint32` and `uint8`.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously, ref.func instructions would be assigned the canonical (i.e. first
parsed) heap type for the referenced function signature rather than the HeapType
actually specified in the type definition. In nominal mode, this could cause
validation failures because the types assigned to ref.func instructions would
not be correct.
Fix the problem by tracking function HeapTypes rather than function Signatures
when parsing the text format.
There can still be validation failures when round-tripping modules because
function HeapTypes are not properly preserved after parsing, but that will be
addressed in a follow-up PR.
|
| |
|
|
|
|
|
|
|
| |
This removes `attribute` field from `Tag` class, making the reserved and
unused field known only to binary encoder and decoder. This also removes
the `attribute` parameter from `makeTag` and `addTag` methods in
wasm-builder.h, C API, and Binaryen JS API.
Suggested in
https://github.com/WebAssembly/binaryen/pull/3946#pullrequestreview-687756523.
|
| |
|
|
|
|
|
|
| |
This attribute is always 0 and reserved for future use. In Binayren's
unofficial text format we were writing this field as `(attr 0)`, but we
have recently come to the conclusion that this is not necessary.
Relevant discussion:
https://github.com/WebAssembly/exception-handling/pull/160#discussion_r653254680
|
| |
|
|
|
|
| |
We don't need to assign a zero value for such locals (and we can't, as no
default value exists for them).
Fixes #3944
|
| |
|
|
|
|
|
|
|
|
|
| |
We recently decided to change 'event' to 'tag', and to 'event section'
to 'tag section', out of the rationale that the section contains a
generalized tag that references a type, which may be used for something
other than exceptions, and the name 'event' can be confusing in the web
context.
See
- https://github.com/WebAssembly/exception-handling/issues/159#issuecomment-857910130
- https://github.com/WebAssembly/exception-handling/pull/161
|
| |
|
|
|
|
|
|
|
|
|
| |
These files are special in that they use define symbols that are not
defined within those files or other files included in those files; they
are supposed to be defined in source files that include these headers.
This has caused clang-tidy to fail every time these files have changed
because they are not compilable per se.
This PR solves the problem by changing their extension to `def`, which
is also used in LLVM codebase. LLVM has dozens of files like this whose
extension is `def`, which makes these not checked by clang-tidy.
|
| |
|
|
|
|
|
|
|
|
| |
RedundantSetElimination checks if a local already has the default
value when we assign the default to it. For a non-nullable local, however,
there is no initial value - it cannot be used before it is assigned to. So
we just need to skip such locals, and not assume they contain a default
value we can compare against (we would assert on trying to create a
"zero" for such a non-nullable type).
Fixes #3942
|
| |
|
|
|
|
|
|
| |
This is a useful alternative to extract-function when you don't know the
function's name.
Also moves the extract-function tests to be lit tests and re-uses them as
extract-function-index tests.
|
| |
|
|
|
| |
Improve the legibility of the option documentation by adding vertical space
between options. This is particularly helpful to delimit the text of options
with longer explanations.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Assertions were previously parsed by replacing "invoke" with "call" and using
the normal s-expr parser. The parseCall method of the s-expr parser uses the
call target to look up the correct signature on the module, but the invoke
targets in assertions use export names rather than internal function names, so
the signature lookups were inserting new bogus entries with default values.
This issue didn't seem to cause any big problems before, but #3935 turns it into
a hard error because the default `HeapType` does not have an associated
signature.
Fix the problem (at least in the common case of trivial arguments and expected
results) by manually construction a `Call` expression rather than depending on
the s-expr parser to construct it.
|
| |
|
|
|
|
|
|
|
|
| |
This is the same as rtt.sub, but creates a "new" rtt each time. See
https://docs.google.com/document/d/1DklC3qVuOdLHSXB5UXghM_syCh-4cMinQ50ICiXnK3Q/edit#
The old Literal implementation of rtts becomes a little more complex here,
as it was designed for the original spec where only structure matters. It may
be worth a complete redesign there, but for now as the spec is in flux I think
the approach here is good enough.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
This adds a new feature flag, GCNNLocals that enables support for
non-nullable locals. No validation is applied to check that they are
actually assigned before their use yet - this just allows experimentation
to begin.
This feature is not enabled by default even with -all. If we enabled it,
then it would take effect in most of our tests and likely confuse current
users as well. Instead, the flag must be opted in explicitly using
--enable-gc-nn-locals. That is, this is an experimental feature flag,
and as such must be explicitly enabled. (Once the spec stabilizes,
we will remove the feature anyhow when we implement the
final status of non-nullability. )
|
| |
|
|
|
|
|
| |
Adds a `--nominal` option to switch the type machinery from equirecursive to
nominal. Implements binary and text parsing and emitting of nominal types using
new type constructor opcodes and an `(extends $super)` text syntax extension.
When not in nominal mode, these extensions will still be parsed but will not
have any effect and will not be used when emitting.
|
| |
|
|
|
|
| |
The new instruction emits a file containing a map between placeholder index and
the name of the split out function that placeholder is replacing in the table.
This map is intended to be useful for debugging, as discussed in
https://github.com/emscripten-core/emscripten/issues/14330.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Add methods to the TypeBuilder interface to declare subtyping relationships
between the built types. These relationships are validated and recorded globally
as part of type building. If the relationships are not valid, a fatal error is
produced. In the future, it would be better to report the error to the
TypeBuilder client code, but this behavior is sufficient for now. Also updates
SubTyper and TypeBounder to be aware of nominal mode so that subtyping and LUBs
are correctly calculated.
Tests of the failing behavior will be added in a future PR that exposes this
functionality to the command line, since the current `example` testing
infrastructure cannot handle testing fatal errors.
|
| |
|
|
| |
This removes the restriction that `try` should have at least one
`catch`/`catch_all`/`delegate`. See WebAssembly/exception-handling#157.
|
| |
|
|
|
|
|
| |
When parsing func.ref instructions, we need to get the HeapType corresponding to
the referenced function's signature. Since constructing HeapTypes from
Signatures can be expensive under equirecursive typing, keep track of the
original function signature HeapTypes directly during parsing rather than
storing them as Signatures.
|
| | |
|
| |
|
|
|
|
| |
The noteBreak call was in the wrong place, causing us to not note breaks
from BrOnNull for example, which could make validation miss errors.
Noticed in #3926
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In nominal mode, HeapType constructors besides the Signature constructor always
produce fresh types distinct from any previously created types. The HeapType
constructor that takes a Signature maintains its previous behavior of
constructing a canonical representative of the given signature because it is
used frequently throughout the code base and never in a situation that would
benefit from creating a fresh type. It is left as future work to clean up this
discrepancy between the Signature HeapType constructor and other HeapType
constructors.
TypeBuilder skips shape and global canonicalization in nominal mode and always
creates a fresh type for each of its entries. For this to work without any
canonicalization, the TypeBuilder allocates temporary types on the global Type
store and does not support building basic HeapTypes in nominal mode.
The new mode is not available in any of the Binaryen tools yet because it is
still missing critical functionality like the ability to declare subtyping
relations and correctly calculate LUBs. This functionality will be implemented
in future PRs.
|
| |
|
| |
fixes part of #3906
|
| |
|
|
|
|
|
| |
Given a list of profiles for the same module, --merge-profiles produces a single
combined profile the contains the minimum timestamp among the original profiles
for each function. When verbose output is enabled, also emit a message for each
profile that could individually be removed without affecting the set of
functions in the combined profile, as suggested in #3912.
|
| |
|
|
|
|
| |
They are basically the flip versions. The only interesting part in the impl is that their
returned typed and sent types are different.
Spec: https://docs.google.com/document/d/1DklC3qVuOdLHSXB5UXghM_syCh-4cMinQ50ICiXnK3Q/edit
|
| |
|
|
|
|
| |
In anticipation of adding a third wasm-split mode, merge-profiles, in addition
to the existing split and instrument modes, refactor wasm-split's option
validation to let the valid modes be declared for each option. This approach is
more scalable and robust than the ad-hoc validation we had previously.
|
| |
|
|
|
|
|
|
|
| |
wasm-split would previously use internal function names to create the external
names of the functions that are newly exported from the primary module to be
imported into the secondary module. When the input module contains full function
names (as is commonly the case when emitting symbol maps), this caused the
function names to be preserved as the export names, even when names are
otherwise being stripped. To save on code size and properly anonymize functions,
generate minimal export names when debuginfo is disabled instead.
|
| |
|
|
| |
Simplifies the public API to not unnecessarily take an index and simplifies the
implementation to use a single integer as state rather than a vector of indices.
|
| |
|
|
|
|
|
|
| |
Spec for it is here:
https://docs.google.com/document/d/1DklC3qVuOdLHSXB5UXghM_syCh-4cMinQ50ICiXnK3Q/edit#
Also reorder some things in wasm.h that were not in the canonical order (that has
no effect, but it is confusing to read).
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Without adding logic there, it simply ignored the branch, which could
lead to bad optimizations (thinking code is unreachable when it was).
There isn't a trivial way to add a static error to force us to add new
classes to CFGWalker. But this PR generalizes the code there to
handle all branches and all unreachable instructions in a generic
way. The only thing we'll need to remember to do in the future is to
add control flow structures. (And normally the fuzzer should quickly
find such bugs, but we don't have full fuzzing enabled for GC yet.)
Fixes #3907
|
| |
|
|
|
|
| |
Even when other names are stripped, it can be useful for wasm-split to preserve
the module name so that the split modules can be differentiated in stack traces.
Adding this option to wasm-split requires adding similar options to ModuleWriter
and WasmBinaryWriter.
|
| | |
|
| |
|
|
|
|
|
|
|
|
| |
`Walker::TaskFunc` has changed from a function pointer to
`std::function` in #3494, mainly to make the EH support for `CFGWalker`
easier. We didn't notice much performance difference then, but it was
recently reported that it creased binaryen.js code size and performance.
This changes `Walker::TaskFunc` back to a function pointer and does a
little more work to manage catch index in `CFGWalker` side.
Hopefully fixes #3857.
|
| |
|
|
|
|
|
|
| |
Imported functions come first when modules are emitted, so to ensure the
function indices are correct, they need to come first in the symbol maps. We
never noticed this bug before because imported functions are always the first
functions when a module is parsed, so the bug never mattered in practice.
However, wasm-split adds new imported functions after parsing and these were
causing the symbol map indices to be incorrect.
|
| |
|
|
| |
We must do that before assuming the type is a heap type in getStructIndex,
or we'd hit an assert there.
|
| |
|
| |
Fixes #3895
|
| |
|
|
|
| |
The new option emits a symbol map file for each of the split modules. The file
names are created by appending ".symbols" to each of the Wasm output file
names.
|
| |
|
|
|
|
|
|
|
|
|
| |
Valmari and Lehtinen's algorithm is broadly similar to Hopcroft's algorithm, but
it more precisely keeps track of which input transitions might be able to split
a partition of states so it ends up doing much less work. Unlike our
implementation of Hopcroft's algorithm, which naively used sets of HeapTypes,
this new algorithm also uses optimized data structures that can split partitions
in constant time and never reallocate.
This change improves the shape canonicalization time for a real-world
unoptimized type section from 40 minutes to 1.5 seconds.
|
| |
|
|
|
|
| |
When things go well, the reducer shrinks the factor by 50% or more, but
when things are slow it kept the factor unchanged. That is annoying in
some cases where you really have no benefit from reduction until the
factor gets small. So this at least reduces it by 10% in each iteration.
|
| |
|
|
|
|
|
|
|
| |
As found in #3682, the current implementation of type ordering is not correct,
and although the immediate issue would be easy to fix, I don't think the current
intended comparison algorithm is correct in the first place. Rather than try to
switch to using a correct algorithm (which I am not sure I know how to
implement, although I have an idea) this PR removes Type ordering entirely. In
places that used Type ordering with std::set or std::map because they require
deterministic iteration order, this PR uses InsertOrdered{Set,Map} instead.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously we would try to stop using the allocation as much as possible,
for example not writing it to locals any more, and leaving it to other passes
to actually remove it (and remove gets of those locals etc.). This seemed
simpler and more modular, but does not actually work in some cases as
the fuzzer has found. Specifically, if we stop writing our allocation to
locals, then if we do a (ref.as_non_null (local.get ..)) of that, then we
will trap on the null present in the local.
Instead, this changes our rewriting to do slightly more work, but it is
simpler in the end. We replace the allocation with a null, and replace
all the places that use it accordingly, for example, updating types to be
nullable, and removing RefAsNonNulls, etc. This literally gets rid of the
allocation and all the places it flows to (leaving less for other passes to
do later).
|
| |
|
|
| |
Similar to struct operations, if the reference is unreachable then we do
not know the heap type, and cannot print the full expression.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
| |
If we run a pass that removes DWARF followed by one that could destroy it, then
there is no possible problem - there is nothing left to destroy. We can run the later
pass with no issues (and no warnings).
Also add an assertion on running a pass runner only once. That has always been
the assumption, and now that we track whether the added passes remove debug
info, we need to check it.
Fixes emscripten-core/emscripten#14161
|
| |
|
|
|
|
| |
Move the InsertOrderedSet and InsertOrderedMap implementations out of
Relooper.h and into a new insert_ordered.h so they can be used more widely. Only
changes the implementation code to use unordered_maps and
`WASM_UNREACHABLE` instead of `abort`.
|
| |
|
|
| |
Affects `printMajor` and `printMedium`. There is no usage of this optional
argument in the source code.
|
