Refactoring (#95)

* Blake2Generator::getInt32 renamed to getUInt32 to avoid confusion
* isPowerOf2 renamed to isZeroOrPowerOf2 to avoid confusion
* added asserts to validate the input/output size of AES functions
* fixed possible overflow in JitCompilerX86::getCodeSize (unused function)

src/aes_hash.cpp

@@ -27,6 +27,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */
 
 #include "soft_aes.h"
+#include <cassert>
 
 #define AES_HASH_1R_STATE0 0xd7983aad, 0xcc82db47, 0x9fa856de, 0x92b52c0d
 #define AES_HASH_1R_STATE1 0xace78057, 0xf59e125a, 0x15c7b798, 0x338d996e
@@ -50,6 +51,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */
 template<bool softAes>
 void hashAes1Rx4(const void *input, size_t inputSize, void *hash) {
+	assert(inputSize % 64 == 0);
 	const uint8_t* inptr = (uint8_t*)input;
 	const uint8_t* inputEnd = inptr + inputSize;
 
@@ -118,6 +120,7 @@ template void hashAes1Rx4<true>(const void *input, size_t inputSize, void *hash)
 */
 template<bool softAes>
 void fillAes1Rx4(void *state, size_t outputSize, void *buffer) {
+	assert(outputSize % 64 == 0);
 	const uint8_t* outptr = (uint8_t*)buffer;
 	const uint8_t* outputEnd = outptr + outputSize;
 
@@ -168,6 +171,7 @@ template void fillAes1Rx4<false>(void *state, size_t outputSize, void *buffer);
 
 template<bool softAes>
 void fillAes4Rx4(void *state, size_t outputSize, void *buffer) {
+	assert(outputSize % 64 == 0);
 	const uint8_t* outptr = (uint8_t*)buffer;
 	const uint8_t* outputEnd = outptr + outputSize;
 

src/assembly_generator_x86.cpp

@@ -446,7 +446,7 @@ namespace randomx {
 
 	void AssemblyGeneratorX86::h_IMUL_RCP(Instruction& instr, int i) {
 		uint64_t divisor = instr.getImm32();
-		if (!isPowerOf2(divisor)) {
+		if (!isZeroOrPowerOf2(divisor)) {
 			registerUsage[instr.dst] = i;
 			asmCode << "\tmov rax, " << randomx_reciprocal(divisor) << std::endl;
 			asmCode << "\timul " << regR[instr.dst] << ", rax" << std::endl;

src/blake2_generator.cpp

@@ -46,7 +46,7 @@ namespace randomx {
 		return data[dataIndex++];
 	}
 
-	uint32_t Blake2Generator::getInt32() {
+	uint32_t Blake2Generator::getUInt32() {
 		checkData(4);
 		auto ret = load32(&data[dataIndex]);
 		dataIndex += 4;

src/blake2_generator.hpp

@@ -36,7 +36,7 @@ namespace randomx {
 	public:
 		Blake2Generator(const void* seed, size_t seedSize, int nonce = 0);
 		uint8_t getByte();
-		uint32_t getInt32();
+		uint32_t getUInt32();
 	private:
 		void checkData(const size_t);
 

src/bytecode_machine.cpp

@@ -244,7 +244,7 @@ namespace randomx {
 
 		if (opcode < ceil_IMUL_RCP) {
 			uint64_t divisor = instr.getImm32();
-			if (!isPowerOf2(divisor)) {
+			if (!isZeroOrPowerOf2(divisor)) {
 				auto dst = instr.dst % RegistersCount;
 				ibc.type = InstructionType::IMUL_R;
 				ibc.idst = &nreg->r[dst];

src/common.hpp

@@ -145,7 +145,7 @@ namespace randomx {
 	constexpr int RegisterNeedsDisplacement = 5; //x86 r13 register
 	constexpr int RegisterNeedsSib = 4; //x86 r12 register
 
-	inline bool isPowerOf2(uint64_t x) {
+	inline bool isZeroOrPowerOf2(uint64_t x) {
 		return (x & (x - 1)) == 0;
 	}
 

src/jit_compiler_x86.cpp

@@ -197,7 +197,7 @@ namespace randomx {
 	static const uint8_t* NOPX[] = { NOP1, NOP2, NOP3, NOP4, NOP5, NOP6, NOP7, NOP8 };
 
 	size_t JitCompilerX86::getCodeSize() {
-		return codePos - prologueSize;
+		return codePos < prologueSize ? 0 : codePos - prologueSize;
 	}
 
 	JitCompilerX86::JitCompilerX86() {
@@ -580,7 +580,7 @@ namespace randomx {
 
 	void JitCompilerX86::h_IMUL_RCP(Instruction& instr, int i) {
 		uint64_t divisor = instr.getImm32();
-		if (!isPowerOf2(divisor)) {
+		if (!isZeroOrPowerOf2(divisor)) {
 			registerUsage[instr.dst] = i;
 			emit(MOV_RAX_I);
 			emit64(randomx_reciprocal_fast(divisor));

src/superscalar.cpp

@@ -37,6 +37,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include "superscalar.hpp"
 #include "intrin_portable.h"
 #include "reciprocal.h"
+#include "common.hpp"
 
 namespace randomx {
 
@@ -334,7 +335,7 @@ namespace randomx {
 			return false;
 
 		if (availableRegisters.size() > 1) {
-			index = gen.getInt32() % availableRegisters.size();
+			index = gen.getUInt32() % availableRegisters.size();
 		}
 		else {
 			index = 0;
@@ -447,7 +448,7 @@ namespace randomx {
 			case SuperscalarInstructionType::IADD_C8:
 			case SuperscalarInstructionType::IADD_C9: {
 				mod_ = 0;
-				imm32_ = gen.getInt32();
+				imm32_ = gen.getUInt32();
 				opGroup_ = SuperscalarInstructionType::IADD_C7;
 				opGroupPar_ = -1;
 			} break;
@@ -456,7 +457,7 @@ namespace randomx {
 			case SuperscalarInstructionType::IXOR_C8:
 			case SuperscalarInstructionType::IXOR_C9: {
 				mod_ = 0;
-				imm32_ = gen.getInt32();
+				imm32_ = gen.getUInt32();
 				opGroup_ = SuperscalarInstructionType::IXOR_C7;
 				opGroupPar_ = -1;
 			} break;
@@ -466,7 +467,7 @@ namespace randomx {
 				mod_ = 0;
 				imm32_ = 0;
 				opGroup_ = SuperscalarInstructionType::IMULH_R;
-				opGroupPar_ = gen.getInt32();
+				opGroupPar_ = gen.getUInt32();
 			} break;
 
 			case SuperscalarInstructionType::ISMULH_R: {
@@ -474,14 +475,14 @@ namespace randomx {
 				mod_ = 0;
 				imm32_ = 0;
 				opGroup_ = SuperscalarInstructionType::ISMULH_R;
-				opGroupPar_ = gen.getInt32();
+				opGroupPar_ = gen.getUInt32();
 			} break;
 
 			case SuperscalarInstructionType::IMUL_RCP: {
 				mod_ = 0;
 				do {
-					imm32_ = gen.getInt32();
+					imm32_ = gen.getUInt32();
-				} while ((imm32_ & (imm32_ - 1)) == 0);
+				} while (isZeroOrPowerOf2(imm32_));
 				opGroup_ = SuperscalarInstructionType::IMUL_RCP;
 				opGroupPar_ = -1;
 			} break;

src/tests/perf-simulation.cpp

@@ -478,7 +478,7 @@ int analyze(randomx::Program& p) {
 
 		if (opcode < randomx::ceil_IMUL_RCP) {
 			uint64_t divisor = instr.getImm32();
-			if (!randomx::isPowerOf2(divisor)) {
+			if (!randomx::isZeroOrPowerOf2(divisor)) {
 				instr.dst = instr.dst % randomx::RegistersCount;
 				instr.opcode |= DST_INT;
 				registerUsage[instr.dst].lastUsed = i;