| Summary: | s390x: memcmp test yields extra "Conditional jump or move depends on uninitialised value(s)" | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Andreas Arnez <arnez> |
| Component: | vex | Assignee: | Julian Seward <jseward> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | ||
| Priority: | NOR | ||
| Version First Reported In: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed/Implemented In: | ||
| Sentry Crash Report: | |||
| Attachments: | Fix for the memcmp test fail | ||
|
Description
Andreas Arnez
2020-03-03 14:46:06 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. 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.
(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 :-) (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. |