[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Libjit] Miscompilation problem
From: |
Aleksey Demakov |
Subject: |
Re: [Libjit] Miscompilation problem |
Date: |
Thu, 22 Mar 2018 02:10:05 +0300 |
Hi David,
I finally applied my version of the fix. Your test seems to work now.
If you still use libjit please test it too.
Regards,
Aleksey
On Thu, Jan 18, 2018 at 2:21 AM, Aleksey Demakov <address@hidden> wrote:
> 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.
>>
>>
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Libjit] Miscompilation problem,
Aleksey Demakov <=