From 67a3f10a517722b07871d995bbfcc3369dadd16d Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Tue, 23 Mar 2021 18:21:20 +0100 Subject: [PATCH 1/2] Checkpoint fix for #19 --- tests/invalid_binary.spv | Bin 0 -> 88 bytes tests/val.rs | 22 ++++++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 tests/invalid_binary.spv diff --git a/tests/invalid_binary.spv b/tests/invalid_binary.spv new file mode 100644 index 0000000000000000000000000000000000000000..e3441ea57c828fab9de7b62381c1a906fb9f41fb GIT binary patch literal 88 zcmZQ(Qf6mhW@KPs;Aa4lARx%V1ZIQihawPulpq5G7Xt@Fa6nkRw?|NXS!qsoVqSc1 XYHof}WqfXaN@@-R9|JR3KZpbXh;0m+ literal 0 HcmV?d00001 diff --git a/tests/val.rs b/tests/val.rs index 63779cd..7e97baf 100644 --- a/tests/val.rs +++ b/tests/val.rs @@ -1,6 +1,7 @@ #![allow(clippy::unnecessary_wraps)] const SPIRV_BIN: &[u8] = include_bytes!("wgpu_example_shader.spv"); +const SPIRV_OTHER: &[u8] = include_bytes!("invalid_binary.spv"); fn validate_compiled(_input: &[u8]) -> Option> { #[cfg(feature = "use-compiled-tools")] @@ -44,3 +45,24 @@ fn gets_error_message() { _ => {} } } + +#[test] +fn verify_error_message() { + let cexpected_msg = "invalid binary:0:0 - No OpEntryPoint instruction was found. This is only allowed if the Linkage capability is being used."; + let texpected_msg = ""; + match (validate_compiled(SPIRV_OTHER), validate_tool(SPIRV_OTHER)) { + (Some(resc), Some(rest)) => { + // assert_eq!(resc, rest); + + assert_eq!(resc.unwrap_err().to_string(), cexpected_msg); + assert_eq!(rest.unwrap_err().to_string(), texpected_msg); + } + (Some(resc), None) => { + assert_eq!(resc.unwrap_err().to_string(), cexpected_msg); + } + (None, Some(rest)) => { + assert_eq!(rest.unwrap_err().to_string(), texpected_msg); + } + _ => {} + } +} From bb929f72f121d2c16f52d7cbbd2acddb20811dd0 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Wed, 24 Mar 2021 14:48:51 +0100 Subject: [PATCH 2/2] Ignore the spirv error code when in Error::display This is due to the lossiness of running the spirv binaries, while needing (more) consistent error output between compiled and tool. --- src/cmd.rs | 5 ++++- src/error.rs | 6 +++--- tests/invalid_binary.spv | Bin 88 -> 0 bytes tests/val.rs | 34 +++++++--------------------------- 4 files changed, 14 insertions(+), 31 deletions(-) delete mode 100644 tests/invalid_binary.spv diff --git a/src/cmd.rs b/src/cmd.rs index 8160b68..c4e3ef8 100644 --- a/src/cmd.rs +++ b/src/cmd.rs @@ -48,7 +48,10 @@ impl From for crate::error::Error { ); Self { - inner: SpirvResult::InternalError, // this isn't really correct + // this isn't really correct, but the spirv binaries don't + // provide the error code in any meaningful way, either by the + // status code of the binary, or in diagnostic output + inner: SpirvResult::InternalError, diagnostic: Some(diagnostic), } } diff --git a/src/error.rs b/src/error.rs index 5d75c47..14ee28f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -15,10 +15,10 @@ impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match &self.diagnostic { Some(diag) => f.write_fmt(format_args!( - "{}:{}:{} - {}", - self.inner, diag.line, diag.column, diag.message + "error:{}:{} - {}", + diag.line, diag.column, diag.message )), - None => f.write_fmt(format_args!("{}", self.inner)), + None => f.write_str("an unknown error occurred"), } } } diff --git a/tests/invalid_binary.spv b/tests/invalid_binary.spv deleted file mode 100644 index e3441ea57c828fab9de7b62381c1a906fb9f41fb..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 88 zcmZQ(Qf6mhW@KPs;Aa4lARx%V1ZIQihawPulpq5G7Xt@Fa6nkRw?|NXS!qsoVqSc1 XYHof}WqfXaN@@-R9|JR3KZpbXh;0m+ diff --git a/tests/val.rs b/tests/val.rs index 7e97baf..f192b3a 100644 --- a/tests/val.rs +++ b/tests/val.rs @@ -1,7 +1,6 @@ #![allow(clippy::unnecessary_wraps)] const SPIRV_BIN: &[u8] = include_bytes!("wgpu_example_shader.spv"); -const SPIRV_OTHER: &[u8] = include_bytes!("invalid_binary.spv"); fn validate_compiled(_input: &[u8]) -> Option> { #[cfg(feature = "use-compiled-tools")] @@ -27,35 +26,16 @@ fn validate_tool(_input: &[u8]) -> Option> { #[test] fn gets_error_message() { - let cexpected_msg = "invalid cfg:0:0 - Loop header 6[%loop_header] is targeted by 2 back-edge blocks but the standard requires exactly one\n %loop_header = OpLabel\n"; - let texpected_msg = "internal error:0:0 - Loop header 6[%loop_header] is targeted by 2 back-edge blocks but the standard requires exactly one"; + let cexpected_msg = "error:0:0 - Loop header 6[%loop_header] is targeted by 2 back-edge blocks but the standard requires exactly one\n %loop_header = OpLabel\n"; + let texpected_msg = "error:0:0 - Loop header 6[%loop_header] is targeted by 2 back-edge blocks but the standard requires exactly one"; match (validate_compiled(SPIRV_BIN), validate_tool(SPIRV_BIN)) { (Some(resc), Some(rest)) => { - // assert_eq!(resc, rest); + let cstr = resc.unwrap_err().to_string(); + let tstr = rest.unwrap_err().to_string(); + assert_eq!(&cstr[..111], &tstr[..111]); - assert_eq!(resc.unwrap_err().to_string(), cexpected_msg); - assert_eq!(rest.unwrap_err().to_string(), texpected_msg); - } - (Some(resc), None) => { - assert_eq!(resc.unwrap_err().to_string(), cexpected_msg); - } - (None, Some(rest)) => { - assert_eq!(rest.unwrap_err().to_string(), texpected_msg); - } - _ => {} - } -} - -#[test] -fn verify_error_message() { - let cexpected_msg = "invalid binary:0:0 - No OpEntryPoint instruction was found. This is only allowed if the Linkage capability is being used."; - let texpected_msg = ""; - match (validate_compiled(SPIRV_OTHER), validate_tool(SPIRV_OTHER)) { - (Some(resc), Some(rest)) => { - // assert_eq!(resc, rest); - - assert_eq!(resc.unwrap_err().to_string(), cexpected_msg); - assert_eq!(rest.unwrap_err().to_string(), texpected_msg); + assert_eq!(cstr, cexpected_msg); + assert_eq!(tstr, texpected_msg); } (Some(resc), None) => { assert_eq!(resc.unwrap_err().to_string(), cexpected_msg);