Skip to content
This repository was archived by the owner on Jan 28, 2023. It is now read-only.

Conversation

@AlexAltea
Copy link
Contributor

@AlexAltea AlexAltea commented Nov 21, 2018

Fixes issue reported at #132 (comment).

The bit-test instructions BT/BTC/BTR/BTS, through the variant bt* mN,rN, allow selecting bytes outside the value referenced by the memory operand. This is now checked for all instructions flagged with INSN_BITOP. Corresponding tests have been added as well (they have been cross-checked with tests run on my CPU).

See also: Intel SDM Vol. 2A: Figure 3-2. Memory Bit Indexing.

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

@raphaelning
Copy link
Contributor

@AlexAltea Thanks. I’m traveling and AFK. May not be able to give you much feedback on these PRs until next Monday...

@AlexAltea
Copy link
Contributor Author

Have a nice trip! Tomorrow I'll head to HITB until Novermber 30th, so I'll be mostly AFK until then as well.

@AlexAltea
Copy link
Contributor Author

All done.

Once this passes your review, I'll squash it into a single commit (the first one) if you want.

Copy link
Contributor

@raphaelning raphaelning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once this passes your review, I'll squash it into a single commit (the first one) if you want.

Thanks, please do. I think the removal of ea &= 0xFF is worth mentioning in the commit message.

The bit-test instructions BT/BTC/BTR/BTS, through the variant `bt* mN,rN`, allow selecting bytes outside the value referenced by the memory operand. This is now checked for all instructions flagged with INSN_BITOP. Corresponding tests have been added as well. See also: Intel SDM Vol. 2A: Figure 3-2. Memory Bit Indexing.

Additionally, the test_read_memory and test_write_memory functions have
dropped the `ea &= 0xFF` line since to prevent subtle bugs related to
memory addressing. The correctness of the memory address will be
verified.

Signed-off-by: Alexandro Sanchez Bach <[email protected]>
@AlexAltea
Copy link
Contributor Author

Once this passes your review, I'll squash it into a single commit (the first one) if you want.

Thanks, please do. I think the removal of ea &= 0xFF is worth mentioning in the commit message.

Done!

@wcwang wcwang merged commit 98c8126 into intel:master Dec 4, 2018
@AlexAltea AlexAltea deleted the fix-bitop branch December 4, 2018 10:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants