Re: [Qemu-devel] [RISU PATCH 0/5] Fix RISU build for i386

[Richard Henderson]

On 4/8/19 8:27 AM, Jan Bobek wrote:
> 1. Most of it is just moving stuff around, however I've implemented
>    reginfo_dump_mismatch (based on reginfo_dump and code in other
>    architectures) and defined EAX as the param register. There is no
>    support for more registers yet, that will need to be added later.

It's probably worthwhile to get x86_64 working at the same time.
This isn't too difficult -- see below.

> 2. Note the '-std=c99' switch in the command-line above; without it,
>    GCC defines the symbol 'i386' to 1 and the preprocessor magic for
>    including arch-specific headers in risu.h breaks. Does anyone have
>    an idea how to fix this in a more robust way?

Adding -U$(ARCH) to the command line is probably as good a fix as any.

> 3. gas (the GNU assembler) chokes on the syntax of test_i386.s; that's
>    why I'm using nasm as the assembler above. Is that intentional? I
>    haven't found the nasm dependency mentioned anywhere.

I think rewriting to not require nasm is better.

I'm not sure why it was done this way.  Perhaps Peter's unfamiliarity with x86
assembler, combined with the fact that the Intel manual is not in AT&T syntax?

In any case...

>    Also, nasm will happily emit the UD1 opcode (0F B9) with no
>    operands (see test_i386.s). That's a bit surprising to me, since
>    Intel's Software Developer's Manual says UD1 has two operands; I'd
>    expect at least a follow-up ModR/M byte. gas refuses to assemble
>    UD1 with no operands, and gdb's disassembler gets confused when I
>    load up the nasm's binary into risu. Is there something obvious
>    that I'm missing?

You are not missing anything -- ud1 should require a modrm byte.

My suggestion is to use only UD1 as the "break" insn, with the different OP_*
codes encoded into the modrm byte.  E.g.

void advance_pc(void *vuc)
{
    ucontext_t *uc = (ucontext_t *) vuc;

    /*
     * We assume that this is UD1 as per get_risuop below.
     * This would need tweaking if we want to test expected undefs.
     */
    uc->uc_mcontext.gregs[REG_E(IP)] += 3;
}

int get_risuop(struct reginfo *ri)
{
    if ((ri->faulting_insn & 0xf8ffff) == 0xc0b90f) { /* UD1 %xxx,%eax */
        return (ri->faulting_insn >> 16) & 7;
    }
    return -1;
}

This leads to:

  I    ENUM            INSN
  0    OP_COMPARE      ud1 %eax,%eax
  1    OP_TESTEND      ud1 %ecx,%eax
  2    OP_SETMEMBLOCK  ud1 %edx,%eax
  3    OP_GETMEMBLOCK  ud1 %ebx,%eax
  4    OP_COMPAREMEM   ud1 %esp,%eax


> P.S. This is my first time using git send-email, so please bear with
>      me if something goes wrong and/or let me know how I can improve
>      my future submissions. Thank you!

You've done well with git send-email.  ;-)

The following set of changes Works For Me for i386 and x86_64.  For clarity(?),
I've included diff from master and diff on top of your patch set.


r~

z-diff-jb
Description: Text document

z-diff-master
Description: Text document