ARM : ldrexd and strexd implementation flawed
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
QEMU |
Fix Released
|
Undecided
|
Unassigned |
Bug Description
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_
static void gen_load_
{
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_
store_reg(s, rt, tmp);
if (size == 3) {
tmp = gen_ld32(addr, IS_USER(s));
}
tcg_
}
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_
...
if (size == 3) {
TCGv tmp2 = new_tmp();
tmp = gen_ld32(addr, IS_USER(s));
}
....
the current code assigns tmp2 as addr + 4, but loads from addr on the next line, not from tmp2(=addr+4).
Changed in qemu: | |
status: | Fix Committed → Fix Released |
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.