Back out rev 1.9 (optimised %eflags save/restore). It isn't quite
right.
Here's a code sequence illustrating the problem. The conditional jump
at the end evidently goes the wrong way sometimes, and the program
goes off into outer space soon after.
0x4017F6F7: addl %ebx,%ecx
12: GETL %ECX, t10
13: ADDL %EBX, t10 (-wOSZACP)
14: PUTL t10, %ECX
15: INCEIPo $2
0x4017F6F9: decl %eax
16: GETL %EAX, t12
17: DECL t12 (-wOSZAP)
18: PUTL t12, %EAX
19: INCEIPo $1
0x4017F6FA: jnb-8 0x4017F710
20: Jnbo $0x4017F710 (-rOSZACP)
21: JMPo $0x4017F6FC
Look carefully at the annotation on # 17. Then look in the Intel docs
and see what flag(s) the Jnb (not-below) condition consults. Bwaha!
It consults the carry flag.
The generated code for 17 (renamed to 12 after some NOP removal, I
guess) is
12: DECL %edx (-wOSZAP) [---d--]
42: 4A
decl %edx
43: 9C 8F 45 20
pushfl ; popl 32(%ebp)
viz, we do the decl, and then copy the real machine's %eflags into
%EFLAGS. Unfortunately this copies the real carry flag into the
simulated one, rather than leaving the simulated one alone.
So the principle is that it's only safe to omit the initial
%EFLAGS->%eflags move prior to the insn if the insn writes _all_ the
flags, and in this case it doesn't.
(after further consideration ...)
At first it seems tempting to play games with subset checks, ie if an
insn writes a _subset_ of the flags, we'd better copy sim'd to real
flags before the insn.
Problem with that is that the D (direction) flag, which specifies the
direction that rep-prefix string ops travel, is part of the "normal"
flag set. So it is conceivable, although highly unlikely, that an app
could
- set the D flag
- do something like ADD (-wOSZACP)
- use the D flag
then we'd still have to do an %EFLAGS->%eflags copy prior to the ADD,
in order to ensure the simulated D flag is preserved.
So it seems to me that it's pretty much impossible to preserve
absolute correctness and do any better than the vg_from_ucode.c rev
1.8 scheme.
Backing out rev 1.9. We can just as easily restore it from cvs if a
better solution is arrived at.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@1185 a5019735-40e9-0310-863c-91ae7b9d1cf9
diff --git a/coregrind/vg_from_ucode.c b/coregrind/vg_from_ucode.c
index aadbe1f..d08c505 100644
--- a/coregrind/vg_from_ucode.c
+++ b/coregrind/vg_from_ucode.c
@@ -1527,29 +1527,29 @@
}
-static void synth_unaryop_reg ( Bool wr_cc,
+static void synth_unaryop_reg ( Bool upd_cc,
Opcode opcode, Int size,
Int reg )
{
/* NB! opcode is a uinstr opcode, not an x86 one! */
switch (size) {
- case 4: //if (rd_cc) emit_get_eflags(); (never needed --njn)
+ case 4: if (upd_cc) emit_get_eflags();
VG_(emit_unaryopv_reg) ( 4, opcode, reg );
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
break;
- case 2: //if (rd_cc) emit_get_eflags(); (never needed --njn)
+ case 2: if (upd_cc) emit_get_eflags();
VG_(emit_unaryopv_reg) ( 2, opcode, reg );
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
break;
case 1: if (reg < 4) {
- //if (rd_cc) emit_get_eflags(); (never needed --njn)
+ if (upd_cc) emit_get_eflags();
VG_(emit_unaryopb_reg) ( opcode, reg );
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
} else {
VG_(emit_swapl_reg_EAX) ( reg );
- //if (rd_cc) emit_get_eflags(); (never needed --njn)
+ if (upd_cc) emit_get_eflags();
VG_(emit_unaryopb_reg) ( opcode, R_AL );
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
VG_(emit_swapl_reg_EAX) ( reg );
}
break;
@@ -1559,19 +1559,19 @@
-static void synth_nonshiftop_reg_reg ( Bool rd_cc, Bool wr_cc,
+static void synth_nonshiftop_reg_reg ( Bool upd_cc,
Opcode opcode, Int size,
Int reg1, Int reg2 )
{
/* NB! opcode is a uinstr opcode, not an x86 one! */
switch (size) {
- case 4: if (rd_cc) emit_get_eflags();
+ case 4: if (upd_cc) emit_get_eflags();
VG_(emit_nonshiftopv_reg_reg) ( 4, opcode, reg1, reg2 );
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
break;
- case 2: if (rd_cc) emit_get_eflags();
+ case 2: if (upd_cc) emit_get_eflags();
VG_(emit_nonshiftopv_reg_reg) ( 2, opcode, reg1, reg2 );
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
break;
case 1: { /* Horrible ... */
Int s1, s2;
@@ -1580,44 +1580,44 @@
sure s1 != s2 and that neither of them equal either reg1 or
reg2. Then use them as temporaries to make things work. */
if (reg1 < 4 && reg2 < 4) {
- if (rd_cc) emit_get_eflags();
+ if (upd_cc) emit_get_eflags();
emit_nonshiftopb_reg_reg(opcode, reg1, reg2);
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
break;
}
for (s1 = 0; s1 == reg1 || s1 == reg2; s1++) ;
if (reg1 >= 4 && reg2 < 4) {
emit_swapl_reg_reg ( reg1, s1 );
- if (rd_cc) emit_get_eflags();
+ if (upd_cc) emit_get_eflags();
emit_nonshiftopb_reg_reg(opcode, s1, reg2);
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
emit_swapl_reg_reg ( reg1, s1 );
break;
}
for (s2 = 0; s2 == reg1 || s2 == reg2 || s2 == s1; s2++) ;
if (reg1 < 4 && reg2 >= 4) {
emit_swapl_reg_reg ( reg2, s2 );
- if (rd_cc) emit_get_eflags();
+ if (upd_cc) emit_get_eflags();
emit_nonshiftopb_reg_reg(opcode, reg1, s2);
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
emit_swapl_reg_reg ( reg2, s2 );
break;
}
if (reg1 >= 4 && reg2 >= 4 && reg1 != reg2) {
emit_swapl_reg_reg ( reg1, s1 );
emit_swapl_reg_reg ( reg2, s2 );
- if (rd_cc) emit_get_eflags();
+ if (upd_cc) emit_get_eflags();
emit_nonshiftopb_reg_reg(opcode, s1, s2);
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
emit_swapl_reg_reg ( reg1, s1 );
emit_swapl_reg_reg ( reg2, s2 );
break;
}
if (reg1 >= 4 && reg2 >= 4 && reg1 == reg2) {
emit_swapl_reg_reg ( reg1, s1 );
- if (rd_cc) emit_get_eflags();
+ if (upd_cc) emit_get_eflags();
emit_nonshiftopb_reg_reg(opcode, s1, s1);
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
emit_swapl_reg_reg ( reg1, s1 );
break;
}
@@ -1629,31 +1629,31 @@
static void synth_nonshiftop_offregmem_reg (
- Bool rd_cc, Bool wr_cc,
+ Bool upd_cc,
Opcode opcode, Int size,
Int off, Int areg, Int reg )
{
switch (size) {
case 4:
- if (rd_cc) emit_get_eflags();
+ if (upd_cc) emit_get_eflags();
emit_nonshiftopv_offregmem_reg ( 4, opcode, off, areg, reg );
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
break;
case 2:
- if (rd_cc) emit_get_eflags();
+ if (upd_cc) emit_get_eflags();
emit_nonshiftopv_offregmem_reg ( 2, opcode, off, areg, reg );
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
break;
case 1:
if (reg < 4) {
- if (rd_cc) emit_get_eflags();
+ if (upd_cc) emit_get_eflags();
emit_nonshiftopb_offregmem_reg ( opcode, off, areg, reg );
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
} else {
VG_(emit_swapl_reg_EAX) ( reg );
- if (rd_cc) emit_get_eflags();
+ if (upd_cc) emit_get_eflags();
emit_nonshiftopb_offregmem_reg ( opcode, off, areg, R_AL );
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
VG_(emit_swapl_reg_EAX) ( reg );
}
break;
@@ -1663,28 +1663,28 @@
}
-static void synth_nonshiftop_lit_reg ( Bool rd_cc, Bool wr_cc,
+static void synth_nonshiftop_lit_reg ( Bool upd_cc,
Opcode opcode, Int size,
UInt lit, Int reg )
{
switch (size) {
- case 4: if (rd_cc) emit_get_eflags();
+ case 4: if (upd_cc) emit_get_eflags();
VG_(emit_nonshiftopv_lit_reg) ( 4, opcode, lit, reg );
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
break;
- case 2: if (rd_cc) emit_get_eflags();
+ case 2: if (upd_cc) emit_get_eflags();
VG_(emit_nonshiftopv_lit_reg) ( 2, opcode, lit, reg );
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
break;
case 1: if (reg < 4) {
- if (rd_cc) emit_get_eflags();
+ if (upd_cc) emit_get_eflags();
emit_nonshiftopb_lit_reg ( opcode, lit, reg );
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
} else {
VG_(emit_swapl_reg_EAX) ( reg );
- if (rd_cc) emit_get_eflags();
+ if (upd_cc) emit_get_eflags();
emit_nonshiftopb_lit_reg ( opcode, lit, R_AL );
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
VG_(emit_swapl_reg_EAX) ( reg );
}
break;
@@ -1739,47 +1739,47 @@
}
-static void synth_shiftop_reg_reg ( Bool rd_cc, Bool wr_cc,
+static void synth_shiftop_reg_reg ( Bool upd_cc,
Opcode opcode, Int size,
Int regs, Int regd )
{
synth_push_reg ( size, regd );
if (regs != R_ECX) emit_swapl_reg_ECX ( regs );
- if (rd_cc) emit_get_eflags();
+ if (upd_cc) emit_get_eflags();
switch (size) {
case 4: emit_shiftopv_cl_stack0 ( 4, opcode ); break;
case 2: emit_shiftopv_cl_stack0 ( 2, opcode ); break;
case 1: emit_shiftopb_cl_stack0 ( opcode ); break;
default: VG_(core_panic)("synth_shiftop_reg_reg");
}
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
if (regs != R_ECX) emit_swapl_reg_ECX ( regs );
synth_pop_reg ( size, regd );
}
-static void synth_shiftop_lit_reg ( Bool rd_cc, Bool wr_cc,
+static void synth_shiftop_lit_reg ( Bool upd_cc,
Opcode opcode, Int size,
UInt lit, Int reg )
{
switch (size) {
- case 4: if (rd_cc) emit_get_eflags();
+ case 4: if (upd_cc) emit_get_eflags();
VG_(emit_shiftopv_lit_reg) ( 4, opcode, lit, reg );
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
break;
- case 2: if (rd_cc) emit_get_eflags();
+ case 2: if (upd_cc) emit_get_eflags();
VG_(emit_shiftopv_lit_reg) ( 2, opcode, lit, reg );
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
break;
case 1: if (reg < 4) {
- if (rd_cc) emit_get_eflags();
+ if (upd_cc) emit_get_eflags();
emit_shiftopb_lit_reg ( opcode, lit, reg );
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
} else {
VG_(emit_swapl_reg_EAX) ( reg );
- if (rd_cc) emit_get_eflags();
+ if (upd_cc) emit_get_eflags();
emit_shiftopb_lit_reg ( opcode, lit, R_AL );
- if (wr_cc) emit_put_eflags();
+ if (upd_cc) emit_put_eflags();
VG_(emit_swapl_reg_EAX) ( reg );
}
break;
@@ -1948,16 +1948,19 @@
/*--- Generate code for a single UInstr. ---*/
/*----------------------------------------------------*/
-static Bool readFlagUse ( UInstr* u )
-{
- return (u->flags_r != FlagsEmpty);
-}
-
-static Bool writeFlagUse ( UInstr* u )
+static __inline__
+Bool writeFlagUse ( UInstr* u )
{
return (u->flags_w != FlagsEmpty);
}
+static __inline__
+Bool anyFlagUse ( UInstr* u )
+{
+ return (u->flags_r != FlagsEmpty || u->flags_w != FlagsEmpty);
+}
+
+
static void emitUInstr ( UCodeBlock* cb, Int i, RRegSet regs_live_before )
{
Int old_emitted_code_used;
@@ -2213,27 +2216,25 @@
break;
}
+ case SBB:
+ case ADC:
case XOR:
case OR:
case AND:
case SUB:
- case ADD:
- vg_assert(! readFlagUse ( u ));
- /* fall thru */
- case SBB:
- case ADC: {
+ case ADD: {
vg_assert(u->tag2 == RealReg);
switch (u->tag1) {
case Literal: synth_nonshiftop_lit_reg (
- readFlagUse(u), writeFlagUse(u),
+ anyFlagUse(u),
u->opcode, u->size, u->lit32, u->val2 );
break;
case RealReg: synth_nonshiftop_reg_reg (
- readFlagUse(u), writeFlagUse(u),
+ anyFlagUse(u),
u->opcode, u->size, u->val1, u->val2 );
break;
case ArchReg: synth_nonshiftop_offregmem_reg (
- readFlagUse(u), writeFlagUse(u),
+ anyFlagUse(u),
u->opcode, u->size,
spillOrArchOffset( u->size, u->tag1, u->val1 ),
R_EBP,
@@ -2244,23 +2245,21 @@
break;
}
+ case RCR:
+ case RCL:
case ROR:
case ROL:
case SAR:
case SHR:
case SHL: {
- vg_assert(! readFlagUse ( u ));
- /* fall thru */
- case RCR:
- case RCL:
vg_assert(u->tag2 == RealReg);
switch (u->tag1) {
case Literal: synth_shiftop_lit_reg (
- readFlagUse(u), writeFlagUse(u),
+ anyFlagUse(u),
u->opcode, u->size, u->lit32, u->val2 );
break;
case RealReg: synth_shiftop_reg_reg (
- readFlagUse(u), writeFlagUse(u),
+ anyFlagUse(u),
u->opcode, u->size, u->val1, u->val2 );
break;
default: VG_(core_panic)("emitUInstr:non-shift-op");
@@ -2273,9 +2272,8 @@
case NEG:
case NOT:
vg_assert(u->tag1 == RealReg);
- vg_assert(! readFlagUse ( u ));
synth_unaryop_reg (
- writeFlagUse(u), u->opcode, u->size, u->val1 );
+ anyFlagUse(u), u->opcode, u->size, u->val1 );
break;
case BSWAP:
@@ -2348,7 +2346,7 @@
vg_assert(u->tag1 == Lit16);
vg_assert(u->tag2 == NoValue);
vg_assert(u->size == 0);
- if (readFlagUse ( u ))
+ if (anyFlagUse ( u ))
emit_get_eflags();
VG_(synth_call) ( False, u->val1 );
if (writeFlagUse ( u ))
@@ -2402,7 +2400,7 @@
case FPU:
vg_assert(u->tag1 == Lit16);
vg_assert(u->tag2 == NoValue);
- if (readFlagUse ( u ))
+ if (anyFlagUse ( u ))
emit_get_eflags();
synth_fpu_no_mem ( (u->val1 >> 8) & 0xFF,
u->val1 & 0xFF );