spirv-val: Check DebugLine Line/Column are valid

source/val/validate_extensions.cpp

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 // Validates correctness of extension SPIR-V instructions.
+#include <cstdint>
 #include <cstdlib>
 #include <sstream>
 #include <string>
@@ -29,6 +30,7 @@
 #include "source/val/instruction.h"
 #include "source/val/validate.h"
 #include "source/val/validation_state.h"
+#include "spirv-tools/libspirv.h"
 #include "spirv/unified1/NonSemanticClspvReflection.h"
 
 namespace spvtools {
@@ -282,6 +284,50 @@ spv_result_t ValidateOperandDebugType(
          << " is not a valid debug type";
 }
 
+spv_result_t ValidateOperandDebugSource(
+    ValidationState_t& _, const Instruction* inst, uint32_t source_index,
+    uint32_t line_index, uint32_t column_index,
+    spv_ext_inst_type_t ext_inst_type,
+    const std::function<std::string()>& ext_inst_name) {
+  auto* debug_source_inst = _.FindDef(inst->word(source_index));
+  const std::vector<uint32_t>& line_lengths =
+      _.GetDebugSourceLineLength(debug_source_inst->id());
+  if (line_lengths.empty())
+    return SPV_SUCCESS;  // Text not provide in DebugSource
+
+  const bool vulkanDebugInfo =
+      ext_inst_type == SPV_EXT_INST_TYPE_NONSEMANTIC_SHADER_DEBUGINFO_100;
+  uint32_t line = 0;
+  uint32_t column = 0;
+  // NonSemantic uses OpConstant for all operands
+  if (vulkanDebugInfo) {
+    bool is_int32 = false, is_const_int32 = false;
+    std::tie(is_int32, is_const_int32, line) =
+        _.EvalInt32IfConst(inst->word(line_index));
+    std::tie(is_int32, is_const_int32, column) =
+        _.EvalInt32IfConst(inst->word(column_index));
+  } else {
+    line = inst->word(line_index);
+    column = inst->word(column_index);
+  }
+
+  if (line > line_lengths.size()) {
+    return _.diag(SPV_ERROR_INVALID_DATA, inst)
+           << ext_inst_name() << ": operand Line (" << line
+           << ") is larger then the " << line_lengths.size()
+           << " lines found in the DebugSource text";
+  } else if (line != 0) {
+    const uint32_t line_length = line_lengths[line - 1];
+    if (column > line_length) {
+      return _.diag(SPV_ERROR_INVALID_DATA, inst)
+             << ext_inst_name() << ": operand Column End (" << column
+             << ") is larger then Line " << line << " column length of "
+             << line_length << " found in the DebugSource text";
+    }
+  }
+  return SPV_SUCCESS;
+}
+
 spv_result_t ValidateClspvReflectionKernel(ValidationState_t& _,
                                            const Instruction* inst,
                                            uint32_t version) {
@@ -1002,6 +1048,70 @@ spv_result_t ValidateClspvReflectionInstruction(ValidationState_t& _,
   return SPV_SUCCESS;
 }
 
+// We build up a vector that is length of the DebugSource lines and get how long
+// they are to make sure anyone using a DebugSource provides valid Line/Columns
+// inside
+void BuildDebugSourceLineLength(ValidationState_t& _, const Instruction* inst,
+                                uint32_t ext_inst_index) {
+  if (ext_inst_index == NonSemanticShaderDebugInfo100DebugSource &&
+      inst->words().size() < 7) {
+    return;  // The optional text was not provided
+  }
+
+  // Validated to be an OpString
+  const uint32_t string_operand =
+      (ext_inst_index == NonSemanticShaderDebugInfo100DebugSource) ? 6 : 5;
+  auto* debug_source_text_insn = _.FindDef(inst->word(string_operand));
+  std::string debug_source_text =
+      debug_source_text_insn->GetOperandAs<std::string>(1);
+
+  // walk back to get DebugSource to update it's line length list
+  uint32_t debug_source_id = inst->id();
+  uint32_t continue_count = 0;
+  while (ext_inst_index == NonSemanticShaderDebugInfo100DebugSourceContinued) {
+    continue_count++;  // might have multiple Continues in a row
+    auto prev_index = inst - &_.ordered_instructions()[0] - continue_count;
+    auto prev_inst = &_.ordered_instructions()[prev_index];
+    debug_source_id = prev_inst->id();
+    ext_inst_index = prev_inst->GetOperandAs<uint32_t>(3);
+  }
+
+  std::vector<uint32_t>& line_lengths =
+      _.GetDebugSourceLineLength(debug_source_id);
+  uint32_t line_start = 0;
+  // If we have a line like "abc", it really column 1-to-4.
+  // Even an empty line should have a column of 1
+  // Add 1 to length to emulate this later
+  uint32_t length = 1;
+
+  for (uint32_t i = 0; i < debug_source_text.size(); ++i) {
+    if (debug_source_text[i] == '\n') {
+      // Unix-style new line
+      line_lengths.push_back(length);
+      length = 1;
+      line_start = i + 1;
+    } else if (debug_source_text[i] == '\r') {
+      // Handle Windows-style \r\n
+      if (i + 1 < debug_source_text.size() &&
+          debug_source_text[i + 1] == '\n') {
+        line_lengths.push_back(length);
+        ++i;  // Skip '\n'
+      } else {
+        line_lengths.push_back(length);
+      }
+      length = 1;
+      line_start = i + 1;
+    } else {
+      ++length;
+    }
+  }
+
+  // Capture last line if the string does not end in a newline
+  if (line_start < debug_source_text.size()) {
+    line_lengths.push_back(length);
+  }
+}
+
 bool IsConstIntScalarTypeWith32Or64Bits(ValidationState_t& _,
                                         Instruction* instr) {
   if (instr->opcode() != spv::Op::OpConstant) return false;
@@ -3232,7 +3342,12 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
               _.EvalInt32IfConst(inst->word(8));
           std::tie(is_int32, is_const_int32, column_end) =
               _.EvalInt32IfConst(inst->word(9));
-          if (line_end < line_start) {
+          if (line_start == 0) {
+            return _.diag(SPV_ERROR_INVALID_DATA, inst)
+                   << ext_inst_name()
+                   << ": operand Line Start (0) is not allowed, source lines "
+                      "start at Line 1";
+          } else if (line_end < line_start) {
             return _.diag(SPV_ERROR_INVALID_DATA, inst)
                    << ext_inst_name() << ": operand Line End (" << line_end
                    << ") is less than Line Start (" << line_start << ")";
@@ -3242,10 +3357,53 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
                    << ") is less than Column Start (" << column_start
                    << ") when Line Start equals Line End";
           }
+
+          // Make sure Line is found in the DebugSource
+          auto* debug_source_inst = _.FindDef(inst->word(5));
+          const std::vector<uint32_t>& line_lengths =
+              _.GetDebugSourceLineLength(debug_source_inst->id());
+          if (!line_lengths.empty()) {
+            if (line_end > line_lengths.size()) {
+              return _.diag(SPV_ERROR_INVALID_DATA, inst)
+                     << ext_inst_name() << ": operand Line End (" << line_end
+                     << ") is larger then the " << line_lengths.size()
+                     << " lines found in the DebugSource text";
+            }
+            if (line_start == line_end) {
+              const uint32_t line_length = line_lengths[line_end - 1];
+              if (column_end > line_length) {
+                return _.diag(SPV_ERROR_INVALID_DATA, inst)
+                       << ext_inst_name() << ": operand Column End ("
+                       << column_end << ") is larger then Line " << line_end
+                       << " column length of " << line_length
+                       << " found in the DebugSource text";
+              }
+            } else {
+              uint32_t line_length = line_lengths[line_start - 1];
+              if (column_start > line_length) {
+                return _.diag(SPV_ERROR_INVALID_DATA, inst)
+                       << ext_inst_name() << ": operand Column Start ("
+                       << column_start << ") is larger then Line " << line_start
+                       << " column length of " << line_length
+                       << " found in the DebugSource text";
+              }
+              line_length = line_lengths[line_end - 1];
+              if (column_end > line_length) {
+                return _.diag(SPV_ERROR_INVALID_DATA, inst)
+                       << ext_inst_name() << ": operand Column End ("
+                       << column_end << ") is larger then Line " << line_end
+                       << " column length of " << line_length
+                       << " found in the DebugSource text";
+              }
+            }
+          }
+
           break;
         }
         case NonSemanticShaderDebugInfo100DebugSourceContinued: {
           CHECK_OPERAND("Text", spv::Op::OpString, 5);
+          // OpenCL didn't have a Continued version
+          BuildDebugSourceLineLength(_, inst, ext_inst_index);
           break;
         }
         case NonSemanticShaderDebugInfo100DebugBuildIdentifier: {
@@ -3305,6 +3463,7 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
         case CommonDebugInfoDebugSource: {
           CHECK_OPERAND("File", spv::Op::OpString, 5);
           if (num_words == 7) CHECK_OPERAND("Text", spv::Op::OpString, 6);
+          BuildDebugSourceLineLength(_, inst, ext_inst_index);
           break;
         }
         case CommonDebugInfoDebugTypeBasic: {
@@ -3419,6 +3578,10 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
           CHECK_DEBUG_OPERAND("Source", CommonDebugInfoDebugSource, 7);
           CHECK_CONST_UINT_OPERAND("Line", 8);
           CHECK_CONST_UINT_OPERAND("Column", 9);
+          if (auto error = ValidateOperandDebugSource(
+                  _, inst, 7, 8, 9, ext_inst_type, ext_inst_name))
+            return error;
+
           auto validate_parent =
               ValidateOperandLexicalScope(_, "Parent", inst, 10, ext_inst_name);
           if (validate_parent != SPV_SUCCESS) return validate_parent;
@@ -3457,6 +3620,10 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
           CHECK_DEBUG_OPERAND("Source", CommonDebugInfoDebugSource, 7);
           CHECK_CONST_UINT_OPERAND("Line", 8);
           CHECK_CONST_UINT_OPERAND("Column", 9);
+          if (auto error = ValidateOperandDebugSource(
+                  _, inst, 7, 8, 9, ext_inst_type, ext_inst_name))
+            return error;
+
           auto validate_parent =
               ValidateOperandLexicalScope(_, "Parent", inst, 10, ext_inst_name);
           if (validate_parent != SPV_SUCCESS) return validate_parent;
@@ -3480,6 +3647,10 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
           CHECK_DEBUG_OPERAND("Source", CommonDebugInfoDebugSource, 7);
           CHECK_CONST_UINT_OPERAND("Line", 8);
           CHECK_CONST_UINT_OPERAND("Column", 9);
+          if (auto error = ValidateOperandDebugSource(
+                  _, inst, 7, 8, 9, ext_inst_type, ext_inst_name))
+            return error;
+
           auto validate_parent =
               ValidateOperandLexicalScope(_, "Parent", inst, 10, ext_inst_name);
           if (validate_parent != SPV_SUCCESS) return validate_parent;
@@ -3521,6 +3692,10 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
           CHECK_DEBUG_OPERAND("Source", CommonDebugInfoDebugSource, 7);
           CHECK_CONST_UINT_OPERAND("Line", 8);
           CHECK_CONST_UINT_OPERAND("Column", 9);
+          if (auto error = ValidateOperandDebugSource(
+                  _, inst, 7, 8, 9, ext_inst_type, ext_inst_name))
+            return error;
+
           // NonSemantic.Shader.DebugInfo doesn't have the Parent operand
           if (vulkanDebugInfo) {
             CHECK_OPERAND("Offset", spv::Op::OpConstant, 10);
@@ -3573,6 +3748,10 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
           CHECK_DEBUG_OPERAND("Source", CommonDebugInfoDebugSource, 7);
           CHECK_CONST_UINT_OPERAND("Line", 8);
           CHECK_CONST_UINT_OPERAND("Column", 9);
+          if (auto error = ValidateOperandDebugSource(
+                  _, inst, 7, 8, 9, ext_inst_type, ext_inst_name))
+            return error;
+
           auto validate_parent =
               ValidateOperandLexicalScope(_, "Parent", inst, 10, ext_inst_name);
           if (validate_parent != SPV_SUCCESS) return validate_parent;
@@ -3608,6 +3787,10 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
           CHECK_DEBUG_OPERAND("Source", CommonDebugInfoDebugSource, 7);
           CHECK_CONST_UINT_OPERAND("Line", 8);
           CHECK_CONST_UINT_OPERAND("Column", 9);
+          if (auto error = ValidateOperandDebugSource(
+                  _, inst, 7, 8, 9, ext_inst_type, ext_inst_name))
+            return error;
+
           auto validate_parent =
               ValidateOperandLexicalScope(_, "Parent", inst, 10, ext_inst_name);
           if (validate_parent != SPV_SUCCESS) return validate_parent;
@@ -3619,6 +3802,10 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
           CHECK_DEBUG_OPERAND("Source", CommonDebugInfoDebugSource, 5);
           CHECK_CONST_UINT_OPERAND("Line", 6);
           CHECK_CONST_UINT_OPERAND("Column", 7);
+          if (auto error = ValidateOperandDebugSource(
+                  _, inst, 5, 6, 7, ext_inst_type, ext_inst_name))
+            return error;
+
           auto validate_parent =
               ValidateOperandLexicalScope(_, "Parent", inst, 8, ext_inst_name);
           if (validate_parent != SPV_SUCCESS) return validate_parent;
@@ -3644,6 +3831,10 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
           CHECK_DEBUG_OPERAND("Source", CommonDebugInfoDebugSource, 7);
           CHECK_CONST_UINT_OPERAND("Line", 8);
           CHECK_CONST_UINT_OPERAND("Column", 9);
+          if (auto error = ValidateOperandDebugSource(
+                  _, inst, 7, 8, 9, ext_inst_type, ext_inst_name))
+            return error;
+
           auto validate_parent =
               ValidateOperandLexicalScope(_, "Parent", inst, 10, ext_inst_name);
           if (validate_parent != SPV_SUCCESS) return validate_parent;
@@ -3723,6 +3914,9 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
           CHECK_DEBUG_OPERAND("Source", CommonDebugInfoDebugSource, 8);
           CHECK_CONST_UINT_OPERAND("Line", 9);
           CHECK_CONST_UINT_OPERAND("Column", 10);
+          if (auto error = ValidateOperandDebugSource(
+                  _, inst, 8, 9, 10, ext_inst_type, ext_inst_name))
+            return error;
           break;
         }
         case CommonDebugInfoDebugGlobalVariable: {
@@ -3733,6 +3927,10 @@ spv_result_t ValidateExtInst(ValidationState_t& _, const Instruction* inst) {
           CHECK_DEBUG_OPERAND("Source", CommonDebugInfoDebugSource, 7);
           CHECK_CONST_UINT_OPERAND("Line", 8);
           CHECK_CONST_UINT_OPERAND("Column", 9);
+          if (auto error = ValidateOperandDebugSource(
+                  _, inst, 7, 8, 9, ext_inst_type, ext_inst_name))
+            return error;
+
           auto validate_scope =
               ValidateOperandLexicalScope(_, "Scope", inst, 10, ext_inst_name);
           if (validate_scope != SPV_SUCCESS) return validate_scope;

source/val/validation_state.cpp

@@ -1827,6 +1827,15 @@ bool ValidationState_t::ContainsUntypedPointer(uint32_t id) const {
   return false;
 }
 
+std::vector<uint32_t>& ValidationState_t::GetDebugSourceLineLength(
+    uint32_t id) {
+  auto it = debug_source_line_length_.find(id);
+  if (it == debug_source_line_length_.end()) {
+    return debug_source_line_length_[id];
+  }
+  return it->second;
+}
+
 bool ValidationState_t::IsValidStorageClass(
     spv::StorageClass storage_class) const {
   if (spvIsVulkanEnv(context()->target_env)) {

source/val/validation_state.h

@@ -859,6 +859,10 @@ class ValidationState_t {
            qcom_image_processing_consumers_.end();
   }
 
+  // Get the list of line lengths for a given result ID of a DebugSource
+  // instruction Will create a new vector if DebugSource is not found
+  std::vector<uint32_t>& GetDebugSourceLineLength(uint32_t id);
+
  private:
   ValidationState_t(const ValidationState_t&);
 
@@ -1023,6 +1027,10 @@ class ValidationState_t {
   // TypePass.
   std::unordered_set<uint32_t> pointer_to_storage_image_;
 
+  /// Maps an id of DebugSource to a vector that contains the length of each
+  /// line side of it. (Also will have the DebugSourceContinued source included)
+  std::unordered_map<uint32_t, std::vector<uint32_t>> debug_source_line_length_;
+
   /// Maps ids to friendly names.
   std::unique_ptr<spvtools::FriendlyNameMapper> friendly_mapper_;
   spvtools::NameMapper name_mapper_;

test/opt/aggressive_dead_code_elim_test.cpp

@@ -7698,8 +7698,7 @@ struct PS_OUTPUT
 } ;
 
 "
-          %6 = OpString "
-PS_OUTPUT MainPs ( )
+          %6 = OpString "PS_OUTPUT MainPs ( )
 {
     PS_OUTPUT ps_output ;
     ps_output . vColor = float4( 1.0, 0.0, 0.0, 0.0 );