diff --git a/CHANGELOG.md b/CHANGELOG.md index a8cd7def49..35f1b731ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -154,6 +154,7 @@ Bottom level categories: - In spv-in, remove unnecessary "gl_PerVertex" name check so unused builtins will always be skipped. By @Imberflur in [#5227](https://github.com/gfx-rs/wgpu/pull/5227). - GLSL 410 does not support layout(binding = ...), enable only for GLSL 420. By @bes in [#5357](https://github.com/gfx-rs/wgpu/pull/5357) - In spv-out, check for acceleration and ray-query types when enabling ray-query extension to prevent validation error. By @Vecvec in [#5463](https://github.com/gfx-rs/wgpu/pull/5463) +- Add a limit for curly brace nesting in WGSL parsing, plus a note about stack size requirements. By @ErichDonGubler in [#5447](https://github.com/gfx-rs/wgpu/pull/5447). #### Tests diff --git a/naga/src/front/wgsl/error.rs b/naga/src/front/wgsl/error.rs index 54aa8296b1..f0b55c70fd 100644 --- a/naga/src/front/wgsl/error.rs +++ b/naga/src/front/wgsl/error.rs @@ -269,6 +269,10 @@ pub enum Error<'a> { scalar: String, inner: ConstantEvaluatorError, }, + ExceededLimitForNestedBraces { + span: Span, + limit: u8, + }, } impl<'a> Error<'a> { @@ -770,6 +774,13 @@ impl<'a> Error<'a> { format!("the expression should have been converted to have {} scalar type", scalar), ] }, + Error::ExceededLimitForNestedBraces { span, limit } => ParseError { + message: "brace nesting limit reached".into(), + labels: vec![(span, "limit reached at this brace".into())], + notes: vec![ + format!("nesting limit is currently set to {limit}"), + ], + }, } } } diff --git a/naga/src/front/wgsl/mod.rs b/naga/src/front/wgsl/mod.rs index b6151fe1c0..aec1e657fc 100644 --- a/naga/src/front/wgsl/mod.rs +++ b/naga/src/front/wgsl/mod.rs @@ -44,6 +44,17 @@ impl Frontend { } } +///
+// NOTE: Keep this in sync with `wgpu::Device::create_shader_module`! +// NOTE: Keep this in sync with `wgpu_core::Global::device_create_shader_module`! +/// +/// This function may consume a lot of stack space. Compiler-enforced limits for parsing recursion +/// exist; if shader compilation runs into them, it will return an error gracefully. However, on +/// some build profiles and platforms, the default stack size for a thread may be exceeded before +/// this limit is reached during parsing. Callers should ensure that there is enough stack space +/// for this, particularly if calls to this method are exposed to user input. +/// +///
pub fn parse_str(source: &str) -> Result { Frontend::new().parse(source) } diff --git a/naga/src/front/wgsl/parse/mod.rs b/naga/src/front/wgsl/parse/mod.rs index 51fc2f013b..6724eb95f9 100644 --- a/naga/src/front/wgsl/parse/mod.rs +++ b/naga/src/front/wgsl/parse/mod.rs @@ -1619,22 +1619,21 @@ impl Parser { lexer: &mut Lexer<'a>, ctx: &mut ExpressionContext<'a, '_, '_>, block: &mut ast::Block<'a>, + brace_nesting_level: u8, ) -> Result<(), Error<'a>> { self.push_rule_span(Rule::Statement, lexer); match lexer.peek() { (Token::Separator(';'), _) => { let _ = lexer.next(); self.pop_rule_span(lexer); - return Ok(()); } (Token::Paren('{'), _) => { - let (inner, span) = self.block(lexer, ctx)?; + let (inner, span) = self.block(lexer, ctx, brace_nesting_level)?; block.stmts.push(ast::Statement { kind: ast::StatementKind::Block(inner), span, }); self.pop_rule_span(lexer); - return Ok(()); } (Token::Word(word), _) => { let kind = match word { @@ -1711,7 +1710,7 @@ impl Parser { let _ = lexer.next(); let condition = self.general_expression(lexer, ctx)?; - let accept = self.block(lexer, ctx)?.0; + let accept = self.block(lexer, ctx, brace_nesting_level)?.0; let mut elsif_stack = Vec::new(); let mut elseif_span_start = lexer.start_byte_offset(); @@ -1722,12 +1721,12 @@ impl Parser { if !lexer.skip(Token::Word("if")) { // ... else { ... } - break self.block(lexer, ctx)?.0; + break self.block(lexer, ctx, brace_nesting_level)?.0; } // ... else if (...) { ... } let other_condition = self.general_expression(lexer, ctx)?; - let other_block = self.block(lexer, ctx)?; + let other_block = self.block(lexer, ctx, brace_nesting_level)?; elsif_stack.push((elseif_span_start, other_condition, other_block)); elseif_span_start = lexer.start_byte_offset(); }; @@ -1759,7 +1758,9 @@ impl Parser { "switch" => { let _ = lexer.next(); let selector = self.general_expression(lexer, ctx)?; - lexer.expect(Token::Paren('{'))?; + let brace_span = lexer.expect_span(Token::Paren('{'))?; + let brace_nesting_level = + Self::increase_brace_nesting(brace_nesting_level, brace_span)?; let mut cases = Vec::new(); loop { @@ -1784,7 +1785,7 @@ impl Parser { }); }; - let body = self.block(lexer, ctx)?.0; + let body = self.block(lexer, ctx, brace_nesting_level)?.0; cases.push(ast::SwitchCase { value, @@ -1794,7 +1795,7 @@ impl Parser { } (Token::Word("default"), _) => { lexer.skip(Token::Separator(':')); - let body = self.block(lexer, ctx)?.0; + let body = self.block(lexer, ctx, brace_nesting_level)?.0; cases.push(ast::SwitchCase { value: ast::SwitchValue::Default, body, @@ -1810,7 +1811,7 @@ impl Parser { ast::StatementKind::Switch { selector, cases } } - "loop" => self.r#loop(lexer, ctx)?, + "loop" => self.r#loop(lexer, ctx, brace_nesting_level)?, "while" => { let _ = lexer.next(); let mut body = ast::Block::default(); @@ -1834,7 +1835,7 @@ impl Parser { span, }); - let (block, span) = self.block(lexer, ctx)?; + let (block, span) = self.block(lexer, ctx, brace_nesting_level)?; body.stmts.push(ast::Statement { kind: ast::StatementKind::Block(block), span, @@ -1857,7 +1858,9 @@ impl Parser { let (_, span) = { let ctx = &mut *ctx; let block = &mut *block; - lexer.capture_span(|lexer| self.statement(lexer, ctx, block))? + lexer.capture_span(|lexer| { + self.statement(lexer, ctx, block, brace_nesting_level) + })? }; if block.stmts.len() != num_statements { @@ -1902,7 +1905,7 @@ impl Parser { lexer.expect(Token::Paren(')'))?; } - let (block, span) = self.block(lexer, ctx)?; + let (block, span) = self.block(lexer, ctx, brace_nesting_level)?; body.stmts.push(ast::Statement { kind: ast::StatementKind::Block(block), span, @@ -1964,13 +1967,15 @@ impl Parser { &mut self, lexer: &mut Lexer<'a>, ctx: &mut ExpressionContext<'a, '_, '_>, + brace_nesting_level: u8, ) -> Result, Error<'a>> { let _ = lexer.next(); let mut body = ast::Block::default(); let mut continuing = ast::Block::default(); let mut break_if = None; - lexer.expect(Token::Paren('{'))?; + let brace_span = lexer.expect_span(Token::Paren('{'))?; + let brace_nesting_level = Self::increase_brace_nesting(brace_nesting_level, brace_span)?; ctx.local_table.push_scope(); @@ -1980,7 +1985,9 @@ impl Parser { // the last thing in the loop body // Expect a opening brace to start the continuing block - lexer.expect(Token::Paren('{'))?; + let brace_span = lexer.expect_span(Token::Paren('{'))?; + let brace_nesting_level = + Self::increase_brace_nesting(brace_nesting_level, brace_span)?; loop { if lexer.skip(Token::Word("break")) { // Branch for the `break if` statement, this statement @@ -2009,7 +2016,7 @@ impl Parser { break; } else { // Otherwise try to parse a statement - self.statement(lexer, ctx, &mut continuing)?; + self.statement(lexer, ctx, &mut continuing, brace_nesting_level)?; } } // Since the continuing block must be the last part of the loop body, @@ -2023,7 +2030,7 @@ impl Parser { break; } // Otherwise try to parse a statement - self.statement(lexer, ctx, &mut body)?; + self.statement(lexer, ctx, &mut body, brace_nesting_level)?; } ctx.local_table.pop_scope(); @@ -2040,15 +2047,17 @@ impl Parser { &mut self, lexer: &mut Lexer<'a>, ctx: &mut ExpressionContext<'a, '_, '_>, + brace_nesting_level: u8, ) -> Result<(ast::Block<'a>, Span), Error<'a>> { self.push_rule_span(Rule::Block, lexer); ctx.local_table.push_scope(); - lexer.expect(Token::Paren('{'))?; + let brace_span = lexer.expect_span(Token::Paren('{'))?; + let brace_nesting_level = Self::increase_brace_nesting(brace_nesting_level, brace_span)?; let mut block = ast::Block::default(); while !lexer.skip(Token::Paren('}')) { - self.statement(lexer, ctx, &mut block)?; + self.statement(lexer, ctx, &mut block, brace_nesting_level)?; } ctx.local_table.pop_scope(); @@ -2135,9 +2144,10 @@ impl Parser { // do not use `self.block` here, since we must not push a new scope lexer.expect(Token::Paren('{'))?; + let brace_nesting_level = 1; let mut body = ast::Block::default(); while !lexer.skip(Token::Paren('}')) { - self.statement(lexer, &mut ctx, &mut body)?; + self.statement(lexer, &mut ctx, &mut body, brace_nesting_level)?; } ctx.local_table.pop_scope(); @@ -2347,4 +2357,30 @@ impl Parser { Ok(tu) } + + const fn increase_brace_nesting( + brace_nesting_level: u8, + brace_span: Span, + ) -> Result> { + // From [spec.](https://gpuweb.github.io/gpuweb/wgsl/#limits): + // + // > § 2.4. Limits + // > + // > … + // > + // > Maximum nesting depth of brace-enclosed statements in a function[:] 127 + // + // _However_, we choose 64 instead because (a) it avoids stack overflows in CI and + // (b) we expect the limit to be decreased to 63 based on this conversation in + // WebGPU CTS upstream: + // + const BRACE_NESTING_MAXIMUM: u8 = 64; + if brace_nesting_level + 1 > BRACE_NESTING_MAXIMUM { + return Err(Error::ExceededLimitForNestedBraces { + span: brace_span, + limit: BRACE_NESTING_MAXIMUM, + }); + } + Ok(brace_nesting_level + 1) + } } diff --git a/naga/tests/wgsl_errors.rs b/naga/tests/wgsl_errors.rs index 46270b6650..74c273e33a 100644 --- a/naga/tests/wgsl_errors.rs +++ b/naga/tests/wgsl_errors.rs @@ -2151,3 +2151,129 @@ fn compaction_preserves_spans() { panic!("Error message has wrong span:\n\n{err:#?}"); } } + +#[test] +fn limit_braced_statement_nesting() { + let too_many_braces = "fn f() {{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{"; + + let expected_diagnostic = r###"error: brace nesting limit reached + ┌─ wgsl:1:72 + │ +1 │ fn f() {{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{ + │ ^ limit reached at this brace + │ + = note: nesting limit is currently set to 64 + +"###; + + // In debug builds, we might actually overflow the stack before exercising this error case, + // depending on the platform and the `RUST_MIN_STACK` env. var. Use a thread with a custom + // stack size that works on all platforms. + std::thread::Builder::new() + .stack_size(1024 * 1024 * 2 /* MB */) + .spawn(|| check(too_many_braces, expected_diagnostic)) + .unwrap() + .join() + .unwrap() +} + +#[test] +fn too_many_unclosed_loops() { + let too_many_braces = "fn f() { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + loop { + "; + + let expected_diagnostic = r###"error: brace nesting limit reached + ┌─ wgsl:65:13 + │ +65 │ loop { + │ ^ limit reached at this brace + │ + = note: nesting limit is currently set to 64 + +"###; + + // In debug builds, we might actually overflow the stack before exercising this error case, + // depending on the platform and the `RUST_MIN_STACK` env. var. Use a thread with a custom + // stack size that works on all platforms. + std::thread::Builder::new() + .stack_size(1024 * 1024 * 2 /* MB */) + .spawn(|| check(too_many_braces, expected_diagnostic)) + .unwrap() + .join() + .unwrap() +} diff --git a/wgpu-core/src/device/global.rs b/wgpu-core/src/device/global.rs index 0c97e1b504..891a62ad23 100644 --- a/wgpu-core/src/device/global.rs +++ b/wgpu-core/src/device/global.rs @@ -1170,6 +1170,20 @@ impl Global { } } + /// Create a shader module with the given `source`. + /// + ///
+ // NOTE: Keep this in sync with `naga::front::wgsl::parse_str`! + // NOTE: Keep this in sync with `wgpu::Device::create_shader_module`! + /// + /// This function may consume a lot of stack space. Compiler-enforced limits for parsing + /// recursion exist; if shader compilation runs into them, it will return an error gracefully. + /// However, on some build profiles and platforms, the default stack size for a thread may be + /// exceeded before this limit is reached during parsing. Callers should ensure that there is + /// enough stack space for this, particularly if calls to this method are exposed to user + /// input. + /// + ///
pub fn device_create_shader_module( &self, device_id: DeviceId, diff --git a/wgpu/src/lib.rs b/wgpu/src/lib.rs index ecf5f88d88..a49c72a1ed 100644 --- a/wgpu/src/lib.rs +++ b/wgpu/src/lib.rs @@ -2296,6 +2296,19 @@ impl Device { } /// Creates a shader module from either SPIR-V or WGSL source code. + /// + ///
+ // NOTE: Keep this in sync with `naga::front::wgsl::parse_str`! + // NOTE: Keep this in sync with `wgpu_core::Global::device_create_shader_module`! + /// + /// This function may consume a lot of stack space. Compiler-enforced limits for parsing + /// recursion exist; if shader compilation runs into them, it will return an error gracefully. + /// However, on some build profiles and platforms, the default stack size for a thread may be + /// exceeded before this limit is reached during parsing. Callers should ensure that there is + /// enough stack space for this, particularly if calls to this method are exposed to user + /// input. + /// + ///
pub fn create_shader_module(&self, desc: ShaderModuleDescriptor<'_>) -> ShaderModule { let (id, data) = DynContext::device_create_shader_module( &*self.context,