Skip to content

Commit 14a58af

Browse files
lkorenclkorenc
authored
Explicitly call __remill_undef for undefined arith flags. (#543)
* x86: Explicitly call __remill_undef for undefined arith flags. * x86: Remove hack needed for pop [rsp] and introduce new semantic fn instead. * test: Update test definitions with proper ignore flags. * test: Propagate name of tests into gtest framework for better messages. * arch: Make assert more verbose on failure.
1 parent d8d3b6c commit 14a58af

File tree

19 files changed

+129
-28
lines changed

19 files changed

+129
-28
lines changed

‎lib/Arch/Arch.cpp‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -688,14 +688,23 @@ const Register *Arch::AddRegister(const char *reg_name_, llvm::Type *val_type,
688688
const_cast<Register *>(reg->parent)->children.push_back(reg);
689689
}
690690

691+
auto maybe_get_reg_name = [](auto reg_ptr) -> std::string {
692+
if (!reg_ptr) {
693+
return "(nullptr)";
694+
}
695+
return reg_ptr->name;
696+
};
697+
691698
// Provide easy access to registers at specific offsets in the `State`
692699
// structure.
693700
for (auto i = reg->offset; i < (reg->offset + reg->size); ++i) {
694701
auto &reg_at_offset = impl->reg_by_offset[i];
695702
if (!reg_at_offset) {
696703
reg_at_offset = reg;
697704
} else if (reg_at_offset) {
698-
CHECK_EQ(reg_at_offset->EnclosingRegister(), reg->EnclosingRegister());
705+
CHECK_EQ(reg_at_offset->EnclosingRegister(), reg->EnclosingRegister())
706+
<< maybe_get_reg_name(reg_at_offset->EnclosingRegister()) << " != "
707+
<< maybe_get_reg_name(reg->EnclosingRegister());;
699708
reg_at_offset = reg;
700709
}
701710
}

‎lib/Arch/X86/Arch.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -467,9 +467,9 @@ static void DecodeMemory(Instruction &inst, const xed_decoded_inst_t *xedd,
467467
auto segment_reg = ignore_segment(deduce_segment(raw_segment_reg));
468468

469469
// Special case: `POP [xSP + ...] uses the value of `xSP` after incrementing
470-
// it by the stack width.
470+
// it by the stack width. For more reasoning see definition of semantics for POP.
471471
if (XED_ICLASS_POP == iclass && XED_REG_RSP == base_wide) {
472-
disp += static_cast<int64_t>(size / 8);
472+
inst.function = "POP_MEM_XSP_" + std::to_string(size);
473473
}
474474

475475
Operand op = {};

‎lib/Arch/X86/Semantics/BINARY.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,10 +248,10 @@ ALWAYS_INLINE static void WriteFlagsMul(State &state, T lhs, T rhs, U res,
248248
V res_trunc) {
249249
const auto new_of = Overflow<tag_mul>::Flag(lhs, rhs, res);
250250
FLAG_CF = new_of;
251-
FLAG_PF = ParityFlag(res); // Technically undefined.
251+
FLAG_PF = BUndefined(); // Technically undefined.
252252
FLAG_AF = BUndefined();
253253
FLAG_ZF = BUndefined();
254-
FLAG_SF = std::is_signed<T>::value ? SignFlag(res_trunc) : BUndefined();
254+
FLAG_SF = BUndefined();
255255
FLAG_OF = new_of;
256256
}
257257

‎lib/Arch/X86/Semantics/BITBYTE.cpp‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -148,12 +148,23 @@ DEF_ISEL(SETBE_GPR8) = SETBE<R8W>;
148148

149149
namespace {
150150

151+
152+
#define _BTClearUndefFlags() \
153+
do { \
154+
UndefFlag(of); \
155+
UndefFlag(sf); \
156+
UndefFlag(zf); \
157+
UndefFlag(af); \
158+
UndefFlag(pf); \
159+
} while (false)
160+
151161
template <typename S1, typename S2>
152162
DEF_SEM(BTreg, S1 src1, S2 src2) {
153163
auto val = Read(src1);
154164
auto bit = ZExtTo<S1>(Read(src2));
155165
auto bit_mask = UShl(Literal<S1>(1), URem(bit, BitSizeOf(src1)));
156166
Write(FLAG_CF, UCmpNeq(UAnd(val, bit_mask), Literal<S1>(0)));
167+
_BTClearUndefFlags();
157168
return memory;
158169
}
159170

@@ -164,6 +175,7 @@ DEF_SEM(BTmem, S1 src1, S2 src2) {
164175
auto index = UDiv(bit, BitSizeOf(src1));
165176
auto val = Read(GetElementPtr(src1, index));
166177
Write(FLAG_CF, UCmpNeq(UAnd(val, bit_mask), Literal<S1>(0)));
178+
_BTClearUndefFlags();
167179
return memory;
168180
}
169181

@@ -174,6 +186,7 @@ DEF_SEM(BTSreg, D dst, S1 src1, S2 src2) {
174186
auto bit_mask = UShl(Literal<S1>(1), URem(bit, BitSizeOf(val)));
175187
WriteZExt(dst, UOr(val, bit_mask));
176188
Write(FLAG_CF, UCmpNeq(UAnd(val, bit_mask), Literal<S1>(0)));
189+
_BTClearUndefFlags();
177190
return memory;
178191
}
179192

@@ -185,6 +198,7 @@ DEF_SEM(BTSmem, D dst, S1 src1, S2 src2) {
185198
auto val = Read(GetElementPtr(src1, index));
186199
Write(GetElementPtr(dst, index), UOr(val, bit_mask));
187200
Write(FLAG_CF, UCmpNeq(UAnd(val, bit_mask), Literal<S1>(0)));
201+
_BTClearUndefFlags();
188202
return memory;
189203
}
190204

@@ -195,6 +209,7 @@ DEF_SEM(BTRreg, D dst, S1 src1, S2 src2) {
195209
auto bit_mask = UShl(Literal<S1>(1), URem(bit, BitSizeOf(src1)));
196210
WriteZExt(dst, UAnd(val, UNot(bit_mask)));
197211
Write(FLAG_CF, UCmpNeq(UAnd(val, bit_mask), Literal<S1>(0)));
212+
_BTClearUndefFlags();
198213
return memory;
199214
}
200215

@@ -206,6 +221,7 @@ DEF_SEM(BTRmem, D dst, S1 src1, S2 src2) {
206221
auto val = Read(GetElementPtr(src1, index));
207222
Write(GetElementPtr(dst, index), UAnd(val, UNot(bit_mask)));
208223
Write(FLAG_CF, UCmpNeq(UAnd(val, bit_mask), Literal<S1>(0)));
224+
_BTClearUndefFlags();
209225
return memory;
210226
}
211227

@@ -216,6 +232,7 @@ DEF_SEM(BTCreg, D dst, S1 src1, S2 src2) {
216232
auto bit_mask = UShl(Literal<S1>(1), URem(bit, BitSizeOf(val)));
217233
WriteZExt(dst, UXor(val, bit_mask));
218234
Write(FLAG_CF, UCmpNeq(UAnd(val, bit_mask), Literal<S1>(0)));
235+
_BTClearUndefFlags();
219236
return memory;
220237
}
221238

@@ -227,9 +244,12 @@ DEF_SEM(BTCmem, D dst, S1 src1, S2 src2) {
227244
auto val = Read(GetElementPtr(src1, index));
228245
Write(GetElementPtr(dst, index), UXor(val, bit_mask));
229246
Write(FLAG_CF, UCmpNeq(UAnd(val, bit_mask), Literal<S1>(0)));
247+
_BTClearUndefFlags();
230248
return memory;
231249
}
232250

251+
#undef _BTClearUndefFlags
252+
233253
} // namespace
234254

235255
DEF_ISEL_Mn_In(BT_MEMv_IMMb, BTmem);
@@ -323,11 +343,11 @@ DEF_SEM(BSR, D dst, S src) {
323343
auto index = USub(USub(BitSizeOf(src), count), Literal<S>(1));
324344
Write(FLAG_ZF, ZeroFlag(val));
325345
auto index_ret = Select(FLAG_ZF, Read(dst), ZExtTo<D>(index));
326-
Write(FLAG_OF, false); // Undefined, but experimentally 0.
327-
Write(FLAG_SF, false); // Undefined, but experimentally 0.
346+
Write(FLAG_OF, BUndefined()); // Undefined, but experimentally 0.
347+
Write(FLAG_SF, BUndefined()); // Undefined, but experimentally 0.
328348
Write(FLAG_PF, ParityFlag(index)); // Undefined, but experimentally 1.
329-
Write(FLAG_AF, false); // Undefined, but experimentally 0.
330-
Write(FLAG_CF, false); // Undefined, but experimentally 0.
349+
Write(FLAG_AF, BUndefined()); // Undefined, but experimentally 0.
350+
Write(FLAG_CF, BUndefined()); // Undefined, but experimentally 0.
331351
Write(dst, index_ret);
332352
return memory;
333353
}
@@ -337,11 +357,11 @@ DEF_SEM(BSF, D dst, S src) {
337357
auto val = Read(src);
338358
Write(FLAG_ZF, ZeroFlag(val));
339359
auto index = Select(FLAG_ZF, Read(dst), ZExtTo<D>(CountTrailingZeros(val)));
340-
Write(FLAG_OF, false); // Undefined, but experimentally 0.
341-
Write(FLAG_SF, false); // Undefined, but experimentally 0.
360+
Write(FLAG_OF, BUndefined()); // Undefined, but experimentally 0.
361+
Write(FLAG_SF, BUndefined()); // Undefined, but experimentally 0.
342362
Write(FLAG_PF, ParityFlag(index));
343-
Write(FLAG_AF, false); // Undefined, but experimentally 0.
344-
Write(FLAG_CF, false); // Undefined, but experimentally 0.
363+
Write(FLAG_AF, BUndefined()); // Undefined, but experimentally 0.
364+
Write(FLAG_CF, BUndefined()); // Undefined, but experimentally 0.
345365
Write(dst, index);
346366
return memory;
347367
}

‎lib/Arch/X86/Semantics/FLAGS.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,8 @@ struct Carry<tag_sub> {
172172

173173
} // namespace
174174

175+
#define UndefFlag(name) do { state.aflag.name = __remill_undefined_8(); } while (false)
176+
175177
#define ClearArithFlags() \
176178
do { \
177179
state.aflag.cf = __remill_undefined_8(); \

‎lib/Arch/X86/Semantics/LOGICAL.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ DEF_SEM(OR, D dst, S1 src1, S2 src2) {
4545
auto res = UOr(lhs, rhs);
4646
WriteZExt(dst, res);
4747
SetFlagsLogical(state, lhs, rhs, res);
48+
UndefFlag(af);
4849
return memory;
4950
}
5051

@@ -55,6 +56,7 @@ DEF_SEM(XOR, D dst, S1 src1, S2 src2) {
5556
auto res = UXor(lhs, rhs);
5657
WriteZExt(dst, res);
5758
SetFlagsLogical(state, lhs, rhs, res);
59+
UndefFlag(af);
5860
return memory;
5961
}
6062

@@ -70,6 +72,7 @@ DEF_SEM(TEST, S1 src1, S2 src2) {
7072
auto rhs = Read(src2);
7173
auto res = UAnd(lhs, rhs);
7274
SetFlagsLogical(state, lhs, rhs, res);
75+
UndefFlag(af);
7376
return memory;
7477
}
7578

‎lib/Arch/X86/Semantics/POP.cpp‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,22 @@ DEF_SEM(POP, D dst) {
3232
return memory;
3333
}
3434

35+
template<typename D>
36+
DEF_SEM(POP_MEM_XSP, D dst) {
37+
addr_t op_size = ZExtTo<D>(ByteSizeOf(dst));
38+
addr_t old_xsp = Read(REG_XSP);
39+
addr_t new_xsp = UAdd(old_xsp, op_size);
40+
41+
// `XSP` will be adjusted by `op_size`. Unfortunately, `dst` at this point
42+
// no longer has any information about the fact it was composed of `XPS`.
43+
// This semantic is a special case of general `POP` and it makes sure that
44+
// the adjustment happens correctly.
45+
// See #PR for more details.
46+
Write(REG_XSP, new_xsp);
47+
WriteZExt(D{dst.addr + op_size}, Read(ReadPtr<D>(old_xsp _IF_32BIT(REG_SS_BASE))));
48+
return memory;
49+
}
50+
3551
#if 32 == ADDRESS_SIZE_BITS
3652
DEF_SEM(DoPOPA) {
3753
Write(REG_DI, PopFromStack<uint16_t>(memory, state));
@@ -123,6 +139,9 @@ DEF_ISEL(POP_GPRv_58_16) = POP<R16W>;
123139
DEF_ISEL_R32or64W(POP_GPRv_51, POP);
124140
DEF_ISEL_R32or64W(POP_GPRv_58, POP);
125141

142+
DEF_ISEL(POP_MEM_XSP_16) = POP_MEM_XSP<M16W>;
143+
DEF_ISEL_M32or64W(POP_MEM_XSP, POP_MEM_XSP);
144+
126145
DEF_ISEL(POP_MEMv_16) = POP<M16W>;
127146
DEF_ISEL_M32or64W(POP_MEMv, POP);
128147

‎lib/Arch/X86/Semantics/ROTATE.cpp‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@ DEF_SEM(ROL, D dst, S1 src1, S2 src2) {
3636
Write(FLAG_CF, UCmpEq(UAnd(new_val, one), one));
3737
if (1 == temp_count) {
3838
Write(FLAG_OF, BXor(FLAG_CF, SignFlag(new_val)));
39+
// OF undefined for `1 != temp_count`.
3940
} else {
40-
Write(FLAG_OF, false);
41+
Write(FLAG_OF, BUndefined());
4142
}
42-
// OF undefined for `1 == temp_count`.
4343
} else {
4444
WriteZExt(dst, val);
4545
}
@@ -62,9 +62,12 @@ DEF_SEM(ROR, D dst, S1 src1, S2 src2) {
6262
UOr(UShr(val, temp_count), UShl(val, USub(op_size, temp_count)));
6363
WriteZExt(dst, new_val);
6464
Write(FLAG_CF, SignFlag(new_val));
65-
Write(FLAG_OF, BXor(FLAG_CF, SignFlag(UShl(new_val, one))));
65+
// OF undefined for `1 != temp_count`.
66+
if (temp_count == 1)
67+
Write(FLAG_OF, BXor(FLAG_CF, SignFlag(UShl(new_val, one))));
68+
else
69+
Write(FLAG_OF, BUndefined());
6670

67-
// OF undefined for `1 == temp_count`.
6871
} else {
6972
WriteZExt(dst, val);
7073
}
@@ -146,9 +149,13 @@ DEF_SEM(RCL, D dst, S1 src1, S2 src2) {
146149
UShr(right, one));
147150
WriteZExt(dst, new_val);
148151
Write(FLAG_CF, SignFlag(UShl(val, USub(temp_count, one))));
149-
Write(FLAG_OF, BXor(FLAG_CF, SignFlag(new_val)));
152+
// OF undefined for `1 != temp_count`.
153+
if (temp_count == 1) {
154+
Write(FLAG_OF, BXor(FLAG_CF, SignFlag(new_val)));
155+
} else {
156+
Write(FLAG_OF, BUndefined());
157+
}
150158

151-
// OF undefined for `1 == temp_count`.
152159
} else {
153160
WriteZExt(dst, val);
154161
}
@@ -180,7 +187,11 @@ DEF_SEM(RCR, D dst, S1 src1, S2 src2) {
180187
UShl(right, one));
181188
WriteZExt(dst, new_val);
182189
Write(FLAG_CF, UCmpNeq(UAnd(left, one), zero));
183-
Write(FLAG_OF, BXor(SignFlag(UShl(new_val, one)), SignFlag(new_val)));
190+
if (temp_count == 1) {
191+
Write(FLAG_OF, BXor(SignFlag(UShl(new_val, one)), SignFlag(new_val)));
192+
} else {
193+
Write(FLAG_OF, BUndefined());
194+
}
184195

185196
// OF undefined for `1 == temp_count`.
186197
} else {

‎lib/Arch/X86/Semantics/SHIFT.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,15 +148,16 @@ DEF_SEM(SHL, D dst, S1 src1, S2 src2) {
148148
new_val = UShl(res, 1);
149149

150150
} else {
151-
new_of = 1; // Undefined, probably 1.
151+
new_of = BUndefined(); // Undefined, probably 1.
152+
// TODO(lukas): Double check.
152153
new_cf = 0; // Undefined, probably 0.
153154
new_val = 0;
154155
}
155156

156157
WriteZExt(dst, new_val);
157158
Write(FLAG_CF, new_cf);
158159
Write(FLAG_PF, ParityFlag(new_val));
159-
Write(FLAG_AF, false); // Undefined, experimentally 0.
160+
Write(FLAG_AF, BUndefined()); // Undefined, experimentally 0.
160161
Write(FLAG_ZF, ZeroFlag(new_val));
161162
Write(FLAG_SF, SignFlag(new_val));
162163
Write(FLAG_OF, new_of);

‎tests/X86/BITBYTE/BT.S‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616

1717
TEST_BEGIN(BTr16i8, 1)
18+
TEST_IGNORE_FLAGS(SF ZF AF PF OF)
1819
TEST_INPUTS(
1920
0,
2021
1)
@@ -23,6 +24,7 @@ TEST_INPUTS(
2324
TEST_END
2425

2526
TEST_BEGIN(BTr16r16, 2)
27+
TEST_IGNORE_FLAGS(SF ZF AF PF OF)
2628
TEST_INPUTS(
2729
0, 0,
2830
1, 0,
@@ -34,6 +36,7 @@ TEST_INPUTS(
3436
TEST_END
3537

3638
TEST_BEGIN(BTr32i8, 1)
39+
TEST_IGNORE_FLAGS(OF SF ZF AF PF)
3740
TEST_INPUTS(
3841
0,
3942
1)
@@ -42,6 +45,7 @@ TEST_INPUTS(
4245
TEST_END
4346

4447
TEST_BEGIN(BTr32r32, 2)
48+
TEST_IGNORE_FLAGS(OF SF ZF AF PF)
4549
TEST_INPUTS(
4650
0, 0,
4751
1, 0,
@@ -56,6 +60,7 @@ TEST_INPUTS(
5660
TEST_END
5761

5862
TEST_BEGIN_64(BTr32i8_64, 1)
63+
TEST_IGNORE_FLAGS(OF SF ZF AF PF)
5964
TEST_INPUTS(
6065
0,
6166
1)
@@ -64,6 +69,7 @@ TEST_INPUTS(
6469
TEST_END_64
6570

6671
TEST_BEGIN_64(BTr64r64_64, 2)
72+
TEST_IGNORE_FLAGS(OF SF ZF AF PF)
6773
TEST_INPUTS(
6874
0, 0,
6975
1, 0,
@@ -81,6 +87,7 @@ TEST_INPUTS(
8187
TEST_END_64
8288

8389
TEST_BEGIN_64(BTm16r16, 1)
90+
TEST_IGNORE_FLAGS(OF SF ZF AF PF)
8491
TEST_INPUTS(
8592
0,
8693
1,
@@ -96,6 +103,7 @@ TEST_INPUTS(
96103
TEST_END_64
97104

98105
TEST_BEGIN_64(BTm32r32, 1)
106+
TEST_IGNORE_FLAGS(OF SF ZF AF PF)
99107
TEST_INPUTS(
100108
0,
101109
1,
@@ -111,6 +119,7 @@ TEST_INPUTS(
111119
TEST_END_64
112120

113121
TEST_BEGIN_64(BTm64r64_64, 1)
122+
TEST_IGNORE_FLAGS(OF SF ZF AF PF)
114123
TEST_INPUTS(
115124
0,
116125
1,

0 commit comments

Comments
 (0)