Validate uses of ids defined in unreachable blocks. (#2146)
* Validate uses of ids defined in unreachable blocks.
For some reason we do not make sure the uses of ids that are defined
in unreachable blocks are dominated by their def. This is causing
invalid code to pass the validator.
Fixes #2143
* Add test for unreachable code after a return.
We want to allow code like:
```
void foo() {
a = ...;
...
return; // for debugging
<use of a>;
...
}
```
I added a test to make sure that something like this is still accepted
by the validator.
* Add test for unreachable def used in phi.
diff --git a/source/val/validate_id.cpp b/source/val/validate_id.cpp
index b3ba480..fcba711 100644
--- a/source/val/validate_id.cpp
+++ b/source/val/validate_id.cpp
@@ -65,7 +65,6 @@
if (inst.id() == 0) continue;
if (const Function* func = inst.function()) {
if (const BasicBlock* block = inst.block()) {
- if (!block->reachable()) continue;
// If the Id is defined within a block then make sure all references to
// that Id appear in a blocks that are dominated by the defining block
for (auto& use_index_pair : inst.uses()) {
diff --git a/test/val/val_id_test.cpp b/test/val/val_id_test.cpp
index 282e91a..c96aa17 100644
--- a/test/val/val_id_test.cpp
+++ b/test/val/val_id_test.cpp
@@ -6074,6 +6074,200 @@
"StorageBuffer storage classes."));
}
+TEST_F(ValidateIdWithMessage, IdDefInUnreachableBlock1) {
+ const std::string spirv = kNoKernelGLSL450MemoryModel + R"(
+%1 = OpTypeVoid
+%2 = OpTypeFunction %1
+%3 = OpTypeFloat 32
+%4 = OpTypeFunction %3
+%5 = OpFunction %1 None %2
+%6 = OpLabel
+%7 = OpFunctionCall %3 %8
+OpUnreachable
+OpFunctionEnd
+%8 = OpFunction %3 None %4
+%9 = OpLabel
+OpReturnValue %7
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv, SPV_ENV_UNIVERSAL_1_3);
+ EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_UNIVERSAL_1_3));
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("ID 7 defined in block 6 does not dominate its use in "
+ "block 9\n %9 = OpLabel"));
+}
+
+TEST_F(ValidateIdWithMessage, IdDefInUnreachableBlock2) {
+ const std::string spirv = kNoKernelGLSL450MemoryModel + R"(
+%1 = OpTypeVoid
+%2 = OpTypeFunction %1
+%3 = OpTypeFloat 32
+%4 = OpTypeFunction %3
+%5 = OpFunction %1 None %2
+%6 = OpLabel
+OpReturn
+%7 = OpLabel
+%8 = OpFunctionCall %3 %9
+OpUnreachable
+OpFunctionEnd
+%9 = OpFunction %3 None %4
+%10 = OpLabel
+OpReturnValue %8
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv, SPV_ENV_UNIVERSAL_1_3);
+ EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_UNIVERSAL_1_3));
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("ID 8 defined in block 7 does not dominate its use in "
+ "block 10\n %10 = OpLabel"));
+}
+
+TEST_F(ValidateIdWithMessage, IdDefInUnreachableBlock3) {
+ const std::string spirv = kNoKernelGLSL450MemoryModel + R"(
+%1 = OpTypeVoid
+%2 = OpTypeFunction %1
+%3 = OpTypeFloat 32
+%4 = OpTypeFunction %3
+%5 = OpFunction %1 None %2
+%6 = OpLabel
+OpReturn
+%7 = OpLabel
+%8 = OpFunctionCall %3 %9
+OpReturn
+OpFunctionEnd
+%9 = OpFunction %3 None %4
+%10 = OpLabel
+OpReturnValue %8
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv, SPV_ENV_UNIVERSAL_1_3);
+ EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_UNIVERSAL_1_3));
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("ID 8 defined in block 7 does not dominate its use in "
+ "block 10\n %10 = OpLabel"));
+}
+
+TEST_F(ValidateIdWithMessage, IdDefInUnreachableBlock4) {
+ const std::string spirv = kNoKernelGLSL450MemoryModel + R"(
+%1 = OpTypeVoid
+%2 = OpTypeFunction %1
+%3 = OpTypeFloat 32
+%4 = OpTypeFunction %3
+%5 = OpFunction %1 None %2
+%6 = OpLabel
+OpReturn
+%7 = OpLabel
+%8 = OpUndef %3
+%9 = OpCopyObject %3 %8
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv, SPV_ENV_UNIVERSAL_1_3);
+ EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_UNIVERSAL_1_3));
+}
+
+TEST_F(ValidateIdWithMessage, IdDefInUnreachableBlock5) {
+ const std::string spirv = kNoKernelGLSL450MemoryModel + R"(
+%1 = OpTypeVoid
+%2 = OpTypeFunction %1
+%3 = OpTypeFloat 32
+%4 = OpTypeFunction %3
+%5 = OpFunction %1 None %2
+%6 = OpLabel
+OpReturn
+%7 = OpLabel
+%8 = OpUndef %3
+OpBranch %9
+%9 = OpLabel
+%10 = OpCopyObject %3 %8
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv, SPV_ENV_UNIVERSAL_1_3);
+ EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_UNIVERSAL_1_3));
+}
+
+TEST_F(ValidateIdWithMessage, IdDefInUnreachableBlock6) {
+ const std::string spirv = kNoKernelGLSL450MemoryModel + R"(
+%1 = OpTypeVoid
+%2 = OpTypeFunction %1
+%3 = OpTypeFloat 32
+%4 = OpTypeFunction %3
+%5 = OpFunction %1 None %2
+%6 = OpLabel
+OpBranch %7
+%8 = OpLabel
+%9 = OpUndef %3
+OpBranch %7
+%7 = OpLabel
+%10 = OpCopyObject %3 %9
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv, SPV_ENV_UNIVERSAL_1_3);
+ EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_UNIVERSAL_1_3));
+ EXPECT_THAT(getDiagnosticString(),
+ HasSubstr("ID 9 defined in block 8 does not dominate its use in "
+ "block 7\n %7 = OpLabel"));
+}
+
+TEST_F(ValidateIdWithMessage, ReachableDefUnreachableUse) {
+ const std::string spirv = kNoKernelGLSL450MemoryModel + R"(
+%1 = OpTypeVoid
+%2 = OpTypeFunction %1
+%3 = OpTypeFloat 32
+%4 = OpTypeFunction %3
+%5 = OpFunction %1 None %2
+%6 = OpLabel
+%7 = OpUndef %3
+OpReturn
+%8 = OpLabel
+%9 = OpCopyObject %3 %7
+OpReturn
+OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv, SPV_ENV_UNIVERSAL_1_3);
+ EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_UNIVERSAL_1_3));
+}
+
+TEST_F(ValidateIdWithMessage, UnreachableDefUsedInPhi) {
+ const std::string spirv = kNoKernelGLSL450MemoryModel + R"(
+ %void = OpTypeVoid
+ %3 = OpTypeFunction %void
+ %float = OpTypeFloat 32
+ %bool = OpTypeBool
+ %6 = OpTypeFunction %float
+ %1 = OpFunction %void None %3
+ %7 = OpLabel
+ %8 = OpUndef %bool
+ OpSelectionMerge %9 None
+ OpBranchConditional %8 %10 %9
+ %10 = OpLabel
+ %11 = OpUndef %float
+ OpBranch %9
+ %12 = OpLabel
+ %13 = OpUndef %float
+ OpUnreachable
+ %9 = OpLabel
+ %14 = OpPhi %float %11 %10 %13 %7
+ OpReturn
+ OpFunctionEnd
+)";
+
+ CompileSuccessfully(spirv, SPV_ENV_UNIVERSAL_1_3);
+ EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_UNIVERSAL_1_3));
+ EXPECT_THAT(
+ getDiagnosticString(),
+ HasSubstr("In OpPhi instruction 14, ID 13 definition does not dominate "
+ "its parent 7\n %14 = OpPhi %float %11 %10 %13 %7"));
+}
} // namespace
} // namespace val
} // namespace spvtools
