[Qemu-devel] Re: dyngen_code in 16 bit

[Clemens Kolbitsch <ck@iseclab.org>]

On Monday 14 April 2008 12:16:08 you wrote:
> Hi!
> For a research project I extended Qemu to include some extra code inside
> the op_XXX instructions that increased the generated TB-code's size to
> quite some extend...
>
> Now I have a problem when having block chaining enabled (that I don't want
> to disable for performance reasons :-/): The code_gen_buffer sometimes
> contains code areas that span more than 0xffff bytes, however, dyngen and
> all functions related to it use 16 bit pointers, etc. Therefore, e.g. the
> dyngen_code function uses the 16 bit pointers to overwrite certain params
> and of course destroys the TB-code.
>
> When working with x86 hosts and guests (both 32 bit), is there a specific
> reason for all these pointers to be 16 bits or has it just been a safe
> assumption up to now?? I have tried rewriting the code to use 32 bit, but
> keep getting segfaults... however, of course, I might have missed some code
> still.

Hi again!
I have further modified the code replacing 16 bit pointers with 32 bits (e.g. 

cpu_gen_code(CPUState *env, TranslationBlock *tb,
                 int max_code_size, int *gen_code_size_ptr)

to 
cpu_gen_code(CPUState *env, TranslationBlock *tb,
                 unsigned int max_code_size, unsigned int *gen_code_size_ptr)

) (note the signed/unsigned) and the problems seem to be gone.

ATTENTION: dyngen_code returns a signed integer!! Although _normal_ code does 
not generate that big TBs, there is a bug if it does... consider the 
following code from translate-all.c:


gen_code_size = dyngen_code(gen_code_buf, tb->tb_next_offset,
#ifdef USE_DIRECT_JUMP
                                    tb->tb_jmp_offset,
#else
                                    NULL,
#endif
                                    gen_opc_buf, gen_opparam_buf, gen_labels);
    }
    *gen_code_size_ptr = gen_code_size;
    
    if (gen_code_buf + gen_code_size > code_gen_buffer + CODE_GEN_BUFFER_SIZE)
    {
      printf("gen_code_buffer overflow!\n");
      exit(1);
    }

gen_code_size is signed as well --> if we handle large buffers, the return 
value will eventually be negative, causing the security-check below to fail, 
although it shouldn't!!!

Do you think it is worth writing a patch?? (Because a sane configuration of 

MAX_OP_PER_INSTR
and
OPC_BUF_SIZE

can circumvent the problem...). If you do think so, I'll post my patch, as 
soon as I get around making one (I'm using a _very_ modified version right 
now...)

Cheers