Re: [Qemu-devel] [PATCH] tcg/aarch64: Fix tcg_out_qemu_{ld, st} for linux-user

["Andreas Färber" <afaerber@suse.de>]

Am 27.08.2015 um 22:30 schrieb Richard Henderson:
> On 08/27/2015 04:47 PM, Andreas Färber wrote:
>> The argument order for the !CONFIG_SOFTMMU case was jumbled up since
>> ffc6372851d8631a9f9fa56ec613b3244dc635b9 ("tcg/aarch64: use 32-bit
>> offset for 32-bit user-mode emulation"), regressing from -rc2 to v2.4.0.
>> Fix their order to avoid segfaults, e.g., in openSUSE's GNU coreutils
>> 8.24.
> 
> Nack.  The argument order is correct, that is...
> 
>> -    tcg_out_qemu_ld_direct(s, memop, ext, data_reg,
>> -                           guest_base ? TCG_REG_GUEST_BASE :
>> TCG_REG_XZR,
>> -                           otype, addr_reg);
>> +    tcg_out_qemu_ld_direct(s, memop, ext, data_reg, addr_reg, otype,
>> +                           guest_base ? TCG_REG_GUEST_BASE :
>> TCG_REG_XZR);
> 
> TCG_REG_GUEST_BASE is definitely the "base" register, holding a 64-bit
> host address, while addr_reg is the "offset" register, holding a
> (potentially) 32-bit guest address.  It is (supposed to be) the "offset"
> register to which the zero-extend is applied.

Huh? Please see
http://git.qemu-project.org/?p=qemu.git;a=blobdiff;f=tcg/aarch64/tcg-target.c;h=4aca883b8e78b2cae6ec22c19a74f0fa9bc22bfe;hp=d1c550886a461581f93c34d5beead2b3ddfebde5;hb=ffc6372851d8631a9f9fa56ec613b3244dc635b9;hpb=6c0f0c0f124718650a8d682ba275044fc02f6fe2

@@ -1210,12 +1210,15 @@ static void tcg_out_qemu_ld(TCGContext *s,
TCGReg data_reg, TCGReg addr_reg,
tcg_insn_unit *label_ptr;

tcg_out_tlb_read(s, addr_reg, s_bits, &label_ptr, mem_index, 1);
- tcg_out_qemu_ld_direct(s, memop, ext, data_reg, addr_reg, TCG_REG_X1);
+ tcg_out_qemu_ld_direct(s, memop, ext, data_reg, addr_reg,
+ TCG_TYPE_I64, TCG_REG_X1);
add_qemu_ldst_label(s, true, oi, ext, data_reg, addr_reg,
s->code_ptr, label_ptr);
#else /* !CONFIG_SOFTMMU */
- tcg_out_qemu_ld_direct(s, memop, ext, data_reg, addr_reg,
- GUEST_BASE ? TCG_REG_GUEST_BASE : TCG_REG_XZR);
+ const TCGType otype = TARGET_LONG_BITS == 64 ? TCG_TYPE_I64 :
TCG_TYPE_I32;
+ tcg_out_qemu_ld_direct(s, memop, ext, data_reg,
+ GUEST_BASE ? TCG_REG_GUEST_BASE : TCG_REG_XZR,
+ otype, addr_reg);
#endif /* CONFIG_SOFTMMU */
}

@@ -1229,12 +1232,15 @@ static void tcg_out_qemu_st(TCGContext *s,
TCGReg data_reg, TCGReg addr_reg,
tcg_insn_unit *label_ptr;

tcg_out_tlb_read(s, addr_reg, s_bits, &label_ptr, mem_index, 0);
- tcg_out_qemu_st_direct(s, memop, data_reg, addr_reg, TCG_REG_X1);
+ tcg_out_qemu_st_direct(s, memop, data_reg, addr_reg,
+ TCG_TYPE_I64, TCG_REG_X1);
add_qemu_ldst_label(s, false, oi, s_bits == MO_64, data_reg, addr_reg,
s->code_ptr, label_ptr);
#else /* !CONFIG_SOFTMMU */
- tcg_out_qemu_st_direct(s, memop, data_reg, addr_reg,
- GUEST_BASE ? TCG_REG_GUEST_BASE : TCG_REG_XZR);
+ const TCGType otype = TARGET_LONG_BITS == 64 ? TCG_TYPE_I64 :
TCG_TYPE_I32;
+ tcg_out_qemu_st_direct(s, memop, data_reg,
+ GUEST_BASE ? TCG_REG_GUEST_BASE : TCG_REG_XZR,
+ otype, addr_reg);
#endif /* CONFIG_SOFTMMU */
}


An otype argument is being inserted as next-to-last argument for the
function definitions. Same for the softmmu callsites. Only in the *-user
callsites the argument order is being changed with addr_reg and off_r
switching order? I don't see why that should be done in this patch. If
it was wrong before, it should've been done in a separate patch.

> If something's wrong, and I'm not currently in a position to verify one
> way or another, it's in tcg_out_insn_3310.

git-bisect pointed to the above commit. You are referring to its
predecessor
http://git.qemu-project.org/?p=qemu.git;a=commit;h=6c0f0c0f124718650a8d682ba275044fc02f6fe2
? Don't spot anything obviously wrong there immediately...

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)