2017-03-23 16:06 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>:
> Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
> ---
> dlls/d3dx9_36/preshader.c | 103 ++++++++++++++++++++++++++++++++++++-------
> dlls/d3dx9_36/tests/effect.c | 38 ++++++++--------
> 2 files changed, 106 insertions(+), 35 deletions(-)
>
> diff --git a/dlls/d3dx9_36/preshader.c b/dlls/d3dx9_36/preshader.c
> index 9aedc99..2974ffd 100644
> --- a/dlls/d3dx9_36/preshader.c
> +++ b/dlls/d3dx9_36/preshader.c
> @@ -191,6 +191,8 @@ struct d3dx_pres_operand
> /* offset is component index, not register index, e. g.
> offset for component c3.y is 13 (3 * 4 + 1) */
> unsigned int offset;
> + enum pres_reg_tables base_reg_table;
> + unsigned int base_reg_offset;
> };
It would be nicer to create a struct for storing together table and
offset and to pass it around to the various functions taking both
arguments, like parse_pres_reg().
It should probably be a separate patch before this one.
> + ptr = parse_pres_reg(ptr + 1, &opr->base_reg_table, &opr->base_reg_offset);
> + }
> + else
> + {
> + opr->base_reg_table = PRES_REGTAB_COUNT;
> + ++ptr;
> }
> - opr->table = reg_table[*ptr++];
> - opr->offset = *ptr++;
> +
> + ptr = parse_pres_reg(ptr, &opr->table, &opr->offset);
This would crash if the previous parse_pres_reg() call returned NULL.
> + size = rs->table_sizes[table] * table_info[table].reg_component_count;
> + if (offset >= size)
> + {
> + if ((offset & 255) < size)
> + offset &= 255;
> + else
> + offset = offset % size;
> + }
Is this just to try and avoid the modulo operation in some cases or is
there a deeper reason?
If the former, you could extend that to all the power-of-two sizes by
first checking if that's the case (!(size & size - 1)) and, if so,
and-ing offset with the mask (size - 1). That's assuming the compiler
doesn't do the same optimization already.
Let me know if there is some other reason instead.
> static double exec_get_arg(struct d3dx_regstore *rs, const struct d3dx_pres_operand *opr, unsigned int comp)
> {
> - if (!regstore_is_val_set_reg(rs, opr->table, (opr->offset + comp) / table_info[opr->table].reg_component_count))
> - WARN("Using uninitialized input, table %u, offset %u.\n", opr->table, opr->offset + comp);
> + unsigned int base_offset, index;
> +
> + if (opr->base_reg_table == PRES_REGTAB_COUNT)
> + index = 0;
> + else
> + index = (int)exec_get_reg_value(rs, opr->base_reg_table, opr->base_reg_offset);
I think it's better with lrint() in place of the cast.
> + if (index >= rs->table_sizes[opr->table] && opr->table == PRES_REGTAB_CONST)
> + {
> + unsigned int size_pow2;
> +
> + for (size_pow2 = 1; size_pow2 < rs->table_sizes[opr->table]; size_pow2 <<= 1)
> + ;
> + index &= (size_pow2 - 1);
> + if (index >= rs->table_sizes[opr->table])
> + return 0.0;
> + }
I don't think we necessarily have to follow native's madness on buffer
overflows to the letter. Since you did, that's certainly fine though.