-
Notifications
You must be signed in to change notification settings - Fork 785
RFC: Add support for multiple tables #642
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Have you seen WebAssembly/design#682? That allows importing tables, which means we can have more than one. Although, I'm not sure of the details yet, as it mentions a "default" table and it's not clear how the others can be accessed. I assume those details will be clarified when this reaches the spec repo (which it hasn't yet). Also, that PR began more general, similar to what you propose here with multiple internal tables, might be interesting to read the discussion of why the path changed somewhat. Very minor implementation note, |
Let me know if there is specific feedback you're looking for, reading this now I don't see anything odd or wrong. |
I will split off the backend changes to the table data structure into a separate patch, while leaving the frontend parsing untouched. This should make it easier to add multiple table support in the future. |
The discussion on WebAssembly/design#727 is probably relevant. Some specific comments:
Ah no, anyfunc is emphatically not the same as a function type with no parameters and results! It is a dynamic function type. It is a new type constructor (i.e., keyword) that we probably have to allow in the type section to enable the model you envision. That is, you want the user to allow defining (type $anyfunc (anyfunc)) Note that this has a number of implications, i.e., you can no longer assume that all type indices are legal to be used in various places, such as func declarations, func imports, or call_indirect.
The table name should be a variable, not a string. Also, to be consistent with the handling of all other name spaces, the name should be optional, i.e., it should be possible to reference tables purely by raw index. I'd also deal with the default declaration differently, see the latest WebAssembly/design#727 comments, but as a temporary experimental solution a flag might be fine.
Note that the $x syntax is exclusively used for user-defined name space indices.
See above, this should be a var, which is a name space index either given as a symbolic $x or a raw number.
I think this is obsolete with WebAssembly/design#727.
|
55a766c
to
f713415
Compare
fc088a2
to
2566183
Compare
191f8f2
to
efa721b
Compare
This commits makes a number of changes to the WebAssembly format, some of which exceed the feature set desired for the MVP. (1) It adds support for updated table definitions, including the default, elementType, initial, and max attributes, plus a name. Currently, the initial and max attributes must be equal to the number of elements. The elementType attribute is interpreted as a FunctionType index, and type homogeneity is enforced on table elements, unless the specified FunctionType has name "anyfunc", which corresponds to a FunctionType with a none parameter and return type none. Format: (table <name> [default] <type> <entries>) (2) It adds support for multiple tables. If tables are used, currently the first table must be default, and the remainder must not. Example: (table "foo" default (type $FUNCSIG$i) $a) (table "bla" (type $anyfunc) $b $c $d) (3) Indirect calls have an immediate argument that specifies the index of the function call table. Example: (call_indirect "foo" $FUNCSIG$i (get_local $1)) (4) Corresponding upstream LLVM changes are required to use multiple tables, but the updated format is backwards compatible. Example: i32.call_indirect $0=, $pop0 i32.call_indirect.1 $0=, $pop0, $1, $2, $3 (5) Generating WebAssembly from code built with Clang/LLVM CFI now utilizes multiple tables. This is the only enabled use case for multiple tables; all others will default to a single table, if tables are used. The value passed in the .indidx assembler directive is now interpreted as the index of the indirect call table to assign.
This was described as for feedback only, so I think it can be closed. |
This pull should not be merged, but I'd like some feedback on the implementation. It adds support for multiple non-homogeneous tables, which exceeds the feature set desired for MVP, but is necessary for testing fine-grained CFI. Below is a summary of changes:
default
,elementType
,initial
, andmax
attributes, plus aname
. Currently, theinitial
andmax
attributes must be equal to the number ofelements
. TheelementType
attribute is interpreted as aFunctionType
index
, and type homogeneity is enforced on table elements, unless the specified FunctionType has name"anyfunc"
, which corresponds to a FunctionType with anone
parameter and return typenone
. Format:(table $<name> [default] (type <type>) <entries>)
(table $foo default (type $FUNCSIG$i) $a)
(table $bla (type $anyfunc) $b $c $d)
(call_indirect $foo $FUNCSIG$i (get_local $1))
default
flag is inferred to betrue
for the first table if unspecified but multiple tables are used, the type of the default table is inferred to be"anyfunc"
if not specified, the target table of indirect calls is inferred to be the default table if the positional argument does not match a known table name, and the first table with index zero is the default table if multiple tables are used.Example:
i32.call_indirect $0=, $pop0
i32.call_indirect.1 $0=, $pop0, $1, $2, $3
(table $aaa $a $b)
(call_indirect $FUNCSIG$ii $a $b $c)
.indidx
assembler directive is now interpreted as the index of the indirect call table that the current function should be assigned to.This pull also requires LLVM support and V8 support.