ARM : ldrexd and strexd implementation flawed

[Junhee Yoo]

The ldrexd / strexd instructions have a flawed implementation : it never works properly. For example, the most simple code something like:

ldrexd r0, r1, [r2]
strexd r4, r0, r1, [r2]

which should usually have r4 as zero (successfully done the exclusive store). However, the current implementation always returns one (unsuccessfully done the exclusive store).

The current trunk (function gen_load_exclusive, gen_store_exclusive, both from target-arm/translate.c) looks like an incorrect implementation:

static void gen_load_exclusive(DisasContext *s, int rt, int rt2,
                               TCGv addr, int size)
{
    TCGv tmp;

    switch (size) {
    case 0:
        tmp = gen_ld8u(addr, IS_USER(s));
        break;
    case 1:
        tmp = gen_ld16u(addr, IS_USER(s));
        break;
    case 2:
    case 3:
        tmp = gen_ld32(addr, IS_USER(s));
        break;
    default:
        abort();
    }
    tcg_gen_mov_i32(cpu_exclusive_val, tmp);
    store_reg(s, rt, tmp);
    if (size == 3) {
        tcg_gen_addi_i32(addr, addr, 4);
        tmp = gen_ld32(addr, IS_USER(s));
        tcg_gen_mov_i32(cpu_exclusive_high, tmp);
        store_reg(s, rt2, tmp);
    }
    tcg_gen_mov_i32(cpu_exclusive_addr, addr);
}

The problem lies when size is 3 (=ldrexd) : normally, cpu_exclusive_addr should be set as addr, but since the current implementation increments addr by 4 (when size is 3) before cpu_exclusive_addr is updated, it results in a wrong value stored on cpu_exclusive_addr.

Another error on gen_store_exclusive():

...
    if (size == 3) {
        TCGv tmp2 = new_tmp();
        tcg_gen_addi_i32(tmp2, addr, 4);
        tmp = gen_ld32(addr, IS_USER(s));
        dead_tmp(tmp2);
        tcg_gen_brcond_i32(TCG_COND_NE, tmp, cpu_exclusive_high, fail_label);
        dead_tmp(tmp);
    }
....

the current code assigns tmp2 as addr + 4, but loads from addr on the next line, not from tmp2(=addr+4).

[Peter Maydell (pmaydell)]

Coincidentally, I was just looking at this yesterday. There's another error you haven't pointed out, which is in the linux-user implementation of do_strex(), which writes both halves of the 64 bit double to the same address, rather than to addr and addr+4.

[Peter Maydell (pmaydell)]

I sent a patch for this to the list: http://patchwork.ozlabs.org/patch/70149/

[Peter Maydell (pmaydell)]

The patch has now been merged into qemu's master git repo:

http://git.qemu.org/qemu.git/commit/?id=2c9adbda721c9996ec6b371cac4a00c1164b818e

(and so this bug should be fixed in the 0.14 release since that has not yet branched.)