Re: [Tarantool-patches] [PATCH luajit 01/19] MIPS: Use precise search for exit jump patching.

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Sergey

Thanks for the patch! LGTM

On 8/9/23 18:35, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Contributed by Djordje Kovacevic and Stefan Pejic.
>
> (cherry-picked from commit 7381b620358c2561e8690149f1d25828fdad6675)
>
> Without the aforementioned checks, some non-branch instructions may be
> interpreted as some branch due to memory address collisions. This patch
> adds the corresponding comparisons masked values with instruction
> opcodes used in the LuaJIT:
> * `MIPSI_BEQ` for `beq` and `bne`,
> * `MIPSI_BLTZ` for `bltz`, `blez`, `bgtz` and `bgez`,
> * `MIPSI_BC1F` for `bc1f` and `bc1t`,
> see <src/lj_target_mips.h> and MIPS Instruction Set Manual [1] for
> details.
>
> To reproduce this failure, we need specific memory mapping, so testcase
> is omitted.
>
> Since MIPS architecture is not supported by Tarantool (at the moment)
> this patch is not necessary for backport. OTOH, it gives to us the
> following benefits:
> * Be in sync with the LuaJIT upstream not only for x86_64, arm64
>    architectures.
> * Avoid conflicts during the future backporting.
> So, it's more useful to backport some of the patches to avoid conflicts
> with the future patch series.
>
> [1]: https://s3-eu-west-1.amazonaws.com/downloads-mips/documents/MD00086-2B-MIPS32BIS-AFP-6.06.pdf
>
> Sergey Kaplun:
> * added the description for the problem
>
> Part of tarantool/tarantool#8825
> ---
>   src/lj_asm_mips.h | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/lj_asm_mips.h b/src/lj_asm_mips.h
> index 03417013..03215821 100644
> --- a/src/lj_asm_mips.h
> +++ b/src/lj_asm_mips.h
> @@ -2472,7 +2472,11 @@ void lj_asm_patchexit(jit_State *J, GCtrace *T, ExitNo exitno, MCode *target)
>     MCode tjump = MIPSI_J|(((uintptr_t)target>>2)&0x03ffffffu);
>     for (p++; p < pe; p++) {
>       if (*p == exitload) {  /* Look for load of exit number. */
> -      if (((p[-1] ^ (px-p)) & 0xffffu) == 0) {  /* Look for exitstub branch. */
> +      /* Look for exitstub branch. Yes, this covers all used branch variants. */
> +      if (((p[-1] ^ (px-p)) & 0xffffu) == 0 &&
> +	  ((p[-1] & 0xf0000000u) == MIPSI_BEQ ||
> +	   (p[-1] & 0xfc1e0000u) == MIPSI_BLTZ ||
> +	   (p[-1] & 0xffe00000u) == MIPSI_BC1F)) {
>   	ptrdiff_t delta = target - p;
>   	if (((delta + 0x8000) >> 16) == 0) {  /* Patch in-range branch. */
>   	patchbranch: