Skip to content
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

[spec/test] Fix scoping of non-imported globals #1525

Merged
merged 4 commits into from
Aug 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion document/core/valid/instructions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1641,7 +1641,7 @@ Constant Expressions
}

.. note::
Currently, constant expressions occurring as initializers of :ref:`globals <syntax-global>` are further constrained in that contained |GLOBALGET| instructions are only allowed to refer to *imported* globals.
Currently, constant expressions occurring in :ref:`globals <syntax-global>`, :ref:`element <syntax-elem>`, or :ref:`data <syntax-data>` segments are further constrained in that contained |GLOBALGET| instructions are only allowed to refer to *imported* globals.
This is enforced in the :ref:`validation rule for modules <valid-module>` by constraining the context :math:`C` accordingly.

The definition of constant expression may be extended in future versions of WebAssembly.
44 changes: 22 additions & 22 deletions document/core/valid/modules.rst
Original file line number Diff line number Diff line change
Expand Up @@ -546,40 +546,40 @@ Instead, the context :math:`C` for validation of the module's content is constru

* all other fields are empty.

* Under the context :math:`C`:
* For each :math:`\functype_i` in :math:`\module.\MTYPES`,
the :ref:`function type <syntax-functype>` :math:`\functype_i` must be :ref:`valid <valid-functype>`.

* For each :math:`\functype_i` in :math:`\module.\MTYPES`,
the :ref:`function type <syntax-functype>` :math:`\functype_i` must be :ref:`valid <valid-functype>`.
* Under the context :math:`C`:

* For each :math:`\func_i` in :math:`\module.\MFUNCS`,
the definition :math:`\func_i` must be :ref:`valid <valid-func>` with a :ref:`function type <syntax-functype>` :math:`\X{ft}_i`.

* If :math:`\module.\MSTART` is non-empty,
then :math:`\module.\MSTART` must be :ref:`valid <valid-start>`.

* For each :math:`\import_i` in :math:`\module.\MIMPORTS`,
the segment :math:`\import_i` must be :ref:`valid <valid-import>` with an :ref:`external type <syntax-externtype>` :math:`\X{it}_i`.

* For each :math:`\export_i` in :math:`\module.\MEXPORTS`,
the segment :math:`\export_i` must be :ref:`valid <valid-export>` with :ref:`external type <syntax-externtype>` :math:`\X{et}_i`.

* Under the context :math:`C'`:

* For each :math:`\table_i` in :math:`\module.\MTABLES`,
the definition :math:`\table_i` must be :ref:`valid <valid-table>` with a :ref:`table type <syntax-tabletype>` :math:`\X{tt}_i`.

* For each :math:`\mem_i` in :math:`\module.\MMEMS`,
the definition :math:`\mem_i` must be :ref:`valid <valid-mem>` with a :ref:`memory type <syntax-memtype>` :math:`\X{mt}_i`.

* For each :math:`\global_i` in :math:`\module.\MGLOBALS`:

* Under the context :math:`C'`,
the definition :math:`\global_i` must be :ref:`valid <valid-global>` with a :ref:`global type <syntax-globaltype>` :math:`\X{gt}_i`.
* For each :math:`\global_i` in :math:`\module.\MGLOBALS`,
the definition :math:`\global_i` must be :ref:`valid <valid-global>` with a :ref:`global type <syntax-globaltype>` :math:`\X{gt}_i`.

* For each :math:`\elem_i` in :math:`\module.\MELEMS`,
the segment :math:`\elem_i` must be :ref:`valid <valid-elem>` with :ref:`reference type <syntax-reftype>` :math:`\X{rt}_i`.

* For each :math:`\data_i` in :math:`\module.\MDATAS`,
the segment :math:`\data_i` must be :ref:`valid <valid-data>`.

* If :math:`\module.\MSTART` is non-empty,
then :math:`\module.\MSTART` must be :ref:`valid <valid-start>`.

* For each :math:`\import_i` in :math:`\module.\MIMPORTS`,
the segment :math:`\import_i` must be :ref:`valid <valid-import>` with an :ref:`external type <syntax-externtype>` :math:`\X{it}_i`.

* For each :math:`\export_i` in :math:`\module.\MEXPORTS`,
the segment :math:`\export_i` must be :ref:`valid <valid-export>` with :ref:`external type <syntax-externtype>` :math:`\X{et}_i`.

* The length of :math:`C.\CMEMS` must not be larger than :math:`1`.

* All export names :math:`\export_i.\ENAME` must be different.
Expand Down Expand Up @@ -607,15 +607,15 @@ Instead, the context :math:`C` for validation of the module's content is constru
\quad
(C \vdashfunc \func : \X{ft})^\ast
\quad
(C \vdashtable \table : \X{tt})^\ast
(C' \vdashtable \table : \X{tt})^\ast
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do tables and memories need to be evaluated under C'?

Copy link
Member Author

@rossberg rossberg Aug 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now the difference isn't visible, but with the funcref proposal we are e.g. adding default initializers to table definitions. Better to be forward-consistent with that.

\quad
(C \vdashmem \mem : \X{mt})^\ast
(C' \vdashmem \mem : \X{mt})^\ast
\quad
(C' \vdashglobal \global : \X{gt})^\ast
\\
(C \vdashelem \elem : \X{rt})^\ast
(C' \vdashelem \elem : \X{rt})^\ast
\quad
(C \vdashdata \data \ok)^n
(C' \vdashdata \data \ok)^n
\quad
(C \vdashstart \start \ok)^?
\quad
Expand Down Expand Up @@ -667,8 +667,8 @@ Instead, the context :math:`C` for validation of the module's content is constru
However, this recursion is just a specification device.
All types needed to construct :math:`C` can easily be determined from a simple pre-pass over the module that does not perform any actual validation.

Globals, however, are not recursive.
The effect of defining the limited context :math:`C'` for validating the module's globals is that their initialization expressions can only access functions and imported globals and nothing else.
Globals, however, are not recursive and not accessible within :ref:`constant expressions <valid-const>` when they are defined locally.
The effect of defining the limited context :math:`C'` for validating certain definitions is that they can only access functions and imported globals and nothing else.

.. note::
The restriction on the number of memories may be lifted in future versions of WebAssembly.
25 changes: 17 additions & 8 deletions test/core/data.wast
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,15 @@
(data (global.get $g) "a")
)

;; Use of internal globals in constant expressions is not allowed in MVP.
;; (module (memory 1) (data (global.get 0) "a") (global i32 (i32.const 0)))
;; (module (memory 1) (data (global.get $g) "a") (global $g i32 (i32.const 0)))
(assert_invalid
(module (memory 1) (global i32 (i32.const 0)) (data (global.get 0) "a"))
"unknown global"
)
(assert_invalid
(module (memory 1) (global $g i32 (i32.const 0)) (data (global.get $g) "a"))
"unknown global"
)


;; Corner cases

Expand Down Expand Up @@ -456,11 +462,14 @@
"constant expression required"
)

;; Use of internal globals in constant expressions is not allowed in MVP.
;; (assert_invalid
;; (module (memory 1) (data (global.get $g)) (global $g (mut i32) (i32.const 0)))
;; "constant expression required"
;; )
(assert_invalid
(module
(global $g (import "test" "g") (mut i32))
(memory 1)
(data (global.get $g))
)
"constant expression required"
)

(assert_invalid
(module
Expand Down
23 changes: 18 additions & 5 deletions test/core/elem.wast
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,16 @@
(assert_return (invoke "call-7") (i32.const 65))
(assert_return (invoke "call-9") (i32.const 66))

(assert_invalid
(module (table 1 funcref) (global i32 (i32.const 0)) (elem (global.get 0) $f) (func $f))
"unknown global"
)
(assert_invalid
(module (table 1 funcref) (global $g i32 (i32.const 0)) (elem (global.get $g) $f) (func $f))
"unknown global"
)


;; Corner cases

(module
Expand Down Expand Up @@ -425,11 +435,14 @@
"constant expression required"
)

;; Use of internal globals in constant expressions is not allowed in MVP.
;; (assert_invalid
;; (module (table 1 funcref) (elem (global.get $g)) (global $g i32 (i32.const 0)))
;; "constant expression required"
;; )
(assert_invalid
(module
(global $g (import "test" "g") (mut i32))
(table 1 funcref)
(elem (global.get $g))
)
"constant expression required"
)

(assert_invalid
(module
Expand Down
9 changes: 9 additions & 0 deletions test/core/global.wast
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,15 @@
"unknown global"
)

(assert_invalid
(module (global i32 (i32.const 0)) (global i32 (global.get 0)))
"unknown global"
)
(assert_invalid
(module (global $g i32 (i32.const 0)) (global i32 (global.get $g)))
"unknown global"
)

(assert_invalid
(module (global i32 (global.get 1)) (global i32 (i32.const 0)))
"unknown global"
Expand Down