Skip to content

Commit 0693158

Browse files
committed
Fix capability extension validation to use ANY-of semantics
Grammar extension lists for capabilities are alternatives: at least one of the listed extensions must be present, not all of them. For example, ShaderViewportIndexLayerEXT can be enabled by either SPV_EXT_shader_viewport_index_layer OR SPV_NV_viewport_array2. The previous code iterated all grammar extensions and failed on the first one not found, effectively requiring ALL extensions. This matches the C++ behavior which uses HasAnyOfExtensions() to check if any single extension from the grammar list is present. Add regression tests for capabilities with alternative extensions.
1 parent 38824bc commit 0693158

File tree

2 files changed

+116
-12
lines changed

2 files changed

+116
-12
lines changed

rust/spirv-tools-core/src/validation/rules/capabilities.rs

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -120,10 +120,16 @@ pub fn validate_capabilities(
120120
&& (always_require_extension || !version_allows_core);
121121

122122
if grammar_requires_extension {
123-
for &required_ext in grammar_requirements.required_extensions {
124-
if !extension_allowed_in_env(required_ext, env) {
123+
// Grammar extension lists are alternatives (ANY suffices).
124+
// Only error if NONE of the alternative extensions is allowed.
125+
let any_ext_allowed = grammar_requirements
126+
.required_extensions
127+
.iter()
128+
.any(|&ext| extension_allowed_in_env(ext, env));
129+
if !any_ext_allowed {
130+
if let Some(&ext) = grammar_requirements.required_extensions.first() {
125131
return Err(ValidationError::DisallowedExtension {
126-
extension: ExtensionName::from(required_ext),
132+
extension: ExtensionName::from(ext),
127133
env,
128134
});
129135
}
@@ -171,19 +177,36 @@ pub fn validate_capabilities(
171177
}
172178

173179
if !grammar_requirements.required_extensions.is_empty() && grammar_requires_extension {
174-
for &required_ext in grammar_requirements.required_extensions {
175-
if !extension_allowed_in_env(required_ext, env) {
180+
// Grammar extension lists are alternatives (ANY suffices).
181+
// The capability is valid if at least one listed extension is
182+
// both allowed in the environment and declared in the module.
183+
let any_ext_satisfied = grammar_requirements
184+
.required_extensions
185+
.iter()
186+
.any(|&ext| extension_allowed_in_env(ext, env) && has_extension(extensions, ext));
187+
if !any_ext_satisfied {
188+
let any_allowed = grammar_requirements
189+
.required_extensions
190+
.iter()
191+
.any(|&ext| extension_allowed_in_env(ext, env));
192+
if !any_allowed {
176193
return Err(ValidationError::DisallowedExtension {
177-
extension: ExtensionName::from(required_ext),
194+
extension: ExtensionName::from(
195+
grammar_requirements.required_extensions[0],
196+
),
178197
env,
179198
});
180199
}
181-
if !has_extension(extensions, required_ext) {
182-
return Err(ValidationError::DisallowedCapabilityMissingExtension {
183-
capability,
184-
required_extension: required_ext.to_string(),
185-
});
186-
}
200+
// At least one is allowed but none is declared
201+
let first_allowed = grammar_requirements
202+
.required_extensions
203+
.iter()
204+
.find(|&&ext| extension_allowed_in_env(ext, env))
205+
.unwrap();
206+
return Err(ValidationError::DisallowedCapabilityMissingExtension {
207+
capability,
208+
required_extension: first_allowed.to_string(),
209+
});
187210
}
188211
}
189212

rust/spirv-tools-core/src/validation/tests/mod.rs

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30277,3 +30277,84 @@ fn udot_signed_result_rejected() {
3027730277
"Expected DotProductResultNotUnsignedIntScalar, got: {error:?}"
3027830278
);
3027930279
}
30280+
30281+
// Grammar extension lists are alternatives: ANY listed extension suffices.
30282+
// ShaderViewportIndexLayerEXT lists two alternative extensions:
30283+
// SPV_EXT_shader_viewport_index_layer OR SPV_NV_viewport_array2
30284+
#[test]
30285+
fn capability_with_alternative_extensions_accepts_any_single_extension() {
30286+
// Should succeed with only SPV_NV_viewport_array2 (not both extensions)
30287+
let text_nv = [
30288+
"OpCapability Shader",
30289+
"OpCapability Geometry",
30290+
"OpCapability MultiViewport",
30291+
"OpCapability ShaderViewportIndexLayerEXT",
30292+
"OpExtension \"SPV_NV_viewport_array2\"",
30293+
"OpMemoryModel Logical GLSL450",
30294+
"OpEntryPoint Vertex %main \"main\"",
30295+
"%void = OpTypeVoid",
30296+
"%fn = OpTypeFunction %void",
30297+
"%main = OpFunction %void None %fn",
30298+
"%entry = OpLabel",
30299+
"OpReturn",
30300+
"OpFunctionEnd",
30301+
]
30302+
.join("\n");
30303+
text_nv
30304+
.as_str()
30305+
.validate(TargetEnv::Vulkan1_1)
30306+
.expect("Capability with one of its alternative extensions should be accepted");
30307+
30308+
// Should succeed with only SPV_EXT_shader_viewport_index_layer
30309+
let text_ext = [
30310+
"OpCapability Shader",
30311+
"OpCapability Geometry",
30312+
"OpCapability MultiViewport",
30313+
"OpCapability ShaderViewportIndexLayerEXT",
30314+
"OpExtension \"SPV_EXT_shader_viewport_index_layer\"",
30315+
"OpMemoryModel Logical GLSL450",
30316+
"OpEntryPoint Vertex %main \"main\"",
30317+
"%void = OpTypeVoid",
30318+
"%fn = OpTypeFunction %void",
30319+
"%main = OpFunction %void None %fn",
30320+
"%entry = OpLabel",
30321+
"OpReturn",
30322+
"OpFunctionEnd",
30323+
]
30324+
.join("\n");
30325+
text_ext
30326+
.as_str()
30327+
.validate(TargetEnv::Vulkan1_1)
30328+
.expect("Capability with the other alternative extension should be accepted");
30329+
}
30330+
30331+
#[test]
30332+
fn capability_with_alternative_extensions_rejects_when_none_declared() {
30333+
// Should fail when neither alternative extension is declared
30334+
let text = [
30335+
"OpCapability Shader",
30336+
"OpCapability Geometry",
30337+
"OpCapability MultiViewport",
30338+
"OpCapability ShaderViewportIndexLayerEXT",
30339+
"OpMemoryModel Logical GLSL450",
30340+
"OpEntryPoint Vertex %main \"main\"",
30341+
"%void = OpTypeVoid",
30342+
"%fn = OpTypeFunction %void",
30343+
"%main = OpFunction %void None %fn",
30344+
"%entry = OpLabel",
30345+
"OpReturn",
30346+
"OpFunctionEnd",
30347+
]
30348+
.join("\n");
30349+
let error = text
30350+
.as_str()
30351+
.validate(TargetEnv::Vulkan1_1)
30352+
.expect_err("Capability without any required extension should be rejected");
30353+
assert!(
30354+
matches!(
30355+
error,
30356+
ValidationError::DisallowedCapabilityMissingExtension { .. }
30357+
),
30358+
"Expected DisallowedCapabilityMissingExtension, got: {error:?}"
30359+
);
30360+
}

0 commit comments

Comments
 (0)