Fixes issue #489.

From the SPIR-V Spec 2.16.1:

A function declaration (an OpFunction with no basic blocks), must have
a Linkage Attributes Decoration with the Import Linkage Type.

A function definition (an OpFunction with basic blocks) cannot be
decorated with the Import Linkage Type.

diff --git a/source/validate_decorations.cpp b/source/validate_decorations.cpp
index 134dea8..e2b7dfb 100644 (file)
--- a/source/validate_decorations.cpp
+++ b/source/validate_decorations.cpp
@@ -23,6 +23,7 @@
 
 using libspirv::Decoration;
 using libspirv::DiagnosticStream;
+using libspirv::Instruction;
 using libspirv::ValidationState_t;
 
 namespace {
@@ -38,12 +39,41 @@ bool isBuiltInStruct(uint32_t struct_id, ValidationState_t& vstate) {
       });
 }
 
-}  // anonymous namespace
+// Returns true if the given ID has the Import LinkageAttributes decoration.
+bool hasImportLinkageAttribute(uint32_t id, ValidationState_t& vstate) {
+  const auto& decorations = vstate.id_decorations(id);
+  return std::any_of(decorations.begin(), decorations.end(),
+                     [](const Decoration& d) {
+                       return SpvDecorationLinkageAttributes == d.dec_type() &&
+                              d.params().size() >= 2u &&
+                              d.params().back() == SpvLinkageTypeImport;
+                     });
+}
 
-namespace libspirv {
+spv_result_t CheckLinkageAttrOfFunctions(ValidationState_t& vstate) {
+  for (const auto& function : vstate.functions()) {
+    if (function.block_count() == 0u) {
+      // A function declaration (an OpFunction with no basic blocks), must have
+      // a Linkage Attributes Decoration with the Import Linkage Type.
+      if (!hasImportLinkageAttribute(function.id(), vstate)) {
+        return vstate.diag(SPV_ERROR_INVALID_BINARY)
+               << "Function declaration (id " << function.id()
+               << ") must have a LinkageAttributes decoration with the Import "
+                  "Linkage type.";
+      }
+    } else {
+      if (hasImportLinkageAttribute(function.id(), vstate)) {
+        return vstate.diag(SPV_ERROR_INVALID_BINARY)
+               << "Function definition (id " << function.id()
+               << ") may not be decorated with Import Linkage type.";
+      }
+    }
+  }
+  return SPV_SUCCESS;
+}
 
-// Validates that decorations have been applied properly.
-spv_result_t ValidateDecorations(ValidationState_t& vstate) {
+// Checks whether an imported variable is initialized by this module.
+spv_result_t CheckImportedVariableInitialization(ValidationState_t& vstate) {
   // According the SPIR-V Spec 2.16.1, it is illegal to initialize an imported
   // variable. This means that a module-scope OpVariable with initialization
   // value cannot be marked with the Import Linkage Type (import type id = 1).
@@ -51,20 +81,18 @@ spv_result_t ValidateDecorations(ValidationState_t& vstate) {
     // Initializer <id> is an optional argument for OpVariable. If initializer
     // <id> is present, the instruction will have 5 words.
     auto variable_instr = vstate.FindDef(global_var_id);
-    if (variable_instr->words().size() == 5u) {
-      for (const auto& decoration : vstate.id_decorations(global_var_id)) {
-        // the Linkage Type is the last parameter of the decoration.
-        if (SpvDecorationLinkageAttributes == decoration.dec_type() &&
-            decoration.params().size() >= 2u &&
-            decoration.params().back() == 1) {
-          return vstate.diag(SPV_ERROR_INVALID_ID)
-                 << "A module-scope OpVariable with initialization value "
-                    "cannot be marked with the Import Linkage Type.";
-        }
-      }
+    if (variable_instr->words().size() == 5u &&
+        hasImportLinkageAttribute(global_var_id, vstate)) {
+      return vstate.diag(SPV_ERROR_INVALID_ID)
+             << "A module-scope OpVariable with initialization value "
+                "cannot be marked with the Import Linkage Type.";
     }
   }
+  return SPV_SUCCESS;
+}
 
+// Checks whether proper decorations have been appied to the entry points.
+spv_result_t CheckDecorationsOfEntryPoints(ValidationState_t& vstate) {
   for (uint32_t entry_point : vstate.entry_points()) {
     const auto& interfaces = vstate.entry_point_interfaces(entry_point);
     int num_builtin_inputs = 0;
@@ -121,9 +149,18 @@ spv_result_t ValidateDecorations(ValidationState_t& vstate) {
       }
     }
   }
+  return SPV_SUCCESS;
+}
 
-  // TODO: Refactor this function into smaller pieces.
+}  // anonymous namespace
+
+namespace libspirv {
 
+// Validates that decorations have been applied properly.
+spv_result_t ValidateDecorations(ValidationState_t& vstate) {
+  if (auto error = CheckImportedVariableInitialization(vstate)) return error;
+  if (auto error = CheckDecorationsOfEntryPoints(vstate)) return error;
+  if (auto error = CheckLinkageAttrOfFunctions(vstate)) return error;
   return SPV_SUCCESS;
 }
 

diff --git a/test/val/val_cfg_test.cpp b/test/val/val_cfg_test.cpp
index 4e101f1..01d92fa 100644 (file)
--- a/test/val/val_cfg_test.cpp
+++ b/test/val/val_cfg_test.cpp
@@ -757,7 +757,10 @@ TEST_P(ValidateCFG, UnreachableBranch) {
 
 TEST_P(ValidateCFG, EmptyFunction) {
   string str = header(GetParam()) + string(types_consts()) +
-               "%func    = OpFunction %voidt None %funct\n" + "OpFunctionEnd\n";
+               R"(%func    = OpFunction %voidt None %funct
+                  %l = OpLabel
+                  OpReturn
+                  OpFunctionEnd)";
 
   CompileSuccessfully(str);
   ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());

diff --git a/test/val/val_decoration_test.cpp b/test/val/val_decoration_test.cpp
index eab9166..03e5535 100644 (file)
--- a/test/val/val_decoration_test.cpp
+++ b/test/val/val_decoration_test.cpp
@@ -360,5 +360,93 @@ TEST_F(ValidateDecorations, EntryPointFunctionHasLinkageAttributeBad) {
                 "an OpEntryPoint instruction."));
 }
 
+TEST_F(ValidateDecorations, FunctionDeclarationWithoutImportLinkageBad) {
+  string spirv = R"(
+               OpCapability Shader
+               OpCapability Linkage
+               OpMemoryModel Logical GLSL450
+     %void = OpTypeVoid
+     %func = OpTypeFunction %void
+       %main = OpFunction %void None %func
+               OpFunctionEnd
+  )";
+  CompileSuccessfully(spirv);
+  EXPECT_EQ(SPV_ERROR_INVALID_BINARY, ValidateAndRetrieveValidationState());
+  EXPECT_THAT(
+      getDiagnosticString(),
+      HasSubstr("Function declaration (id 3) must have a LinkageAttributes "
+                "decoration with the Import Linkage type."));
+}
+
+TEST_F(ValidateDecorations, FunctionDeclarationWithImportLinkageGood) {
+  string spirv = R"(
+               OpCapability Shader
+               OpCapability Linkage
+               OpMemoryModel Logical GLSL450
+               OpDecorate %main LinkageAttributes "link_fn" Import
+     %void = OpTypeVoid
+     %func = OpTypeFunction %void
+       %main = OpFunction %void None %func
+               OpFunctionEnd
+  )";
+  CompileSuccessfully(spirv);
+  EXPECT_EQ(SPV_SUCCESS, ValidateAndRetrieveValidationState());
+}
+
+TEST_F(ValidateDecorations, FunctionDeclarationWithExportLinkageBad) {
+  string spirv = R"(
+               OpCapability Shader
+               OpCapability Linkage
+               OpMemoryModel Logical GLSL450
+               OpDecorate %main LinkageAttributes "link_fn" Export
+     %void = OpTypeVoid
+     %func = OpTypeFunction %void
+       %main = OpFunction %void None %func
+               OpFunctionEnd
+  )";
+  CompileSuccessfully(spirv);
+  EXPECT_EQ(SPV_ERROR_INVALID_BINARY, ValidateAndRetrieveValidationState());
+  EXPECT_THAT(
+      getDiagnosticString(),
+      HasSubstr("Function declaration (id 1) must have a LinkageAttributes "
+                "decoration with the Import Linkage type."));
+}
+
+TEST_F(ValidateDecorations, FunctionDefinitionWithImportLinkageBad) {
+  string spirv = R"(
+               OpCapability Shader
+               OpCapability Linkage
+               OpMemoryModel Logical GLSL450
+               OpDecorate %main LinkageAttributes "link_fn" Import
+     %void = OpTypeVoid
+     %func = OpTypeFunction %void
+       %main = OpFunction %void None %func
+      %label = OpLabel
+               OpReturn
+               OpFunctionEnd
+  )";
+  CompileSuccessfully(spirv);
+  EXPECT_EQ(SPV_ERROR_INVALID_BINARY, ValidateAndRetrieveValidationState());
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Function definition (id 1) may not be decorated with "
+                        "Import Linkage type."));
+}
+
+TEST_F(ValidateDecorations, FunctionDefinitionWithoutImportLinkageGood) {
+  string spirv = R"(
+               OpCapability Shader
+               OpCapability Linkage
+               OpMemoryModel Logical GLSL450
+     %void = OpTypeVoid
+     %func = OpTypeFunction %void
+       %main = OpFunction %void None %func
+      %label = OpLabel
+               OpReturn
+               OpFunctionEnd
+  )";
+  CompileSuccessfully(spirv);
+  EXPECT_EQ(SPV_SUCCESS, ValidateAndRetrieveValidationState());
+}
+
 }  // anonymous namespace
 

diff --git a/test/val/val_id_test.cpp b/test/val/val_id_test.cpp
index 07deb4d..f8ede50 100644 (file)
--- a/test/val/val_id_test.cpp
+++ b/test/val/val_id_test.cpp
@@ -2675,6 +2675,8 @@ TEST_F(ValidateIdWithMessage, OpFunctionGood) {
 %2 = OpTypeInt 32 0
 %3 = OpTypeFunction %1 %2 %2
 %4 = OpFunction %1 None %3
+%5 = OpLabel
+     OpReturn
      OpFunctionEnd)";
   CompileSuccessfully(spirv.c_str());
   EXPECT_EQ(SPV_SUCCESS, ValidateInstructions());
@@ -2683,9 +2685,11 @@ TEST_F(ValidateIdWithMessage, OpFunctionResultTypeBad) {
   string spirv = kGLSL450MemoryModel + R"(
 %1 = OpTypeVoid
 %2 = OpTypeInt 32 0
-%5 = OpConstant %2 42
-%3 = OpTypeFunction %1 %2 %2
-%4 = OpFunction %2 None %3
+%3 = OpConstant %2 42
+%4 = OpTypeFunction %1 %2 %2
+%5 = OpFunction %2 None %4
+%6 = OpLabel
+     OpReturn
      OpFunctionEnd)";
   CompileSuccessfully(spirv.c_str());
   EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
@@ -2698,6 +2702,8 @@ TEST_F(ValidateIdWithMessage, OpFunctionFunctionTypeBad) {
 %1 = OpTypeVoid
 %2 = OpTypeInt 32 0
 %4 = OpFunction %1 None %2
+%5 = OpLabel
+     OpReturn
 OpFunctionEnd)";
   CompileSuccessfully(spirv.c_str());
   EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());

diff --git a/test/val/val_layout_test.cpp b/test/val/val_layout_test.cpp
index 172f14a..3a99008 100644 (file)
--- a/test/val/val_layout_test.cpp
+++ b/test/val/val_layout_test.cpp
@@ -107,13 +107,15 @@ const vector<string>& getInstructions() {
     "OpLine %str 3 4",
     "OpNoLine",
     "%func     = OpFunction %voidt None %vfunct",
-    "OpFunctionEnd",
+    "%l = OpLabel",
+    "OpReturn ; %func return",
+    "OpFunctionEnd ; %func end",
     "%func2    = OpFunction %voidt None %viifunct",
     "%funcp1   = OpFunctionParameter %intt",
     "%funcp2   = OpFunctionParameter %intt",
     "%fLabel   = OpLabel",
-    "            OpNop",
-    "            OpReturn",
+    "OpNop",
+    "OpReturn ; %func2 return",
     "OpFunctionEnd"
   };
   return instructions;
@@ -151,16 +153,16 @@ INSTANTIATE_TEST_CASE_P(InstructionsOrder,
                      , make_tuple(string("OpTypeVoid")                , Range<17, 30>()        , Range<0, 25>())
                      , make_tuple(string("OpTypeFloat")               , Range<17, 30>()        , Range<0,20>())
                      , make_tuple(string("OpTypeInt")                 , Range<17, 30>()        , Range<0, 20>())
-                     , make_tuple(string("OpTypeVector %floatt 4")      , Range<17, 30>()        , Range<19, 23>())
+                     , make_tuple(string("OpTypeVector %floatt 4")    , Range<17, 30>()        , Range<19, 23>())
                      , make_tuple(string("OpTypeMatrix %vec4 4")      , Range<17, 30>()        , Range<22, kRangeEnd>())
                      , make_tuple(string("OpTypeStruct")              , Range<17, 30>()        , Range<24, kRangeEnd>())
                      , make_tuple(string("%vfunct   = OpTypeFunction"), Range<17, 30>()        , Range<20, 30>())
                      , make_tuple(string("OpConstant")                , Range<17, 30>()        , Range<20, kRangeEnd>())
                      , make_tuple(string("OpLine ")                   , Range<17, kRangeEnd>() , Range<7, kRangeEnd>())
                      , make_tuple(string("OpNoLine")                  , Range<17, kRangeEnd>() , All)
-                     , make_tuple(string("OpLabel")                   , Equals<36>             , All)
-                     , make_tuple(string("OpNop")                     , Equals<37>             , All)
-                     , make_tuple(string("OpReturn")                  , Equals<38>             , All)
+                     , make_tuple(string("%fLabel   = OpLabel")       , Equals<38>             , All)
+                     , make_tuple(string("OpNop")                     , Equals<39>             , Range<39,kRangeEnd>())
+                     , make_tuple(string("OpReturn ; %func2 return")  , Equals<40>             , All)
     )),);
 // clang-format on