Improved test_insn_xN_xN semantics

[AlexAltea]

• Removed unnecessary readonly_dst parameter and enforced identical destination in each test case (only affects the test instruction so far).

• Templated over integer parameters N and M representing the width of both instruction operands: test_insn_xN_xM. Unless explicitly specified, M will be equal to N by default.

• Enforced sign-aware truncation of immediate types via the imm_t type. To verify this, the second operand in BT/BTC/BTR/BTS has changed from 0x7F to 0xFF, in their very last test case (this operand gets casted to -1).

Signed-off-by: Alexandro Sanchez Bach [email protected]

[AlexAltea]

The main reason, in my opinion, to remove readonly_dst is that the test_insn_xN_xN functions were supposed to encode only the syntax of the instructions (i.e. number and type of operands), and not their behavior (i.e. effect on source and destination operands).

The latter can be applied directly at test-case level by simply copying in_src to in_dst.

[raphaelning]

Thanks! I learned some useful C++11 syntax along the way :-)

To verify this, the second operand in BT/BTC/BTR/BTS has changed from 0x7F to 0xFF, in their very last test case (this operand gets casted to -1).

I get the part where 0xFF is cast to -1, but Intel SDM doesn't seem to allow this... It only allows the Register BitOffset to be negative, when BitBase is a memory address. See Vol. 2A Table 3-2: Range of Bit Positions Specified by Bit Offset Operands, as well as Figure 3-2: Memory Bit Indexing.

[AlexAltea]

All done. I've also fixed the snprintf argument indentation in the functions below test_insn_xN_xN_xN, and in test_insn_rN_rM, i.e. all remaining cases. Changes are force-pushed on top of the existing commit.

Also, ignore the build issue. This is due to a Chocolatey package update, and is fixed in #133.
Probably that PR should be merged before this or any other patches.

---

I get the part where 0xFF is cast to -1, but Intel SDM doesn't seem to allow this....

I've noticed it, but this seems in contradiction with Intel SDM Vol. 2A: BT—Bit Test (and BTC, BTR, BTS). Quoting below:

If the bit base operand specifies a register, the instruction takes the modulo 16, 32, or 64 of the bit offset operand (modulo size depends on the mode and register size; 64-bit operands are available only in 64-bit mode). This allows any bit position to be selected.

The section you are quoting, Intel SDM Vol. 2A: Table 3-2: Range of Bit Positions Specified by Bit Offset Operands, mentions:

If the BitBase is a register, the BitOffset can be in the range 0 to [15, 31, 63] depending on the mode and register size.

Probably, rather than a contradiction what the SDM is trying to say (for BT/BTC/BTR/BTS) is that the modulo of the bit offset is taken before applying the Bit "macro", i,e, Bit(BitBase, BitOffset % Width), so the Bit "macro" always receives an operand in range 0 to [15, 31, 63]. If so, specifying 0xFF/-1 should be fine.

EDIT: This also seems to match with the behavior on my CPU:

[raphaelning]

Thanks! I agree with your interpretation of the BT rN, imm8 case, which you have also verified by experiment. I still wonder what should be the correct behavior of BT mN, imm8 when imm8 is negative BT mN, rN when rN stores a negative value. Does the CPU take the bit from an address lower than mN (BitBase), as Figure 3-2 suggests? Or does it honor the operand size (N) and translate imm8 rN (BitOffset) to an offset between [0, N)? My bet is on the latter. Or maybe we already know the answer, since test_bt<64> with 0xFF yields the expected result for test_insn_mN_iM<64>. Is the test_cpu_t::mem array initialized to all-zero?

EDIT: Oops, sorry, wrong question. BT mN, imm8 is unambiguous, because Table 3-2 has made it clear that 0 <= Immediate BitOffset < N. But I'm still curious about BT mN, rN (rN < 0). We don't have a test case for this yet (which is fine; this is a corner case and we don't need to be pedantic).

[raphaelning]

I'd like to test this PR using Travis CI (since #133 has added support for running the unit tests), but I don't know how to make it retry the build... If there's no easy way, I'll just test it manually.

[raphaelning]

Ah, this time Travis rebuild started instantly, and finished with flying colors.

[AlexAltea]

But I'm still curious about BT mN, rN (rN < 0). We don't have a test case for this yet (which is fine; this is a corner case and we don't need to be pedantic).

@raphaelning You are right, the scenario bt* mN, rN has an edge-case that we are emulating wrong, since it can go out of bounds whenever rN < 0 or rN ≥ N. Thanks for the finding! See:

The problem here is that our "fast emulator" works by converting memory-operands into register-operands, which unfortunately have a different behavior here. This seems a weird design choice of x86, but we have to comply with it. The best approach will be, I think, adding INSN_BITOP to the instruction flags and modifying resulting memory address based on the offset operand.

If you agree, I'll implement it that way and prepare more tests for BT/BTC/BTR/BTS.

(EDIT: Fixed in #135)

---

Also, note that this out-of-bounds behaviour does not occur on bt* rN, rN/i8 (due to modulo operation), or bt* mN, i8 (as you said, due to constraints of the immediate operand specified in Table 3-2, although internally it does modulo, see image below).