[PATCH v2] target/i386: do not zero-extend BSR/BSF dest when source is zero

For the bsr and bsf instructions per the Intel SDM: "If the content
of the source operand is 0, the content of the destination operand
is undefined." The AMD64 Architecture Programmer's Manual is more
specific: it states the destination operand remains unchanged when
the source is zero. Testing on real hardware (multiple Intel and
AMD systems) confirms that when the source operand is zero, the CPU
leaves the entire 64-bit destination register untouched, including
the upper 32 bits, even when executing the 32-bit form of the
instruction (e.g. "bsr edx, ecx") in 64-bit mode.

gen_BSF()/gen_BSR() already encode this intent (see the existing
comment) by arranging for T0 to hold the correct full-width
passthrough value when the source is zero. However, that correct
value was then handed to the generic register writeback path
(gen_writeback), which for a 32-bit destination unconditionally
applies tcg_gen_ext32u_tl() and clears the upper 32 bits regardless
of what gen_BSF()/gen_BSR() had just computed.

Fix this at the operand-decode level instead: give the destination
(G) and its 2op copy the d64 size class instead of v, so gen_writeback
treats the write as full-width in the one case that was buggy (64-bit
mode, 32-bit operand size), leaving every other case unchanged. The
source (E) must stay v, or the decoder would scan the full 64-bit
source register instead of just its low 32 bits. No changes to
gen_BSF()/gen_BSR() are needed.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4132
Signed-off-by: Simon Scherer <[email protected]>
---
 target/i386/tcg/decode-new.c.inc | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/decode-new.c.inc
index ac181308ca..85f9d27c56 100644
--- a/target/i386/tcg/decode-new.c.inc
+++ b/target/i386/tcg/decode-new.c.inc
@@ -659,12 +659,18 @@ static void decode_0FB8(DisasContext *s, CPUX86State *env, X86OpEntry *entry, ui
 
 static void decode_0FBC(DisasContext *s, CPUX86State *env, X86OpEntry *entry, uint8_t *b)
 {
-    /* For BSF, pass 2op as the third operand so that we can use zextT0 */
+    /*
+     * For BSF, pass 2op as the third operand so that we can use zextT0.
+     * G/2op use d64 because ctz already zero-extends the full 64-bit result,
+     * and v would zero-extend the output register if the input is zero.
+     * E stays v: forcing it to d64 too would scan the full 64-bit register
+     * instead of just the 32-bit source.
+     */
     static const X86OpEntry opcodes_0FBC[4] = {
-        X86_OP_ENTRY3(BSF,    G,v, E,v, 2op,v, zextT0),
-        X86_OP_ENTRY3(BSF,    G,v, E,v, 2op,v, zextT0), /* 0x66 */
-        X86_OP_ENTRYwr(TZCNT, G,v, E,v,        zextT0), /* 0xf3 */
-        X86_OP_ENTRY3(BSF,    G,v, E,v, 2op,v, zextT0), /* 0xf2 */
+        X86_OP_ENTRY3(BSF,    G,d64, E,v, 2op,d64, zextT0),
+        X86_OP_ENTRY3(BSF,    G,d64, E,v, 2op,d64, zextT0), /* 0x66 */
+        X86_OP_ENTRYwr(TZCNT, G,v,   E,v,          zextT0), /* 0xf3 */
+        X86_OP_ENTRY3(BSF,    G,d64, E,v, 2op,d64, zextT0), /* 0xf2 */
     };
     if (!(s->cpuid_ext3_features & CPUID_EXT3_ABM)) {
         *entry = opcodes_0FBC[0];
@@ -675,12 +681,18 @@ static void decode_0FBC(DisasContext *s, CPUX86State *env, X86OpEntry *entry, ui
 
 static void decode_0FBD(DisasContext *s, CPUX86State *env, X86OpEntry *entry, uint8_t *b)
 {
-    /* For BSR, pass 2op as the third operand so that we can use zextT0 */
+    /*
+     * For BSR, pass 2op as the third operand so that we can use zextT0.
+     * G/2op use d64 because clz already zero-extends the full 64-bit result,
+     * and v would zero-extend the output register if the input is zero.
+     * E stays v: forcing it to d64 too would scan the full 64-bit register
+     * instead of just the 32-bit source.
+     */
     static const X86OpEntry opcodes_0FBD[4] = {
-        X86_OP_ENTRY3(BSR,    G,v, E,v, 2op,v, zextT0),
-        X86_OP_ENTRY3(BSR,    G,v, E,v, 2op,v, zextT0), /* 0x66 */
-        X86_OP_ENTRYwr(LZCNT, G,v, E,v,        zextT0), /* 0xf3 */
-        X86_OP_ENTRY3(BSR,    G,v, E,v, 2op,v, zextT0), /* 0xf2 */
+        X86_OP_ENTRY3(BSR,    G,d64, E,v, 2op,d64, zextT0),
+        X86_OP_ENTRY3(BSR,    G,d64, E,v, 2op,d64, zextT0), /* 0x66 */
+        X86_OP_ENTRYwr(LZCNT, G,v,   E,v,          zextT0), /* 0xf3 */
+        X86_OP_ENTRY3(BSR,    G,d64, E,v, 2op,d64, zextT0), /* 0xf2 */
     };
     if (!(s->cpuid_7_0_ebx_features & CPUID_7_0_EBX_BMI1)) {
         *entry = opcodes_0FBD[0];
-- 
2.53.0