fix(naga): Reject unsupported enable extensions

Previously we required that the enable extension be declared and that
the capability be present if the functionality was used, but allowed the
extension to be declared if the functionality was not used.

Author: andyleiserson

CHANGELOG.md

@@ -89,6 +89,10 @@ Bottom level categories:
 
 ### Changes
 
+#### General
+
+- Naga and `wgpu` now reject shaders with an `enable` directive for functionality that is not available, even if that functionality is not used by the shader. By @andyleiserson in [#8913](https://github.com/gfx-rs/wgpu/pull/8913).
+
 #### Naga
 
 - Prevent UB from incorrectly using ray queries on HLSL. By @Vecvec in [#8763](https://github.com/gfx-rs/wgpu/pull/8763).

naga-cli/src/bin/naga.rs

@@ -146,6 +146,14 @@ struct Args {
     /// defines to be passed to the parser (only glsl is supported)
     #[argh(option, short = 'D')]
     defines: Vec<Defines>,
+
+    /// capabilities for parsing and validation.
+    ///
+    /// Can be a comma-separated list of capability names (e.g.,
+    /// "shader_float16,dual_source_blending"), a numeric bitflags value (e.g.,
+    /// "67108864"), the string "none", or the string "all".
+    #[argh(option, default = "CapabilitiesArg(naga::valid::Capabilities::all())")]
+    capabilities: CapabilitiesArg,
 }
 
 /// Newtype so we can implement [`FromStr`] for `BoundsCheckPolicy`.
@@ -336,6 +344,39 @@ impl FromStr for Defines {
     }
 }
 
+#[derive(Debug, Clone, Copy)]
+struct CapabilitiesArg(naga::valid::Capabilities);
+
+impl FromStr for CapabilitiesArg {
+    type Err = String;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        use naga::valid::Capabilities;
+
+        let s = s.to_uppercase();
+
+        if s == "NONE" {
+            Ok(Self(Capabilities::empty()))
+        } else if s == "ALL" {
+            Ok(Self(Capabilities::all()))
+        } else if let Ok(bits) = s.parse::<u64>() {
+            Capabilities::from_bits(bits)
+                .map(Self)
+                .ok_or_else(|| format!("Invalid capabilities bitflags value: {bits}"))
+        } else {
+            s.split(',')
+                .fold(Ok(Capabilities::empty()), |acc, s| {
+                    acc.and_then(|acc| {
+                        Capabilities::from_name(s.trim())
+                            .map(|cap| acc | cap)
+                            .ok_or(format!("Unknown capability {}", s.trim()))
+                    })
+                })
+                .map(Self)
+        }
+    }
+}
+
 #[derive(Default)]
 struct Parameters<'a> {
     validation_flags: naga::valid::ValidationFlags,
@@ -352,6 +393,7 @@ struct Parameters<'a> {
     input_kind: Option<InputKind>,
     shader_stage: Option<ShaderStage>,
     defines: FastHashMap<String, String>,
+    capabilities: naga::valid::Capabilities,
 
     /// We use this copy of `args.compact` to know whether we should pass the
     /// entrypoint to `process_overrides`, which will result in removal from
@@ -505,6 +547,7 @@ fn run() -> anyhow::Result<()> {
     );
 
     params.compact = args.compact;
+    params.capabilities = args.capabilities.0;
 
     if args.bulk_validate {
         return bulk_validate(&args, &params);
@@ -682,7 +725,12 @@ fn parse_input(input_path: &Path, input: Vec<u8>, params: &Parameters) -> anyhow
         },
         InputKind::Wgsl => {
             let input = String::from_utf8(input)?;
-            let result = naga::front::wgsl::parse_str(&input);
+            let options = naga::front::wgsl::Options {
+                parse_doc_comments: false,
+                capabilities: params.capabilities,
+            };
+            let mut frontend = naga::front::wgsl::Frontend::new_with_options(options);
+            let result = frontend.parse(&input);
             match result {
                 Ok(v) => Parsed {
                     module: v,
@@ -961,7 +1009,7 @@ fn bulk_validate(args: &Args, params: &Parameters) -> anyhow::Result<()> {
         };
 
         let mut validator =
-            naga::valid::Validator::new(params.validation_flags, naga::valid::Capabilities::all());
+            naga::valid::Validator::new(params.validation_flags, params.capabilities);
         validator.subgroup_stages(naga::valid::ShaderStages::all());
         validator.subgroup_operations(naga::valid::SubgroupOperationSet::all());
 

naga-test/src/lib.rs

@@ -84,6 +84,7 @@ impl From<&WgslInParameters> for naga::front::wgsl::Options {
     fn from(value: &WgslInParameters) -> Self {
         Self {
             parse_doc_comments: value.parse_doc_comments,
+            capabilities: naga::valid::Capabilities::all(),
         }
     }
 }

naga/src/front/wgsl/error.rs

@@ -381,6 +381,10 @@ pub(crate) enum Error<'a> {
         kind: EnableExtension,
         span: Span,
     },
+    EnableExtensionNotSupported {
+        kind: EnableExtension,
+        span: Span,
+    },
     LanguageExtensionNotYetImplemented {
         kind: UnimplementedLanguageExtension,
         span: Span,
@@ -1240,6 +1244,17 @@ impl<'a> Error<'a> {
                     ]
                 },
             },
+            Error::EnableExtensionNotSupported { kind, span } => ParseError {
+                message: format!(
+                    "the `{}` extension is not supported in the current environment",
+                    kind.to_ident()
+                ),
+                labels: vec![(
+                    span,
+                    "unsupported enable-extension".into(),
+                )],
+                notes: vec![],
+            },
             Error::LanguageExtensionNotYetImplemented { kind, span } => ParseError {
                 message: format!(
                     "the `{}` language extension is not yet supported",

naga/src/front/wgsl/parse/directive/enable_extension.rs

@@ -174,6 +174,22 @@ pub enum ImplementedEnableExtension {
     WgpuCooperativeMatrix,
 }
 
+impl ImplementedEnableExtension {
+    /// Returns the capability required for this enable extension.
+    pub const fn capability(self) -> crate::valid::Capabilities {
+        use crate::valid::Capabilities as C;
+        match self {
+            Self::F16 => C::SHADER_FLOAT16,
+            Self::DualSourceBlending => C::DUAL_SOURCE_BLENDING,
+            Self::ClipDistances => C::CLIP_DISTANCE,
+            Self::WgpuMeshShader => C::MESH_SHADER,
+            Self::WgpuRayQuery => C::RAY_QUERY,
+            Self::WgpuRayQueryVertexReturn => C::RAY_HIT_VERTEX_POSITION,
+            Self::WgpuCooperativeMatrix => C::COOPERATIVE_MATRIX,
+        }
+    }
+}
+
 /// A variant of [`EnableExtension::Unimplemented`].
 #[derive(Clone, Copy, Debug, Hash, Eq, PartialEq)]
 pub enum UnimplementedEnableExtension {

naga/src/front/wgsl/parse/mod.rs

@@ -287,13 +287,16 @@ impl<'a> BindingParser<'a> {
 pub struct Options {
     /// Controls whether the parser should parse doc comments.
     pub parse_doc_comments: bool,
+    /// Capabilities to enable during parsing.
+    pub capabilities: crate::valid::Capabilities,
 }
 
 impl Options {
-    /// Creates a new [`Options`] without doc comments parsing.
+    /// Creates a new default [`Options`].
     pub const fn new() -> Self {
         Options {
             parse_doc_comments: false,
+            capabilities: crate::valid::Capabilities::all(),
         }
     }
 }
@@ -3280,6 +3283,14 @@ impl Parser {
                                     }))
                                 }
                             };
+                            // Check if the required capability is supported
+                            let required_capability = extension.capability();
+                            if !options.capabilities.contains(required_capability) {
+                                return Err(Box::new(Error::EnableExtensionNotSupported {
+                                    kind,
+                                    span,
+                                }));
+                            }
                             enable_extensions.add(extension);
                             Ok(())
                         })?;

naga/tests/naga/wgsl_errors.rs

@@ -4724,3 +4724,129 @@ fn mesh_shader_enable_extension() {
         })
     );
 }
+
+#[track_caller]
+fn check_with_capabilities(input: &str, snapshot: &str, capabilities: Capabilities) {
+    let mut options = naga::front::wgsl::Options::new();
+    options.capabilities = capabilities;
+    let mut frontend = naga::front::wgsl::Frontend::new_with_options(options);
+    let output = match frontend.parse(input) {
+        Ok(_) => panic!("expected parser error, but parsing succeeded!"),
+        Err(err) => err.emit_to_string(input),
+    };
+    if output != snapshot {
+        for diff in diff::lines(snapshot, &output) {
+            match diff {
+                diff::Result::Left(l) => println!("-{l}"),
+                diff::Result::Both(l, _) => println!(" {l}"),
+                diff::Result::Right(r) => println!("+{r}"),
+            }
+        }
+        panic!("Error snapshot failed");
+    }
+}
+
+#[test]
+fn enable_f16_without_capability() {
+    check_with_capabilities(
+        "enable f16;",
+        r###"error: the `f16` extension is not supported in the current environment
+  ┌─ wgsl:1:8
+  │
+1 │ enable f16;
+  │        ^^^ unsupported enable-extension
+
+"###,
+        !Capabilities::SHADER_FLOAT16,
+    );
+}
+
+#[test]
+fn enable_dual_source_blending_without_capability() {
+    check_with_capabilities(
+        "enable dual_source_blending;",
+        r###"error: the `dual_source_blending` extension is not supported in the current environment
+  ┌─ wgsl:1:8
+  │
+1 │ enable dual_source_blending;
+  │        ^^^^^^^^^^^^^^^^^^^^ unsupported enable-extension
+
+"###,
+        !Capabilities::DUAL_SOURCE_BLENDING,
+    );
+}
+
+#[test]
+fn enable_clip_distances_without_capability() {
+    check_with_capabilities(
+        "enable clip_distances;",
+        r###"error: the `clip_distances` extension is not supported in the current environment
+  ┌─ wgsl:1:8
+  │
+1 │ enable clip_distances;
+  │        ^^^^^^^^^^^^^^ unsupported enable-extension
+
+"###,
+        !Capabilities::CLIP_DISTANCE,
+    );
+}
+
+#[test]
+fn enable_wgpu_mesh_shader_without_capability() {
+    check_with_capabilities(
+        "enable wgpu_mesh_shader;",
+        r###"error: the `wgpu_mesh_shader` extension is not supported in the current environment
+  ┌─ wgsl:1:8
+  │
+1 │ enable wgpu_mesh_shader;
+  │        ^^^^^^^^^^^^^^^^ unsupported enable-extension
+
+"###,
+        !Capabilities::MESH_SHADER,
+    );
+}
+
+#[test]
+fn enable_wgpu_ray_query_without_capability() {
+    check_with_capabilities(
+        "enable wgpu_ray_query;",
+        r###"error: the `wgpu_ray_query` extension is not supported in the current environment
+  ┌─ wgsl:1:8
+  │
+1 │ enable wgpu_ray_query;
+  │        ^^^^^^^^^^^^^^ unsupported enable-extension
+
+"###,
+        !Capabilities::RAY_QUERY,
+    );
+}
+
+#[test]
+fn enable_wgpu_ray_query_vertex_return_without_capability() {
+    check_with_capabilities(
+        "enable wgpu_ray_query_vertex_return;",
+        r###"error: the `wgpu_ray_query_vertex_return` extension is not supported in the current environment
+  ┌─ wgsl:1:8
+  │
+1 │ enable wgpu_ray_query_vertex_return;
+  │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unsupported enable-extension
+
+"###,
+        !Capabilities::RAY_HIT_VERTEX_POSITION,
+    );
+}
+
+#[test]
+fn enable_wgpu_cooperative_matrix_without_capability() {
+    check_with_capabilities(
+        "enable wgpu_cooperative_matrix;",
+        r###"error: the `wgpu_cooperative_matrix` extension is not supported in the current environment
+  ┌─ wgsl:1:8
+  │
+1 │ enable wgpu_cooperative_matrix;
+  │        ^^^^^^^^^^^^^^^^^^^^^^^ unsupported enable-extension
+
+"###,
+        !Capabilities::COOPERATIVE_MATRIX,
+    );
+}

tests/tests/wgpu-gpu/dual_source_blending.rs

@@ -18,7 +18,7 @@ fn vs_main() -> @builtin(position) vec4f {
 "#;
 
 const FRAGMENT_SHADER_WITHOUT_DUAL_SOURCE_BLENDING: &str = r#"
-@fragment 
+@fragment
 fn fs_main() -> @location(0) vec4f {
     return vec4f(1.0);
 }
@@ -31,7 +31,7 @@ struct FragmentOutput {
     @location(0) @blend_src(1) output1_: vec4<f32>,
 }
 
-@fragment 
+@fragment
 fn fs_main() -> FragmentOutput {
     return FragmentOutput(vec4<f32>(0.4f, 0.3f, 0.2f, 0.1f), vec4<f32>(0.9f, 0.8f, 0.7f, 0.6f));
 }
@@ -112,7 +112,7 @@ async fn dual_source_blending_disabled(ctx: TestingContext) {
                 source: ShaderSource::Wgsl(FRAGMENT_SHADER_WITH_DUAL_SOURCE_BLENDING.into()),
             });
         },
-        Some("Capability Capabilities(DUAL_SOURCE_BLENDING) is not supported"),
+        Some("the `dual_source_blending` extension is not supported in the current environment"),
     );
 }