Skip to content

Commit c7c79a0

Browse files
andyleisersonandyleiserson
authored
Disallow taking the address of a vector component (gfx-rs#7284)
1 parent efbfa36 commit c7c79a0

File tree

9 files changed

+182
-112
lines changed

9 files changed

+182
-112
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,8 @@ By @wumpf in [#7144](https://github.com/gfx-rs/wgpu/pull/7144)
218218
The original names (e.g. `naga::Module`) remain present for compatibility.
219219
By @kpreid in [#7365](https://github.com/gfx-rs/wgpu/pull/7365).
220220
- Refactored `use` statements to simplify future `no_std` support. By @bushrat011899 in [#7256](https://github.com/gfx-rs/wgpu/pull/7256)
221+
- Naga's WGSL frontend no longer allows using the `&` operator to take the address of a component of a vector,
222+
which is not permitted by the WGSL specification. By @andyleiserson in [#7284](https://github.com/gfx-rs/wgpu/pull/7284)
221223
222224
#### Vulkan
223225

‎naga/src/front/wgsl/error.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ pub(crate) enum Error<'a> {
215215
},
216216
DeclMissingTypeAndInit(Span),
217217
MissingAttribute(&'static str, Span),
218+
InvalidAddrOfOperand(Span),
218219
InvalidAtomicPointer(Span),
219220
InvalidAtomicOperandType(Span),
220221
InvalidRayQueryPointer(Span),
@@ -676,6 +677,11 @@ impl<'a> Error<'a> {
676677
)],
677678
notes: vec![],
678679
},
680+
Error::InvalidAddrOfOperand(span) => ParseError {
681+
message: "cannot take the address of a vector component".to_string(),
682+
labels: vec![(span, "invalid operand for address-of".into())],
683+
notes: vec![],
684+
},
679685
Error::InvalidAtomicPointer(span) => ParseError {
680686
message: "atomic operation is done on a pointer to a non-atomic".to_string(),
681687
labels: vec![(span, "atomic pointer is invalid".into())],

‎naga/src/front/wgsl/lower/mod.rs‎

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2077,6 +2077,21 @@ impl<'source, 'temp> Lowerer<'source, 'temp> {
20772077
// reference is required, the Load Rule is not applied.
20782078
match self.expression_for_reference(expr, ctx)? {
20792079
Typed::Reference(handle) => {
2080+
let expr = &ctx.runtime_expression_ctx(span)?.function.expressions[handle];
2081+
if let &crate::Expression::Access { base, .. }
2082+
| &crate::Expression::AccessIndex { base, .. } = expr
2083+
{
2084+
if let Some(ty) = resolve_inner!(ctx, base).pointer_base_type() {
2085+
if matches!(
2086+
*ty.inner_with(&ctx.module.types),
2087+
crate::TypeInner::Vector { .. },
2088+
) {
2089+
return Err(Box::new(Error::InvalidAddrOfOperand(
2090+
ctx.get_expression_span(handle),
2091+
)));
2092+
}
2093+
}
2094+
}
20802095
// No code is generated. We just declare the reference a pointer now.
20812096
return Ok(Typed::Plain(handle));
20822097
}
@@ -2149,30 +2164,13 @@ impl<'source, 'temp> Lowerer<'source, 'temp> {
21492164
}
21502165
}
21512166

2152-
let temp_inner;
2167+
let temp_ty;
21532168
let composite_type: &crate::TypeInner = match lowered_base {
21542169
Typed::Reference(handle) => {
2155-
let inner = resolve_inner!(ctx, handle);
2156-
match *inner {
2157-
crate::TypeInner::Pointer { base, .. } => &ctx.module.types[base].inner,
2158-
crate::TypeInner::ValuePointer {
2159-
size: None, scalar, ..
2160-
} => {
2161-
temp_inner = crate::TypeInner::Scalar(scalar);
2162-
&temp_inner
2163-
}
2164-
crate::TypeInner::ValuePointer {
2165-
size: Some(size),
2166-
scalar,
2167-
..
2168-
} => {
2169-
temp_inner = crate::TypeInner::Vector { size, scalar };
2170-
&temp_inner
2171-
}
2172-
_ => unreachable!(
2173-
"In Typed::Reference(handle), handle must be a Naga pointer"
2174-
),
2175-
}
2170+
temp_ty = resolve_inner!(ctx, handle)
2171+
.pointer_base_type()
2172+
.expect("In Typed::Reference(handle), handle must be a Naga pointer");
2173+
temp_ty.inner_with(&ctx.module.types)
21762174
}
21772175

21782176
Typed::Plain(handle) => {

‎naga/src/proc/type_methods.rs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,25 @@ impl crate::TypeInner {
128128
}
129129
}
130130

131+
/// If `self` is a pointer type, return its base type.
132+
pub const fn pointer_base_type(&self) -> Option<TypeResolution> {
133+
match *self {
134+
crate::TypeInner::Pointer { base, .. } => Some(TypeResolution::Handle(base)),
135+
crate::TypeInner::ValuePointer {
136+
size: None, scalar, ..
137+
} => Some(TypeResolution::Value(crate::TypeInner::Scalar(scalar))),
138+
crate::TypeInner::ValuePointer {
139+
size: Some(size),
140+
scalar,
141+
..
142+
} => Some(TypeResolution::Value(crate::TypeInner::Vector {
143+
size,
144+
scalar,
145+
})),
146+
_ => None,
147+
}
148+
}
149+
131150
pub fn is_atomic_pointer(&self, types: &crate::UniqueArena<crate::Type>) -> bool {
132151
match *self {
133152
crate::TypeInner::Pointer { base, .. } => match types[base].inner {

‎naga/src/valid/function.rs‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use super::{
77
};
88
use crate::arena::{Arena, UniqueArena};
99
use crate::arena::{Handle, HandleSet};
10+
use crate::proc::TypeResolution;
1011
use crate::span::WithSpan;
1112
use crate::span::{AddSpan as _, MapErrWithSpan as _};
1213

@@ -299,6 +300,10 @@ impl<'a> BlockContext<'a> {
299300
fn resolve_pointer_type(&self, handle: Handle<crate::Expression>) -> &crate::TypeInner {
300301
self.info[handle].ty.inner_with(self.types)
301302
}
303+
304+
fn inner_type<'t>(&'t self, ty: &'t TypeResolution) -> &'t crate::TypeInner {
305+
ty.inner_with(self.types)
306+
}
302307
}
303308

304309
impl super::Validator {
@@ -1039,23 +1044,15 @@ impl super::Validator {
10391044
}
10401045

10411046
let pointer_ty = context.resolve_pointer_type(pointer);
1042-
1043-
let good = match *pointer_ty {
1044-
Ti::Pointer { base, space: _ } => match context.types[base].inner {
1045-
Ti::Atomic(scalar) => *value_ty == Ti::Scalar(scalar),
1046-
ref other => value_ty == other,
1047-
},
1048-
Ti::ValuePointer {
1049-
size: Some(size),
1050-
scalar,
1051-
space: _,
1052-
} => *value_ty == Ti::Vector { size, scalar },
1053-
Ti::ValuePointer {
1054-
size: None,
1055-
scalar,
1056-
space: _,
1057-
} => *value_ty == Ti::Scalar(scalar),
1058-
_ => false,
1047+
let good = match pointer_ty
1048+
.pointer_base_type()
1049+
.as_ref()
1050+
.map(|ty| context.inner_type(ty))
1051+
{
1052+
// The Naga IR allows storing a scalar to an atomic.
1053+
Some(&Ti::Atomic(ref scalar)) => *value_ty == Ti::Scalar(*scalar),
1054+
Some(other) => *value_ty == *other,
1055+
None => false,
10591056
};
10601057
if !good {
10611058
return Err(FunctionError::InvalidStoreTypes { pointer, value }

‎naga/tests/in/wgsl/pointers.wgsl‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
fn f() {
2-
var v: vec2<i32>;
3-
let px = &v.x;
4-
*px = 10;
2+
var v: mat2x2<f32>;
3+
let px = &v[0];
4+
*px = vec2<f32>(10.0);
55
}
66

77
struct DynamicArray {

‎naga/tests/out/spv/pointers.spvasm‎

Lines changed: 68 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
; SPIR-V
22
; Version: 1.2
33
; Generator: rspirv
4-
; Bound: 43
4+
; Bound: 46
55
OpCapability Shader
66
OpCapability Linkage
77
OpExtension "SPV_KHR_storage_buffer_storage_class"
88
%1 = OpExtInstImport "GLSL.std.450"
99
OpMemoryModel Logical GLSL450
1010
%3 = OpString "pointers.wgsl"
1111
OpSource Unknown 0 %3 "fn f() {
12-
var v: vec2<i32>;
13-
let px = &v.x;
14-
*px = 10;
12+
var v: mat2x2<f32>;
13+
let px = &v[0];
14+
*px = vec2<f32>(10.0);
1515
}
1616

1717
struct DynamicArray {
@@ -35,80 +35,84 @@ fn index_dynamic_array(i: i32, v: u32) {
3535
(*p)[i] = val + v;
3636
}
3737
"
38-
OpMemberName %8 0 "arr"
39-
OpName %8 "DynamicArray"
40-
OpName %9 "dynamic_array"
41-
OpName %12 "f"
42-
OpName %15 "v"
43-
OpName %23 "i"
44-
OpName %24 "v"
45-
OpName %25 "index_unsized"
46-
OpName %35 "i"
47-
OpName %36 "v"
48-
OpName %37 "index_dynamic_array"
49-
OpDecorate %7 ArrayStride 4
50-
OpMemberDecorate %8 0 Offset 0
51-
OpDecorate %8 Block
52-
OpDecorate %9 DescriptorSet 0
53-
OpDecorate %9 Binding 0
38+
OpMemberName %9 0 "arr"
39+
OpName %9 "DynamicArray"
40+
OpName %11 "dynamic_array"
41+
OpName %14 "f"
42+
OpName %18 "v"
43+
OpName %26 "i"
44+
OpName %27 "v"
45+
OpName %28 "index_unsized"
46+
OpName %38 "i"
47+
OpName %39 "v"
48+
OpName %40 "index_dynamic_array"
49+
OpDecorate %8 ArrayStride 4
50+
OpMemberDecorate %9 0 Offset 0
51+
OpDecorate %9 Block
52+
OpDecorate %11 DescriptorSet 0
53+
OpDecorate %11 Binding 0
5454
%2 = OpTypeVoid
55-
%4 = OpTypeInt 32 1
56-
%5 = OpTypeVector %4 2
57-
%6 = OpTypeInt 32 0
58-
%7 = OpTypeRuntimeArray %6
59-
%8 = OpTypeStruct %7
60-
%10 = OpTypePointer StorageBuffer %8
61-
%9 = OpVariable %10 StorageBuffer
62-
%13 = OpTypeFunction %2
63-
%14 = OpConstant %4 10
64-
%16 = OpTypePointer Function %5
65-
%17 = OpConstantNull %5
55+
%6 = OpTypeFloat 32
56+
%5 = OpTypeVector %6 2
57+
%4 = OpTypeMatrix %5 2
58+
%7 = OpTypeInt 32 0
59+
%8 = OpTypeRuntimeArray %7
60+
%9 = OpTypeStruct %8
61+
%10 = OpTypeInt 32 1
62+
%12 = OpTypePointer StorageBuffer %9
63+
%11 = OpVariable %12 StorageBuffer
64+
%15 = OpTypeFunction %2
65+
%16 = OpConstant %6 10.0
66+
%17 = OpConstantComposite %5 %16 %16
6667
%19 = OpTypePointer Function %4
67-
%20 = OpConstant %6 0
68-
%26 = OpTypeFunction %2 %4 %6
69-
%28 = OpTypePointer StorageBuffer %7
70-
%29 = OpTypePointer StorageBuffer %6
71-
%12 = OpFunction %2 None %13
72-
%11 = OpLabel
73-
%15 = OpVariable %16 Function %17
74-
OpBranch %18
75-
%18 = OpLabel
68+
%20 = OpConstantNull %4
69+
%22 = OpTypePointer Function %5
70+
%23 = OpConstant %7 0
71+
%29 = OpTypeFunction %2 %10 %7
72+
%31 = OpTypePointer StorageBuffer %8
73+
%32 = OpTypePointer StorageBuffer %7
74+
%14 = OpFunction %2 None %15
75+
%13 = OpLabel
76+
%18 = OpVariable %19 Function %20
77+
OpBranch %21
78+
%21 = OpLabel
7679
OpLine %3 3 14
80+
OpLine %3 4 10
7781
OpLine %3 4 4
78-
%21 = OpAccessChain %19 %15 %20
79-
OpStore %21 %14
82+
%24 = OpAccessChain %22 %18 %23
83+
OpStore %24 %17
8084
OpReturn
8185
OpFunctionEnd
82-
%25 = OpFunction %2 None %26
83-
%23 = OpFunctionParameter %4
84-
%24 = OpFunctionParameter %6
85-
%22 = OpLabel
86-
OpBranch %27
87-
%27 = OpLabel
86+
%28 = OpFunction %2 None %29
87+
%26 = OpFunctionParameter %10
88+
%27 = OpFunctionParameter %7
89+
%25 = OpLabel
90+
OpBranch %30
91+
%30 = OpLabel
8892
OpLine %3 17 14
89-
%30 = OpAccessChain %29 %9 %20 %23
90-
%31 = OpLoad %6 %30
93+
%33 = OpAccessChain %32 %11 %23 %26
94+
%34 = OpLoad %7 %33
9195
OpLine %3 18 4
92-
%32 = OpIAdd %6 %31 %24
96+
%35 = OpIAdd %7 %34 %27
9397
OpLine %3 18 4
94-
%33 = OpAccessChain %29 %9 %20 %23
95-
OpStore %33 %32
98+
%36 = OpAccessChain %32 %11 %23 %26
99+
OpStore %36 %35
96100
OpReturn
97101
OpFunctionEnd
98-
%37 = OpFunction %2 None %26
99-
%35 = OpFunctionParameter %4
100-
%36 = OpFunctionParameter %6
101-
%34 = OpLabel
102-
OpBranch %38
103-
%38 = OpLabel
102+
%40 = OpFunction %2 None %29
103+
%38 = OpFunctionParameter %10
104+
%39 = OpFunctionParameter %7
105+
%37 = OpLabel
106+
OpBranch %41
107+
%41 = OpLabel
104108
OpLine %3 22 51
105109
OpLine %3 24 14
106-
%39 = OpAccessChain %29 %9 %20 %35
107-
%40 = OpLoad %6 %39
110+
%42 = OpAccessChain %32 %11 %23 %38
111+
%43 = OpLoad %7 %42
108112
OpLine %3 25 4
109-
%41 = OpIAdd %6 %40 %36
113+
%44 = OpIAdd %7 %43 %39
110114
OpLine %3 25 4
111-
%42 = OpAccessChain %29 %9 %20 %35
112-
OpStore %42 %41
115+
%45 = OpAccessChain %32 %11 %23 %38
116+
OpStore %45 %44
113117
OpReturn
114118
OpFunctionEnd

‎naga/tests/out/wgsl/pointers.wgsl‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ struct DynamicArray {
66
var<storage, read_write> dynamic_array: DynamicArray;
77

88
fn f() {
9-
var v: vec2<i32>;
9+
var v: mat2x2<f32>;
1010

11-
let px = (&v.x);
12-
(*px) = 10i;
11+
let px = (&v[0]);
12+
(*px) = vec2(10f);
1313
return;
1414
}
1515

0 commit comments

Comments
 (0)