Re: [Libjit] Miscompilation problem

[Aleksey Demakov]

Hi David,

Unfortunately not yet. Your patch is probably correct but perhaps
there is a somewhat simpler fix. Something like replacing in
reset_value() this line:

value->in_global_register = 0;

with

value->in_global_register = value->has_global_register;

I'm not sure this will work either, didn't check it yet.

Regards,
Aleksey

On Wed, Jan 17, 2018 at 10:39 PM, David Meyer <address@hidden> wrote:
> Hi Aleksey,
>
> Any progress on this? I am using the following patch to fix the bug for my 
> own purposes, although I’m not sure if this solution is ideal:
>
> diff --git a/jit/jit-compile.c b/jit/jit-compile.c
> index ba310e4..7b34514 100644
> --- a/jit/jit-compile.c
> +++ b/jit/jit-compile.c
> @@ -308,6 +308,7 @@ reset_value(jit_value_t value)
>         value->reg = -1;
>         value->in_register = 0;
>         value->in_global_register = 0;
> +       value->has_global_register = 0;
>         value->in_frame = 0;
>  }
>
> @@ -743,6 +744,9 @@ compile(_jit_compile_t *state, jit_function_t func)
>                 /* Clean up the compilation state */
>                 cleanup_on_restart(&state->gen, state->func);
>
> +               /* Prepare data needed for code generation */
> +               codegen_prepare(state);
> +
>                 /* Allocate more space */
>                 memory_realloc(state);
>         }
>
> - David
>
> On 1/9/18, 10:18 PM, "Aleksey Demakov" <address@hidden> wrote:
>
>     Thanks for the report, I will look into this on the weekend.
>
>     On Mon, Jan 8, 2018 at 4:39 PM, David Meyer <address@hidden> wrote:
>     > I’ve been encountering this sporadically for several weeks. I finally 
> have
>     > an isolated example (attached, minimal.c). It will attempt to reproduce 
> the
>     > miscompilation. There is some trial and error involved, since it 
> requires
>     > triggering an out-of-memory compile restart at just the right time. 
> When it
>     > detects a miscompile, it dumps the corresponding object code to
>     > /tmp/minimal.dump. Here is what it looks like when it miscompiles:
>     >
>     >     7ffff7fb402f:       55                      push   %rbp
>     >
>     >     7ffff7fb4030:       48 8b ec                mov    %rsp,%rbp
>     >
>     >     7ffff7fb4033:       48 83 ec 30             sub    $0x30,%rsp
>     >
>     >     7ffff7fb4037:       4c 89 34 24             mov    %r14,(%rsp)
>     >
>     >     7ffff7fb403b:       4c 89 7c 24 08          mov    %r15,0x8(%rsp)
>     >
>     >     7ffff7fb4040:       4c 8b f7                mov    %rdi,%r14
>     >
>     >     7ffff7fb4043:       89 75 f8                mov    %esi,-0x8(%rbp)
>     >
>     >     7ffff7fb4046:       89 55 f0                mov    %edx,-0x10(%rbp)
>     >
>     >     7ffff7fb4049:       e9 78 78 00 00          jmpq   0x7ffff7fbb8c6
>     >
>     >     7ffff7fb404e:       44 8b 7d e8             mov    -0x18(%rbp),%r15d
>     > <<<<<<<<<<
>     >
>     >     7ffff7fb4052:       41 83 c7 00             add    $0x0,%r15d
>     >
>     >     7ffff7fb4056:       e9 7b 78 00 00          jmpq   0x7ffff7fbb8d6
>     >
>     >
>     >                ... large dummy jumptable section...
>     >
>     >
>     >     7ffff7fbb8c6:       45 8b fe                mov    %r14d,%r15d
>     >
>     >     7ffff7fbb8c9:       44 03 7d f8             add    -0x8(%rbp),%r15d
>     >
>     >     7ffff7fbb8cd:       44 03 7d f0             add    -0x10(%rbp),%r15d
>     >
>     >     7ffff7fbb8d1:       e9 78 87 ff ff          jmpq   0x7ffff7fb404e
>     >
>     >     7ffff7fbb8d6:       41 83 fe 64             cmp    $0x64,%r14d
>     >
>     >     7ffff7fbb8da:       0f 84 7b 87 ff ff       je     0x7ffff7fb405b
>     >
>     >     7ffff7fbb8e0:       41 8b c7                mov    %r15d,%eax
>     >
>     >     7ffff7fbb8e3:       4c 8b 34 24             mov    (%rsp),%r14
>     >
>     >     7ffff7fbb8e7:       4c 8b 7c 24 08          mov    0x8(%rsp),%r15
>     >
>     >     7ffff7fbb8ec:       48 8b e5                mov    %rbp,%rsp
>     >
>     >     7ffff7fbb8ef:       5d                      pop    %rbp
>     >
>     >     7ffff7fbb8f0:       c3                      retq
>     >
>     >
>     >
>     >
>     > The code at the top of the function is confused about where the value 
> “v”
>     > (defined on line 44 of minimal.c) lives. The code at the bottom assumes 
> that
>     > the value lives in register %r15, but the code at the top pulls it from 
> the
>     > stack at -0x18(%rbp), even through it is never written there.
>     >
>     > I have only a vague idea of what might be happening. The value “v” is 
> bound
>     > to a global register by codegen_prepare() (which runs on the first 
> compile,
>     > but not after restarts). The memory exception triggers reset_value() on 
> “v”
>     > (from cleanup_on_restart), which clears v->in_global_register and
>     > v->in_register, leaving “v” in a slightly different state the second 
> time
>     > codegen starts. So, when the second compilation hits the add 
> instruction at
>     > the very top (the first instruction that uses v in the block/instruction
>     > iteration order), it assumes it needs to load it from the stack.
>     >
>     >
>     > I’m not sure what the correct fix is here, but it seems like the value 
> state
>     > should be consistent across compile restarts.
>
>