runtime (gc): memory corruption due to untracked allocations

[deadprogram]

The latest dev branch has some issues when running the https://github.com/tinygo-org/drivers/tree/master/examples/wifinina/mqttsub example.

After some period of time, somewhere between 100 and 1500 messages sent/received, the program on the Arduino Nano33 IoT crashes, giving one of the following outputs:

panic: runtime error: out of memory

fatal error: HardFault with sp=0x200007b4 pc=0xf7f8fbbd

panic: runtime error: goroutine stack overflow

panic: runtime error: nil pointer dereference

Note that it is one of the above when it crashes, not all of them.

[aykevl]

This looks like a memory corruption. Does it also happen with -scheduler=coroutines?

[aykevl]

Also, it would help a lot if you could bisect this issue to the offending commit (assuming that the master branch works).

[deadprogram]

It runs without errors when I use

tinygo flash -target arduino-nano33 -scheduler=coroutines ./examples/wifinina/mqttsub/

I will bisect to try to find the commit that breaks things.

[deadprogram]

It appears this is the answer:

# first bad commit: [6a50f25a48385fc78397b6f0f1c83650130d8d16] refactor coroutine lowering and tasks

[aykevl]

Okay, great. That's at least something. @jaddr2line can you perhaps take a look?

[niaow]

I suspect that this is due to a small change in stack usage causing an overflow. Can you try raising the stack size a bit? Those numbers sound like potential garbage collection times.

[niaow]

Also the only relevant change I can think of from that commit is that the task data is no longer on top of the stack. It may have overflowed before, but is now corrupting something other than the task data.

[aykevl]

Here is the stack size you can try raising (for example to 2048):

tinygo/src/internal/task/task_stack_cortexm.go

Line 7 in 57320c0

const stackSize = 1024

[deadprogram]

Raising the stack size to 2048 had no effect.

[niaow]

There appears to be an issue with GC tracking right now. This patch forces the conservative GC to reclaim memory earlier for debugging purposes:

diff --git a/src/runtime/gc_conservative.go b/src/runtime/gc_conservative.go
index 319e68b..5fb87b3 100644
--- a/src/runtime/gc_conservative.go
+++ b/src/runtime/gc_conservative.go
@@ -219,8 +219,11 @@ func alloc(size uintptr) unsafe.Pointer {
 
        neededBlocks := (size + (bytesPerBlock - 1)) / bytesPerBlock
 
+       GC()
+
        // Continue looping until a run of free blocks has been found that fits the
        // requested size.
+       nextAlloc = 0
        index := nextAlloc
        numFreeBlocks := uintptr(0)
        heapScanCount := uint8(0)

This causes several compiler tests to fail, and some have errors similar to what have been discovered here. It also triggers the bug discovered at #1012 (comment).

[niaow]

With stack slots, it appears that we are not properly tracking phi nodes:

package main

import "math/rand"

func gen(n int) *int {
	return &n
}

func check(ptr *int, n int) {
	if *ptr != n {
		panic("corrupted memory")
	}
}

func main() {
	a, b := 0, 1
	aptr, bptr := gen(a), gen(b)
	for i := 0; i < 1000; i++ {
		println(i)

		if rand.Intn(2) == 0 {
			a, b = b, a
			aptr, bptr = bptr, aptr
		}

		a = rand.Int()
		aptr = gen(a)

		check(aptr, a)
		check(bptr, b)
	}
}

However, this does not explain corruption on bare-metal platforms.

[niaow]

I have confirmed that some of the problems on baremetal are caused by references being stored in callee-saved registers. We do not currently scan these, and thus the allocations are freed early.