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

Emit clear error for index into zero-sized Arrays #171

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
3 changes: 0 additions & 3 deletions crates/rustc_codegen_spirv/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -745,9 +745,6 @@ fn trans_aggregate<'tcx>(cx: &CodegenCx<'tcx>, span: Span, ty: TyAndLayout<'tcx>
element: element_type,
}
.def(span, cx)
} else if count == 0 {
// spir-v doesn't support zero-sized arrays
create_zst(cx, span, ty)
} else {
let count_const = cx.constant_u32(span, count as u32);
let element_spv = cx.lookup_type(element_type);
Expand Down
20 changes: 17 additions & 3 deletions crates/rustc_codegen_spirv/src/builder/builder_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let ptr = ptr.strip_ptrcasts();
let mut leaf_ty = match self.lookup_type(ptr.ty) {
SpirvType::Pointer { pointee } => pointee,
other => self.fatal(format!("non-pointer type: {other:?}")),
SpirvType::Adt { .. } => return None,
other => self.fatal(format!("adjust_pointer for non-pointer type: {other:?}")),
};

// FIXME(eddyb) this isn't efficient, `recover_access_chain_from_offset`
Expand Down Expand Up @@ -531,8 +532,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let stride = ty_kind.sizeof(self)?;
ty_size = MaybeSized::Sized(stride);

indices.push((offset.bytes() / stride.bytes()).try_into().ok()?);
offset = Size::from_bytes(offset.bytes() % stride.bytes());
if stride.bytes() == 0 {
indices.push(0);
offset = Size::from_bytes(0);
} else {
indices.push((offset.bytes() / stride.bytes()).try_into().ok()?);
offset = Size::from_bytes(offset.bytes() % stride.bytes());
}
}
_ => return None,
}
Expand Down Expand Up @@ -569,6 +575,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
.iter()
.map(|index| {
result_pointee_type = match self.lookup_type(result_pointee_type) {
SpirvType::Array { count, element }
if self.builder.lookup_const_scalar(count) == Some(0) =>
{
self.fatal(format!(
"evaluation of constant value failed: cannot index into [{}; 0]",
self.debug_type(element)
))
}
SpirvType::Array { element, .. } | SpirvType::RuntimeArray { element } => {
element
}
Expand Down
5 changes: 5 additions & 0 deletions crates/rustc_codegen_spirv/src/codegen_cx/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,11 @@ impl<'tcx> CodegenCx<'tcx> {
}
self.constant_composite(ty, values.into_iter())
}
SpirvType::Array { count, .. }
if self.builder.lookup_const_scalar(count) == Some(0) =>
{
self.undef(ty)
}
SpirvType::Array { element, count } => {
let count = self.builder.lookup_const_scalar(count).unwrap() as usize;
let values = (0..count).map(|_| {
Expand Down
3 changes: 3 additions & 0 deletions crates/rustc_codegen_spirv/src/spirv_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,9 @@ impl SpirvType<'_> {
.bytes(),
)
.expect("alignof: Vectors must have power-of-2 size"),
Self::Array { count, .. } if cx.builder.lookup_const_scalar(count) == Some(0) => {
Align::from_bytes(0).unwrap()
}
Self::Array { element, .. }
| Self::RuntimeArray { element }
| Self::Matrix { element, .. } => cx.lookup_type(element).alignof(cx),
Expand Down
2 changes: 2 additions & 0 deletions tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,5 @@ the resulting binary with spirv-val). Because of this, there might be some stran
the point isn't to make a fully functional shader every time (that would take an annoying amount of
effort), but rather validate that specific parts of the compiler are doing their job correctly
(either succeeding as they should, or erroring as they should).

For more docs on compiletests, check the [rustc docs](https://rustc-dev-guide.rust-lang.org/tests/compiletest.html).
11 changes: 11 additions & 0 deletions tests/ui/lang/core/array/array_0.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#![allow(unconditional_panic)]

// build-fail
#![cfg_attr(target_arch = "spirv", no_std)]
use spirv_std::spirv;

#[spirv(compute(threads(1, 1, 1)))]
pub fn compute() {
let array = [0; 0];
let x = array[0];
}
8 changes: 8 additions & 0 deletions tests/ui/lang/core/array/array_0.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: evaluation of constant value failed: cannot index into [i32; 0]
--> $DIR/array_0.rs:10:13
|
10 | let x = array[0];
| ^^^^^^^^

error: aborting due to 1 previous error

9 changes: 9 additions & 0 deletions tests/ui/lang/core/array/array_entry_param.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// build-fail
#![allow(unconditional_panic)]

#![cfg_attr(target_arch = "spirv", no_std)]
use spirv_std::spirv;

// note that &mut [usize; 0] will cause an even worse panic
#[spirv(compute(threads(1, 1, 1)))]
pub fn compute(m: [usize; 0]) {}
8 changes: 8 additions & 0 deletions tests/ui/lang/core/array/array_entry_param.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: entry point parameter type not yet supported (`[usize; 0]` has size `0`)
--> $DIR/array_entry_param.rs:9:19
|
9 | pub fn compute(m: [usize; 0]) {}
| ^^^^^^^^^^

error: aborting due to 1 previous error

12 changes: 12 additions & 0 deletions tests/ui/lang/core/array/array_zst_0.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// build-pass
#![cfg_attr(target_arch = "spirv", no_std)]
use spirv_std::spirv;

#[spirv(compute(threads(1, 1, 1)))]
pub fn compute() {
let mut array = [(); 0];
for i in 0..array.len() {
array[i] = ();
}
let () = array[0];
}
10 changes: 10 additions & 0 deletions tests/ui/lang/core/array/array_zst_0.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: this operation will panic at runtime
--> $DIR/array_zst_0.rs:11:14
|
11 | let () = array[0];
| ^^^^^^^^ index out of bounds: the length is 0 but the index is 0
|
= note: `#[deny(unconditional_panic)]` on by default

error: aborting due to 1 previous error

16 changes: 16 additions & 0 deletions tests/ui/lang/core/array/gep0.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// build-fail

#![cfg_attr(target_arch = "spirv", no_std)]
use spirv_std::spirv;

fn example<const N: usize>() {
let mut array = [0; N];
for i in 0..N {
array[i] += i;
}
}

#[spirv(compute(threads(1, 1, 1)))]
pub fn compute() {
example::<0>();
}
8 changes: 8 additions & 0 deletions tests/ui/lang/core/array/gep0.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: evaluation of constant value failed: cannot index into [u32; 0]
--> $DIR/gep0.rs:9:9
|
9 | array[i] += i;
| ^^^^^^^^^^^^^

error: aborting due to 1 previous error

16 changes: 16 additions & 0 deletions tests/ui/lang/core/array/oob_0_array.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// build-pass
// compile-flags: -C llvm-args=--disassemble-entry=compute
#![allow(unconditional_panic)]
JulianKnodt marked this conversation as resolved.
Show resolved Hide resolved

#![cfg_attr(target_arch = "spirv", no_std)]
use spirv_std::spirv;

#[spirv(compute(threads(1, 1, 1)))]
pub fn compute() {
let mut array = [0; 0];
// writes to an array compile fine although they should be a panic.
// (&mut array)[0] = 1; // this fails to compile, but it seems that below is
// optimized out, and I'm not sure where.
// GEP is not being hit, and neither is any load/store.
array[0] = 1;
}
14 changes: 14 additions & 0 deletions tests/ui/lang/core/array/oob_0_array.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
%1 = OpFunction %2 None %3
%4 = OpLabel
OpLine %5 15 4
%6 = OpULessThan %7 %8 %8
OpNoLine
OpSelectionMerge %9 None
OpBranchConditional %6 %10 %11
%10 = OpLabel
OpBranch %9
%11 = OpLabel
OpReturn
%9 = OpLabel
OpReturn
OpFunctionEnd
7 changes: 7 additions & 0 deletions tests/ui/lang/core/array/unused_array_0.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#![cfg_attr(target_arch = "spirv", no_std)]
use spirv_std::spirv;

#[spirv(compute(threads(1, 1, 1)))]
pub fn compute() {
let array = [0; 0];
}