slang/RHI: nullptr deref in ShaderObjectImpl::writeSamplerDescriptor

[aleino-nv]

Affected tests:

• tests/bugs/user-attribute-lookup.slang

sampler is nullptr for some reason:

void ShaderObjectImpl::writeSamplerDescriptor(
    RootBindingContext& context,
    BindingOffset const& offset,
    span<RefPtr<SamplerImpl>> samplers
)
{
    Index count = samplers.size();
    for (Index i = 0; i < count; ++i)
    {
        const RefPtr<SamplerImpl>& sampler = samplers[i];

        WGPUBindGroupEntry entry = {};
        entry.binding = offset.binding + i;
        entry.sampler = sampler->m_sampler;
        writeDescriptor(context, offset.bindingSet, entry);
    }
}

[cheneym2]

@aleino-nv I'm not able to reproduce this.

There's a line in the test to disable wgsl, and I removed that "DISABLE" line, but the test still runs fine (no segfault).

Is the issue still present at TOT for you?

tests/bugs/user-attribute-lookup.slang

[aleino-nv]

@cheneym2 Yeah, I'm still seeing the same issue with this test on 79056cd.
You can assign this one to me if you want.

C:\Users\aleino\workspaces\slang>cd C:\Users\aleino\workspaces\slang   && C:\Users\aleino\workspaces\slang\build\Debug\bin\slang-test -bindir C:\Users\aleino\workspaces\slang\build\Debug\bin -category full -api wgpu tests/bugs/user-attribute-lookup.slang   || exit 1 
Supported backends: fxc dxc glslang spirv-dis visualstudio genericcpp llvm spirv-opt tint
Check vk,vulkan: Supported
Check dx12,d3d12: Supported
Check dx11,d3d11: Supported
Check cuda: Not Supported
Check wgpu,webgpu: Supported
ignored test: 'tests/bugs/user-attribute-lookup.slang (dx11)' 
ignored test: 'tests/bugs/user-attribute-lookup.slang.1 (vk)' 
Warning: Old OnSubmittedWorkDone APIs are deprecated. If using C please pass a CallbackInfo struct that has two userdatas. Otherwise, if using C++, please use templated helpers.
error: ERROR:
EXPECTED{{{
result code = 0
standard error = {
}
standard output = {
}
}}}
ACTUAL{{{
result code = 1
standard error = {
}
standard output = {
Unhandled exception}
}}}

error: ERROR:
EXPECTED{{{
0
1
2
3
}}}
ACTUAL{{{
}}}

ignored test: 'tests/bugs/user-attribute-lookup.slang.2 syn (wgpu)' 
Warning: Old OnSubmittedWorkDone APIs are deprecated. If using C please pass a CallbackInfo struct that has two userdatas. Otherwise, if using C++, please use templated helpers.
error: ERROR:
EXPECTED{{{
result code = 0
standard error = {
}
standard output = {
}
}}}
ACTUAL{{{
result code = 1
standard error = {
}
standard output = {
Unhandled exception}
}}}

error: ERROR:
EXPECTED{{{
0
1
2
3
}}}
ACTUAL{{{
}}}

Retrying 1 failed tests...
FAILED test: 'tests/bugs/user-attribute-lookup.slang.2 syn (wgpu)' 

===
0% of tests passed (0/1), 3 tests ignored
===

failing tests:
---
tests/bugs/user-attribute-lookup.slang.2 syn (wgpu)
---

[cheneym2]

Oh, nevermind. I was bit again by #5462.

I'm able to see the crash now that I have the wgsl backend running correctly again in my new branch. Sorry about that.

[cheneym2]

Looks like the user defined attribute is not relevant to the repro, rather it's having a SamplerState declaration without matching TEST_INPUT: Sampler

In the test snippet,

// Define an attribute that will appear in reflection
[__AttributeUsage(_AttributeTargets.Var)]
struct StaticSamplerAttribute {};

struct Thing
{
    [StaticSampler] SamplerState samplerState1;
    [StaticSampler] SamplerState samplerState2;
}

[StaticSampler] SamplerState samplerState;

The bug can be reproduced replacing all that with just

SamplerState samplerState;

And then independent of which of the two SamplerState declarations you use, the following additional line fixes the crash:

//TEST_INPUT: Sampler:name samplerState

[cheneym2]

Is it legal to declare the SamplerState object without providing slang-test corresponding input data?

I might just do a null check right where there's the segfault in the wgpu rhi backend. It causes the test to fail gracefully(?) with the following validation spew.

WGPU error: Number of entries (1) did not match the expected number of entries (2) for [BindGroupLayout (unlabeled)].
Expected layout: [{ binding: 0, visibility: ShaderStage::(Fragment|Compute), buffer: {type: BufferBindingType::Storage, minBindingSize: 0, hasDynamicOffset: 0} }, { binding: 1, visibility: ShaderStage::(Vertex|Fragment|Compute), sampler: {type: SamplerBindingType::Filtering} }]
 - While validating [BindGroupDescriptor] against [BindGroupLayout (unlabeled)]
 - While calling [Device].CreateBindGroup([BindGroupDescriptor]).

WGPU error: [Invalid BindGroup (unlabeled)] is invalid.
 - While encoding [ComputePassEncoder (unlabeled)].SetBindGroup(0, [Invalid BindGroup (unlabeled)], 0, ...).
 - While finishing [CommandEncoder (unlabeled)].

WGPU error: [Invalid CommandBuffer] is invalid.
 - While calling [Queue].Submit([[Invalid CommandBuffer]])

Warning: Old MapAsync APIs are deprecated. If using C please pass a CallbackInfo struct that has two userdatas. Otherwise, if using C++, please use templated helpers.
error: ERROR:
EXPECTED{{{
0
1
2
3
}}}
ACTUAL{{{
0
0
0
0
}}}

FAILED test: 'tests/bugs/user-attribute-lookup.slang.2 syn (wgpu)'

The above seems like an improvement over crashing. Then the test passes altogether if I add this line:

//TEST_INPUT: Sampler:name samplerState

[csyonghe]

For most targets, this will be illegal. Let's modify the test to provide the TEST_INPUT line and close this.

[cheneym2]

Will do, thanks!

[cheneym2]

#5510