Skip to content

Commit

Permalink
Address additional review of WebAssembly#222, also fixing label types…
Browse files Browse the repository at this point in the history
… everywhere. (WebAssembly#241)

This addresses additional review comments to PR WebAssembly#222, that were made after it was merged.
The last review comment in the discussion suggests to adjust all validation labels to use label types instead of just result types.
Should address all occurrences of validation labels.

Additionally adds a boolean catch_label to control frames in the validation algorithm,
and some related functionality, fixing the cases for opcodes `catch` and `catch_all`.

* Apply suggestions from code review

Co-authored-by: Andreas Rossberg <[email protected]>
Co-authored-by: Heejin Ahn <[email protected]>

* Reverting changes to typing of CAUGHTadm.

Changes to this rule are now done in PR WebAssembly#244
  • Loading branch information
ioannad committed Dec 23, 2022
1 parent b56918d commit a8056e5
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 24 deletions.
2 changes: 1 addition & 1 deletion document/core/appendix/algorithm.rst
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ The control stack is likewise manipulated through auxiliary functions:
ctrls.pop()
return frame
func label_types(frame : ctrl_frame) : list(val_types) =
func label_types(frame : ctrl_frame) : list(val_type) =
return (if frame.opcode == loop then frame.start_types else frame.end_types)
func unreachable() =
Expand Down
5 changes: 2 additions & 3 deletions document/core/appendix/properties.rst
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ To that end, all previous typing judgements :math:`C \vdash \X{prop}` are genera
\frac{
S; C,\CLABELS\,[t^\ast] \vdashinstrseq \instr^\ast : [] \to [t^\ast]
\qquad
C.\CLABELS[l] = [t_0^\ast]
C.\CLABELS[l] = \LCATCH^?~[t_0^\ast]
}{
S; C,\CLABELS\,[t^\ast] \vdashadmininstr \DELEGATEadm\{l\}~\instr^\ast~\END : [] \to [t^\ast]
}
Expand All @@ -685,7 +685,6 @@ To that end, all previous typing judgements :math:`C \vdash \X{prop}` are genera

* Then the compound instruction is valid under context :math:`C''` with type :math:`[] \to [t^\ast]`.


.. math::
\frac{
S \vdashexternval \EVTAG~\tagaddr : \ETTAG~[t_0^\ast]\to[]
Expand Down Expand Up @@ -722,7 +721,7 @@ To that end, all previous typing judgements :math:`C \vdash \X{prop}` are genera

* The instruction sequence :math:`\instr_0^\ast` must be :ref:`valid <valid-instr-seq>` with some type :math:`[t_1^n] \to [t_2^*]`.

* Let :math:`C'` be the same :ref:`context <context>` as :math:`C`, but with the :ref:`result type <syntax-resulttype>` :math:`[t_1^n]` prepended to the |CLABELS| vector.
* Let :math:`C'` be the same :ref:`context <context>` as :math:`C`, but with the :ref:`label type <syntax-labeltype>` :math:`[t_1^n]` prepended to the |CLABELS| vector.

* Under context :math:`C'`,
the instruction sequence :math:`\instr^\ast` must be :ref:`valid <valid-instr-seq>` with type :math:`[] \to [t_2^*]`.
Expand Down
40 changes: 21 additions & 19 deletions document/core/valid/instructions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1234,7 +1234,7 @@ Memory Instructions
}
.. index:: control instructions, structured control, label, block, branch, block type, label index, function index, type index, tag index, vector, polymorphism, context
.. index:: control instructions, structured control, label, block, branch, block type, label index, label type, function index, type index, tag index, vector, polymorphism, context
pair: validation; instruction
single: abstract syntax; instruction
.. _valid-label:
Expand Down Expand Up @@ -1281,7 +1281,7 @@ Control Instructions

* The :ref:`block type <syntax-blocktype>` must be :ref:`valid <valid-blocktype>` as some :ref:`function type <syntax-functype>` :math:`[t_1^\ast] \to [t_2^\ast]`.

* Let :math:`C'` be the same :ref:`context <context>` as :math:`C`, but with the :ref:`result type <syntax-resulttype>` :math:`[t_2^\ast]` prepended to the |CLABELS| vector.
* Let :math:`C'` be the same :ref:`context <context>` as :math:`C`, but with the :ref:`label type <syntax-labeltype>` :math:`[t_2^\ast]` prepended to the |CLABELS| vector.

* Under context :math:`C'`,
the instruction sequence :math:`\instr^\ast` must be :ref:`valid <valid-instr-seq>` with type :math:`[t_1^\ast] \to [t_2^\ast]`.
Expand All @@ -1308,7 +1308,7 @@ Control Instructions

* The :ref:`block type <syntax-blocktype>` must be :ref:`valid <valid-blocktype>` as some :ref:`function type <syntax-functype>` :math:`[t_1^\ast] \to [t_2^\ast]`.

* Let :math:`C'` be the same :ref:`context <context>` as :math:`C`, but with the :ref:`result type <syntax-resulttype>` :math:`[t_1^\ast]` prepended to the |CLABELS| vector.
* Let :math:`C'` be the same :ref:`context <context>` as :math:`C`, but with the :ref:`label type <syntax-labeltype>` :math:`[t_1^\ast]` prepended to the |CLABELS| vector.

* Under context :math:`C'`,
the instruction sequence :math:`\instr^\ast` must be :ref:`valid <valid-instr-seq>` with type :math:`[t_1^\ast] \to [t_2^\ast]`.
Expand All @@ -1335,7 +1335,7 @@ Control Instructions

* The :ref:`block type <syntax-blocktype>` must be :ref:`valid <valid-blocktype>` as some :ref:`function type <syntax-functype>` :math:`[t_1^\ast] \to [t_2^\ast]`.

* Let :math:`C'` be the same :ref:`context <context>` as :math:`C`, but with the :ref:`result type <syntax-resulttype>` :math:`[t_2^\ast]` prepended to the |CLABELS| vector.
* Let :math:`C'` be the same :ref:`context <context>` as :math:`C`, but with the :ref:`label type <syntax-labeltype>` :math:`[t_2^\ast]` prepended to the |CLABELS| vector.

* Under context :math:`C'`,
the instruction sequence :math:`\instr_1^\ast` must be :ref:`valid <valid-instr-seq>` with type :math:`[t_1^\ast] \to [t_2^\ast]`.
Expand Down Expand Up @@ -1368,23 +1368,25 @@ Control Instructions

* The :ref:`block type <syntax-blocktype>` must be :ref:`valid <valid-blocktype>` as some :ref:`function type <syntax-functype>` :math:`[t_1^\ast] \to [t_2^\ast]`.

* Let :math:`C'` be the same :ref:`context <context>` as :math:`C`, but with the :ref:`result type <syntax-resulttype>` :math:`[t_2^\ast]` prepended to the |CLABELS| vector.
* Let :math:`C'` be the same :ref:`context <context>` as :math:`C`, but with the :ref:`label type <syntax-labeltype>` :math:`[t_2^\ast]` prepended to the |CLABELS| vector.

* Under context :math:`C'`,
the instruction sequence :math:`\instr_1^\ast` must be :ref:`valid <valid-instr-seq>` with type :math:`[t_1^\ast] \to [t_2^\ast]`.

* Let :math:`C''` be the same :ref:`context <context>` as :math:`C`, but with the :ref:`label <exec-label>` :math:`\LCATCH~[t_2^\ast]` prepended to the |CLABELS| vector.
* Let :math:`C''` be the same :ref:`context <context>` as :math:`C`, but with the :ref:`label type <syntax-labeltype>` :math:`\LCATCH~[t_2^\ast]` prepended to the |CLABELS| vector.

* For every :math:`(\CATCH~x~\instr_2^\ast)`:
* For every :math:`x_i` and :math:`\instr_{2i}^\ast` in :math:`(\CATCH~x~\instr_2^\ast)^\ast`:

* The tag :math:`C.\CTAGS[x]` must be defined in the context :math:`C`.
* The tag :math:`C.\CTAGS[x_i]` must be defined in the context :math:`C`.

* Let :math:`[t^\ast] \to []` be its :ref:`tag type <syntax-tagtype>`.
* Let :math:`[t_{3i}^\ast] \to [t_{4i}^\ast]` be the :ref:`tag type <syntax-tagtype>` :math:`C.\CTAGS[x_i]`.

* The :ref:`result type <syntax-resulttype>` :math:`[t_{4i}^\ast]` must be empty.

* Under context :math:`C''`,
the instruction sequence :math:`\instr_2^\ast` must be :ref:`valid <valid-instr-seq>` with type :math:`[t^\ast] \to [t_2^\ast]`.
the instruction sequence :math:`\instr_{2i}^\ast` must be :ref:`valid <valid-instr-seq>` with type :math:`[t_{3i}^\ast] \to [t_2^\ast]`.

* If there is a :math:`(\CATCHALL~\instr_3^\ast)`, then:
* If :math:`(\CATCHALL~\instr_3^\ast)^?` is not empty, then:

* Under context :math:`C''`,
the instruction sequence :math:`\instr_3^\ast` must be :ref:`valid <valid-instr-seq>` with type :math:`[] \to [t_2^\ast]`.
Expand Down Expand Up @@ -1471,9 +1473,9 @@ Control Instructions

* The label :math:`C.\CLABELS[l]` must be defined in the context.

* Let :math:`(\LCATCH^?~[t^\ast])` be the :ref:`label type <labeltype>` :math:`C.\CLABELS[l]`.
* Let :math:`(\LCATCH^?~[t^\ast])` be the :ref:`label type <syntax-labeltype>` :math:`C.\CLABELS[l]`.

* The |LCATCH| must be present in the :ref:`label type <labeltype>` :math:`C.\CLABELS[l]`.
* The |LCATCH| must be present in the :ref:`label type <syntax-labeltype>` :math:`C.\CLABELS[l]`.

* Then the instruction is valid with type :math:`[t_1^\ast] \to [t_2^\ast]`, for any sequences of :ref:`value types <syntax-valtype>` :math:`t_1^\ast` and :math:`t_2^\ast`.

Expand All @@ -1498,19 +1500,19 @@ Control Instructions

* The label :math:`C.\CLABELS[l]` must be defined in the context.

* Let :math:`[t^\ast]` be the :ref:`result type <syntax-resulttype>` :math:`C.\CLABELS[l]`.
* Let :math:`\LCATCH^?~[t^\ast]` be the :ref:`label type <syntax-labeltype>` :math:`C.\CLABELS[l]`.

* Then the instruction is valid with type :math:`[t_1^\ast~t^\ast] \to [t_2^\ast]`, for any sequences of :ref:`value types <syntax-valtype>` :math:`t_1^\ast` and :math:`t_2^\ast`.

.. math::
\frac{
C.\CLABELS[l] = [t^\ast]
C.\CLABELS[l] = \LCATCH^?~[t^\ast]
}{
C \vdashinstr \BR~l : [t_1^\ast~t^\ast] \to [t_2^\ast]
}
.. note::
The :ref:`label index <syntax-labelidx>` space in the :ref:`context <context>` :math:`C` contains the most recent label first, so that :math:`C.\CLABELS[l]` performs a relative lookup as expected.
The :ref:`label index <syntax-labelidx>` space in the :ref:`context <context>` :math:`C` contains the most recent label type first, so that :math:`C.\CLABELS[l]` performs a relative lookup as expected.

The |BR| instruction is :ref:`stack-polymorphic <polymorphism>`.

Expand All @@ -1522,19 +1524,19 @@ Control Instructions

* The label :math:`C.\CLABELS[l]` must be defined in the context.

* Let :math:`[t^\ast]` be the :ref:`result type <syntax-resulttype>` :math:`C.\CLABELS[l]`.
* Let :math:`\LCATCH^?~[t^\ast]` be the :ref:`label type <syntax-labeltype>` :math:`C.\CLABELS[l]`.

* Then the instruction is valid with type :math:`[t^\ast~\I32] \to [t^\ast]`.

.. math::
\frac{
C.\CLABELS[l] = [t^\ast]
C.\CLABELS[l] = \LCATCH^?~[t^\ast]
}{
C \vdashinstr \BRIF~l : [t^\ast~\I32] \to [t^\ast]
}
.. note::
The :ref:`label index <syntax-labelidx>` space in the :ref:`context <context>` :math:`C` contains the most recent label first, so that :math:`C.\CLABELS[l]` performs a relative lookup as expected.
The :ref:`label index <syntax-labelidx>` space in the :ref:`context <context>` :math:`C` contains the most recent label type first, so that :math:`C.\CLABELS[l]` performs a relative lookup as expected.


.. _valid-br_table:
Expand Down
2 changes: 1 addition & 1 deletion document/core/valid/modules.rst
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Functions :math:`\func` are classified by :ref:`function types <syntax-functype>

* |CLOCALS| set to the sequence of :ref:`value types <syntax-valtype>` :math:`t_1^\ast~t^\ast`, concatenating parameters and locals,

* |CLABELS| set to the singular sequence containing only :ref:`result type <syntax-resulttype>` :math:`[t_2^\ast]`.
* |CLABELS| set to the singular sequence containing only :ref:`label type <syntax-labeltype>` :math:`[t_2^\ast]`.

* |CRETURN| set to the :ref:`result type <syntax-resulttype>` :math:`[t_2^\ast]`.

Expand Down

0 comments on commit a8056e5

Please sign in to comment.