spirv-fuzz: Use validator to check break/continue dominance conditions#3089
Merged
afd merged 1 commit intoKhronosGroup:masterfrom Dec 6, 2019
Merged
Conversation
The passes that add dead breaks and continues suffer from the challenge that a new control flow graph edge can change dominance information, leading to the potenital for definitions to no longer dominate their uses. The attempt at guarding against this was known to be incomplete. This change calls on the SPIR-V validator to do the necessary checking: in deciding whether adding such an edge would be legitimate, we clone the module, add the edge, and use the validator to check whether the transformed clone is valid. This strategy is heavy-weight, and should be used sparingly, but seems like a good option when the validity of transformations is intricate, to avoid reimplementing swathes of validation logic in the fuzzer. Fixes KhronosGroup#2919.
paulthomson
approved these changes
Dec 6, 2019
Contributor
paulthomson
left a comment
There was a problem hiding this comment.
Looks good. As discussed, we could might be able to save performance a bit by: adding the edge, recompute dominance in opt, check dominance using opt, removing the edge if needed. But recomputing dominance might be the bottleneck anyway.
dneto0
pushed a commit
to dneto0/SPIRV-Tools
that referenced
this pull request
Sep 14, 2024
Roll third_party/glslang/ 0de87ee..6c47979 (4 commits) KhronosGroup/glslang@0de87ee...6c47979 $ git log 0de87ee..6c47979 --date=short --no-merges --format='%ad %ae %s' 2019-12-09 cepheus Fix KhronosGroup#2020: PR KhronosGroup#1977 broke HLSL member consistency, this finishes it... 2019-12-09 cepheus Fix: KhronosGroup#2014: Don't do "extension-on && version >= ..." keyword checks. 2019-12-09 cepheus Fix KhronosGroup#2007: Fix a couple relative header paths in header files. 2019-12-09 cepheus Fix KhronosGroup#1993: Fully exclude ftransform() from SPIR-V semantics. Roll third_party/googletest/ ae8d1fc81..78fdd6c00 (6 commits) google/googletest@ae8d1fc...78fdd6c $ git log ae8d1fc81..78fdd6c00 --date=short --no-merges --format='%ad %ae %s' 2019-12-05 absl-team Googletest export 2019-12-05 absl-team Googletest export 2019-12-05 absl-team Googletest export 2019-11-27 krystian.kuzniarek Revert "remove MSVC workaround: wmain link error in the static library" 2019-11-27 krystian.kuzniarek Revert "unify googletest and googlemock main functions" 2019-11-17 krystian.kuzniarek remove MSVC workaround: cease const dropping Roll third_party/re2/ bb8e77755..6a86f6b3f (1 commit) google/re2@bb8e777...6a86f6b $ git log bb8e77755..6a86f6b3f --date=short --no-merges --format='%ad %ae %s' 2019-12-09 junyer Simplify the bytecode for the 80-10FFFF rune range. Roll third_party/spirv-cross/ 15b860eb1..f912c3289 (2 commits) KhronosGroup/SPIRV-Cross@15b860e...f912c32 $ git log 15b860eb1..f912c3289 --date=short --no-merges --format='%ad %ae %s' 2019-12-10 post GLSL: Fix array of input patch variables. 2019-12-09 post GLSL: Fix EmitStreamVertex/Primitive. Roll third_party/spirv-tools/ e82a428..0a2b38d (2 commits) KhronosGroup/SPIRV-Tools@e82a428...0a2b38d $ git log e82a428..0a2b38d --date=short --no-merges --format='%ad %ae %s' 2019-12-10 afdx spirv-fuzz: function outlining fuzzer pass (KhronosGroup#3078) 2019-12-06 afdx spirv-fuzz: Use validator to check break/continue dominance conditions (KhronosGroup#3089) Created with: roll-dep third_party/effcee third_party/glslang third_party/googletest third_party/re2 third_party/spirv-cross third_party/spirv-headers third_party/spirv-tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The passes that add dead breaks and continues suffer from the
challenge that a new control flow graph edge can change dominance
information, leading to the potenital for definitions to no longer
dominate their uses. The attempt at guarding against this was known
to be incomplete. This change calls on the SPIR-V validator to do the
necessary checking: in deciding whether adding such an edge would be
legitimate, we clone the module, add the edge, and use the validator
to check whether the transformed clone is valid.
This strategy is heavy-weight, and should be used sparingly, but seems
like a good option when the validity of transformations is intricate,
to avoid reimplementing swathes of validation logic in the fuzzer.
Fixes #2919.