[PATCH 1/2] d3d10/effect: Parse through value expressions.

[Matteo Bruni]

On Fri, Nov 19, 2021 at 2:47 PM Nikolay Sivov <nsivov at codeweavers.com> wrote:
> On 11/19/21 4:41 PM, Matteo Bruni wrote:
> > On Tue, Nov 16, 2021 at 6:15 PM Nikolay Sivov <nsivov at codeweavers.com> wrote:
> >> Signed-off-by: Nikolay Sivov <nsivov at codeweavers.com>
> >> ---
> >>  dlls/d3d10/d3d10_private.h |  1 +
> >>  dlls/d3d10/effect.c        | 35 +++++++++++++++++
> >>  dlls/d3d10/tests/effect.c  | 78 ++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 114 insertions(+)
> > Sorry for the delay.
> >
> >> diff --git a/dlls/d3d10/d3d10_private.h b/dlls/d3d10/d3d10_private.h
> >> index 7f83fb1182a..b35711b55e5 100644
> >> --- a/dlls/d3d10/d3d10_private.h
> >> +++ b/dlls/d3d10/d3d10_private.h
> >> @@ -52,6 +52,7 @@ enum d3d10_effect_object_operation
> >>      D3D10_EOO_CONST_INDEX = 3,
> >>      D3D10_EOO_VAR_INDEX = 4,
> >>      D3D10_EOO_INDEX_EXPRESSION = 5,
> >> +    D3D10_EOO_VALUE_EXPRESSION = 6,
> >>      D3D10_EOO_ANONYMOUS_SHADER = 7,
> >>  };
> >>
> >> diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c
> >> index 3f35e9cc28a..9c2012768dd 100644
> >> --- a/dlls/d3d10/effect.c
> >> +++ b/dlls/d3d10/effect.c
> >> @@ -289,6 +289,10 @@ struct d3d10_effect_prop_dependency
> >>              struct d3d10_effect_variable *v;
> >>              struct d3d10_effect_preshader index;
> >>          } index_expr;
> >> +        struct
> >> +        {
> >> +            struct d3d10_effect_preshader value;
> >> +        } value_expr;
> >>      };
> >>  };
> >>
> >> @@ -392,6 +396,9 @@ static void d3d10_effect_clear_prop_dependencies(struct d3d10_effect_prop_depend
> >>              case D3D10_EOO_INDEX_EXPRESSION:
> >>                  d3d10_effect_preshader_clear(&dep->index_expr.index);
> >>                  break;
> >> +            case D3D10_EOO_VALUE_EXPRESSION:
> >> +                d3d10_effect_preshader_clear(&dep->value_expr.value);
> >> +                break;
> >>          }
> >>      }
> >>      heap_free(d->entries);
> >> @@ -2632,6 +2639,34 @@ static HRESULT parse_fx10_property_assignment(const char *data, size_t data_size
> >>
> >>              return d3d10_effect_add_prop_dependency(d, &dep);
> >>
> >> +        case D3D10_EOO_VALUE_EXPRESSION:
> >> +
> >> +            if (value_offset >= data_size || !require_space(value_offset, 1, sizeof(uint16_t), data_size))
> >> +            {
> >> +                WARN("Invalid offset %#x (data size %#lx).\n", value_offset, (long)data_size);
> >> +                return E_FAIL;
> >> +            }
> > Pretty sure you want sizeof(DWORD) here.
> >
> >> +
> >> +            data_ptr = data + value_offset;
> >> +            read_dword(&data_ptr, &blob_size);
> >> +
> >> +            if (!require_space(value_offset, 1, sizeof(uint16_t) + blob_size, data_size))
> >> +            {
> >> +                WARN("Invalid offset %#x (data size %#lx).\n", value_offset, (long)data_size);
> >> +                return E_FAIL;
> >> +            }
> > Not sure about this sizeof(uint16_t) either, what's the idea?
> >
> > BTW, we probably want this kind of require_space(..., blob_size, ...)
> > in the D3D10_EOO_INDEX_EXPRESSION case too.
> Yes, it should have been uint32_t in both places.
>
> First check is to read 4 bytes at "value_offset" offset, which would
> give blob_size. Additional size in second check is to account for
> blob_size that's been read already. Alternatively this could do
> value_offset += 4.

Ah yes, that seems right. It's okay as it is (with the fixed sizeof()
of course).