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.

3 files changed, 73 insertions, 38 deletions

diff --git a/lib/system.nim b/lib/system.nim
index 3d7d4bd28..e0bfbe8ea 100644
--- a/lib/system.nim
+++ b/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
diff --git a/lib/system/mmdisp.nim b/lib/system/mmdisp.nim
index bf006f68a..ecf25ceee 100644
--- a/lib/system/mmdisp.nim
+++ b/lib/system/mmdisp.nim
@@ -66,41 +66,34 @@ proc raiseOutOfMem() {.noinline.} =
   quit(1)
 
 when defined(boehmgc):
-  when defined(windows):
-    const boehmLib = "boehmgc.dll"
-  elif defined(macosx):
-    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 boehmGCinit {.importc: "GC_init", boehmGC.}
+  proc boehmGC_disable {.importc: "GC_disable", boehmGC.}
+  proc boehmGC_enable {.importc: "GC_enable", boehmGC.}
   proc boehmGCincremental {.
-    importc: "GC_enable_incremental", dynlib: boehmLib.}
-  proc boehmGCfullCollect {.importc: "GC_gcollect", dynlib: boehmLib.}
-  proc boehmAlloc(size: int): pointer {.
-    importc: "GC_malloc", dynlib: boehmLib.}
+    importc: "GC_enable_incremental", boehmGC.}
+  proc boehmGCfullCollect {.importc: "GC_gcollect", boehmGC.}
+  proc boehmAlloc(size: int): pointer {.importc: "GC_malloc", boehmGC.}
   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.}
-  proc boehmDealloc(p: pointer) {.importc: "GC_free", dynlib: boehmLib.}
+    importc: "GC_realloc", boehmGC.}
+  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",
-    dynlib: boehmLib.}
+  proc boehmGetBytesSinceGC: int {.importc: "GC_get_bytes_since_gc", boehmGC.}
     ## Return the number of bytes allocated since the last collection.
 
-  proc boehmGetTotalBytes: int {.importc: "GC_get_total_bytes",
-    dynlib: boehmLib.}
+  proc boehmGetTotalBytes: int {.importc: "GC_get_total_bytes", boehmGC.}
     ## 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)
diff --git a/lib/system/threads.nim b/lib/system/threads.nim
index c7cb8d9df..c5de841f8 100644
--- a/lib/system/threads.nim
+++ b/lib/system/threads.nim
@@ -304,22 +304,53 @@ type
 when not defined(boehmgc) and not hasSharedHeap and not defined(gogc):
   proc deallocOsPages()
 
+when defined(boehmgc):
+  type GCStackBaseProc = proc(sb: pointer, t: pointer) {.noconv.}
+  proc boehmGC_call_with_stack_base(sbp: GCStackBaseProc, p: pointer)
+    {.importc: "GC_call_with_stack_base", boehmGC.}
+  proc boehmGC_register_my_thread(sb: pointer)
+    {.importc: "GC_register_my_thread", boehmGC.}
+  proc boehmGC_unregister_my_thread()
+    {.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):
+      thrd.stackBottom = addr(thrd)
+      registerThread(thrd)
+    p(thrd)
+    when declared(registerThread): unregisterThread(thrd)
+    when declared(deallocOsPages): deallocOsPages()
+  else:
+    threadProcWrapDispatch(thrd)
+
 template threadProcWrapperBody(closure: expr) {.immediate.} =
   when declared(globalsSlot): threadVarSetValue(globalsSlot, closure)
-  var t = cast[ptr Thread[TArg]](closure)
-  when useStackMaskHack:
-    var tls: ThreadLocalStorage
-  when not defined(boehmgc) and not defined(gogc) and not defined(nogc) and not hasSharedHeap:
-    # init the GC for this thread:
-    setStackBottom(addr(t))
-    initGC()
-  when declared(registerThread):
-    t.stackBottom = addr(t)
-    registerThread(t)
-  when TArg is void: t.dataFn()
-  else: t.dataFn(t.data)
-  when declared(registerThread): unregisterThread(t)
-  when declared(deallocOsPages): deallocOsPages()
+  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):