Bug 418435 - s390x: memcmp test yields extra "Conditional jump or move depends on uninitialised value(s)"
Summary: s390x: memcmp test yields extra "Conditional jump or move depends on uninitia...
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: vex (other bugs)
Version First Reported In: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-03 14:46 UTC by Andreas Arnez
Modified: 2020-03-04 17:24 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments
Fix for the memcmp test fail (855 bytes, patch)
2020-03-03 16:18 UTC, Andreas Arnez
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Arnez 2020-03-03 14:46:06 UTC
The test memcheck/tests/memcmp currently fails on s390x because it yields the expected error message twice instead of just once:

Conditional jump or move depends on uninitialised value(s)
   at 0x........: bcmp (vg_replace_strmem.c:...)
   by 0x........: main (memcmptest.c:13)

Conditional jump or move depends on uninitialised value(s)
   at 0x........: bcmp (vg_replace_strmem.c:...)
   by 0x........: main (memcmptest.c:13)
Comment 1 Andreas Arnez 2020-03-03 15:41:33 UTC
The issue seems to be caused by the handling of the s390x instruction
CLC, see s390_irgen_CLC_EX().  When comparing two bytes from the two
input strings, the implementation uses the comparison result for a
conditional branch to the next instruction.  But if no further bytes
need to be compared, the comparison result is also used for generating
the resulting condition code.  There are two cases:
1. The inputs are equal.  Then the resulting condition code is zero.
   This is what happens in the memcmp test case.
2. The inputs are different.  Then the resulting condition code is 1 or
   2 if the first or second operand is greater, respectively.

At least in the first case it is easy to avoid the additional
dependency, by clearing the condition code explicitly.
Comment 2 Andreas Arnez 2020-03-03 16:18:02 UTC
Created attachment 126571 [details]
Fix for the memcmp test fail

This is a simple fix for the memcmp test fail.  It addresses only the case where the two input strings are equal and then clears the condition code explicitly, to avoid the additional data dependency.
Comment 3 Julian Seward 2020-03-04 12:07:22 UTC
(In reply to Andreas Arnez from comment #2)
> This is a simple fix for the memcmp test fail.  It addresses only the case
> where the two input strings are equal and then clears the condition code
> explicitly, to avoid the additional data dependency.

If this fix is correct in the sense that it can never cause incorrect
instruction emulation, then I would say, land it.  FWIW the x86/amd64
front ends (and may be others) similarly zero out bits of the flags 
related state so as to avoid false data dependencies, so you're in good
company :-)
Comment 4 Andreas Arnez 2020-03-04 17:24:53 UTC
(In reply to Julian Seward from comment #3)
> If this fix is correct in the sense that it can never cause incorrect
> instruction emulation, then I would say, land it.
The patch shouldn't change the emulation; it just clears the condition code in the case where it's already cleared.
So OK, I pushed it as 91bf2d2d8cbf5942d73dc748150945a5c24ecae2.