This is similar to bug #387712 (about cgijnl), but a newer gcc uses cgijl now: 723 /* Check whether this is the initial frame or a signal frame. 724 Then we need to unwind from the original, unadjusted PC. */ 725 if (! state->initial_frame && ! state->signal_frame) 0x000000000487e53c <+44>: lg %r1,16(%r11) 0x000000000487e542 <+50>: lg %r3,176(%r15) 0x000000000487e548 <+56>: tmhh %r1,16384 0x000000000487e54c <+60>: jne 0x487e560 <__libdwfl_frame_unwind+80> => 0x000000000487e550 <+64>: cgijl %r1,0,0x487e560 <__libdwfl_frame_unwind+80> The following should fix it: diff --git a/VEX/priv/guest_s390_helpers.c b/VEX/priv/guest_s390_helpers.c index 52e4ce936..aeda67704 100644 --- a/VEX/priv/guest_s390_helpers.c +++ b/VEX/priv/guest_s390_helpers.c @@ -1935,6 +1935,14 @@ guest_s390x_spechelper(const HChar *function_name, IRExpr **args, return unop(Iop_1Uto32, binop(Iop_CmpNE64, cc_dep1, cc_dep2)); } if (cond == 4 || cond == 4 + 1) { + if (isC64_exactly(cc_dep2, 0)) { + /* dep1 <signed 0 + --> m.s.bit of dep1 == 1 */ + return unop(Iop_64to32, + binop(Iop_And64, + binop(Iop_Shr64, cc_dep1, mkU8(63)), + mkU64(1))); + } return unop(Iop_1Uto32, binop(Iop_CmpLT64S, cc_dep1, cc_dep2)); } if (cond == 8 + 4 || cond == 8 + 4 + 1) {
looks sane: normally we compare that dep1 < dep2 (if yes ----> 1, otherwise 0) the special case is now: dep2 == 0 we then shift right dep1 by 63, (taking the msb of dep1) then we AND that with 1 -> if msb is set --> 1 otherwise the result is 0.
The patch looks sane to me. For adding spec rules like this, I use the following testing strategy: * first, slightly mis-implement it. For example, do Iop_Shr64 of 62 bits instead of 63. * then check that your test program (or at least, _some_ program) crashes or fails bizarrely, with this "fix" * then change it so it's really correct and check that said program now works correctly. This gives at least some confidence that the spec rule is being used and is correct.
(In reply to Julian Seward from comment #2) > The patch looks sane to me. For adding spec rules like this, I use > the following testing strategy: > > * first, slightly mis-implement it. For example, do Iop_Shr64 of 62 > bits instead of 63. > > * then check that your test program (or at least, _some_ program) > crashes or fails bizarrely, with this "fix" > > * then change it so it's really correct and check that said program > now works correctly. > > This gives at least some confidence that the spec rule is being used > and is correct. That is a fun idea. Implementing it wrongly (shifting by 1) does show programs that get into infinite loops and blow up the stack. Without the fix we just get a false Conditional jump or move depends on uninitialised value(s) report. With the correct fix everything looks fine. commit 51736549e33fc8468e47861031a70c7f8cadd691 Author: Mark Wielaard <mark@klomp.org> Date: Mon Sep 3 12:56:53 2018 +0200 Bug 398066 s390x: cgijl dep1, 0 reports false uninitialised values warning. This is similar to bug #387712 (about cgijnl), but a newer gcc uses cgijl now. So use a similar fix when cc_dep2 is zero, only check whether the most significant bit of cc_dep1 is set to 1.