From 89aba8092595f18946f5b30bf5d84749b69f13e4 Mon Sep 17 00:00:00 2001 From: tevador <37503146+tevador@users.noreply.github.com> Date: Wed, 3 Jul 2019 18:13:20 +0200 Subject: [PATCH] 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 | 4 ++++ src/assembly_generator_x86.cpp | 2 +- src/blake2_generator.cpp | 2 +- src/blake2_generator.hpp | 2 +- src/bytecode_machine.cpp | 2 +- src/common.hpp | 2 +- src/jit_compiler_x86.cpp | 4 ++-- src/superscalar.cpp | 15 ++++++++------- src/tests/perf-simulation.cpp | 2 +- 9 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/aes_hash.cpp b/src/aes_hash.cpp index a5c0797..896395e 100644 --- a/src/aes_hash.cpp +++ b/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 #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 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(const void *input, size_t inputSize, void *hash) */ template 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(void *state, size_t outputSize, void *buffer); template 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; diff --git a/src/assembly_generator_x86.cpp b/src/assembly_generator_x86.cpp index 645fd9d..e7e5258 100644 --- a/src/assembly_generator_x86.cpp +++ b/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; diff --git a/src/blake2_generator.cpp b/src/blake2_generator.cpp index dcf51ce..3f2d028 100644 --- a/src/blake2_generator.cpp +++ b/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; diff --git a/src/blake2_generator.hpp b/src/blake2_generator.hpp index b5ac080..5e7f61f 100644 --- a/src/blake2_generator.hpp +++ b/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); diff --git a/src/bytecode_machine.cpp b/src/bytecode_machine.cpp index 20c4e10..7d8e902 100644 --- a/src/bytecode_machine.cpp +++ b/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]; diff --git a/src/common.hpp b/src/common.hpp index 0c504fe..39bd8ed 100644 --- a/src/common.hpp +++ b/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; } diff --git a/src/jit_compiler_x86.cpp b/src/jit_compiler_x86.cpp index 054a171..81e0ef7 100644 --- a/src/jit_compiler_x86.cpp +++ b/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)); diff --git a/src/superscalar.cpp b/src/superscalar.cpp index 39d772f..b3c52c0 100644 --- a/src/superscalar.cpp +++ b/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(); - } while ((imm32_ & (imm32_ - 1)) == 0); + imm32_ = gen.getUInt32(); + } while (isZeroOrPowerOf2(imm32_)); opGroup_ = SuperscalarInstructionType::IMUL_RCP; opGroupPar_ = -1; } break; diff --git a/src/tests/perf-simulation.cpp b/src/tests/perf-simulation.cpp index 1f9b3f2..1068a40 100644 --- a/src/tests/perf-simulation.cpp +++ b/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;