Re: [Qemu-devel] [RISU RFC PATCH v1 2/7] risugen_x86_asm: add module

2019-06-28 Thread Jan Bobek
On 6/27/19 5:05 AM, Richard Henderson wrote:
> On 6/19/19 7:04 AM, Jan Bobek wrote:
>> +sub rex_encode(%)
>> +{
>> +my (%args) = @_;
>> +
>> +$args{w} = 0 unless defined $args{w};
>> +$args{r} = 0 unless defined $args{w};
>> +$args{x} = 0 unless defined $args{w};
>> +$args{b} = 0 unless defined $args{w};
> 
> What makes you believe that REX.[RXB] are dependent on REX.W?
> Or are these merely cut-and-paste errors?

Oops, that's a typo.

>> +sub modrm_encode(%)
>> +{
>> +my (%args) = @_;
>> +
>> +die "MOD field out-of-range: $args{mod}"
>> +unless 0 <= $args{mod} && $args{mod} <= 3;
>> +die "REG field out-of-range: $args{reg}"
>> +unless 0 <= $args{reg} && $args{reg} <= 7;
>> +die "RM field out-of-range: $args{rm}"
>> +unless 0 <= $args{rm} && $args{rm} <= 7;
>> +
>> +return (value =>
>> +($args{mod} << 6)
>> +| ($args{reg} << 3)
>> +| $args{rm},
>> +len => 1);
>> +}
>> +
>> +sub sib_encode(%)
>> +{
>> +my (%args) = @_;
>> +
>> +die "SS field out-of-range: $args{ss}"
>> +unless 0 <= $args{ss} && $args{ss} <= 3;
>> +die "INDEX field out-of-range: $args{index}"
>> +unless 0 <= $args{index} && $args{index} <= 7;
>> +die "BASE field out-of-range: $args{base}"
>> +unless 0 <= $args{base} && $args{base} <= 7;
>> +
>> +return (value =>
>> +($args{ss} << 6)
>> +| ($args{index} << 3)
>> +| $args{base},
>> +len => 1);
>> +}
> 
> These are interdependent, in that SIB requires MODRM.RM == 4.  But I don't see
> anything that either enforces that or validates it, either here or within
> write_insn below.

It also requires MODRM.MOD != 3. You are right, this is not validated
by the risugen_x86_asm module at all; the caller is supposed to know
what they're doing. I also don't validate if displacement is present
when required; the exact conditions are somewhat complicated, already
present in one form or another in risugen_x86 and it generally seemed
like not worth the effort.

-Jan



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [RISU RFC PATCH v1 2/7] risugen_x86_asm: add module

2019-06-27 Thread Richard Henderson
On 6/19/19 7:04 AM, Jan Bobek wrote:
> +sub rex_encode(%)
> +{
> +my (%args) = @_;
> +
> +$args{w} = 0 unless defined $args{w};
> +$args{r} = 0 unless defined $args{w};
> +$args{x} = 0 unless defined $args{w};
> +$args{b} = 0 unless defined $args{w};

What makes you believe that REX.[RXB] are dependent on REX.W?
Or are these merely cut-and-paste errors?


> +sub modrm_encode(%)
> +{
> +my (%args) = @_;
> +
> +die "MOD field out-of-range: $args{mod}"
> +unless 0 <= $args{mod} && $args{mod} <= 3;
> +die "REG field out-of-range: $args{reg}"
> +unless 0 <= $args{reg} && $args{reg} <= 7;
> +die "RM field out-of-range: $args{rm}"
> +unless 0 <= $args{rm} && $args{rm} <= 7;
> +
> +return (value =>
> +($args{mod} << 6)
> +| ($args{reg} << 3)
> +| $args{rm},
> +len => 1);
> +}
> +
> +sub sib_encode(%)
> +{
> +my (%args) = @_;
> +
> +die "SS field out-of-range: $args{ss}"
> +unless 0 <= $args{ss} && $args{ss} <= 3;
> +die "INDEX field out-of-range: $args{index}"
> +unless 0 <= $args{index} && $args{index} <= 7;
> +die "BASE field out-of-range: $args{base}"
> +unless 0 <= $args{base} && $args{base} <= 7;
> +
> +return (value =>
> +($args{ss} << 6)
> +| ($args{index} << 3)
> +| $args{base},
> +len => 1);
> +}

These are interdependent, in that SIB requires MODRM.RM == 4.  But I don't see
anything that either enforces that or validates it, either here or within
write_insn below.


r~