interp: ptrtoint integer size does not equal pointer size

[niaow]

I think this error was supposed to be used internally for rollback, but it seems that it replicates on AVR:

package main

import "fmt"

func printOne() {
        fmt.Println("one")
}

func main() {
        f := printOne
        f()
}

This example is pulled from #349 which I was checking on.

[niaow@finch tinygo]$ build/tinygo run -target arduino /tmp/bleh.go
error: interp: ptrtoint integer size does not equal pointer size

[dgryski]

Is this the issue that was mentioned in the patch for the conservative collector here: f870ed5 ?

[niaow]

No, it is not. Although idk if the patch might impact it.

[dgryski]

Is this patch then still something I should create a PR for?

diff --git a/src/runtime/gc_conservative.go b/src/runtime/gc_conservative.go
index cabd41427..faa4091a5 100644
--- a/src/runtime/gc_conservative.go
+++ b/src/runtime/gc_conservative.go
@@ -410,7 +410,7 @@ func markRoots(start, end uintptr) {
 		}
 	}
 
-	for addr := start; addr < end; addr += unsafe.Alignof(addr) {
+	for addr := start; addr+unsafe.Sizeof(unsafe.Pointer(nil)) <= end; addr += unsafe.Alignof(addr) {
 		root := *(*uintptr)(unsafe.Pointer(addr))
 		markRoot(addr, root)
 	}

[niaow]

I do not know. I will need to look into that again. That should not be related to this issue.

[niaow]

So the answer there is that yes that code still needs to be fixed but also my fix had a bug in it because markRoots already had another load-bearing bug.

[niaow]

For future reference, that patch has been turned into #2396

[dgryski]

This issue is also happening when trying to build https://github.com/google/open-location-code/, and I'm pretty sure that's a recent failure.

[aykevl]

This issue is most likely unrelated to any runtime GC that might happen. The interp package intercepts calls to runtime.alloc and replaces them with creating a new global.

I think this error was supposed to be used internally for rollback

No, what it actually shows is that there is a bug somewhere in the interp package. It is usually caused when something stores a pointer in a non-pointer field (such as int, uintptr, [4]byte, etc). It might be fixable by passing a correct layout parameter to runtime.alloc: there are many places where this isn't yet done (for example, in llvmutil.EmitPointerPack).

[niaow]

While working on #2458, I found that this can be triggered by testdata/json.go, testdata/stdlib.go, and testdata/testing.go on AVR.

[markoxley]

Not sure if this will help, but I had a similar issue until I removed all fmt and log references from my code

[dgryski]

(This is no longer crashing open-location-code/go either wasi or native.)

[dkegel-fastly]

#2565 includes a regression test for this, but doesn't run it yet from Makefile, as it is still failing.

[kedare]

I was playing with some basic code and got the same issue as soon as I added a use of math/random in my code

package main

import (
	"machine"
	"math/rand"
	"time"
)

func main() {
	led := machine.LED
	led.Configure(machine.PinConfig{Mode: machine.PinOutput})
	for {
		led.Low()
		pause := rand.Intn(5000)
		time.Sleep(time.Millisecond * time.Duration(pause))

		led.High()
		time.Sleep(time.Millisecond * 50)
	}
}

[dkegel-fastly]

I reduced one of the above testcases. To reproduce:

$ tinygo build -target arduino -o main  main.go

where main.go is

package main
func main() {
	println(Foo())
}
func Foo() int { return global.Foo() }
var global = &Bar{NewThing()}
type Bar struct {
	src Thing
}
func (r *Bar) Foo() int { return r.src.Foo() }
type Thing interface {
	Foo() int
}
func NewThing() Thing {
	return &subThing{}
}
type subThing struct {}
func (s *subThing) Foo() int {
	return 7
}

[stevegt]

@deadprogram @dkegel-fastly In the spirit of prioritizing a fix for #2732, does it make sense to go ahead and enable the regression test Dan mentions above and in #2565? I'd suggest the build should fail until the fmt regression is fixed -- not supporting the arduino is not only blocking adoption but may be masking some other issues, like the apparent println() regression I also mention over there.

[dkegel-fastly]

I've marked the two bugs with the 0.23 milestone, that might suffice.

[stevegt]

Thanks Dan -- that should help. After I wrote that I realized that all I'm suggesting is TDD, but decided not to press it.

Noticing that this bug is causing some uncertainty in other repos as well -- tinygo-org/drivers#328 for instance.

[dkegel-fastly]

Yeah, it's serious, and now that it has a minimal-ish reproducer, the compiler guys have something to chew on...
thanks for making a ruckus.

[dkegel-fastly]

I think I heard this is AVR-specific, and the AVR backend is so unstable we had to disable tests.

See also #2492

Removing 0.23 milestone; who knows, maybe llvm 14 will improve things...?

[Emreu]

Got same error with rand.Intn() while building for target=arduino-nano. I'm totally new for LLVM and tinygo, so could be wrong, but there is some clues - code compiles well for target=arduino-mega2560, on smaller platform culprit is global struct math/rand.globalRand and if you disable code generating error (at interp/memory.go:857-862) - you get anoter:

ld: /tmp/***/main section `.data' will not fit in region `RAM'
ld: region `RAM' overflowed by 3644 bytes

So may be this is a bug with correct error reporting?