nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.44k stars 1.47k forks source link

[ARC] Incorrect sinking of an argument? #14233

Closed ghost closed 4 years ago

ghost commented 4 years ago

This code works fine with the default GC but SIGSEGV's with ARC. Found in https://github.com/trustable-code/NiGui.

Example

type
  Control = ref object of RootObj
    fOnMouseButtonDown: MouseButtonProc

  ControlImpl = ref object of Control

  MouseEvent = ref object
    control: Control
    x: int
    y: int
    button: int

  MouseButtonProc = proc(event: MouseEvent)

proc handleMouseButtonDownEvent(control: Control, event: MouseEvent) = 
  let callback = control.fOnMouseButtonDown
  if callback != nil:
    callback(event)

proc pDefaultControlButtonPressSignal(widget: pointer, event: MouseEvent, data: pointer): bool =
  let control = cast[ControlImpl](data)
  let x = event.x.int
  let y = event.y.int
  var evt = new MouseEvent
  evt.control = control
  case event.button
  of 1: evt.button = 1
  of 2: evt.button = 2
  of 3: evt.button = 3
  else: return
  evt.x = x
  evt.y = y

  control.handleMouseButtonDownEvent(evt)

var c: ControlImpl = new ControlImpl

c.fOnMouseButtonDown = proc(event: MouseEvent) = 
  echo "click"

let cptr = cast[pointer](Control(c))
let cimplptr = cast[pointer](c)
let evt = MouseEvent(control: c, button: 3)

discard pDefaultControlButtonPressSignal(cptr, evt, cimplptr)

Current Output

Traceback (most recent call last)
/home/dian/Projects/data/af.nim(45) af
/home/dian/Projects/data/af.nim(34) pDefaultControlButtonPressSignal
/home/dian/Projects/data/af.nim(16) handleMouseButtonDownEvent
SIGSEGV: Illegal storage access. (Attempt to read from nil?)

Expected Output

click
$ nim -v
Nim Compiler Version 1.3.1 [Linux: amd64]
Compiled at 2020-05-05
Copyright (c) 2006-2020 by Andreas Rumpf

git hash: e86a6d24d5ffe43e9dc7f230b80c274167376225
active boot switches: -d:release
ghost commented 4 years ago

Reduced a bit more:

type
  Control = ref object of RootObj
    fOnMouseButtonDown: MouseButtonProc

  MouseEvent = ref object
    control: Control
    button: int

  MouseButtonProc = proc(event: MouseEvent)

proc handleMouseButtonDownEvent(control: Control, event: MouseEvent) = 
  let callback = control.fOnMouseButtonDown
  if callback != nil:
    callback(event)

proc pDefaultControlButtonPressSignal(widget: Control, event: MouseEvent, data: Control): bool =
  let control = data
  var evt = MouseEvent()
  evt.control = control
  case event.button
  of 1: evt.button = 1
  of 2: evt.button = 2
  of 3: evt.button = 3
  # Works with else: discard instead
  else: return

  control.handleMouseButtonDownEvent(evt)

var c = Control()

c.fOnMouseButtonDown = proc(event: MouseEvent) = 
  echo "click"

discard pDefaultControlButtonPressSignal(c, MouseEvent(control: c, button: 3), c)
ghost commented 4 years ago

Simplified even more:

type
  Control = ref object
    x: int

  MouseEvent = ref object
    control: Control
    button: int

proc run(data: Control) =
  var evt = MouseEvent(button: 1)
  evt.control = data
  if evt.button == 1:
    discard
  else: 
    return

  echo data.x

var c = Control()

run(c)