Skip to content

Commit d46aeb0

Browse files
committed
Fix NameExprType returning deleted canonical type when it's in a generic parent.
fixes shader-slang#339 `NamedExpressionType::CreateCanonicalType()` may return a deleted pointer. The original implementation is as follows: ``` Type* NamedExpressionType::CreateCanonicalType() { return GetType(declRef)->GetCanonicalType(); } ``` If `GetType()` returns a newly constructed Type (this happens when the `typedef` is defined inside a generic parent, which triggers a non-trivial substitution), the temporary type will be deleted when the function returns. The fix is to store the temporary type as a field of NamedExpressionType (`innerType`). A relevant fix (though not the true cause of issue shader-slang#339) is to have `Type::GetCanonicalType()` also hold a `RefPtr` to the constructed canonical type, when the canonical type is not `this`. This prevents a returned canonical type being assigned to a RefPtr, which makes it possible for that RefPtr to be the sole owner of the canonical type and deleteing the canonical type when that RefPtr is destroyed.
1 parent 57d9498 commit d46aeb0

File tree

5 files changed

+50
-2
lines changed

5 files changed

+50
-2
lines changed

source/slang/syntax-base-defs.h

+1
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ RAW(
121121

122122
virtual Type* CreateCanonicalType() = 0;
123123
Type* canonicalType = nullptr;
124+
RefPtr<Type> canonicalTypeRefPtr;
124125

125126
Session* session = nullptr;
126127
)

source/slang/syntax.cpp

+5-1
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,8 @@ void Type::accept(IValVisitor* visitor, void* extra)
189189
{
190190
// TODO(tfoley): worry about thread safety here?
191191
et->canonicalType = et->CreateCanonicalType();
192+
if (dynamic_cast<Type*>(et->canonicalType) != this)
193+
et->canonicalTypeRefPtr = et->canonicalType;
192194
SLANG_ASSERT(et->canonicalType);
193195
}
194196
return et->canonicalType;
@@ -861,7 +863,9 @@ void Type::accept(IValVisitor* visitor, void* extra)
861863

862864
Type* NamedExpressionType::CreateCanonicalType()
863865
{
864-
return GetType(declRef)->GetCanonicalType();
866+
if (!innerType)
867+
innerType = GetType(declRef);
868+
return innerType->GetCanonicalType();
865869
}
866870

867871
int NamedExpressionType::GetHashCode()

source/slang/type-defs.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -420,9 +420,10 @@ END_SYNTAX_CLASS()
420420

421421
// A type alias of some kind (e.g., via `typedef`)
422422
SYNTAX_CLASS(NamedExpressionType, Type)
423-
DECL_FIELD(DeclRef<TypeDefDecl>, declRef)
423+
DECL_FIELD(DeclRef<TypeDefDecl>, declRef)
424424

425425
RAW(
426+
RefPtr<Type> innerType;
426427
NamedExpressionType()
427428
{}
428429
NamedExpressionType(

tests/compute/typedef-member.slang

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
//TEST(compute):COMPARE_COMPUTE:-xslang -use-ir
2+
//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):dxbinding(0),glbinding(0),out
3+
4+
// Confirm that a struct type defined in a generic parent works
5+
6+
RWStructuredBuffer<float> outputBuffer;
7+
8+
struct SubType<T>
9+
{
10+
T x;
11+
};
12+
13+
struct GenStruct<T>
14+
{
15+
typedef SubType<T> SubTypeT;
16+
T getVal(SubTypeT v)
17+
{
18+
return v.x;
19+
}
20+
};
21+
22+
float test(float val)
23+
{
24+
GenStruct<float>.SubTypeT sb;
25+
sb.x = val;
26+
GenStruct<float> obj;
27+
return obj.getVal(sb);
28+
}
29+
30+
31+
[numthreads(4, 1, 1)]
32+
void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID)
33+
{
34+
uint tid = dispatchThreadID.x;
35+
float inVal = float(tid);
36+
float outVal = test(inVal);
37+
outputBuffer[tid] = outVal.x;
38+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
0
2+
3F800000
3+
40000000
4+
40400000

0 commit comments

Comments
 (0)