Re: [Qemu-devel] [RISU RFC PATCH v1 4/7] risugen_x86: add module

[Jan Bobek]

On 6/27/19 6:29 AM, Richard Henderson wrote:
> On 6/19/19 7:04 AM, Jan Bobek wrote:
>> +sub write_mov_reg_imm($$)
>> +{
>> +    my ($reg, $imm) = @_;
>> +
>> +    my %insn = (opcode => {value => 0xB8 | ($reg & 0x7), len => 1},
>> +                imm => {value => $imm, len => $is_x86_64 ? 8 : 4});
>> +
>> +    $insn{rex}{w} = 1 if $is_x86_64;
>> +    $insn{rex}{b} = 1 if $reg >= 8;
>> +
>> +    write_insn(%insn);
>> +}
> 
> There are 3 different insns that x86_64 can use for different ranges of
> immediates; you are always using the full 10 byte version.
> 
> Using 0xb8 without REX.W may be used for any unsigned 32-bit value.
> Using 0xc7 with REX.W may be used for any signed 32-bit value.
> Using 0xb8 with REX.W of course allows any 64-bit value.
> 
> It's not terribly important, but the code size does get large.

Funnily enough, the very first version of this function did try to
save bytes by using different instructions, but when I started writing
write_mem_getoffset, I ran into trouble with signedness. In response,
I hard-coded the 10 byte version for debugging and then never switched
it back.

I'll try to do something about this in v2.

>> +sub rand_insn_modrm($$)
>> +{
>> +    my ($opts, $insn) = @_;
>> +    my $modrm;
>> +
>> +    while (1) {
>> +        $modrm = rand_fill({mod => {bitlen => 2},
>> +                            reg => {bitlen => 3},
>> +                            rm => {bitlen => 3}},
>> +                           $opts);
>> +
>> +        if ($modrm->{mod} != MOD_DIRECT) {
>> +            # Displacement only; we cannot use this since we
>> +            # don't know absolute address of the memblock.
>> +            next if $modrm->{mod} == MOD_INDIRECT && $modrm->{rm} == 
>> REG_EBP;
> ...
>> +sub rand_insn_rex($$)
>> +{
>> +    my ($opts, $insn) = @_;
>> +
>> +    $opts->{w} = 0 unless defined $opts->{w};
>> +    $opts->{x} = 0 unless defined $opts->{x} || defined $insn->{sib};
>> +
>> +    my $rex = rand_fill({w => {bitlen => 1},
>> +                         r => {bitlen => 1},
>> +                         b => {bitlen => 1},
>> +                         x => {bitlen => 1}},
>> +                        $opts);
> 
> I don't think it's a good idea to generate reg/rm/sib.index separate from
> rex.r/b/x.  In particular, all of your tests vs EBP and ESP are not quite
> right, since the final insn may be referencing R12 or R13.

That's true. (Although not in all cases; see Table 2-5 in the Intel Manual,
Volume 2, Chapter 2, Section 2.2.1 "REX Prefixes" for some cases when REX.B
is not decoded.) This is a compromise that I've accepted, at least for v1
of the patch series. Note that this problem is also present in config entries
such as

PMOVMSKB        SSE     00001111 11010111 !emit { modrm(mod => MOD_DIRECT, reg 
=> ~REG_ESP); }

Here, we force MODRM.REG != 4, but this avoids not only ESP/RSP, but
also R12.

Hmmm... I suppose I have some ideas on how to do it better. I'll try
to fix this, though I suspect getting it 100 % right might be
difficult and time-consuming.

> What is your plan for handling the unary insns for which modrm.r is opcode and
> not a register?  This doesn't seem to allow for those.  How about insns for
> which modrm.mod must be 3 (register) or must be != 3 (memory/address)?

Both of these translate to forcing or avoiding a certain value in a field,
which is handled by the $opts argument to rand_fill; I mention this in my
email about the first patch in this series.

> Is this
> simply going to fall into "testing of illegal encodings"?

Speaking of testing of illegal encodings, it occurred to me that it
might be quite hard: since x86 uses variable-length encoding, you
cannot tell how many bytes should be skipped if an invalid instruction
is encountered.

Perhaps we could use a multi-byte NOP with the length embedded in some
of the ignored fields? We could then teach RISU to look at the
preceding instruction and skip the correct number of bytes when
appropriate. Do you think it would be worth the effort? RISU currently
doesn't seem to support this use-case, but it should be
straightforward to implement: just add a record to the trace file
that the instruction generates a SIGILL.

-Jan

signature.asc
Description: OpenPGP digital signature