[OPT] Identify arrays with unknown length in copy prop arrays (KhronosGroup#5570)

* [OPT] Identify arrays with unknown length in copy prop arrays

The code in copy propagate arrays assumes that the length of an
OpTypeArray is known at compile time, but that is not true when the size
is an OpSpecConstant. We try to fix that assumption.

Fixes https://crbug.com/oss-fuzz/66634

Author: s-perron

source/opt/copy_prop_arrays.cpp

@@ -35,6 +35,32 @@ bool IsDebugDeclareOrValue(Instruction* di) {
          dbg_opcode == CommonDebugInfoDebugValue;
 }
 
+// Returns the number of members in |type|.  If |type| is not a composite type
+// or the number of components is not known at compile time, the return value
+// will be 0.
+uint32_t GetNumberOfMembers(const analysis::Type* type, IRContext* context) {
+  if (const analysis::Struct* struct_type = type->AsStruct()) {
+    return static_cast<uint32_t>(struct_type->element_types().size());
+  } else if (const analysis::Array* array_type = type->AsArray()) {
+    const analysis::Constant* length_const =
+        context->get_constant_mgr()->FindDeclaredConstant(
+            array_type->LengthId());
+
+    if (length_const == nullptr) {
+      // This can happen if the length is an OpSpecConstant.
+      return 0;
+    }
+    assert(length_const->type()->AsInteger());
+    return length_const->GetU32();
+  } else if (const analysis::Vector* vector_type = type->AsVector()) {
+    return vector_type->element_count();
+  } else if (const analysis::Matrix* matrix_type = type->AsMatrix()) {
+    return matrix_type->element_count();
+  } else {
+    return 0;
+  }
+}
+
 }  // namespace
 
 Pass::Status CopyPropagateArrays::Process() {
@@ -357,22 +383,9 @@ CopyPropagateArrays::BuildMemoryObjectFromInsert(Instruction* insert_inst) {
 
   analysis::DefUseManager* def_use_mgr = context()->get_def_use_mgr();
   analysis::TypeManager* type_mgr = context()->get_type_mgr();
-  analysis::ConstantManager* const_mgr = context()->get_constant_mgr();
   const analysis::Type* result_type = type_mgr->GetType(insert_inst->type_id());
 
-  uint32_t number_of_elements = 0;
-  if (const analysis::Struct* struct_type = result_type->AsStruct()) {
-    number_of_elements =
-        static_cast<uint32_t>(struct_type->element_types().size());
-  } else if (const analysis::Array* array_type = result_type->AsArray()) {
-    const analysis::Constant* length_const =
-        const_mgr->FindDeclaredConstant(array_type->LengthId());
-    number_of_elements = length_const->GetU32();
-  } else if (const analysis::Vector* vector_type = result_type->AsVector()) {
-    number_of_elements = vector_type->element_count();
-  } else if (const analysis::Matrix* matrix_type = result_type->AsMatrix()) {
-    number_of_elements = matrix_type->element_count();
-  }
+  uint32_t number_of_elements = GetNumberOfMembers(result_type, context());
 
   if (number_of_elements == 0) {
     return nullptr;
@@ -800,23 +813,8 @@ uint32_t CopyPropagateArrays::MemoryObject::GetNumberOfMembers() {
   std::vector<uint32_t> access_indices = GetAccessIds();
   type = type_mgr->GetMemberType(type, access_indices);
 
-  if (const analysis::Struct* struct_type = type->AsStruct()) {
-    return static_cast<uint32_t>(struct_type->element_types().size());
-  } else if (const analysis::Array* array_type = type->AsArray()) {
-    const analysis::Constant* length_const =
-        context->get_constant_mgr()->FindDeclaredConstant(
-            array_type->LengthId());
-    assert(length_const->type()->AsInteger());
-    return length_const->GetU32();
-  } else if (const analysis::Vector* vector_type = type->AsVector()) {
-    return vector_type->element_count();
-  } else if (const analysis::Matrix* matrix_type = type->AsMatrix()) {
-    return matrix_type->element_count();
-  } else {
-    return 0;
-  }
+  return opt::GetNumberOfMembers(type, context);
 }
-
 template <class iterator>
 CopyPropagateArrays::MemoryObject::MemoryObject(Instruction* var_inst,
                                                 iterator begin, iterator end)

source/opt/copy_prop_arrays.h

@@ -101,7 +101,8 @@ class CopyPropagateArrays : public MemPass {
     bool IsMember() const { return !access_chain_.empty(); }
 
     // Returns the number of members in the object represented by |this|.  If
-    // |this| does not represent a composite type, the return value will be 0.
+    // |this| does not represent a composite type or the number of components is
+    // not known at compile time, the return value will be 0.
     uint32_t GetNumberOfMembers();
 
     // Returns the owning variable that the memory object is contained in.
@@ -207,7 +208,7 @@ class CopyPropagateArrays : public MemPass {
 
   // Returns the memory object that at some point was equivalent to the result
   // of |insert_inst|.  If a memory object cannot be identified, the return
-  // value is |nullptr\.  The opcode of |insert_inst| must be
+  // value is |nullptr|.  The opcode of |insert_inst| must be
   // |OpCompositeInsert|.  This function looks for a series of
   // |OpCompositeInsert| instructions that insert the elements one at a time in
   // order from beginning to end.

test/opt/copy_prop_array_test.cpp

@@ -1942,6 +1942,41 @@ OpFunctionEnd
 
   SinglePassRunAndCheck<CopyPropagateArrays>(text, text, false);
 }
+
+// If the size of an array used in an OpCompositeInsert is not known at compile
+// time, then we should not propagate the array, because we do not have a single
+// array that represents the final value.
+TEST_F(CopyPropArrayPassTest, SpecConstSizedArray) {
+  const std::string text = R"(OpCapability Shader
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %2 "main"
+OpExecutionMode %2 OriginUpperLeft
+%void = OpTypeVoid
+%4 = OpTypeFunction %void
+%int = OpTypeInt 32 1
+%uint = OpTypeInt 32 0
+%7 = OpSpecConstant %uint 32
+%_arr_int_7 = OpTypeArray %int %7
+%int_63 = OpConstant %int 63
+%uint_0 = OpConstant %uint 0
+%bool = OpTypeBool
+%int_0 = OpConstant %int 0
+%int_587202566 = OpConstant %int 587202566
+%false = OpConstantFalse %bool
+%_ptr_Function__arr_int_7 = OpTypePointer Function %_arr_int_7
+%16 = OpUndef %_arr_int_7
+%2 = OpFunction %void None %4
+%17 = OpLabel
+%18 = OpVariable %_ptr_Function__arr_int_7 Function
+%19 = OpCompositeInsert %_arr_int_7 %int_0 %16 0
+OpStore %18 %19
+OpReturn
+OpFunctionEnd
+)";
+
+  SinglePassRunAndCheck<CopyPropagateArrays>(text, text, false);
+}
 }  // namespace
 }  // namespace opt
 }  // namespace spvtools