[PATCH 2/5] d3d10/effect: Fix indexing in GetOutputSignatureElementDesc().

[Matteo Bruni]

On Mon, Aug 23, 2021 at 10:23 AM Nikolay Sivov <nsivov at codeweavers.com> wrote:
>
> Signed-off-by: Nikolay Sivov <nsivov at codeweavers.com>
> ---
>  dlls/d3d10/effect.c       | 52 ++++++++++++++++++++++++++++-----------
>  dlls/d3d10/tests/effect.c | 26 +++++++++++++-------
>  2 files changed, 55 insertions(+), 23 deletions(-)
>
> diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c
> index 3f0916e60d5..e261e5a7cd7 100644
> --- a/dlls/d3d10/effect.c
> +++ b/dlls/d3d10/effect.c
> @@ -6999,6 +6999,43 @@ static HRESULT STDMETHODCALLTYPE d3d10_effect_shader_variable_GetPixelShader(
>      return S_OK;
>  }
>
> +static HRESULT d3d10_get_shader_variable_signature(struct d3d10_effect_variable *v,
> +        UINT shader_index, UINT element_index, BOOL output, D3D10_SIGNATURE_PARAMETER_DESC *desc)
> +{
> +    struct d3d10_effect_shader_variable *s;
> +    unsigned int i;
> +
> +    if (v->type->element_count)
> +        v = &v->elements[0];
> +
> +    if (shader_index == 0)
> +    {
> +        s = &v->u.shader;
> +    }
> +    else
> +    {
> +        /* Index is used as an offset from this variable. */
> +
> +        for (i = 0; i < v->effect->used_shader_count; ++i)
> +        {
> +            if (v == v->effect->used_shaders[i]) break;
> +        }
> +
> +        if (i + shader_index >= v->effect->used_shader_count)
> +        {
> +            WARN("This should crash!\n");
> +            return E_FAIL;
> +        }

I know this follows the trend of existing WARN() messages in the DLL
but I'd rather we print a more explicit and less excited message, like
"Shader index out of range" or something along those lines.

> diff --git a/dlls/d3d10/tests/effect.c b/dlls/d3d10/tests/effect.c
> index df0d1a99ef4..1b8e765a8e6 100644
> --- a/dlls/d3d10/tests/effect.c
> +++ b/dlls/d3d10/tests/effect.c
> @@ -3656,25 +3656,23 @@ todo_wine
>      ps = v->lpVtbl->AsShader(v);
>
>      hr = ps->lpVtbl->GetOutputSignatureElementDesc(ps, 0, 0, &sign);
> -todo_wine
>      ok(hr == S_OK, "Unexpected hr %#x.\n", hr);
> -    if (SUCCEEDED(hr))
> -        ok(!strcmp(sign.SemanticName, "SV_Target"), "Unexpected semantic %s.\n", sign.SemanticName);
> +    ok(!strcmp(sign.SemanticName, "SV_Target"), "Unexpected semantic %s.\n", sign.SemanticName);
> +
> +    hr = ps->lpVtbl->GetOutputSignatureElementDesc(ps, 4, 0, &sign);
> +    ok(hr == S_OK, "Unexpected hr %#x.\n", hr);
> +    ok(!strcmp(sign.SemanticName, "SV_POSITION"), "Unexpected semantic %s.\n", sign.SemanticName);
>
>      v = effect->lpVtbl->GetVariableByName(effect, "v");
>      vs = v->lpVtbl->AsShader(v);
>
>      hr = vs->lpVtbl->GetOutputSignatureElementDesc(vs, 0, 0, &sign);
> -todo_wine
>      ok(hr == S_OK, "Unexpected hr %#x.\n", hr);
> -    if (SUCCEEDED(hr))
> -        ok(!strcmp(sign.SemanticName, "SV_POSITION"), "Unexpected semantic %s.\n", sign.SemanticName);
> +    ok(!strcmp(sign.SemanticName, "SV_POSITION"), "Unexpected semantic %s.\n", sign.SemanticName);
>
>      hr = vs->lpVtbl->GetOutputSignatureElementDesc(vs, 1, 0, &sign);
> -todo_wine
>      ok(hr == S_OK, "Unexpected hr %#x.\n", hr);
> -    if (SUCCEEDED(hr))
> -        ok(!strcmp(sign.SemanticName, "SV_POSITION"), "Unexpected semantic %s.\n", sign.SemanticName);
> +    ok(!strcmp(sign.SemanticName, "SV_POSITION"), "Unexpected semantic %s.\n", sign.SemanticName);
>
>      /* NULL shader variable */
>      v = effect->lpVtbl->GetVariableByName(effect, "v0");
> @@ -3683,6 +3681,16 @@ todo_wine
>      hr = vs->lpVtbl->GetOutputSignatureElementDesc(vs, 0, 0, &sign);
>      ok(hr == D3DERR_INVALIDCALL, "Unexpected hr %#x.\n", hr);
>
> +    hr = vs->lpVtbl->GetOutputSignatureElementDesc(vs, 1, 0, &sign);
> +    ok(hr == D3DERR_INVALIDCALL, "Unexpected hr %#x.\n", hr);
> +
> +    hr = vs->lpVtbl->GetOutputSignatureElementDesc(vs, 2, 0, &sign);
> +    ok(hr == D3DERR_INVALIDCALL, "Unexpected hr %#x.\n", hr);
> +
> +    hr = vs->lpVtbl->GetOutputSignatureElementDesc(vs, 3, 0, &sign);
> +    ok(hr == S_OK, "Unexpected hr %#x.\n", hr);
> +    ok(!strcmp(sign.SemanticName, "SV_POSITION"), "Unexpected semantic %s.\n", sign.SemanticName);
> +
>      effect->lpVtbl->Release(effect);
>
>      refcount = ID3D10Device_Release(device);

BTW, that's some amazing behavior by native...