target/i386: apply mod to immediate count of an RCL/RCR operation
RCR and RCL instructions with a count of 9 are the same as if the count was 0, but they generated incorrect code because the can_be_zero flag is false. This causes 0 to underflow into -1 at tcg_gen_subi_tl(count, count, 1). Fix by absorbing the modulo computation into gen_shift_count(), now renamed gen_shift_count_1(), so that it can handle both reductions. Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3452 Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo Bonzini committed
May 28, 2026 at 17:02 UTC
e38d0afade7c134cd4d675f54d26c394cc3cc31f
1 file changed
+35
-29
target/i386/tcg/emit.c.inc
+35
-29
@@ -3244,8 +3244,9 @@ static void gen_PUSHF(DisasContext *s, X86DecodedInsn *decode)
3244
assume_cc_op(s, CC_OP_EFLAGS);
3245
}
3246
3247
-static MemOp gen_shift_count(DisasContext *s, X86DecodedInsn *decode,
3248
- bool *can_be_zero, TCGv *count, int unit)
3247
+static MemOp gen_shift_count_1(DisasContext *s, X86DecodedInsn *decode,
3248
+ bool *can_be_zero, TCGv *count, int unit,
3249
+ int mod)
3250
{
3251
MemOp ot = decode->op[0].ot;
3252
int mask = (ot <= MO_32 ? 0x1f : 0x3f);
@@ -3255,16 +3256,31 @@ static MemOp gen_shift_count(DisasContext *s, X86DecodedInsn *decode,
3256
case X86_OP_INT:
3257
*count = tcg_temp_new();
3258
tcg_gen_andi_tl(*count, cpu_regs[R_ECX], mask);
3259
+
3260
+ if (mod < mask) {
3261
+ TCGv temp = tcg_temp_new();
3262
+ assert(mod * 4 >= mask);
3263
+ if (mod * 2 < mask) {
3264
+ tcg_gen_subi_tl(temp, *count, mod * 2);
3265
+ tcg_gen_movcond_tl(TCG_COND_GE, *count, temp, tcg_constant_tl(0), temp, *count);
3266
+ }
3267
+ tcg_gen_subi_tl(temp, *count, mod);
3268
+ tcg_gen_movcond_tl(TCG_COND_GE, *count, temp, tcg_constant_tl(0), temp, *count);
3269
+ }
3270
*can_be_zero = true;
3271
break;
3272
3273
case X86_OP_IMM:
3262
- if ((decode->immediate & mask) == 0) {
3274
+ decode->immediate &= mask;
3275
+ if (mod < mask) {
3276
+ decode->immediate %= mod;
3277
+ }
3278
+ if (decode->immediate == 0) {
3279
*count = NULL;
3280
break;
3281
}
3282
*count = tcg_temp_new();
3267
- tcg_gen_movi_tl(*count, decode->immediate & mask);
3283
+ tcg_gen_movi_tl(*count, decode->immediate);
3284
break;
3285
3286
case X86_OP_SKIP:
@@ -3279,6 +3295,13 @@ static MemOp gen_shift_count(DisasContext *s, X86DecodedInsn *decode,
3295
return ot;
3296
}
3297
3298
+static MemOp gen_shift_count(DisasContext *s, X86DecodedInsn *decode,
3299
+ bool *can_be_zero, TCGv *count, int unit)
3300
+{
3301
+ return gen_shift_count_1(s, decode, can_be_zero, count, unit,
3302
+ INT_MAX);
3303
+}
3304
+
3305
/*
3306
* Compute existing flags in decode->cc_src, for gen_* functions that wants
3307
* to set the cc_op set to CC_OP_ADCOX. In particular, this allows rotate
@@ -3397,29 +3420,14 @@ static void gen_rot_overflow(X86DecodedInsn *decode, TCGv result, TCGv old,
3420
/*
3421
* RCx operations are invariant modulo 8*operand_size+1. For 8 and 16-bit operands,
3422
* this is less than 0x1f (the mask applied by gen_shift_count) so reduce further.
3423
+ * FIXME: are flags updated if the count is nonzero, but a multiple of (8 << op) + 1?
3424
*/
3401
-static void gen_rotc_mod(MemOp ot, TCGv count)
3425
+static MemOp gen_rotc_count(DisasContext *s, X86DecodedInsn *decode,
3426
+ bool *can_be_zero, TCGv *count, int unit)
3427
{
3403
- TCGv temp;
3404
-
3405
- switch (ot) {
3406
- case MO_8:
3407
- temp = tcg_temp_new();
3408
- tcg_gen_subi_tl(temp, count, 18);
3409
- tcg_gen_movcond_tl(TCG_COND_GE, count, temp, tcg_constant_tl(0), temp, count);
3410
- tcg_gen_subi_tl(temp, count, 9);
3411
- tcg_gen_movcond_tl(TCG_COND_GE, count, temp, tcg_constant_tl(0), temp, count);
3412
- break;
3413
-
3414
- case MO_16:
3415
- temp = tcg_temp_new();
3416
- tcg_gen_subi_tl(temp, count, 17);
3417
- tcg_gen_movcond_tl(TCG_COND_GE, count, temp, tcg_constant_tl(0), temp, count);
3418
- break;
3419
-
3420
- default:
3421
- break;
3422
- }
3428
+ MemOp ot = decode->op[0].ot;
3429
+ return gen_shift_count_1(s, decode, can_be_zero, count, unit,
3430
+ (8 << ot) + 1);
3431
}
3432
3433
/*
@@ -3440,7 +3448,7 @@ static void gen_RCL(DisasContext *s, X86DecodedInsn *decode)
3448
bool have_1bit_cin, can_be_zero;
3449
TCGv count;
3450
TCGLabel *zero_label = NULL;
3443
- MemOp ot = gen_shift_count(s, decode, &can_be_zero, &count, decode->op[2].unit);
3451
+ MemOp ot = gen_rotc_count(s, decode, &can_be_zero, &count, decode->op[2].unit);
3452
TCGv low, high, low_count;
3453
3454
if (!count) {
@@ -3451,7 +3459,6 @@ static void gen_RCL(DisasContext *s, X86DecodedInsn *decode)
3459
high = tcg_temp_new();
3460
low_count = tcg_temp_new();
3461
3454
- gen_rotc_mod(ot, count);
3462
have_1bit_cin = gen_eflags_adcox(s, decode, true, can_be_zero);
3463
if (can_be_zero) {
3464
zero_label = gen_new_label();
@@ -3492,7 +3499,7 @@ static void gen_RCR(DisasContext *s, X86DecodedInsn *decode)
3499
bool have_1bit_cin, can_be_zero;
3500
TCGv count;
3501
TCGLabel *zero_label = NULL;
3495
- MemOp ot = gen_shift_count(s, decode, &can_be_zero, &count, decode->op[2].unit);
3502
+ MemOp ot = gen_rotc_count(s, decode, &can_be_zero, &count, decode->op[2].unit);
3503
TCGv low, high, high_count;
3504
3505
if (!count) {
@@ -3503,7 +3510,6 @@ static void gen_RCR(DisasContext *s, X86DecodedInsn *decode)
3510
high = tcg_temp_new();
3511
high_count = tcg_temp_new();
3512
3506
- gen_rotc_mod(ot, count);
3513
have_1bit_cin = gen_eflags_adcox(s, decode, true, can_be_zero);
3514
if (can_be_zero) {
3515
zero_label = gen_new_label();