"tinygo test os" fails due to varargs problems on M1 mac

[dkegel-fastly]

trafficstars

Apple documents varargs as being different on darwin arm64 (see https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms) but it appears tinygo dev branch doesn't quite implement that yet.

This shows up as test failures in tinygo test os. When TestFstat tries to create a file, it ends up with the wrong permissions.

The open system call is apparently varargs, and the 3rd argument is only used if the 2nd argument has O_CREAT set.

Here's a minimal test that uses that call:

package syscall_test
import (
	"syscall"
	"testing"
)
func TestOpen(t *testing.T) {
	f, err := syscall.Open("/tmp/foo.dat", syscall.O_RDWR|syscall.O_CREAT, 0644)
	if err != nil {
		t.Fatalf("Open: %v", err)
	}
	if err := syscall.Close(f); err != nil {
		t.Fatalf("Close: %v", err)
	}
}

After running this on amd64, ls -l /tmp/foo.dat shows the right permissions. Removing the file and running on arm64 creates the file with incorrect permissions.

Here's a log showing lldb's bt thinks we passed the 3rd argument properly; evidently you can't trust bt to print varargs properly?

% grep O_ syscall_libc_darwin.go
        O_RDWR   = 0x2
        O_CREAT  = 0x200
        ...
% perl -e "print 0x202"
514
% grep O_ open_test.go
        f, err := syscall.Open("/tmp/foo.dat", syscall.O_RDWR|syscall.O_CREAT, 0644)
% perl -e "print 0644"
420

--------- works on amd64 ---------
% GOARCH=amd64 ../../build/tinygo  test -c syscall
% file syscall.test
syscall.test: Mach-O 64-bit executable x86_64
% rm -f /tmp/foo.dat
% arch -x86_64 ./syscall.test
% ls -l /tmp/foo.dat
-rw-r--r--  1 dkegel  wheel  0 Jul 15 12:16 /tmp/foo.dat

--------- fails on arm64 ---------
% ../../build/tinygo test -c syscall
% file syscall.test
syscall.test: Mach-O 64-bit executable arm64
% rm -f /tmp/foo.dat
% ./syscall.test
% ls -l /tmp/foo.dat
-r--r-x---  1 dkegel  wheel  0 Jul 15 12:20 /tmp/foo.dat

--------- are wrong args passed to open()?  Well, lldb bt shows args look correct, but ... ---------

% lldb syscall.test
% b open
% r
...
% c
...
% c
Process 86029 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
    frame #0: 0x00000001a00fdfb8 libsystem_kernel.dylib`open
% bt
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
  * frame #0: 0x00000001a00fdfb8 libsystem_kernel.dylib`open
    frame #1: 0x0000000100000ba8 syscall.test`syscall.Open(path=<unavailable>, flag=514, mode=420) at syscall_libc.go:68:20 [opt]

[aykevl]

One possible way to solve this is using a C wrapper. Unfortunately we can't use CGo directly (because the go toolchain thinks that CGo requires the syscall package which leads to an import cycle), but what we can do is add a new C file (like src/runtime/asm_arm64.S) that wraps the vararg call so it can be called like a normal function, like this:

#include <fcntl.h>
int libc_open(const char *pathname, int flags, mode_t mode) {
        return open(pathname, flags, mode);
}

It can then be called as usual, with an unimplemented //export function.

With ThinLTO, it should be inlined into the Go code so there shouldn't be any extra overhead.

[aykevl]

I've looked at this but it seems like there is more going on here. When I use the -v flag, it should have printed the test name before running, but it doesn't:

$ tinygo test -v os
panic: runtime error: nil pointer dereference
FAIL	os	0.253s
FAIL

Looks like this panic is happening much earlier.

[aykevl]

Update: with -opt=0 I see that it really is crashing inside TestFstat.

[dkegel-fastly]

Maybe, but this bug is about the minimal reproducer in the top comment, where TestFstat isn't being run.

[aykevl]

Tested this today and tinygo test os is indeed fixed on darwin/arm64.

[deadprogram]

0.27.0 has now been released, so closing. Thank you!