Skip to content

Commit

Permalink
threads: add validation for global constant expressions
Browse files Browse the repository at this point in the history
Previously, it was possible to use an unshared global to initialize a
shared global, table element. This change makes that invalid by
propagating sharedness in to the constant expression validator.
  • Loading branch information
abrown committed Nov 7, 2024
1 parent 3e0dda0 commit d99b51d
Show file tree
Hide file tree
Showing 14 changed files with 275 additions and 174 deletions.
54 changes: 45 additions & 9 deletions crates/wasmparser/src/validator/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,13 @@ impl ModuleState {
) -> Result<()> {
self.module
.check_global_type(&mut global.ty, features, types, offset)?;
self.check_const_expr(&global.init_expr, global.ty.content_type, features, types)?;
self.check_const_expr(
&global.init_expr,
global.ty.content_type,
global.ty.shared,
features,
types,
)?;
self.module.assert_mut().globals.push(global.ty);
Ok(())
}
Expand Down Expand Up @@ -162,7 +168,13 @@ impl ModuleState {
the function-references proposal"
);
}
self.check_const_expr(expr, table.ty.element_type.into(), features, types)?;
self.check_const_expr(
expr,
table.ty.element_type.into(),
table.ty.shared,
features,
types,
)?;
}
}
self.module.assert_mut().tables.push(table.ty);
Expand All @@ -182,8 +194,8 @@ impl ModuleState {
memory_index,
offset_expr,
} => {
let ty = self.module.memory_at(memory_index, offset)?.index_type();
self.check_const_expr(&offset_expr, ty, features, types)
let ty = self.module.memory_at(memory_index, offset)?;
self.check_const_expr(&offset_expr, ty.index_type(), ty.shared, features, types)
}
}
}
Expand All @@ -195,8 +207,8 @@ impl ModuleState {
types: &TypeList,
offset: usize,
) -> Result<()> {
// the `funcref` value type is allowed all the way back to the MVP, so
// don't check it here
// The `funcref` value type is allowed all the way back to the MVP, so
// don't check it here.
let element_ty = match &mut e.items {
crate::ElementItems::Functions(_) => RefType::FUNC,
crate::ElementItems::Expressions(ty, _) => {
Expand All @@ -221,8 +233,13 @@ impl ModuleState {
offset,
));
}

self.check_const_expr(&offset_expr, table.index_type(), features, types)?;
self.check_const_expr(
&offset_expr,
table.index_type(),
table.shared,
features,
types,
)?;
}
ElementKind::Passive | ElementKind::Declared => {
if !features.bulk_memory() {
Expand Down Expand Up @@ -256,8 +273,18 @@ impl ModuleState {
}
crate::ElementItems::Expressions(ty, reader) => {
validate_count(reader.count())?;
let shared = match ty.heap_type() {
HeapType::Abstract { shared, .. } => shared,
HeapType::Concrete(unpacked_index) => {
if let Some(id) = unpacked_index.as_core_type_id() {
types[id].composite_type.shared
} else {
todo!()
}
}
};
for expr in reader {
self.check_const_expr(&expr?, ValType::Ref(ty), features, types)?;
self.check_const_expr(&expr?, ValType::Ref(ty), shared, features, types)?;
}
}
}
Expand All @@ -269,6 +296,7 @@ impl ModuleState {
&mut self,
expr: &ConstExpr<'_>,
expected_ty: ValType,
shared: bool,
features: &WasmFeatures,
types: &TypeList,
) -> Result<()> {
Expand All @@ -286,6 +314,7 @@ impl ModuleState {
module: &mut self.module,
},
features,
shared,
};

let mut ops = expr.get_operators_reader();
Expand All @@ -309,6 +338,7 @@ impl ModuleState {
resources: OperatorValidatorResources<'a>,
order: Order,
features: &'a WasmFeatures,
shared: bool,
}

impl VisitConstOperator<'_> {
Expand Down Expand Up @@ -374,6 +404,12 @@ impl ModuleState {
self.offset,
));
}
if self.shared && !global.shared {
return Err(BinaryReaderError::new(
"invalid type: constant expression must be shared",
self.offset,
));
}
Ok(())
}

Expand Down
12 changes: 12 additions & 0 deletions tests/local/shared-everything-threads/globals.wast
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,18 @@
(global (shared mut v128) (v128.const i64x2 0 0))
)

;; Check that we can only use shared globals to initialize other shared globals.
(module
(global $a (shared i32) (i32.const 0))
(global $b (shared i32) (global.get $a))
)
(assert_invalid
(module
(global $a i32 (i32.const 0))
(global $b (shared i32) (global.get $a))
)
"invalid type")

(assert_malformed
(module quote "(global (mut shared i64) (i64.const -1))")
"unexpected token")
Expand Down
17 changes: 17 additions & 0 deletions tests/local/shared-everything-threads/tables.wast
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,23 @@
(table shared 0 (ref $t)))
"shared tables must have a shared element type")

;; A shared table must be initialized from shared objects.
(module
(type $f (shared (func)))
(global $g (shared (ref null $f)) (ref.null $f))
(table $t shared 1 (ref null $f))
(elem (ref null $f) (global.get $g))
)
(assert_invalid
(module
(type $f (shared (func)))
(global $g (ref null $f) (ref.null $f))
(table $t shared 1 (ref null $f))
;; When we initialize a shared element, everything must be shared, including
;; the used global.
(elem (ref null $f) (global.get $g)))
"invalid type")

;; Check `table.atomic.*` instructions.
(module (;eq;)
(table $a (import "spectest" "table_eq") shared 1 (ref null (shared eq)))
Expand Down
Loading

0 comments on commit d99b51d

Please sign in to comment.