Bug 343802

Summary: s390x memcheck reports spurious "conditional jump or move depends on unitialised value(s)" error
Product: [Developer tools] valgrind Reporter: Siddhesh Poyarekar <siddhesh.poyarekar>
Component: memcheckAssignee: Julian Seward <jseward>
Status: RESOLVED FIXED    
Severity: normal CC: arnez, borntraeger, flo2030, mark
Priority: NOR    
Version First Reported In: unspecified   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:
Attachments: Assembler output from the program

Description Siddhesh Poyarekar 2015-02-05 06:04:10 UTC 
This is similar to bug 308427, i.e. instead of an lg+jhe combination, you have an lt+jl combination that only tests the MSB for the jump.

The test program:

struct obstack
{
  unsigned use_extra_arg:1;
  unsigned extra_bitfield:1;
};

void
myfree (void *f)
{
}

void (*freefun) (void *) = myfree;

void
obstack_free (struct obstack *h, void *obj)
{
  if (h->use_extra_arg)
    freefun ((void *) 0x42);
  else
    freefun ((void *) 0x0);
}

int
main (int argc, char **argv)
{
  struct obstack ob;

  ob.use_extra_arg = 0;

  obstack_free (&ob, (void *) 0);
  return 0;
}

The disassembly at the point of error:

   0x00000000800005a8 <+0>:     lt      %r1,0(%r2)
   0x00000000800005ae <+6>:     larl    %r1,0x80002030 <freefun>
=> 0x00000000800005b4 <+12>:    jl      0x800005c4 <obstack_free+28>
   0x00000000800005b8 <+16>:    lghi    %r2,0
   0x00000000800005bc <+20>:    lg      %r1,0(%r1)
   0x00000000800005c2 <+26>:    br      %r1
   0x00000000800005c4 <+28>:    lghi    %r2,66
   0x00000000800005c8 <+32>:    lg      %r1,0(%r1)
   0x00000000800005ce <+38>:    br      %r1
Comment 1 Julian Seward 2015-02-05 08:08:14 UTC 
For similar problems on x86/amd64, I added rewrite rules in the
front end condition code specialisation helper for the case 
"signed < 0 after subtract" and return the top bit of the result.
Maybe something similar would work here?

      /* 8, 9 */
      if (isU64(cc_op, AMD64G_CC_OP_SUBB) && isU64(cond, AMD64CondS)
                                          && isU64(cc_dep2, 0)) {
         /* byte sub/cmp of zero, then S --> test (dst-0 <s 0)
                                         --> test dst <s 0
                                         --> (ULong)dst[7]
            This is yet another scheme by which gcc figures out if the
            top bit of a byte is 1 or 0.  See also LOGICB/CondS below. */
         /* Note: isU64(cc_dep2, 0) is correct, even though this is
            for an 8-bit comparison, since the args to the helper
            function are always U64s. */
         return binop(Iop_And64,
                      binop(Iop_Shr64,cc_dep1,mkU8(7)),
                      mkU64(1));
      }
Comment 2 Christian Borntraeger 2015-02-05 08:18:55 UTC 
This is actually really similar to  bug 308427.
there was ltg + jhe (jump if >=0 == highest bit is not set)
this is ltg + jl (jump if <0 == highest bit set)

Can you provide the the opstack.s file (gcc -S ) as my compiler does not reproduce the issue and we need a reproducer to verify a fix
Comment 3 Christian Borntraeger 2015-02-05 08:57:36 UTC 
Can you retry with 

Index: VEX/priv/guest_s390_helpers.c
===================================================================
--- VEX/priv/guest_s390_helpers.c	(Revision 2998)
+++ VEX/priv/guest_s390_helpers.c	(Arbeitskopie)
@@ -1958,7 +1958,10 @@
             return unop(Iop_1Uto32, binop(Iop_CmpNE64, cc_dep1, mkU64(0)));
          }
          if (cond == 4 || cond == 4 + 1) {
-            return unop(Iop_1Uto32, binop(Iop_CmpLT64S, cc_dep1, mkU64(0)));
+             /* Special case cc_dep < 0. Only check the MSB to avoid bogus
+               memcheck complaints due to gcc magic. Fixes 343802
+             */
+            return unop(Iop_64to32, binop(Iop_Shr64, cc_dep1, mkU8(63)));
          }
          if (cond == 8 + 4 || cond == 8 + 4 + 1) {
             return unop(Iop_1Uto32, binop(Iop_CmpLE64S, cc_dep1, mkU64(0)));
Comment 4 Siddhesh Poyarekar 2015-02-05 09:41:26 UTC 
Created attachment 90919 [details]
Assembler output from the program

Sorry, I forgot to mention the compiler command line:

gcc -O2 -o obs{,.c}

assuming that the source I mentioned above is obs.c.  I have attached the generated assembly in any case, using:

# rpm -q gcc
gcc-4.9.2-5.fc22.s390x
Comment 5 Siddhesh Poyarekar 2015-02-05 10:15:54 UTC 
(In reply to Christian Borntraeger from comment #3)
> Can you retry with 
> 

This patch works, thanks.
Comment 6 Christian Borntraeger 2015-02-05 11:10:01 UTC 
Fixed in  VEX r3083 / VALGRIND r14905.
Julian, please set to fixed (I cant)