[Qemu-devel] another tcg bug? test r,r case in tcg_out_brcond2()

[Juergen Lock]

Hi!

 While trying to find out why a FreeBSD/amd64 guest on an i386 host
fails to mount its /usr from ad0s1d on a 5G images passed as -hda
partitioned like this:

zsh saturn# fdisk -s ad0
/dev/ad0: 652 cyl 255 hd 63 sec
Part        Start        Size Type Flags
   1:          63    10485153 0xa5 0x80
zsh saturn# bsdlabel ad0s1
# /dev/ad0s1:
8 partitions:
#        size   offset    fstype   [fsize bsize bps/cpg]
  a:  1048576  1048576    4.2BSD     2048 16384     8 
  b:  1048576        0      swap                    
  c: 10485153        0    unused        0     0         # "raw" part, don't edit
  d:  8388001  2097152    4.2BSD     2048 16384 28528

 (the sizes and offsets are in 512 byte blocks, so its a 512 MB swap on
ad0s1b, followed by a 512 MB / on ad0s1a and a 4 GB /usr on ad0s1d),
I came across the following translation of a 
        if (bp->bio_offset < 0)
line that checks the disk offset in g_io_check() in:
        http://svn.freebsd.org/viewvc/base/head/sys/geom/geom_io.c

IN: 
0xffffffff8044b2d0:  test   %r8,%r8
0xffffffff8044b2d3:  js     0xffffffff8044b2db

OUT: [size=135]
0x2945c960:  mov    0x40(%ebp),%eax
0x2945c963:  mov    0x44(%ebp),%edx
0x2945c966:  mov    0x40(%ebp),%ecx
0x2945c969:  mov    0x44(%ebp),%ebx
0x2945c96c:  and    %ecx,%eax
0x2945c96e:  and    %ebx,%edx
0x2945c970:  mov    $0x19,%ecx
0x2945c975:  mov    %ecx,0xa0(%ebp)
0x2945c97b:  mov    %eax,0x98(%ebp)
0x2945c981:  mov    %edx,0x9c(%ebp)
0x2945c987:  test   %edx,%edx
0x2945c989:  jl     0x2945c9c2
0x2945c98f:  jne    0x2945c99d          ; ?????
0x2945c995:  test   %eax,%eax           ; ?????
0x2945c997:  jl     0x2945c9c2          ; ?????
0x2945c99d:  jmp    0x2945c9a2
0x2945c9a2:  mov    $0x8044b2d5,%eax
0x2945c9a7:  mov    %eax,0x80(%ebp)
0x2945c9ad:  mov    $0xffffffff,%eax
0x2945c9b2:  mov    %eax,0x84(%ebp)
0x2945c9b8:  mov    $0x112e5580,%eax
0x2945c9bd:  jmp    0x83bdbe8
0x2945c9c2:  jmp    0x2945c9c7
0x2945c9c7:  mov    $0x8044b2db,%eax
0x2945c9cc:  mov    %eax,0x80(%ebp)
0x2945c9d2:  mov    $0xffffffff,%eax
0x2945c9d7:  mov    %eax,0x84(%ebp)
0x2945c9dd:  mov    $0x112e5581,%eax
0x2945c9e2:  jmp    0x83bdbe8

 Am I right the marked insns cause the branch to also be taken when the
higher half of r8 is zero and the lower half has bit 31 set? (like the
start offset of the /usr partition of the mentioned images gets...)
And if yes, is the following patch correct that adds special handling for
the `test r,r' case to the i386 version of tcg_out_brcond2()?

Index: qemu/tcg/i386/tcg-target.c
@@ -350,49 +350,83 @@
     label_next = gen_new_label();
     switch(args[4]) {
     case TCG_COND_EQ:
+    cond_eq:
         tcg_out_brcond(s, TCG_COND_NE, args[0], args[2], const_args[2], 
label_next);
         tcg_out_brcond(s, TCG_COND_EQ, args[1], args[3], const_args[3], 
args[5]);
         break;
     case TCG_COND_NE:
+    cond_ne:
         tcg_out_brcond(s, TCG_COND_NE, args[0], args[2], const_args[2], 
args[5]);
         tcg_out_brcond(s, TCG_COND_NE, args[1], args[3], const_args[3], 
args[5]);
         break;
     case TCG_COND_LT:
         tcg_out_brcond(s, TCG_COND_LT, args[1], args[3], const_args[3], 
args[5]);
+        if (const_args[2] && !args[2])
+            /* test r,r */
+            break;
         tcg_out_jxx(s, JCC_JNE, label_next);
         tcg_out_brcond(s, TCG_COND_LT, args[0], args[2], const_args[2], 
args[5]);
         break;
     case TCG_COND_LE:
         tcg_out_brcond(s, TCG_COND_LT, args[1], args[3], const_args[3], 
args[5]);
         tcg_out_jxx(s, JCC_JNE, label_next);
+        if (const_args[2] && !args[2]) {
+            /* test r,r */
+            tcg_out_brcond(s, TCG_COND_EQ, args[0], args[2], const_args[2], 
args[5]);
+            break;
+        }
         tcg_out_brcond(s, TCG_COND_LE, args[0], args[2], const_args[2], 
args[5]);
         break;
     case TCG_COND_GT:
         tcg_out_brcond(s, TCG_COND_GT, args[1], args[3], const_args[3], 
args[5]);
         tcg_out_jxx(s, JCC_JNE, label_next);
+        if (const_args[2] && !args[2]) {
+            /* test r,r */
+            tcg_out_brcond(s, TCG_COND_NE, args[0], args[2], const_args[2], 
args[5]);
+            break;
+        }
         tcg_out_brcond(s, TCG_COND_GT, args[0], args[2], const_args[2], 
args[5]);
         break;
     case TCG_COND_GE:
+        if (const_args[2] && !args[2]) {
+            /* test r,r */
+            tcg_out_brcond(s, TCG_COND_GE, args[1], args[3], const_args[3], 
args[5]);
+            break;
+        }
         tcg_out_brcond(s, TCG_COND_GT, args[1], args[3], const_args[3], 
args[5]);
         tcg_out_jxx(s, JCC_JNE, label_next);
         tcg_out_brcond(s, TCG_COND_GE, args[0], args[2], const_args[2], 
args[5]);
         break;
     case TCG_COND_LTU:
+        if (const_args[2] && !args[2])
+            /* test r,r */
+            break;
         tcg_out_brcond(s, TCG_COND_LTU, args[1], args[3], const_args[3], 
args[5]);
         tcg_out_jxx(s, JCC_JNE, label_next);
         tcg_out_brcond(s, TCG_COND_LTU, args[0], args[2], const_args[2], 
args[5]);
         break;
     case TCG_COND_LEU:
+        if (const_args[2] && !args[2])
+            /* test r,r */
+            goto cond_eq;
         tcg_out_brcond(s, TCG_COND_LTU, args[1], args[3], const_args[3], 
args[5]);
         tcg_out_jxx(s, JCC_JNE, label_next);
         tcg_out_brcond(s, TCG_COND_LEU, args[0], args[2], const_args[2], 
args[5]);
         break;
     case TCG_COND_GTU:
+        if (const_args[2] && !args[2])
+            /* test r,r */
+            goto cond_ne;
         tcg_out_brcond(s, TCG_COND_GTU, args[1], args[3], const_args[3], 
args[5]);
         tcg_out_jxx(s, JCC_JNE, label_next);
         tcg_out_brcond(s, TCG_COND_GTU, args[0], args[2], const_args[2], 
args[5]);
         break;
     case TCG_COND_GEU:
+        if (const_args[2] && !args[2]) {
+            /* test r,r */
+            tcg_out_jxx(s, JCC_JMP, args[5]);
+            break;
+        }
         tcg_out_brcond(s, TCG_COND_GTU, args[1], args[3], const_args[3], 
args[5]);
         tcg_out_jxx(s, JCC_JNE, label_next);
         tcg_out_brcond(s, TCG_COND_GEU, args[0], args[2], const_args[2], 
args[5]);

 Unfortunately that patch doesnt seem to be enough i.e. /usr still fails
to mount.  Have I missed another case of this problem?  The following
        if (bp->bio_offset > pp->mediasize)
line that might also cause the mount failure gets translated like the
follwoing, can anyone see a bug there?

IN: 
0xffffffff8044b2d5:  cmp    0x48(%rdi),%r8
0xffffffff8044b2d9:  jle    0xffffffff8044b33a

OUT: [size=227]
0x29487ec0:  mov    0x38(%ebp),%eax
0x29487ec3:  mov    0x3c(%ebp),%edx
0x29487ec6:  add    $0x48,%eax
0x29487ec9:  adc    $0x0,%edx
0x29487ecc:  mov    %eax,%ecx
0x29487ece:  mov    %edx,%ebx
0x29487ed0:  mov    %ecx,%edx
0x29487ed2:  mov    %ecx,%eax
0x29487ed4:  shr    $0x7,%edx
0x29487ed7:  and    $0xfffff007,%eax
0x29487edd:  and    $0x1fe0,%edx
0x29487ee3:  lea    0x460(%edx,%ebp,1),%edx
0x29487eea:  cmp    (%edx),%eax
0x29487eec:  mov    %ecx,%eax
0x29487eee:  jne    0x29487ef5
0x29487ef0:  cmp    0x4(%edx),%ebx
0x29487ef3:  je     0x29487f00
0x29487ef5:  mov    %ebx,%edx
0x29487ef7:  xor    %ecx,%ecx
0x29487ef9:  call   0x8157a18
0x29487efe:  jmp    0x29487f0a
0x29487f00:  add    0x18(%edx),%eax
0x29487f03:  mov    %eax,%edx
0x29487f05:  mov    (%edx),%eax
0x29487f07:  mov    0x4(%edx),%edx
0x29487f0a:  mov    0x40(%ebp),%ecx
0x29487f0d:  mov    0x44(%ebp),%ebx
0x29487f10:  mov    %eax,%esi
0x29487f12:  mov    %edx,%edi
0x29487f14:  sub    %eax,%ecx
0x29487f16:  sbb    %edx,%ebx
0x29487f18:  mov    %ecx,%eax
0x29487f1a:  mov    %ebx,%edx
0x29487f1c:  add    %esi,%eax
0x29487f1e:  adc    %edi,%edx
0x29487f20:  mov    %ecx,0x98(%ebp)
0x29487f26:  mov    $0x11,%ecx
0x29487f2b:  mov    %ecx,0xa0(%ebp)
0x29487f31:  mov    %esi,0x90(%ebp)
0x29487f37:  mov    %edi,0x94(%ebp)
0x29487f3d:  mov    %ebx,0x9c(%ebp)
0x29487f43:  cmp    %edi,%edx
0x29487f45:  jl     0x29487f7e
0x29487f4b:  jne    0x29487f59
0x29487f51:  cmp    %esi,%eax
0x29487f53:  jle    0x29487f7e
0x29487f59:  jmp    0x29487f5e
0x29487f5e:  mov    $0x8044b2db,%eax
0x29487f63:  mov    %eax,0x80(%ebp)
0x29487f69:  mov    $0xffffffff,%eax
0x29487f6e:  mov    %eax,0x84(%ebp)
0x29487f74:  mov    $0x112f1328,%eax
0x29487f79:  jmp    0x83bd948
0x29487f7e:  jmp    0x29487f83
0x29487f83:  mov    $0x8044b33a,%eax
0x29487f88:  mov    %eax,0x80(%ebp)
0x29487f8e:  mov    $0xffffffff,%eax
0x29487f93:  mov    %eax,0x84(%ebp)
0x29487f99:  mov    $0x112f1329,%eax
0x29487f9e:  jmp    0x83bd948

 Thanx,
        Juergen