[dev.cc] runtime: delete scalararg, ptrarg; rename onM to systemstack

Scalararg and ptrarg are not "signal safe".
Go code filling them out can be interrupted by a signal,
and then the signal handler runs, and if it also ends up
in Go code that uses scalararg or ptrarg, now the old
values have been smashed.
For the pieces of code that do need to run in a signal handler,
we introduced onM_signalok, which is really just onM
except that the _signalok is meant to convey that the caller
asserts that scalarg and ptrarg will be restored to their old
values after the call (instead of the usual behavior, zeroing them).

Scalararg and ptrarg are also untyped and therefore error-prone.

Go code can always pass a closure instead of using scalararg
and ptrarg; they were only really necessary for C code.
And there's no more C code.

For all these reasons, delete scalararg and ptrarg, converting
the few remaining references to use closures.

Once those are gone, there is no need for a distinction between
onM and onM_signalok, so replace both with a single function
equivalent to the current onM_signalok (that is, it can be called
on any of the curg, g0, and gsignal stacks).

The name onM and the phrase 'm stack' are misnomers,
because on most system an M has two system stacks:
the main thread stack and the signal handling stack.

Correct the misnomer by naming the replacement function systemstack.

Fix a few references to "M stack" in code.

The main motivation for this change is to eliminate scalararg/ptrarg.
Rick and I have already seen them cause problems because
the calling sequence m.ptrarg[0] = p is a heap pointer assignment,
so it gets a write barrier. The write barrier also uses onM, so it has
all the same problems as if it were being invoked by a signal handler.
We worked around this by saving and restoring the old values
and by calling onM_signalok, but there's no point in keeping this nice
home for bugs around any longer.

This CL also changes funcline to return the file name as a result
instead of filling in a passed-in *string. (The *string signature is
left over from when the code was written in and called from C.)
That's arguably an unrelated change, except that once I had done
the ptrarg/scalararg/onM cleanup I started getting false positives
about the *string argument escaping (not allowed in package runtime).
The compiler is wrong, but the easiest fix is to write the code like
Go code instead of like C code. I am a bit worried that the compiler
is wrong because of some use of uninitialized memory in the escape
analysis. If that's the reason, it will go away when we convert the
compiler to Go. (And if not, we'll debug it the next time.)

LGTM=khr
R=r, khr
CC=austin, golang-codereviews, iant, rlh
https://codereview.appspot.com/174950043

1 files changed, 16 insertions, 44 deletions

diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index 569bf5ddc..0bb735355 100644
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -1058,7 +1058,7 @@ func sweepone() uintptr {
 
 func gosweepone() uintptr {
 	var ret uintptr
-	onM(func() {
+	systemstack(func() {
 		ret = sweepone()
 	})
 	return ret
@@ -1152,7 +1152,7 @@ func updatememstats(stats *gcstats) {
 	}
 
 	// Flush MCache's to MCentral.
-	onM(flushallmcaches)
+	systemstack(flushallmcaches)
 
 	// Aggregate local stats.
 	cachestats()
@@ -1193,13 +1193,6 @@ func updatememstats(stats *gcstats) {
 	memstats.heap_objects = memstats.nmalloc - memstats.nfree
 }
 
-// Structure of arguments passed to function gc().
-// This allows the arguments to be passed via mcall.
-type gc_args struct {
-	start_time int64 // start time of GC in ns (just before stoptheworld)
-	eagersweep bool
-}
-
 func gcinit() {
 	if unsafe.Sizeof(workbuf{}) != _WorkbufSize {
 		gothrow("runtime: size of Workbuf is suboptimal")
@@ -1211,21 +1204,18 @@ func gcinit() {
 	gcbssmask = unrollglobgcprog((*byte)(unsafe.Pointer(&gcbss)), uintptr(unsafe.Pointer(&ebss))-uintptr(unsafe.Pointer(&bss)))
 }
 
-func gc_m() {
+func gc_m(start_time int64, eagersweep bool) {
 	_g_ := getg()
 	gp := _g_.m.curg
 	casgstatus(gp, _Grunning, _Gwaiting)
 	gp.waitreason = "garbage collection"
 
-	var a gc_args
-	a.start_time = int64(_g_.m.scalararg[0]) | int64(uintptr(_g_.m.scalararg[1]))<<32
-	a.eagersweep = _g_.m.scalararg[2] != 0
-	gc(&a)
+	gc(start_time, eagersweep)
 
 	if nbadblock > 0 {
 		// Work out path from root to bad block.
 		for {
-			gc(&a)
+			gc(start_time, eagersweep)
 			if nbadblock >= int32(len(badblock)) {
 				gothrow("cannot find path to bad pointer")
 			}
@@ -1235,7 +1225,7 @@ func gc_m() {
 	casgstatus(gp, _Gwaiting, _Grunning)
 }
 
-func gc(args *gc_args) {
+func gc(start_time int64, eagersweep bool) {
 	if _DebugGCPtrs {
 		print("GC start\n")
 	}
@@ -1246,8 +1236,8 @@ func gc(args *gc_args) {
 
 	_g_ := getg()
 	_g_.m.traceback = 2
-	t0 := args.start_time
-	work.tstart = args.start_time
+	t0 := start_time
+	work.tstart = start_time
 
 	var t1 int64
 	if debug.gctrace > 0 {
@@ -1367,7 +1357,7 @@ func gc(args *gc_args) {
 	sweep.spanidx = 0
 	unlock(&mheap_.lock)
 
-	if _ConcurrentSweep && !args.eagersweep {
+	if _ConcurrentSweep && !eagersweep {
 		lock(&gclock)
 		if !sweep.started {
 			go bgsweep()
@@ -1394,11 +1384,7 @@ func gc(args *gc_args) {
 	}
 }
 
-func readmemstats_m() {
-	_g_ := getg()
-	stats := (*mstats)(_g_.m.ptrarg[0])
-	_g_.m.ptrarg[0] = nil
-
+func readmemstats_m(stats *MemStats) {
 	updatememstats(nil)
 
 	// Size of the trailing by_size array differs between Go and C,
@@ -1406,14 +1392,14 @@ func readmemstats_m() {
 	memmove(unsafe.Pointer(stats), unsafe.Pointer(&memstats), sizeof_C_MStats)
 
 	// Stack numbers are part of the heap numbers, separate those out for user consumption
-	stats.stacks_sys = stats.stacks_inuse
-	stats.heap_inuse -= stats.stacks_inuse
-	stats.heap_sys -= stats.stacks_inuse
+	stats.StackSys = stats.StackInuse
+	stats.HeapInuse -= stats.StackInuse
+	stats.HeapSys -= stats.StackInuse
 }
 
 //go:linkname readGCStats runtime/debug.readGCStats
 func readGCStats(pauses *[]uint64) {
-	onM(func() {
+	systemstack(func() {
 		readGCStats_m(pauses)
 	})
 }
@@ -1578,16 +1564,7 @@ func unrollglobgcprog(prog *byte, size uintptr) bitvector {
 	return bitvector{int32(masksize * 8), &mask[0]}
 }
 
-func unrollgcproginplace_m() {
-	_g_ := getg()
-
-	v := _g_.m.ptrarg[0]
-	typ := (*_type)(_g_.m.ptrarg[1])
-	size := _g_.m.scalararg[0]
-	size0 := _g_.m.scalararg[1]
-	_g_.m.ptrarg[0] = nil
-	_g_.m.ptrarg[1] = nil
-
+func unrollgcproginplace_m(v unsafe.Pointer, typ *_type, size, size0 uintptr) {
 	pos := uintptr(0)
 	prog := (*byte)(unsafe.Pointer(uintptr(typ.gc[1])))
 	for pos != size0 {
@@ -1613,12 +1590,7 @@ func unrollgcproginplace_m() {
 var unroll mutex
 
 // Unrolls GC program in typ.gc[1] into typ.gc[0]
-func unrollgcprog_m() {
-	_g_ := getg()
-
-	typ := (*_type)(_g_.m.ptrarg[0])
-	_g_.m.ptrarg[0] = nil
-
+func unrollgcprog_m(typ *_type) {
 	lock(&unroll)
 	mask := (*byte)(unsafe.Pointer(uintptr(typ.gc[0])))
 	if *mask == 0 {