Fix crash when handling uninitialized resource type (#6338)

* Fix crash when handling uninitialized resource type close #6328. When declare a var with struct type, if the struct has a resource type field and it doesn't provide explicit constructor, because slang won't implicit construct such variable at declare site if user doesn't explicitly call the initializer list, we should report the resource type field uninitialized as an error to prevent future possible crash when legalize any use of such variable.
shader-slang · Feb 12, 2025 · 3f102af · 3f102af
1 parent 0fee8c1
commit 3f102af
Show file tree

Hide file tree

Showing 7 changed files with 160 additions and 19 deletions.
diff --git a/source/slang/slang-diagnostic-defs.h b/source/slang/slang-diagnostic-defs.h
@@ -2551,6 +2551,8 @@ DIAGNOSTIC(
     attemptToQuerySizeOfUnsizedArray,
     "cannot obtain the size of an unsized array.")
 
+DIAGNOSTIC(56003, Fatal, useOfUninitializedOpaqueHandle, "use of uninitialized opaque handle '$0'.")
+
 // Metal
 DIAGNOSTIC(
     56100,

diff --git a/source/slang/slang-ir-legalize-types.cpp b/source/slang/slang-ir-legalize-types.cpp
@@ -91,7 +91,10 @@ LegalVal LegalVal::wrappedBuffer(LegalVal const& baseVal, LegalElementWrapping c
 
 //
 
-IRTypeLegalizationContext::IRTypeLegalizationContext(TargetProgram* target, IRModule* inModule)
+IRTypeLegalizationContext::IRTypeLegalizationContext(
+    TargetProgram* target,
+    IRModule* inModule,
+    DiagnosticSink* sink)
 {
     targetProgram = target;
 
@@ -100,6 +103,8 @@ IRTypeLegalizationContext::IRTypeLegalizationContext(TargetProgram* target, IRMo
 
     builderStorage = IRBuilder(inModule);
     builder = &builderStorage;
+
+    m_sink = sink;
 }
 
 static void registerLegalizedValue(
@@ -2046,6 +2051,23 @@ static LegalVal coerceToLegalType(IRTypeLegalizationContext* context, LegalType
     }
 }
 
+static LegalVal legalizeUndefined(IRTypeLegalizationContext* context, IRInst* inst)
+{
+    List<IRType*> opaqueTypes;
+    if (isOpaqueType(inst->getFullType(), opaqueTypes))
+    {
+        auto opaqueType = opaqueTypes[0];
+        auto containerType = opaqueTypes.getCount() > 1 ? opaqueTypes[1] : opaqueType;
+        SourceLoc loc = findBestSourceLocFromUses(inst);
+
+        if (!loc.isValid())
+            loc = getDiagnosticPos(containerType);
+
+        context->m_sink->diagnose(loc, Diagnostics::useOfUninitializedOpaqueHandle, opaqueType);
+    }
+    return LegalVal();
+}
+
 static LegalVal legalizeInst(
     IRTypeLegalizationContext* context,
     IRInst* inst,
@@ -2122,7 +2144,7 @@ static LegalVal legalizeInst(
         result = legalizePrintf(context, args);
         break;
     case kIROp_undefined:
-        return LegalVal();
+        return legalizeUndefined(context, inst);
     case kIROp_GpuForeach:
         // This case should only happen when compiling for a target that does not support
         // GpuForeach
@@ -4015,8 +4037,8 @@ static void legalizeTypes(IRTypeLegalizationContext* context)
 //
 struct IRResourceTypeLegalizationContext : IRTypeLegalizationContext
 {
-    IRResourceTypeLegalizationContext(TargetProgram* target, IRModule* module)
-        : IRTypeLegalizationContext(target, module)
+    IRResourceTypeLegalizationContext(TargetProgram* target, IRModule* module, DiagnosticSink* sink)
+        : IRTypeLegalizationContext(target, module, sink)
     {
     }
 
@@ -4046,8 +4068,11 @@ struct IRResourceTypeLegalizationContext : IRTypeLegalizationContext
 //
 struct IRExistentialTypeLegalizationContext : IRTypeLegalizationContext
 {
-    IRExistentialTypeLegalizationContext(TargetProgram* target, IRModule* module)
-        : IRTypeLegalizationContext(target, module)
+    IRExistentialTypeLegalizationContext(
+        TargetProgram* target,
+        IRModule* module,
+        DiagnosticSink* sink)
+        : IRTypeLegalizationContext(target, module, sink)
     {
     }
 
@@ -4087,8 +4112,8 @@ struct IRExistentialTypeLegalizationContext : IRTypeLegalizationContext
 // a public function signature.
 struct IREmptyTypeLegalizationContext : IRTypeLegalizationContext
 {
-    IREmptyTypeLegalizationContext(TargetProgram* target, IRModule* module)
-        : IRTypeLegalizationContext(target, module)
+    IREmptyTypeLegalizationContext(TargetProgram* target, IRModule* module, DiagnosticSink* sink)
+        : IRTypeLegalizationContext(target, module, sink)
     {
     }
 
@@ -4129,28 +4154,21 @@ void legalizeResourceTypes(TargetProgram* target, IRModule* module, DiagnosticSi
 {
     SLANG_PROFILE;
 
-    SLANG_UNUSED(sink);
-
-    IRResourceTypeLegalizationContext context(target, module);
+    IRResourceTypeLegalizationContext context(target, module, sink);
     legalizeTypes(&context);
 }
 
 void legalizeExistentialTypeLayout(TargetProgram* target, IRModule* module, DiagnosticSink* sink)
 {
     SLANG_PROFILE;
 
-    SLANG_UNUSED(module);
-    SLANG_UNUSED(sink);
-
-    IRExistentialTypeLegalizationContext context(target, module);
+    IRExistentialTypeLegalizationContext context(target, module, sink);
     legalizeTypes(&context);
 }
 
 void legalizeEmptyTypes(TargetProgram* target, IRModule* module, DiagnosticSink* sink)
 {
-    SLANG_UNUSED(sink);
-
-    IREmptyTypeLegalizationContext context(target, module);
+    IREmptyTypeLegalizationContext context(target, module, sink);
     legalizeTypes(&context);
 }
 

diff --git a/source/slang/slang-legalize-types.cpp b/source/slang/slang-legalize-types.cpp
@@ -198,6 +198,64 @@ bool isResourceType(IRType* type)
     return false;
 }
 
+bool isOpaqueType(IRType* type, List<IRType*>& opaqueTypes)
+{
+    if (isResourceType(type))
+    {
+        opaqueTypes.add(type);
+        return true;
+    }
+
+    if (auto structType = as<IRStructType>(type))
+    {
+        for (auto field : structType->getFields())
+        {
+            if (isOpaqueType(field->getFieldType(), opaqueTypes))
+            {
+                opaqueTypes.add(type);
+                return true;
+            }
+        }
+    }
+
+    if (auto arrayType = as<IRArrayTypeBase>(type))
+    {
+        if (isOpaqueType(arrayType->getElementType(), opaqueTypes))
+        {
+            opaqueTypes.add(type);
+            return true;
+        }
+    }
+
+    if (auto tupleType = as<IRTupleTypeBase>(type))
+    {
+        for (UInt i = 0; i < tupleType->getOperandCount(); i++)
+        {
+            if (auto elementType = as<IRType>(tupleType->getOperand(i)))
+            {
+                if (isOpaqueType(elementType, opaqueTypes))
+                {
+                    opaqueTypes.add(type);
+                    return true;
+                }
+            }
+        }
+    }
+
+    return false;
+}
+
+SourceLoc findBestSourceLocFromUses(IRInst* inst)
+{
+    for (auto use = inst->firstUse; use; use = use->nextUse)
+    {
+        auto user = use->getUser();
+        if (user->sourceLoc.isValid())
+            return user->sourceLoc;
+    }
+
+    return inst->sourceLoc;
+}
 // Helper wrapper function around isResourceType that checks if the given
 // type is a pointer to a resource type or a physical storage buffer.
 bool isPointerToResourceType(IRType* type)

diff --git a/source/slang/slang-legalize-types.h b/source/slang/slang-legalize-types.h
@@ -592,8 +592,9 @@ struct IRTypeLegalizationContext
     IRBuilder* builder;
     TargetProgram* targetProgram;
     IRBuilder builderStorage;
+    DiagnosticSink* m_sink;
 
-    IRTypeLegalizationContext(TargetProgram* target, IRModule* inModule);
+    IRTypeLegalizationContext(TargetProgram* target, IRModule* inModule, DiagnosticSink* sink);
 
     // When inserting new globals, put them before this one.
     IRInst* insertBeforeGlobal = nullptr;
@@ -702,7 +703,9 @@ void legalizeEmptyTypes(TargetProgram* target, IRModule* module, DiagnosticSink*
 
 bool isResourceType(IRType* type);
 
+bool isOpaqueType(IRType* type, List<IRType*>& opaqueTypes);
 
+SourceLoc findBestSourceLocFromUses(IRInst* inst);
 } // namespace Slang
 
 #endif
diff --git a/tests/diagnostics/uninitialized-resource-type.slang b/tests/diagnostics/uninitialized-resource-type.slang
@@ -0,0 +1,42 @@
+//DIAGNOSTIC_TEST:SIMPLE: -target hlsl -DTEST_1
+//DIAGNOSTIC_TEST:SIMPLE: -target hlsl -DTEST_2
+
+SamplerState sampler;
+
+struct Foo
+{
+    bool sample_bar = false;
+#ifdef TEST_1
+    Texture2D<float4> bar = {};
+#elif defined(TEST_2)
+    Texture2D<float4> bar[2];
+#endif
+};
+
+struct Result
+{
+    float4 color = float4(0.0);
+};
+
+Result process(in Foo foo)
+{
+    Result result = {};
+
+    if (foo.sample_bar) {
+#ifdef TEST_1
+        result.color = foo.bar.Sample(sampler, float2(0.0, 0.0));
+#elif defined(TEST_2)
+        result.color = foo.bar[0].Sample(sampler, float2(0.0, 0.0));
+#endif
+    }
+
+    return result;
+}
+
+[shader("compute")]
+[numthreads(8, 8, 1)]
+float4 cs_main(uint3 thread_id : SV_DispatchThreadID) {
+    Foo foo;
+    const let result = process(foo);
+    return result.color;
+}
diff --git a/tests/diagnostics/uninitialized-resource-type.slang.1.expected b/tests/diagnostics/uninitialized-resource-type.slang.1.expected
@@ -0,0 +1,9 @@
+result code = -1
+standard error = {
+tests/diagnostics/uninitialized-resource-type.slang(40): warning 41016: use of uninitialized variable 'foo'
+    const let result = process(foo);
+                              ^
+tests/diagnostics/uninitialized-resource-type.slang(40): error 56003: use of uninitialized opaque handle 'array<Texture2D,2>'.
+    const let result = process(foo);
+                              ^
+}
diff --git a/tests/diagnostics/uninitialized-resource-type.slang.expected b/tests/diagnostics/uninitialized-resource-type.slang.expected
@@ -0,0 +1,9 @@
+result code = -1
+standard error = {
+tests/diagnostics/uninitialized-resource-type.slang(40): warning 41016: use of uninitialized variable 'foo'
+    const let result = process(foo);
+                              ^
+tests/diagnostics/uninitialized-resource-type.slang(40): error 56003: use of uninitialized opaque handle 'Texture2D'.
+    const let result = process(foo);
+                              ^
+}