cmd/compile: internal compiler error: expected struct { F uintptr; L lua.Lua_State } value to have type struct { F uintptr; L lua.Lua_State } #62498

raochq · 2023-09-07T04:23:13Z

What version of Go are you using (`go version`)?

$ go version
go version go1.21.0 windows/amd64

Does this issue reproduce with the latest release?

This issue only occurs in go 1.21
It work well before go1.21;
It work well in go1.21 with '-gcflags "all=-l"';

What operating system and processor architecture are you using (`go env`)?

go env Output

$ go env
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\arao\AppData\Local\go-build
set GOENV=C:\Users\arao\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=D:\NN4\server\server\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=D:\NN4\server\server\
set GOPRIVATE=
set GOPROXY=https://goproxy.cn,direct
set GOROOT=C:\go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.21.0
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=NUL
set GOWORK=D:\NN4\server\server\go.work
set CGO_CFLAGS=-ID:\NN4\server\server\local\include -ID:\NN4\server\server\local\include\luajit-2.1
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-LD:\NN4\server\server\local\lib\windows_amd64 -ltolua -lpbc -lluajit-5.1 -lmingwex
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\arao\AppData\Local\Temp\go-build2469763229=/tmp/go-build -gno-record-gcc-switches

What did you do?

go install battlesvr

What did you expect to see?

build success

What did you see instead?

# nikki/golua
src\nikki\golua/luastack.go:19:12: internal compiler error: expected struct { F uintptr; L *lua.Lua_State } value to have type struct { F uintptr; L *lua.Lua_State }

Please file a bug report including a short program that triggers the error.
https://go.dev/issue/new

code

////luastack.go
package golua

import (
	"nikki/golua/lua"
	"nikki/golua/tolua"

	"fmt"
	"runtime"
	"unsafe"
)

type LuaStack struct {
	l *lua.Lua_State
}

func NewLuaStack() *LuaStack {
	L := lua.LuaL_newstate()
	lua.LuaL_openlibs(L)
	tolua.Open(L) // The problem occurs in this line
	stack := &LuaStack{
		l: L,
	}
	runtime.SetFinalizer(stack, (*LuaStack).Close)
	return stack
}

//// tolua.go
package tolua

/*
#include <tolua/tolua.h>
#include <stdlib.h>
*/
import "C"

import (
	"nikki/golua/lua"
	"unsafe"
)
// The problem occurs in this func
func Open(L *lua.Lua_State) {
	C.tolua_open((*C.lua_State)(L))
}

////lua.go
package lua

/*
#include <lua.h>
#include <stdlib.h>

LUA_API const char *(Lua_pushfstring) (lua_State *L, const char *s) { return lua_pushfstring(L, s); }
*/
import "C"

import (
	"fmt"
	"unsafe"
)
type Lua_State C.lua_State
func LuaL_newstate() *Lua_State { return (*Lua_State)(C.luaL_newstate()) }

The text was updated successfully, but these errors were encountered:

egonelbre · 2023-09-07T08:10:44Z

Similar issue can happen when you are typedef-in the same struct from multiple places via headers.

Does the problem persist when you move the code into a single file?

In other words, you should have only one single definition for *lua.Lua_State.

raochq · 2023-09-07T09:21:58Z

This problem is no longer present when I write function Open not in one line.

func Open(L *lua.Lua_State) {
	l := (*C.lua_State)(L)
	C.tolua_open(l)
}

egonelbre · 2023-09-07T10:31:27Z

So, yes, *C.lua_State and *lua.Lua_state are distinct types for Go.

See https://pkg.go.dev/cmd/cgo#hdr-Go_references_to_C

Cgo translates C types into equivalent unexported Go types. Because the translations are unexported, a Go package should not expose C types in its exported API: a C type used in one Go package is different from the same C type used in another.

There's a separate issue for having a nicer solution for this problem: #13467

egonelbre · 2023-09-07T12:05:02Z

Although, I don't think the compiler should ICE due to that.

cherrymui · 2023-09-07T19:07:53Z

cc @golang/compiler

mdempsky · 2023-09-08T02:47:28Z

I'll look into this tomorrow, but if you can simplify the test case to work with only standard C (or at least POSIX) types/includes that will simplify reproducing it for me. Thanks.

raochq · 2023-09-08T04:22:38Z

This is the simplify code。 Hope this helps troubleshooting the issue.
https://github.com/raochq/cgotest.git

cuonglm · 2023-09-08T17:06:44Z

The problem seems to be a more agressive inlining of closures in go1.21

The closure generated for C.tolua_open in function:

func Open(L *lua.Lua_State) {
	C.tolua_open((*C.lua_State)(L))
}

can now be inlined, there's no generated Go type for (*C.lua_State)(L) in package tolua anymore.

It ends up making two closures with different capture vars: C.luaL_openlibs and C.tolua_open, having the same link string struct { F uintptr; L *github.com/raochq/cgotest/golua/lua.Lua_State }, but are not identical, because the closure pointer F have different symbol: lua..F and tolua..F.

mdempsky · 2023-09-08T23:42:06Z

@raochq Thank you, I can reproduce the failure.

mdempsky · 2023-09-09T00:39:14Z

Here's a standalone repro of the issue: https://go.dev/play/p/5eDYV-mzqg9

There's nothing wrong with this code, and it compiled in Go 1.20. So I think this is a regression that should be backported.

@gopherbot Please backport to Go 1.21. This is a compilation failure of valid code that used to work in Go 1.20.

gopherbot · 2023-09-09T00:39:52Z

Backport issue(s) opened: #62544 (for 1.20), #62545 (for 1.21).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

mdempsky · 2023-09-09T00:49:45Z

@cuonglm

but are not identical, because the closure pointer F have different symbol: lua..F and tolua..F.

Thanks, I concur. That's an issue: exported struct fields are supposed to always use ir.LocalPkg for their Sym.

mdempsky · 2023-09-09T00:51:59Z

Oh wait, the field name is actually .F, which isn't exported.

But Type.LinkString is rewriting this to just F, which is a problem, because LinkString is supposed to match type identity.

gopherbot · 2023-09-09T01:39:15Z

Change https://go.dev/cl/527135 mentions this issue: cmd/compile/internal/typecheck: fix closure field naming

mdempsky · 2023-09-09T01:56:18Z

This problem is no longer present when I write function Open not in one line.

This is because the problem relates to closures capturing local variables with uppercase names. (There aren't closures in your code as written, but cgo introduces them as part of its rewriting.)

I've uploaded a fix, and I hope to have it included in the next Go minor release. In the mean time, disabling inlining like you discovered should work. Alternatively, you could rename your variables to start with a lowercase letter.

Sorry for the inconvenience. Thank you for the report and for preparing the standalone reproduction.

gopherbot · 2023-10-12T23:42:55Z

Change https://go.dev/cl/534916 mentions this issue: [release-branch.go1.21] cmd/compile/internal/typecheck: fix closure field naming

…ield naming When creating the struct type to hold variables captured by a function literal, we currently reuse the captured variable names as fields. However, there's no particular reason to do this: these struct types aren't visible to users, and it adds extra complexity in making sure fields belong to the correct packages. Further, it turns out we were getting that subtly wrong. If two function literals from different packages capture variables with identical names starting with an uppercase letter (and in the same order and with corresponding identical types) end up in the same function (e.g., due to inlining), then we could end up creating closure struct types that are "different" (i.e., not types.Identical) yet end up with equal LinkString representations (which violates LinkString's contract). The easy fix is to just always use simple, exported, generated field names in the struct. This should allow further struct reuse across packages too, and shrink binary sizes slightly. For #62498. Fixes #62545. Change-Id: I9c973f5087bf228649a8f74f7dc1522d84a26b51 Reviewed-on: https://go-review.googlesource.com/c/go/+/527135 Auto-Submit: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> (cherry picked from commit e3ce312) Reviewed-on: https://go-review.googlesource.com/c/go/+/534916 Reviewed-by: Matthew Dempsky <mdempsky@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>

gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Sep 7, 2023

cherrymui added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. compiler/runtime Issues related to the Go compiler and/or runtime. and removed compiler/runtime Issues related to the Go compiler and/or runtime. labels Sep 7, 2023

cherrymui added this to the Go1.22 milestone Sep 7, 2023

mdempsky self-assigned this Sep 8, 2023

gopherbot closed this as completed in e3ce312 Sep 11, 2023

dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/compile: internal compiler error: expected struct { F uintptr; L lua.Lua_State } value to have type struct { F uintptr; L lua.Lua_State } #62498

cmd/compile: internal compiler error: expected struct { F uintptr; L lua.Lua_State } value to have type struct { F uintptr; L lua.Lua_State } #62498

raochq commented Sep 7, 2023 •

edited

egonelbre commented Sep 7, 2023

raochq commented Sep 7, 2023

egonelbre commented Sep 7, 2023

egonelbre commented Sep 7, 2023

cherrymui commented Sep 7, 2023

mdempsky commented Sep 8, 2023

raochq commented Sep 8, 2023

cuonglm commented Sep 8, 2023

mdempsky commented Sep 8, 2023

mdempsky commented Sep 9, 2023

gopherbot commented Sep 9, 2023

mdempsky commented Sep 9, 2023

mdempsky commented Sep 9, 2023

gopherbot commented Sep 9, 2023

mdempsky commented Sep 9, 2023

gopherbot commented Oct 12, 2023

cmd/compile: internal compiler error: expected struct { F uintptr; L *lua.Lua_State } value to have type struct { F uintptr; L *lua.Lua_State } #62498

cmd/compile: internal compiler error: expected struct { F uintptr; L *lua.Lua_State } value to have type struct { F uintptr; L *lua.Lua_State } #62498

Comments

raochq commented Sep 7, 2023 • edited

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

What did you expect to see?

What did you see instead?

code

egonelbre commented Sep 7, 2023

raochq commented Sep 7, 2023

egonelbre commented Sep 7, 2023

egonelbre commented Sep 7, 2023

cherrymui commented Sep 7, 2023

mdempsky commented Sep 8, 2023

raochq commented Sep 8, 2023

cuonglm commented Sep 8, 2023

mdempsky commented Sep 8, 2023

mdempsky commented Sep 9, 2023

gopherbot commented Sep 9, 2023

mdempsky commented Sep 9, 2023

mdempsky commented Sep 9, 2023

gopherbot commented Sep 9, 2023

mdempsky commented Sep 9, 2023

gopherbot commented Oct 12, 2023

cmd/compile: internal compiler error: expected struct { F uintptr; L lua.Lua_State } value to have type struct { F uintptr; L lua.Lua_State } #62498

cmd/compile: internal compiler error: expected struct { F uintptr; L lua.Lua_State } value to have type struct { F uintptr; L lua.Lua_State } #62498

raochq commented Sep 7, 2023 •

edited

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?