From c3b76cd420dbcabb8c900d61adc2db5fa26f9e55 Mon Sep 17 00:00:00 2001 From: Mamy Ratsimbazafy Date: Sun, 22 Oct 2023 03:54:09 +0200 Subject: [PATCH] 32-bit fixes (#288) * fix the new div2n1n_vartime on 32-bit - regression from #286 * remove unnecessary defensive programming * reactivate 32-bit CI to check on #244 * 32-bit: centralize OS, ISA and env variable config * enable assemble on x86 32-bit --- .github/workflows/ci.yml | 18 ++++++----- benchmarks/bench_blueprint.nim | 2 +- benchmarks/bench_evm_modexp_dos.nim | 16 ++++++---- benchmarks/bench_fp_double_precision.nim | 2 +- constantine/ethereum_evm_precompiles.nim | 3 -- .../finite_fields_double_precision.nim | 2 +- constantine/platforms/abstractions.nim | 23 +++++--------- constantine/platforms/config.nim | 31 +++++++++++++++++++ .../platforms/constant_time/ct_types.nim | 9 ++---- .../platforms/constant_time/multiplexers.nim | 5 +-- .../intrinsics/addcarry_subborrow.nim | 4 ++- constantine/platforms/intrinsics/bitops.nim | 2 +- .../intrinsics/extended_precision.nim | 1 + .../extended_precision_64bit_uint128.nim | 6 ++-- .../intrinsics/extended_precision_vartime.nim | 25 ++++++++------- .../platforms/isa/macro_assembler_x86.nim | 2 +- .../platforms/isa/macro_assembler_x86_att.nim | 22 +++++++------ .../isa/macro_assembler_x86_intel.nim | 29 ++++++++++------- constantine/platforms/primitives.nim | 2 ++ .../t_bigints_mod.nim | 7 +++-- tests/t_ethereum_evm_modexp.nim | 2 +- 21 files changed, 126 insertions(+), 87 deletions(-) create mode 100644 constantine/platforms/config.nim diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 192164c..6a1921d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,21 +9,24 @@ jobs: matrix: nim_version: [version-1-6] # [version-1-4, devel] target: + - os: linux + cpu: i386 + TEST_LANG: c + BACKEND: NO_ASM + - os: linux + cpu: i386 + TEST_LANG: c + BACKEND: ASM + - os: linux cpu: amd64 TEST_LANG: c BACKEND: NO_ASM - - # 32-bit CI failing with https://github.com/mratsim/constantine/issues/244 - # "E: Unable to correct problems, you have held broken packages."" - # - os: linux - # cpu: i386 - # TEST_LANG: c - # BACKEND: NO_ASM - os: linux cpu: amd64 TEST_LANG: c BACKEND: ASM + - os: windows cpu: amd64 TEST_LANG: c @@ -32,6 +35,7 @@ jobs: cpu: amd64 TEST_LANG: c BACKEND: ASM + - os: macos cpu: amd64 TEST_LANG: c diff --git a/benchmarks/bench_blueprint.nim b/benchmarks/bench_blueprint.nim index ffe713e..a3a4bd5 100644 --- a/benchmarks/bench_blueprint.nim +++ b/benchmarks/bench_blueprint.nim @@ -60,7 +60,7 @@ echo " release: ", defined(release) echo " danger: ", defined(danger) echo " inline assembly: ", UseASM_X86_64 -when (sizeof(int) == 4) or defined(CTT_32): +when CTT_32: echo "⚠️ Warning: using Constantine with 32-bit limbs" else: echo "Using Constantine with 64-bit limbs" diff --git a/benchmarks/bench_evm_modexp_dos.nim b/benchmarks/bench_evm_modexp_dos.nim index 312e36c..b174faa 100644 --- a/benchmarks/bench_evm_modexp_dos.nim +++ b/benchmarks/bench_evm_modexp_dos.nim @@ -7,7 +7,8 @@ import proc report(op: string, elapsedNs: int64, elapsedCycles: int64, iters: int) = let ns = elapsedNs div iters - let cycles = elapsedCycles div iters + when SupportsGetTicks: + let cycles = elapsedCycles div iters let throughput = 1e9 / float64(ns) when SupportsGetTicks: echo &"{op:<70} {throughput:>15.3f} ops/s {ns:>16} ns/op {cycles:>12} CPU cycles (approx)" @@ -17,12 +18,15 @@ proc report(op: string, elapsedNs: int64, elapsedCycles: int64, iters: int) = template bench(fnCall: untyped, ticks, ns: var int64): untyped = block: let startTime = getMonotime() - let startClock = getTicks() + when SupportsGetTicks: + let startClock = getTicks() fnCall - let stopClock = getTicks() + when SupportsGetTicks: + let stopClock = getTicks() let stopTime = getMonotime() - ticks += stopClock - startClock + when SupportsGetTicks: + ticks += stopClock - startClock ns += inNanoseconds(stopTime-startTime) func computeGasFee(inputs: openArray[byte]): tuple[eip128, eip2565: int] = @@ -91,8 +95,8 @@ func computeGasFee(inputs: openArray[byte]): tuple[eip128, eip2565: int] = baseStop = baseStart+baseByteLen-1 expStart = baseStop+1 expStop = expStart+exponentByteLen-1 - modStart = expStop+1 - modStop = modStart+modulusByteLen-1 + # modStart = expStop+1 + # modStop = modStart+modulusByteLen-1 template exponent(): untyped = inputs.toOpenArray(expStart, expStop) diff --git a/benchmarks/bench_fp_double_precision.nim b/benchmarks/bench_fp_double_precision.nim index fb5d3fb..e6ae67c 100644 --- a/benchmarks/bench_fp_double_precision.nim +++ b/benchmarks/bench_fp_double_precision.nim @@ -61,7 +61,7 @@ echo " release: ", defined(release) echo " danger: ", defined(danger) echo " inline assembly: ", UseASM_X86_64 -when (sizeof(int) == 4) or defined(CTT_32): +when CTT_32: echo "⚠️ Warning: using Constantine with 32-bit limbs" else: echo "Using Constantine with 64-bit limbs" diff --git a/constantine/ethereum_evm_precompiles.nim b/constantine/ethereum_evm_precompiles.nim index aa73a25..53e398d 100644 --- a/constantine/ethereum_evm_precompiles.nim +++ b/constantine/ethereum_evm_precompiles.nim @@ -433,9 +433,6 @@ func eth_evm_modexp*(r: var openArray[byte], inputs: openArray[byte]): CttEVMSta if r.len != modulusByteLen: return cttEVM_InvalidOutputSize - if baseWordLen > modulusWordLen: - return cttEVM_InvalidInputSize - # Special cases # ---------------------- if paddedLengths.len + baseByteLen + exponentByteLen >= inputs.len: diff --git a/constantine/math/arithmetic/finite_fields_double_precision.nim b/constantine/math/arithmetic/finite_fields_double_precision.nim index c4c4131..4b270a7 100644 --- a/constantine/math/arithmetic/finite_fields_double_precision.nim +++ b/constantine/math/arithmetic/finite_fields_double_precision.nim @@ -136,7 +136,7 @@ func sum2xMod*(r: var FpDbl, a, b: FpDbl) = # Conditional reduction by 2ⁿp staticFor i, 0, N: - SecretBool(overflowed).ccopy(r.limbs2x[i+N], t[i]) + overflowed.ccopy(r.limbs2x[i+N], t[i]) func neg2xMod*(r: var FpDbl, a: FpDbl) = ## Double-precision modular substraction diff --git a/constantine/platforms/abstractions.nim b/constantine/platforms/abstractions.nim index dac0ddc..9128c7e 100644 --- a/constantine/platforms/abstractions.nim +++ b/constantine/platforms/abstractions.nim @@ -17,28 +17,19 @@ import ./metering/tracer export primitives, tracer -# ------------------------------------------------------------ - -const CTT_ASM {.booldefine.} = true -const UseASM_X86_32* = CTT_ASM and X86 and GCC_Compatible -const UseASM_X86_64* = sizeof(pointer)*8 == 64 and UseASM_X86_32 - -# We use Nim effect system to track vartime subroutines -type VarTime* = object - # ############################################################ # # Secret Words # # ############################################################ -when sizeof(int) == 8 and not defined(CTT_32): +when CTT_32: type - BaseType* = uint64 + BaseType* = uint32 ## Physical BigInt for conversion in "normal integers" else: type - BaseType* = uint32 + BaseType* = uint64 ## Physical BigInt for conversion in "normal integers" type @@ -131,12 +122,12 @@ debug: # Don't allow printing secret words by default type SignedSecretWord* = distinct SecretWord -when sizeof(int) == 8 and not defined(CTT_32): - type - SignedBaseType* = int64 -else: +when CTT_32: type SignedBaseType* = int32 +else: + type + SignedBaseType* = int64 template fmap(x: SignedSecretWord, op: untyped, y: SignedSecretWord): SignedSecretWord = ## Unwrap x and y from their distinct type diff --git a/constantine/platforms/config.nim b/constantine/platforms/config.nim new file mode 100644 index 0000000..190c6dc --- /dev/null +++ b/constantine/platforms/config.nim @@ -0,0 +1,31 @@ +# Constantine +# Copyright (c) 2018-2019 Status Research & Development GmbH +# Copyright (c) 2020-Present Mamy André-Ratsimbazafy +# Licensed and distributed under either of +# * MIT license (license terms in the root directory or at http://opensource.org/licenses/MIT). +# * Apache v2 license (license terms in the root directory or at http://www.apache.org/licenses/LICENSE-2.0). +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +# Compiler and CPU architecture configuration +# ------------------------------------------------------------ + +const GCC_Compatible* = defined(gcc) or defined(clang) or defined(llvm_gcc) +const X86* = defined(amd64) or defined(i386) + +when sizeof(int) == 8 and GCC_Compatible: + type + uint128*{.importc: "unsigned __int128".} = object + int128*{.importc: "__int128".} = object + +# Env variable configuration +# ------------------------------------------------------------ + +const CTT_ASM {.booldefine.} = true +const CTT_32* {.booldefine.} = bool(sizeof(pointer)*8 == 32) +const UseASM_X86_32* = CTT_ASM and X86 and GCC_Compatible +const UseASM_X86_64* = not(CTT_32) and UseASM_X86_32 + +when UseASM_X86_64: + static: doAssert bool(sizeof(pointer)*8 == 64), "Only 32-bit and 64-bit platforms are supported" + +const UseAsmSyntaxIntel* {.booldefine.} = true diff --git a/constantine/platforms/constant_time/ct_types.nim b/constantine/platforms/constant_time/ct_types.nim index c4bb049..1077743 100644 --- a/constantine/platforms/constant_time/ct_types.nim +++ b/constantine/platforms/constant_time/ct_types.nim @@ -33,10 +33,5 @@ type Carry* = Ct[uint8] # distinct range[0'u8 .. 1] Borrow* = Ct[uint8] # distinct range[0'u8 .. 1] -const GCC_Compatible* = defined(gcc) or defined(clang) or defined(llvm_gcc) -const X86* = defined(amd64) or defined(i386) - -when sizeof(int) == 8 and GCC_Compatible: - type - uint128*{.importc: "unsigned __int128".} = object - int128*{.importc: "__int128".} = object \ No newline at end of file + VarTime* = object + ## For use with Nim effect system to track vartime subroutines diff --git a/constantine/platforms/constant_time/multiplexers.nim b/constantine/platforms/constant_time/multiplexers.nim index a7faa43..9dbc81c 100644 --- a/constantine/platforms/constant_time/multiplexers.nim +++ b/constantine/platforms/constant_time/multiplexers.nim @@ -6,8 +6,9 @@ # * Apache v2 license (license terms in the root directory or at http://www.apache.org/licenses/LICENSE-2.0). # at your option. This file may not be copied, modified, or distributed except according to those terms. -import ./ct_types -import ../isa/macro_assembler_x86 +import + ./ct_types, + ../config # ############################################################ # diff --git a/constantine/platforms/intrinsics/addcarry_subborrow.nim b/constantine/platforms/intrinsics/addcarry_subborrow.nim index 01b6ea9..8109550 100644 --- a/constantine/platforms/intrinsics/addcarry_subborrow.nim +++ b/constantine/platforms/intrinsics/addcarry_subborrow.nim @@ -6,7 +6,9 @@ # * Apache v2 license (license terms in the root directory or at http://www.apache.org/licenses/LICENSE-2.0). # at your option. This file may not be copied, modified, or distributed except according to those terms. -import ../constant_time/ct_types +import + ../config, + ../constant_time/ct_types # ############################################################ # diff --git a/constantine/platforms/intrinsics/bitops.nim b/constantine/platforms/intrinsics/bitops.nim index 00a7796..3df0df3 100644 --- a/constantine/platforms/intrinsics/bitops.nim +++ b/constantine/platforms/intrinsics/bitops.nim @@ -6,7 +6,7 @@ # * Apache v2 license (license terms in the root directory or at http://www.apache.org/licenses/LICENSE-2.0). # at your option. This file may not be copied, modified, or distributed except according to those terms. -import ../constant_time/ct_types +import ../config when GCC_Compatible: func builtin_clz(n: uint32): cint {.importc: "__builtin_clz", nodecl.} diff --git a/constantine/platforms/intrinsics/extended_precision.nim b/constantine/platforms/intrinsics/extended_precision.nim index 927a821..02453d6 100644 --- a/constantine/platforms/intrinsics/extended_precision.nim +++ b/constantine/platforms/intrinsics/extended_precision.nim @@ -13,6 +13,7 @@ # ############################################################ import + ../config, ./addcarry_subborrow, ../constant_time/ct_types, ../constant_time/ct_routines diff --git a/constantine/platforms/intrinsics/extended_precision_64bit_uint128.nim b/constantine/platforms/intrinsics/extended_precision_64bit_uint128.nim index b8d2393..53894be 100644 --- a/constantine/platforms/intrinsics/extended_precision_64bit_uint128.nim +++ b/constantine/platforms/intrinsics/extended_precision_64bit_uint128.nim @@ -6,7 +6,9 @@ # * Apache v2 license (license terms in the root directory or at http://www.apache.org/licenses/LICENSE-2.0). # at your option. This file may not be copied, modified, or distributed except according to those terms. -import ../constant_time/ct_types +import + ../config, + ../constant_time/ct_types # ############################################################ # @@ -86,7 +88,7 @@ func smul*(hi, lo: var Ct[uint64], a, b: Ct[uint64]) {.inline.} = ## ## Inputs are intentionally unsigned ## as we use their unchecked raw representation for cryptography - ## + ## ## This is constant-time on most hardware ## See: https://www.bearssl.org/ctmul.html block: diff --git a/constantine/platforms/intrinsics/extended_precision_vartime.nim b/constantine/platforms/intrinsics/extended_precision_vartime.nim index 9a5021c..24a3425 100644 --- a/constantine/platforms/intrinsics/extended_precision_vartime.nim +++ b/constantine/platforms/intrinsics/extended_precision_vartime.nim @@ -15,8 +15,8 @@ import ../abstractions -func div2n1n_nim_vartime[T: SomeUnsignedInt](q, r: var T, n_hi, n_lo, d: T) {.tags:[VarTime].}= - ## Division uint128 by uint64 +func div2n1n_nim_vartime[T: SomeUnsignedInt](q, r: var T, n_hi, n_lo, d: T) {.used, tags:[VarTime].}= + ## Division uint128 by uint64 or uint64 by uint32 ## Warning ⚠️ : ## - if n_hi == d, quotient does not fit in an uint64 and will throw SIGFPE ## - if n_hi > d result is undefined @@ -59,7 +59,7 @@ func div2n1n_nim_vartime[T: SomeUnsignedInt](q, r: var T, n_hi, n_lo, d: T) {.ta q = (q1 shl halfSize) or q2 r = r2 -when sizeof(int) == 8 and defined(vcc): +when not(CTT_32) and defined(vcc): func udiv128_vartime(highDividend, lowDividend, divisor: uint64, remainder: var uint64): uint64 {.importc:"_udiv128", header: "", nodecl, tags:[VarTime].} ## Division 128 by 64, Microsoft only, 64-bit only, ## returns quotient as return value remainder as var parameter @@ -74,7 +74,7 @@ when sizeof(int) == 8 and defined(vcc): ## - if n_hi > d result is undefined q = udiv128_vartime(n_hi, n_lo, d, r) -elif sizeof(int) == 8 and GCC_Compatible: +elif not(CTT_32) and GCC_Compatible: type uint128{.importc: "unsigned __int128".} = object @@ -108,15 +108,16 @@ func div2n1n_vartime*(q, r: var SecretWord, n_hi, n_lo, d: SecretWord) {.inline. ## To avoid issues, n_hi, n_lo, d should be normalized. ## i.e. shifted (== multiplied by the same power of 2) ## so that the most significant bit in d is set. - when sizeof(int) == 4: + when CTT_32: let dividend = (uint64(n_hi) shl 32) or uint64(n_lo) let divisor = uint64(d) - q = uint32(dividend div divisor) - r = uint32(dividend mod divisor) - when nimvm: - div2n1n_nim_vartime(BaseType q, BaseType r, BaseType n_hi, BaseType n_lo, BaseType d) + q = SecretWord(dividend div divisor) + r = SecretWord(dividend mod divisor) else: - when declared(div2n1n_128_vartime): - div2n1n_128_vartime(BaseType q, BaseType r, BaseType n_hi, BaseType n_lo, BaseType d) - else: + when nimvm: div2n1n_nim_vartime(BaseType q, BaseType r, BaseType n_hi, BaseType n_lo, BaseType d) + else: + when declared(div2n1n_128_vartime): + div2n1n_128_vartime(BaseType q, BaseType r, BaseType n_hi, BaseType n_lo, BaseType d) + else: + div2n1n_nim_vartime(BaseType q, BaseType r, BaseType n_hi, BaseType n_lo, BaseType d) diff --git a/constantine/platforms/isa/macro_assembler_x86.nim b/constantine/platforms/isa/macro_assembler_x86.nim index e63c309..ba4e15e 100644 --- a/constantine/platforms/isa/macro_assembler_x86.nim +++ b/constantine/platforms/isa/macro_assembler_x86.nim @@ -6,7 +6,7 @@ # * Apache v2 license (license terms in the root directory or at http://www.apache.org/licenses/LICENSE-2.0). # at your option. This file may not be copied, modified, or distributed except according to those terms. -const UseAsmSyntaxIntel* {.booldefine.} = true +import ../config when UseAsmSyntaxIntel: # We need Intel syntax. diff --git a/constantine/platforms/isa/macro_assembler_x86_att.nim b/constantine/platforms/isa/macro_assembler_x86_att.nim index 01b0511..d63d3fb 100644 --- a/constantine/platforms/isa/macro_assembler_x86_att.nim +++ b/constantine/platforms/isa/macro_assembler_x86_att.nim @@ -6,7 +6,9 @@ # * Apache v2 license (license terms in the root directory or at http://www.apache.org/licenses/LICENSE-2.0). # at your option. This file may not be copied, modified, or distributed except according to those terms. -import std/[macros, strutils, sets, hashes, algorithm] +import + std/[macros, strutils, sets, hashes, algorithm], + ../config # A compile-time inline assembler @@ -39,15 +41,7 @@ type # Clobbered register ClobberedReg -when sizeof(int) == 8 and not defined(CTT_32): - type - Register* = enum - rbx - rdx - r8 - rax - xmm0 -else: +when CTT_32: type Register* = enum rbx = "ebx" @@ -55,6 +49,14 @@ else: r8 = "r8d" rax = "eax" xmm0 +else: + type + Register* = enum + rbx + rdx + r8 + rax + xmm0 type Constraint* = enum diff --git a/constantine/platforms/isa/macro_assembler_x86_intel.nim b/constantine/platforms/isa/macro_assembler_x86_intel.nim index ca8959b..4ada7d1 100644 --- a/constantine/platforms/isa/macro_assembler_x86_intel.nim +++ b/constantine/platforms/isa/macro_assembler_x86_intel.nim @@ -6,7 +6,9 @@ # * Apache v2 license (license terms in the root directory or at http://www.apache.org/licenses/LICENSE-2.0). # at your option. This file may not be copied, modified, or distributed except according to those terms. -import std/[macros, strutils, sets, hashes, algorithm] +import + std/[macros, strutils, sets, hashes, algorithm], + ../config # A compile-time inline assembler @@ -39,15 +41,7 @@ type # Clobbered register ClobberedReg -when sizeof(int) == 8 and not defined(CTT_32): - type - Register* = enum - rbx - rdx - r8 - rax - xmm0 -else: +when CTT_32: type Register* = enum rbx = "ebx" @@ -55,6 +49,14 @@ else: r8 = "r8d" rax = "eax" xmm0 +else: + type + Register* = enum + rbx + rdx + r8 + rax + xmm0 type Constraint* = enum @@ -464,14 +466,17 @@ func getStrOffset(a: Assembler_x86, op: Operand): string = if op.desc.rm in {Mem, MemOffsettable}: # Directly accessing memory if defined(gcc): + # https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#x86-Operand-Modifiers + # q: Print the DImode name of the register. + # k: Print the SImode name of the register. if a.wordBitWidth == 64: if op.offset == 0: return "%q" & op.desc.asmId return "%q" & op.desc.asmId & " + " & $(op.offset * a.wordSize) else: if op.offset == 0: - return "%d" & op.desc.asmId - return "%d" & op.desc.asmId & " + " & $(op.offset * a.wordSize) + return "%k" & op.desc.asmId + return "%k" & op.desc.asmId & " + " & $(op.offset * a.wordSize) elif defined(clang): if a.wordBitWidth == 64: if op.offset == 0: diff --git a/constantine/platforms/primitives.nim b/constantine/platforms/primitives.nim index 43836ca..56de579 100644 --- a/constantine/platforms/primitives.nim +++ b/constantine/platforms/primitives.nim @@ -7,6 +7,7 @@ # at your option. This file may not be copied, modified, or distributed except according to those terms. import + ./config, constant_time/[ ct_types, ct_routines, @@ -23,6 +24,7 @@ import ./allocs export + config, ct_types, ct_routines, multiplexers, diff --git a/tests/math_arbitrary_precision/t_bigints_mod.nim b/tests/math_arbitrary_precision/t_bigints_mod.nim index 2d839ae..6c17f3c 100644 --- a/tests/math_arbitrary_precision/t_bigints_mod.nim +++ b/tests/math_arbitrary_precision/t_bigints_mod.nim @@ -4,7 +4,8 @@ import ../../constantine/math/[ arithmetic, io/io_bigints], - ../../constantine/math_arbitrary_precision/arithmetic/limbs_divmod_vartime + ../../constantine/math_arbitrary_precision/arithmetic/limbs_divmod_vartime, + ../../constantine/platforms/abstractions let a = BigInt[64].fromUint(0xa0e5cb56a1c08396'u64) let M = BigInt[64].fromUint(0xae57180eceb0206f'u64) @@ -14,9 +15,9 @@ var r, r2: BigInt[64] r.reduce(a, M) doAssert r2.limbs.reduce_vartime(a.limbs, M.limbs) -let rU64 = 0xa0e5cb56a1c08396'u64 mod 0xae57180eceb0206f'u64 +let rBase = cast[BaseType](0xa0e5cb56a1c08396'u64) mod cast[BaseType](0xae57180eceb0206f'u64) # echo r.toHex() -doAssert rU64 == a.limbs[0].uint64 +doAssert rBase == a.limbs[0].BaseType doAssert bool(a == r) echo "SUCCESS: t_bigints_mod.nim" \ No newline at end of file diff --git a/tests/t_ethereum_evm_modexp.nim b/tests/t_ethereum_evm_modexp.nim index ddecfb8..f6dfcbb 100644 --- a/tests/t_ethereum_evm_modexp.nim +++ b/tests/t_ethereum_evm_modexp.nim @@ -206,7 +206,7 @@ suite "EVM ModExp precompile (EIP-198)": 0xa0] var r = newSeq[byte](1) let status = r.eth_evm_modexp(input) - doAssert status == cttEVM_Success + doAssert status == cttEVM_Success, "Failure status: " & $status test "Audit #6 - DOS Vector 1": let input = [