From 097310cd00cd405c0ca6ae11ac57a4b22b670b7b Mon Sep 17 00:00:00 2001 From: Stanislav Shwartsman Date: Thu, 30 Mar 2017 21:53:39 +0000 Subject: [PATCH] fixed integer overflow while computing shift flags, avoid using bx_bool while working with flags for more robust code --- bochs/cpu/arith32.cc | 40 ++++++++++------------------------------ bochs/cpu/cpu.h | 34 +++++++++++++++++----------------- bochs/cpu/shift16.cc | 24 ++++++++++++++++-------- bochs/cpu/shift32.cc | 24 ++++++++++++++++-------- bochs/cpu/shift64.cc | 24 ++++++++++++++++-------- bochs/cpu/shift8.cc | 20 ++++++++++++++------ 6 files changed, 89 insertions(+), 77 deletions(-) diff --git a/bochs/cpu/arith32.cc b/bochs/cpu/arith32.cc index 7cc650093..3b7b03ba8 100644 --- a/bochs/cpu/arith32.cc +++ b/bochs/cpu/arith32.cc @@ -92,9 +92,7 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::ADD_GdEdM(bxInstruction_c *i) BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::ADC_EdGdM(bxInstruction_c *i) { - bx_bool temp_CF = getB_CF(); - - Bit32u op1_32, op2_32, sum_32; + Bit32u op1_32, op2_32, sum_32, temp_CF = getB_CF(); bx_address eaddr = BX_CPU_RESOLVE_ADDR(i); @@ -110,9 +108,7 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::ADC_EdGdM(bxInstruction_c *i) BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::ADC_GdEdR(bxInstruction_c *i) { - bx_bool temp_CF = getB_CF(); - - Bit32u op1_32, op2_32, sum_32; + Bit32u op1_32, op2_32, sum_32, temp_CF = getB_CF(); op1_32 = BX_READ_32BIT_REG(i->dst()); op2_32 = BX_READ_32BIT_REG(i->src()); @@ -126,9 +122,7 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::ADC_GdEdR(bxInstruction_c *i) BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::ADC_GdEdM(bxInstruction_c *i) { - bx_bool temp_CF = getB_CF(); - - Bit32u op1_32, op2_32, sum_32; + Bit32u op1_32, op2_32, sum_32, temp_CF = getB_CF(); bx_address eaddr = BX_CPU_RESOLVE_ADDR(i); @@ -144,9 +138,7 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::ADC_GdEdM(bxInstruction_c *i) BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::SBB_EdGdM(bxInstruction_c *i) { - bx_bool temp_CF = getB_CF(); - - Bit32u op1_32, op2_32, diff_32; + Bit32u op1_32, op2_32, diff_32, temp_CF = getB_CF(); bx_address eaddr = BX_CPU_RESOLVE_ADDR(i); @@ -162,9 +154,7 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::SBB_EdGdM(bxInstruction_c *i) BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::SBB_GdEdR(bxInstruction_c *i) { - bx_bool temp_CF = getB_CF(); - - Bit32u op1_32, op2_32, diff_32; + Bit32u op1_32, op2_32, diff_32, temp_CF = getB_CF(); op1_32 = BX_READ_32BIT_REG(i->dst()); op2_32 = BX_READ_32BIT_REG(i->src()); @@ -178,9 +168,7 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::SBB_GdEdR(bxInstruction_c *i) BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::SBB_GdEdM(bxInstruction_c *i) { - bx_bool temp_CF = getB_CF(); - - Bit32u op1_32, op2_32, diff_32; + Bit32u op1_32, op2_32, diff_32, temp_CF = getB_CF(); bx_address eaddr = BX_CPU_RESOLVE_ADDR(i); @@ -196,9 +184,7 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::SBB_GdEdM(bxInstruction_c *i) BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::SBB_EdIdM(bxInstruction_c *i) { - bx_bool temp_CF = getB_CF(); - - Bit32u op1_32, op2_32 = i->Id(), diff_32; + Bit32u op1_32, op2_32 = i->Id(), diff_32, temp_CF = getB_CF(); bx_address eaddr = BX_CPU_RESOLVE_ADDR(i); @@ -213,9 +199,7 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::SBB_EdIdM(bxInstruction_c *i) BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::SBB_EdIdR(bxInstruction_c *i) { - bx_bool temp_CF = getB_CF(); - - Bit32u op1_32, op2_32 = i->Id(), diff_32; + Bit32u op1_32, op2_32 = i->Id(), diff_32, temp_CF = getB_CF(); op1_32 = BX_READ_32BIT_REG(i->dst()); diff_32 = op1_32 - (op2_32 + temp_CF); @@ -421,9 +405,7 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::ADD_EdIdR(bxInstruction_c *i) BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::ADC_EdIdM(bxInstruction_c *i) { - bx_bool temp_CF = getB_CF(); - - Bit32u op1_32, op2_32 = i->Id(), sum_32; + Bit32u op1_32, op2_32 = i->Id(), sum_32, temp_CF = getB_CF(); bx_address eaddr = BX_CPU_RESOLVE_ADDR(i); @@ -438,9 +420,7 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::ADC_EdIdM(bxInstruction_c *i) BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::ADC_EdIdR(bxInstruction_c *i) { - bx_bool temp_CF = getB_CF(); - - Bit32u op1_32, op2_32 = i->Id(), sum_32; + Bit32u op1_32, op2_32 = i->Id(), sum_32, temp_CF = getB_CF(); op1_32 = BX_READ_32BIT_REG(i->dst()); sum_32 = op1_32 + op2_32 + temp_CF; diff --git a/bochs/cpu/cpu.h b/bochs/cpu/cpu.h index 472bf1b3c..d2c58d65a 100644 --- a/bochs/cpu/cpu.h +++ b/bochs/cpu/cpu.h @@ -565,17 +565,17 @@ BOCHSAPI extern BX_CPU_C bx_cpu; // The macro is used once for each flag bit // Do not use for arithmetic flags ! #define DECLARE_EFLAG_ACCESSOR(name,bitnum) \ - BX_SMF BX_CPP_INLINE Bit32u get_##name (); \ - BX_SMF BX_CPP_INLINE bx_bool getB_##name (); \ + BX_SMF BX_CPP_INLINE unsigned get_##name (); \ + BX_SMF BX_CPP_INLINE unsigned getB_##name (); \ BX_SMF BX_CPP_INLINE void assert_##name (); \ BX_SMF BX_CPP_INLINE void clear_##name (); \ BX_SMF BX_CPP_INLINE void set_##name (bx_bool val); #define IMPLEMENT_EFLAG_ACCESSOR(name,bitnum) \ - BX_CPP_INLINE bx_bool BX_CPU_C::getB_##name () { \ + BX_CPP_INLINE unsigned BX_CPU_C::getB_##name () { \ return 1 & (BX_CPU_THIS_PTR eflags >> bitnum); \ } \ - BX_CPP_INLINE Bit32u BX_CPU_C::get_##name () { \ + BX_CPP_INLINE unsigned BX_CPU_C::get_##name () { \ return BX_CPU_THIS_PTR eflags & (1 << bitnum); \ } @@ -1354,11 +1354,11 @@ public: // for now... } // ZF - BX_SMF BX_CPP_INLINE bx_bool getB_ZF(void) { + BX_SMF BX_CPP_INLINE unsigned getB_ZF(void) { return (0 == BX_CPU_THIS_PTR oszapc.result); } - BX_SMF BX_CPP_INLINE bx_bool get_ZF(void) { return getB_ZF(); } + BX_SMF BX_CPP_INLINE unsigned get_ZF(void) { return getB_ZF(); } BX_SMF BX_CPP_INLINE void set_ZF(bx_bool val) { if (val) assert_ZF(); @@ -1383,12 +1383,12 @@ public: // for now... } // SF - BX_SMF BX_CPP_INLINE bx_bool getB_SF(void) { + BX_SMF BX_CPP_INLINE unsigned getB_SF(void) { return ((BX_CPU_THIS_PTR oszapc.result >> BX_LF_SIGN_BIT) ^ (BX_CPU_THIS_PTR oszapc.auxbits >> LF_BIT_SD)) & 1; } - BX_SMF BX_CPP_INLINE bx_bool get_SF(void) { return getB_SF(); } + BX_SMF BX_CPP_INLINE unsigned get_SF(void) { return getB_SF(); } BX_SMF BX_CPP_INLINE void set_SF(bx_bool val) { bx_bool temp_sf = getB_SF(); @@ -1405,14 +1405,14 @@ public: // for now... } // PF - bit 2 in EFLAGS, represented by lower 8 bits of oszapc.result - BX_SMF BX_CPP_INLINE bx_bool getB_PF(void) { + BX_SMF BX_CPP_INLINE unsigned getB_PF(void) { Bit32u temp = (255 & BX_CPU_THIS_PTR oszapc.result); temp = temp ^ (255 & (BX_CPU_THIS_PTR oszapc.auxbits >> LF_BIT_PDB)); temp = (temp ^ (temp >> 4)) & 0x0F; return (0x9669U >> temp) & 1; } - BX_SMF BX_CPP_INLINE bx_bool get_PF(void) { return getB_PF(); } + BX_SMF BX_CPP_INLINE unsigned get_PF(void) { return getB_PF(); } BX_SMF BX_CPP_INLINE void set_PF(bx_bool val) { Bit32u temp_pdb = (255 & BX_CPU_THIS_PTR oszapc.result) ^ (!val); @@ -1430,11 +1430,11 @@ public: // for now... } // AF - bit 4 in EFLAGS, represented by bit LF_BIT_AF of oszapc.auxbits - BX_SMF BX_CPP_INLINE bx_bool getB_AF(void) { + BX_SMF BX_CPP_INLINE unsigned getB_AF(void) { return ((BX_CPU_THIS_PTR oszapc.auxbits >> LF_BIT_AF) & 1); } - BX_SMF BX_CPP_INLINE bx_bool get_AF(void) { + BX_SMF BX_CPP_INLINE unsigned get_AF(void) { return (BX_CPU_THIS_PTR oszapc.auxbits & LF_MASK_AF); } @@ -1452,11 +1452,11 @@ public: // for now... } // CF - BX_SMF BX_CPP_INLINE bx_bool getB_CF(void) { + BX_SMF BX_CPP_INLINE unsigned getB_CF(void) { return ((BX_CPU_THIS_PTR oszapc.auxbits >> LF_BIT_CF) & 1); } - BX_SMF BX_CPP_INLINE bx_bool get_CF(void) { + BX_SMF BX_CPP_INLINE unsigned get_CF(void) { return (BX_CPU_THIS_PTR oszapc.auxbits & LF_MASK_CF); } @@ -1476,11 +1476,11 @@ public: // for now... } // OF - BX_SMF BX_CPP_INLINE bx_bool getB_OF(void) { + BX_SMF BX_CPP_INLINE unsigned getB_OF(void) { return ((BX_CPU_THIS_PTR oszapc.auxbits + (1U << LF_BIT_PO)) >> LF_BIT_CF) & 1; } - BX_SMF BX_CPP_INLINE bx_bool get_OF(void) { + BX_SMF BX_CPP_INLINE unsigned get_OF(void) { return (BX_CPU_THIS_PTR oszapc.auxbits + (1U << LF_BIT_PO)) & (1U << LF_BIT_CF); } @@ -1495,7 +1495,7 @@ public: // for now... } BX_SMF BX_CPP_INLINE void assert_OF(void) { - bx_bool temp_cf = getB_CF(); + unsigned temp_cf = getB_CF(); SET_FLAGS_OxxxxC((1), temp_cf); } diff --git a/bochs/cpu/shift16.cc b/bochs/cpu/shift16.cc index 1ec73a992..e865ae048 100644 --- a/bochs/cpu/shift16.cc +++ b/bochs/cpu/shift16.cc @@ -377,15 +377,17 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::RCL_EwM(bxInstruction_c *i) /* pointer, segment address pair */ Bit16u op1_16 = read_RMW_virtual_word(i->seg(), eaddr); + unsigned temp_CF = getB_CF(); + if (count) { if (count==1) { - result_16 = (op1_16 << 1) | getB_CF(); + result_16 = (op1_16 << 1) | temp_CF; } else if (count==16) { - result_16 = (getB_CF() << 15) | (op1_16 >> 1); + result_16 = (temp_CF << 15) | (op1_16 >> 1); } else { // 2..15 - result_16 = (op1_16 << count) | (getB_CF() << (count - 1)) | + result_16 = (op1_16 << count) | (temp_CF << (count - 1)) | (op1_16 >> (17 - count)); } @@ -412,17 +414,19 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::RCL_EwR(bxInstruction_c *i) count = (count & 0x1f) % 17; + unsigned temp_CF = getB_CF(); + if (count) { Bit16u op1_16 = BX_READ_16BIT_REG(i->dst()); if (count==1) { - result_16 = (op1_16 << 1) | getB_CF(); + result_16 = (op1_16 << 1) | temp_CF; } else if (count==16) { - result_16 = (getB_CF() << 15) | (op1_16 >> 1); + result_16 = (temp_CF << 15) | (op1_16 >> 1); } else { // 2..15 - result_16 = (op1_16 << count) | (getB_CF() << (count - 1)) | + result_16 = (op1_16 << count) | (temp_CF << (count - 1)) | (op1_16 >> (17 - count)); } @@ -453,7 +457,9 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::RCR_EwM(bxInstruction_c *i) Bit16u op1_16 = read_RMW_virtual_word(i->seg(), eaddr); if (count) { - Bit16u result_16 = (op1_16 >> count) | (getB_CF() << (16 - count)) | + unsigned temp_CF = getB_CF(); + + Bit16u result_16 = (op1_16 >> count) | (temp_CF << (16 - count)) | (op1_16 << (17 - count)); write_RMW_linear_word(result_16); @@ -481,7 +487,9 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::RCR_EwR(bxInstruction_c *i) if (count) { Bit16u op1_16 = BX_READ_16BIT_REG(i->dst()); - Bit16u result_16 = (op1_16 >> count) | (getB_CF() << (16 - count)) | + unsigned temp_CF = getB_CF(); + + Bit16u result_16 = (op1_16 >> count) | (temp_CF << (16 - count)) | (op1_16 << (17 - count)); BX_WRITE_16BIT_REG(i->dst(), result_16); diff --git a/bochs/cpu/shift32.cc b/bochs/cpu/shift32.cc index 0a90aa98a..97db76b5b 100644 --- a/bochs/cpu/shift32.cc +++ b/bochs/cpu/shift32.cc @@ -297,11 +297,13 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::RCL_EdM(bxInstruction_c *i) BX_NEXT_INSTR(i); } + Bit32u temp_CF = getB_CF(); + if (count==1) { - result_32 = (op1_32 << 1) | getB_CF(); + result_32 = (op1_32 << 1) | temp_CF; } else { - result_32 = (op1_32 << count) | (getB_CF() << (count - 1)) | + result_32 = (op1_32 << count) | (temp_CF << (count - 1)) | (op1_32 >> (33 - count)); } @@ -333,11 +335,13 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::RCL_EdR(bxInstruction_c *i) Bit32u op1_32 = BX_READ_32BIT_REG(i->dst()); + Bit32u temp_CF = getB_CF(); + if (count==1) { - result_32 = (op1_32 << 1) | getB_CF(); + result_32 = (op1_32 << 1) | temp_CF; } else { - result_32 = (op1_32 << count) | (getB_CF() << (count - 1)) | + result_32 = (op1_32 << count) | (temp_CF << (count - 1)) | (op1_32 >> (33 - count)); } @@ -371,11 +375,13 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::RCR_EdM(bxInstruction_c *i) BX_NEXT_INSTR(i); } + Bit32u temp_CF = getB_CF(); + if (count==1) { - result_32 = (op1_32 >> 1) | (getB_CF() << 31); + result_32 = (op1_32 >> 1) | (temp_CF << 31); } else { - result_32 = (op1_32 >> count) | (getB_CF() << (32 - count)) | + result_32 = (op1_32 >> count) | (temp_CF << (32 - count)) | (op1_32 << (33 - count)); } @@ -408,11 +414,13 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::RCR_EdR(bxInstruction_c *i) Bit32u op1_32 = BX_READ_32BIT_REG(i->dst()); + Bit32u temp_CF = getB_CF(); + if (count==1) { - result_32 = (op1_32 >> 1) | (getB_CF() << 31); + result_32 = (op1_32 >> 1) | (temp_CF << 31); } else { - result_32 = (op1_32 >> count) | (getB_CF() << (32 - count)) | + result_32 = (op1_32 >> count) | (temp_CF << (32 - count)) | (op1_32 << (33 - count)); } diff --git a/bochs/cpu/shift64.cc b/bochs/cpu/shift64.cc index 6e387eae5..10671ccf0 100644 --- a/bochs/cpu/shift64.cc +++ b/bochs/cpu/shift64.cc @@ -287,11 +287,13 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::RCL_EqM(bxInstruction_c *i) BX_NEXT_INSTR(i); } + Bit64u temp_CF = getB_CF(); + if (count==1) { - result_64 = (op1_64 << 1) | getB_CF(); + result_64 = (op1_64 << 1) | temp_CF; } else { - result_64 = (op1_64 << count) | (getB_CF() << (count - 1)) | + result_64 = (op1_64 << count) | (temp_CF << (count - 1)) | (op1_64 >> (65 - count)); } @@ -323,11 +325,13 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::RCL_EqR(bxInstruction_c *i) Bit64u op1_64 = BX_READ_64BIT_REG(i->dst()); + Bit64u temp_CF = getB_CF(); + if (count==1) { - result_64 = (op1_64 << 1) | getB_CF(); + result_64 = (op1_64 << 1) | temp_CF; } else { - result_64 = (op1_64 << count) | (getB_CF() << (count - 1)) | + result_64 = (op1_64 << count) | (temp_CF << (count - 1)) | (op1_64 >> (65 - count)); } @@ -361,11 +365,13 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::RCR_EqM(bxInstruction_c *i) BX_NEXT_INSTR(i); } + Bit64u temp_CF = getB_CF(); + if (count==1) { - result_64 = (op1_64 >> 1) | (((Bit64u) getB_CF()) << 63); + result_64 = (op1_64 >> 1) | (temp_CF << 63); } else { - result_64 = (op1_64 >> count) | (getB_CF() << (64 - count)) | + result_64 = (op1_64 >> count) | (temp_CF << (64 - count)) | (op1_64 << (65 - count)); } @@ -397,11 +403,13 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::RCR_EqR(bxInstruction_c *i) Bit64u op1_64 = BX_READ_64BIT_REG(i->dst()); + Bit64u temp_CF = getB_CF(); + if (count==1) { - result_64 = (op1_64 >> 1) | (((Bit64u) getB_CF()) << 63); + result_64 = (op1_64 >> 1) | (temp_CF << 63); } else { - result_64 = (op1_64 >> count) | (getB_CF() << (64 - count)) | + result_64 = (op1_64 >> count) | (temp_CF << (64 - count)) | (op1_64 << (65 - count)); } diff --git a/bochs/cpu/shift8.cc b/bochs/cpu/shift8.cc index b23ae83a5..2655e46bc 100644 --- a/bochs/cpu/shift8.cc +++ b/bochs/cpu/shift8.cc @@ -201,11 +201,13 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::RCL_EbR(bxInstruction_c *i) Bit8u op1_8 = BX_READ_8BIT_REGx(i->dst(), i->extend8bitL()); + unsigned temp_CF = getB_CF(); + if (count==1) { - result_8 = (op1_8 << 1) | getB_CF(); + result_8 = (op1_8 << 1) | temp_CF; } else { - result_8 = (op1_8 << count) | (getB_CF() << (count - 1)) | + result_8 = (op1_8 << count) | (temp_CF << (count - 1)) | (op1_8 >> (9 - count)); } @@ -239,11 +241,13 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::RCL_EbM(bxInstruction_c *i) BX_NEXT_INSTR(i); } + unsigned temp_CF = getB_CF(); + if (count==1) { - result_8 = (op1_8 << 1) | getB_CF(); + result_8 = (op1_8 << 1) | temp_CF; } else { - result_8 = (op1_8 << count) | (getB_CF() << (count - 1)) | + result_8 = (op1_8 << count) | (temp_CF << (count - 1)) | (op1_8 >> (9 - count)); } @@ -271,7 +275,9 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::RCR_EbR(bxInstruction_c *i) if (count) { Bit8u op1_8 = BX_READ_8BIT_REGx(i->dst(), i->extend8bitL()); - Bit8u result_8 = (op1_8 >> count) | (getB_CF() << (8 - count)) | + unsigned temp_CF = getB_CF(); + + Bit8u result_8 = (op1_8 >> count) | (temp_CF << (8 - count)) | (op1_8 << (9 - count)); BX_WRITE_8BIT_REGx(i->dst(), i->extend8bitL(), result_8); @@ -301,7 +307,9 @@ BX_INSF_TYPE BX_CPP_AttrRegparmN(1) BX_CPU_C::RCR_EbM(bxInstruction_c *i) count = (count & 0x1f) % 9; if (count) { - Bit8u result_8 = (op1_8 >> count) | (getB_CF() << (8 - count)) | + unsigned temp_CF = getB_CF(); + + Bit8u result_8 = (op1_8 >> count) | (temp_CF << (8 - count)) | (op1_8 << (9 - count)); write_RMW_linear_byte(result_8);