Re: [Qemu-devel] [PATCH] target-i386: fix pcmpxstrx equal-ordered (strstr) mode

[Richard Henderson <rth@twiddle.net>]

On 10/12/2015 08:50 PM, Paolo Bonzini wrote:
> In this mode, referring an invalid element of the source forces the
> result to false (table 4-7, last column) but referring an invalid
> element of the destination forces the result to true, so the outer
> loop should still be run even if some elements of the destination
> will be invalid.  They will be culled in the inner loop, which
> correctly bounds "i" to validd.
>
> This fix tst_strstr in glibc 2.17.
>
> Reported-by: Florian Weimer <fweimer@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target-i386/ops_sse.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-i386/ops_sse.h b/target-i386/ops_sse.h
> index 7aa693a..268f3e1 100644
> --- a/target-i386/ops_sse.h
> +++ b/target-i386/ops_sse.h
> @@ -2037,7 +2037,7 @@ static inline unsigned pcmpxstrx(CPUX86State *env, Reg *d, Reg *s,
>           }
>           break;
>       case 3:
> -        for (j = valids - validd; j >= 0; j--) {
> +        for (j = valids; j >= 0; j--) {
>               res <<= 1;
>               v = 1;
>               for (i = MIN(upper - j, validd); i >= 0; i--) {

I don't see how the bounding is properly done.  In particular,

>                 v &= (pcmp_val(s, ctrl, i + j) == pcmp_val(d, ctrl, i));

We're bounding j by valids, but accessing i+j?

I think this would be a lot simpler if we simply followed the pseudocode in 
table 4-3, doing overrideIfDataInvalid after comparison.


r~