Nim icon indicating copy to clipboard operation
Nim copied to clipboard

static (?) - Codegen bug / Memory Corruption same type in Nim, different type in C

Open mratsim opened this issue 7 years ago • 2 comments
trafficstars

This is a very involved bug that requires 5 different files. The issue is that the same type is different in different files:

  1. The file to run mem_corrupt_bug.nim
# File mem_corrupt_bug

import  ./mem_corrupt_bug_add,
        ./mem_corrupt_bug_mul

when isMainModule:

  import typetraits
  import ./mem_corrupt_bug_convert

  let a = toMpUint(10'u32)

  echo "a: " & $a
  echo "a+a: " & $(a+a)

  let z = a * a
  echo "a * a: " & $z # How did the result value change?
  echo "a * a type: " & $z.type.name

  # Compile without release: memory corruption
  # In release: no corruption
  # Comment out the "naiveMul" in mul_impl: no corruption
  echo "Is memory corrupted: " & $(z != toMpUint(100'u32))

# Output on my machine
#
# a: (lo: 10, hi: 0)
# +: (lo: 20, hi: 0)
# a+a: (lo: 20, hi: 0)
# naiveMul cast16:(lo: 100, hi: 0)
# naiveMul cast16:(lo: 0, hi: 0)
# naiveMul cast16:(lo: 0, hi: 0)
# +: (lo: 0, hi: 0)
# Within `*` result: (lo: 100, hi: 0)
# Within `*` result type: MpUint[32]
# a * a: (lo: 100, hi: 3924)
# a * a type: MpUint[32]
# Is memory corrupted: true
  1. The type declaration mem_corrupt_bug_type.nim
# File mem_corrupt_bug_type

type
  # TODO: The following is a hacky workaround
  # due to:
  #   - https://github.com/nim-lang/Nim/issues/7230
  #   - https://github.com/nim-lang/Nim/issues/7378
  #   - https://github.com/nim-lang/Nim/issues/7379

  BitsHolder[bits: static[int]] = object

type
  MpUintImpl[bh] = object
    when bh is BitsHolder[32]: lo*, hi*: uint16
    elif bh is BitsHolder[16]: lo*, hi*: uint8

  MpUint*[bits: static[int]] = MpUintImpl[BitsHolder[bits]]
  1. Conversion of uint to MpUint mem_corrupt_bug_convert.nim Interesting things happen here
import ./mem_corrupt_bug_type

# Commenting the uint16 proc (that is never called)
# Or
# Removing the inline pragma of the uint32 proc solves the issue

proc toMpUint*(n: uint16): MpUint[16] {.inline.}=
  ## Cast an integer to the corresponding size MpUint
  cast[MpUint[16]](n)

proc toMpUint*(n: uint32): MpUint[32] {.inline.}=
  ## Cast an integer to the corresponding size MpUint
  cast[MpUint[32]](n)
  1. Multi-precision addition mem_corrupt_bug_add.nim
# File mem_corrupt_bug_add

import ./mem_corrupt_bug_type

proc `+=`*(x: var MpUint, y: MpUint) =
  ## In-place addition for multi-precision unsigned int
  type SubT = type x.lo
  let tmp = x.lo

  x.lo += y.lo
  x.hi += SubT(x.lo < tmp) + y.hi

proc `+`*(x, y: MpUint): MpUint =
  # Addition for multi-precision unsigned int
  result = x
  result += y

  debugEcho "+: " & $result
  1. Multiplication mem_corrupt_bug_mul.nim
# File mem_corrupt_bug_mul

import ./mem_corrupt_bug_type, ./mem_corrupt_bug_convert
import ./mem_corrupt_bug_add

import typetraits

# Comment the following to remove the memory corruption
proc naiveMul*(x, y: uint8): MpUint[16] =
  # Multiplication in extended precision
  result = toMpuint(x.uint16 * y.uint16)

proc naiveMul*(x, y: uint16): MpUint[32] =
  # Multiplication in extended precision
  result = toMpuint(x.uint32 * y.uint32)
  debugEcho "naiveMul cast16:" & $result

proc `*`*(x, y: MpUint): MpUint =
  ## Multiplication for multi-precision unsigned uint

  result = naiveMul(x.lo, y.lo)
  result.hi += (naiveMul(x.hi, y.lo) + naiveMul(x.lo, y.hi)).lo

  debugEcho "Within `*` result: " & $result
  debugEcho "Within `*` result type: " & $result.type.name

The C file mpint_mem_corrupt_bug_mul.c

...
#undef unix
typedef struct tyObject_MpUintImpl_SMV2Q1UFq9a33CCVE59b3rog tyObject_MpUintImpl_SMV2Q1UFq9a33CCVE59b3rog;
typedef struct NimStringDesc NimStringDesc;
typedef struct TGenericSeq TGenericSeq;
struct tyObject_MpUintImpl_SMV2Q1UFq9a33CCVE59b3rog {
NU8 lo;
NU8 hi;
};

The C file mpint_mem_corrupt_bug.c

...
#undef unix
typedef struct tyObject_MpUintImpl_SMV2Q1UFq9a33CCVE59b3rog tyObject_MpUintImpl_SMV2Q1UFq9a33CCVE59b3rog;
typedef struct NimStringDesc NimStringDesc;
typedef struct TGenericSeq TGenericSeq;
struct tyObject_MpUintImpl_SMV2Q1UFq9a33CCVE59b3rog {
NU16 lo;
NU16 hi;
};

Notice the NU8 / NU16 change for the same struct

mratsim avatar Mar 20 '18 16:03 mratsim