Properly register threads with the Boehm GC.

In order to be able to scan thread stacks, the Boehm GC needs to know
about newly created threads. We establish the end of the stack by using
GC_call_with_stack_base (this works properly also with the dual-stack
Itanium architecture) and then GC_register_my_thread() to register a
thrad and GC_unregister_my_thread() to unregister it again.

This patch also includes a modification for the refc and markandsweep
collectors to set the stack bottom for thread stacks correctly even if
an optimizer aggressively inlines and optimizes procedures (this is
already being done for the stack of the main thread).

Finally, we use the {.noconv.} pragma for the Boehm GC, as the Boehm
API uses no specific calling convention.

lib/system.nim

@@ -1200,6 +1200,15 @@ const
   hasSharedHeap = defined(boehmgc) or defined(gogc) # don't share heaps; every thread has its own
   taintMode = compileOption("taintmode")
 
+when defined(boehmgc):
+  when defined(windows):
+    const boehmLib = "boehmgc.dll"
+  elif defined(macosx):
+    const boehmLib = "libgc.dylib"
+  else:
+    const boehmLib = "libgc.so.1"
+  {.pragma: boehmGC, noconv, dynlib: boehmLib.}
+
 when taintMode:
   type TaintedString* = distinct string ## a distinct string type that
                                         ## is `tainted`:idx:. It is an alias for

lib/system/mmdisp.nim

@@ -66,41 +66,34 @@ proc raiseOutOfMem() {.noinline.} =
   quit(1)
 
 when defined(boehmgc):
-  when defined(windows):
+  proc boehmGCinit {.importc: "GC_init", boehmGC.}
-    const boehmLib = "boehmgc.dll"
+  proc boehmGC_disable {.importc: "GC_disable", boehmGC.}
-  elif defined(macosx):
+  proc boehmGC_enable {.importc: "GC_enable", boehmGC.}
-    const boehmLib = "libgc.dylib"
-  else:
-    const boehmLib = "libgc.so.1"
-
-  proc boehmGCinit {.importc: "GC_init", dynlib: boehmLib.}
-  proc boehmGC_disable {.importc: "GC_disable", dynlib: boehmLib.}
-  proc boehmGC_enable {.importc: "GC_enable", dynlib: boehmLib.}
   proc boehmGCincremental {.
-    importc: "GC_enable_incremental", dynlib: boehmLib.}
+    importc: "GC_enable_incremental", boehmGC.}
-  proc boehmGCfullCollect {.importc: "GC_gcollect", dynlib: boehmLib.}
+  proc boehmGCfullCollect {.importc: "GC_gcollect", boehmGC.}
-  proc boehmAlloc(size: int): pointer {.
+  proc boehmAlloc(size: int): pointer {.importc: "GC_malloc", boehmGC.}
-    importc: "GC_malloc", dynlib: boehmLib.}
   proc boehmAllocAtomic(size: int): pointer {.
-    importc: "GC_malloc_atomic", dynlib: boehmLib.}
+    importc: "GC_malloc_atomic", boehmGC.}
   proc boehmRealloc(p: pointer, size: int): pointer {.
-    importc: "GC_realloc", dynlib: boehmLib.}
+    importc: "GC_realloc", boehmGC.}
-  proc boehmDealloc(p: pointer) {.importc: "GC_free", dynlib: boehmLib.}
+  proc boehmDealloc(p: pointer) {.importc: "GC_free", boehmGC.}
+  when hasThreadSupport:
+    proc boehmGC_allow_register_threads {.
+      importc: "GC_allow_register_threads", boehmGC.}
 
-  proc boehmGetHeapSize: int {.importc: "GC_get_heap_size", dynlib: boehmLib.}
+  proc boehmGetHeapSize: int {.importc: "GC_get_heap_size", boehmGC.}
     ## Return the number of bytes in the heap.  Excludes collector private
     ## data structures. Includes empty blocks and fragmentation loss.
     ## Includes some pages that were allocated but never written.
 
-  proc boehmGetFreeBytes: int {.importc: "GC_get_free_bytes", dynlib: boehmLib.}
+  proc boehmGetFreeBytes: int {.importc: "GC_get_free_bytes", boehmGC.}
     ## Return a lower bound on the number of free bytes in the heap.
 
-  proc boehmGetBytesSinceGC: int {.importc: "GC_get_bytes_since_gc",
+  proc boehmGetBytesSinceGC: int {.importc: "GC_get_bytes_since_gc", boehmGC.}
-    dynlib: boehmLib.}
     ## Return the number of bytes allocated since the last collection.
 
-  proc boehmGetTotalBytes: int {.importc: "GC_get_total_bytes",
+  proc boehmGetTotalBytes: int {.importc: "GC_get_total_bytes", boehmGC.}
-    dynlib: boehmLib.}
     ## Return the total number of bytes allocated in this process.
     ## Never decreases.
 
@@ -158,6 +151,8 @@ when defined(boehmgc):
 
   proc initGC() =
     boehmGCinit()
+    when hasThreadSupport:
+      boehmGC_allow_register_threads()
 
   proc newObj(typ: PNimType, size: int): pointer {.compilerproc.} =
     if ntfNoRefs in typ.flags: result = allocAtomic(size)

lib/system/threads.nim

@@ -304,22 +304,53 @@ type
 when not defined(boehmgc) and not hasSharedHeap and not defined(gogc):
   proc deallocOsPages()
 
-template threadProcWrapperBody(closure: expr) {.immediate.} =
+when defined(boehmgc):
-  when declared(globalsSlot): threadVarSetValue(globalsSlot, closure)
+  type GCStackBaseProc = proc(sb: pointer, t: pointer) {.noconv.}
-  var t = cast[ptr Thread[TArg]](closure)
+  proc boehmGC_call_with_stack_base(sbp: GCStackBaseProc, p: pointer)
-  when useStackMaskHack:
+    {.importc: "GC_call_with_stack_base", boehmGC.}
-    var tls: ThreadLocalStorage
+  proc boehmGC_register_my_thread(sb: pointer)
-  when not defined(boehmgc) and not defined(gogc) and not defined(nogc) and not hasSharedHeap:
+    {.importc: "GC_register_my_thread", boehmGC.}
-    # init the GC for this thread:
+  proc boehmGC_unregister_my_thread()
-    setStackBottom(addr(t))
+    {.importc: "GC_unregister_my_thread", boehmGC.}
+
+  proc threadProcWrapDispatch[TArg](sb: pointer, thrd: pointer) {.noconv.} =
+    boehmGC_register_my_thread(sb)
+    let thrd = cast[ptr Thread[TArg]](thrd)
+    when TArg is void:
+      thrd.dataFn()
+    else:
+      thrd.dataFn(thrd.data)
+    boehmGC_unregister_my_thread()
+else:
+  proc threadProcWrapDispatch[TArg](thrd: ptr Thread[TArg]) =
+    when TArg is void:
+      thrd.dataFn()
+    else:
+      thrd.dataFn(thrd.data)
+
+proc threadProcWrapStackFrame[TArg](thrd: ptr Thread[TArg]) =
+  when defined(boehmgc):
+    boehmGC_call_with_stack_base(threadProcWrapDispatch[TArg], thrd)
+  elif not defined(nogc) and not defined(gogc):
+    var p {.volatile.}: proc(a: ptr Thread[TArg]) {.nimcall.} =
+      threadProcWrapDispatch[TArg]
+    when not hasSharedHeap:
+      # init the GC for refc/markandsweep
+      setStackBottom(addr(p))
       initGC()
     when declared(registerThread):
-    t.stackBottom = addr(t)
+      thrd.stackBottom = addr(thrd)
-    registerThread(t)
+      registerThread(thrd)
-  when TArg is void: t.dataFn()
+    p(thrd)
-  else: t.dataFn(t.data)
+    when declared(registerThread): unregisterThread(thrd)
-  when declared(registerThread): unregisterThread(t)
     when declared(deallocOsPages): deallocOsPages()
+  else:
+    threadProcWrapDispatch(thrd)
+
+template threadProcWrapperBody(closure: expr) {.immediate.} =
+  when declared(globalsSlot): threadVarSetValue(globalsSlot, closure)
+  var thrd = cast[ptr Thread[TArg]](closure)
+  threadProcWrapStackFrame(thrd)
   # Since an unhandled exception terminates the whole process (!), there is
   # no need for a ``try finally`` here, nor would it be correct: The current
   # exception is tried to be re-raised by the code-gen after the ``finally``!
@@ -327,7 +358,7 @@ template threadProcWrapperBody(closure: expr) {.immediate.} =
   # page!
 
   # mark as not running anymore:
-  t.dataFn = nil
+  thrd.dataFn = nil
 
 {.push stack_trace:off.}
 when defined(windows):