Re: [Lightning] jit_qdivr_u trashes JIT_R0 on x86_64

[Paulo César Pereira de Andrade]

  Hi Paul,

Em qua, 28 de ago de 2019 às 20:41, Paul Cercueil <address@hidden> escreveu:

[...]
> I tried, but I could not succeed to make a test case that shows the
> issue... All I tried have been working just fine.
>
> The actual code that triggers the issue is this one:
> https://gist.github.com/pcercuei/8db789b415f6ced73abb01a6504b64c7
> As it's generated, it's not the easiest to understand, sorry...
>
> The thing to see, is that line 26 I write register 0 (rax), which is
> then untouched until line 58 or 71.
>
> This is what Lightning generates:
> https://gist.github.com/pcercuei/6b4afef692682b139a46449665454681
>
> As you can see, line 30 %rax is written to, and %eax is written back to
> memory on lines 69 or 84.
> But also, line 49 %rax is unconditionally overwritten.

  The problem is that after the code does JIT_R0 = JIT_V1 + JIT_V2, that
is %rax = %r13 + %r14 -- lea 0x0(%r13,%r14,1),%rax -- there is a function
call. %rax is not callee save, you need to save it to memory, and restore
once the function returns. The called function is free to change %rax.
this is normal for all JIT_Rx. Only JIT_Vx registers are not modified.
If you are 100% sure the called function does not change JIT_R0, you could
add jit_live(JIT_R0) after the jit_callr(JIT_V0), but that would not work
because it would normally choose %rax as a function pointer (this is a long
TODO to generate %rip displacement calls), or, you could cause it to choose
another register as the function pointer, by creating a jump over the
function call or some other construct, but that also would not work if
calling a varargs function, because the abi requires adding the number of
FPR register/arguments in %rax.
  Basically, if there is a function call, jit_jmpi to unknown location (not
to a jit_label_t) or a jit_jmpr, all non callee save registers are
considered dead in the next instruction. It does this also for jit_jmpi
(to unknown location) and jit_jmpr because otherwise the register allocator
would run out of registers, as it cannot follow where the jump will land,
and what will happen there.

  The closest/minimal reproducer for the issue does not even need the
qdivr, because once jit_qdivr is called, %rax is already considered dead, is:

"""
.data    32
fmt:
.c    "%d\n"
.code
    jmpi main
func:
    prolog
    //reti 2
    epilog
main:
    prolog
    movi %v1 2
    movi %v2 3
    addr %r0 %v1 %v2
    movi %v(3) func
    callr %v(3)
    //movi %r0 5
    //qdivr %v1 %v2 %v2 %v1
    prepare
        pushargi fmt
        ellipsis
        pushargr %r0
    finishi @printf
    ret
    epilog
"""
  If you uncomment the 'reti 2' it will print 5, otherwise it will print 2.
  BTW, I see you found another way to workaround the issues with %rax
described above by manually using callr. So, even if no code touchs %rax,
it will be considered dead in jit_qdivr and it will not save/restore it.
  If you uncomment only the qdivr above, %rax will be 1, and if you also
uncomment the 'movi %r0 5' (to simulate quickly a reload from memory)
you will notice that the qdvir will not clobber %rax, because then it
understands it is live.

> Cheers,
> -Paul

Thanks,
Paulo