Skip to content

cmd/link: Go 1.24.3 and 1.23.9 regression - duplicated definition of symbol dlopen #73617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
djedward opened this issue May 7, 2025 · 12 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Critical A critical problem that affects the availability or correctness of production systems built using Go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone

Comments

@djedward
Copy link

djedward commented May 7, 2025

Go version

go 1.24.3/1.23.9 (linux amd64 --> darwin amd64)

Output of go env in your module/workspace:

AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE=''
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/home/user/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/home/user/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1648213215=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/dev/null'
GOMODCACHE='/home/user/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/user/go'
GOPRIVATE=''
GOPROXY='direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/user/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.24.3'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

I went to release Go 1.24.3 and Go 1.23.9 into our build system and some packages began failing when trying to cross-compile to darwin/amd64:

I've reproduced it down to a small example:

mkdir repro

cd repro

go mod init example.com/failure

cat > main.go << EOF
package main

import (
	_ "github.com/ebitengine/purego"
)

func main() {
}
EOF

go mod tidy

GOOS=darwin GOARCH=amd64 go build -o main main.go

What did you see happen?

I received an error:
link: duplicated definition of symbol dlopen, from github.com/ebitengine/purego and github.com/ebitengine/purego

What did you expect to see?

I expected it to continue to compile, as it did in Go 1.24.2

@Sunyue
Copy link

Sunyue commented May 7, 2025

The same issue also happens to me natively on darwin/amd64

@seankhliao seankhliao changed the title link: Go 1.24.3 and 1.23.9 regression - duplicated definition of symbol dlopen cmd/link: Go 1.24.3 and 1.23.9 regression - duplicated definition of symbol dlopen May 7, 2025
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label May 7, 2025
@corani
Copy link

corani commented May 7, 2025

I'm guessing it may have been introduced here: https://go-review.googlesource.com/c/go/+/662335/3/src/cmd/link/internal/loader/loader.go

Maybe a corner case where old size == new size?

@prattmic
Copy link
Member

prattmic commented May 7, 2025

cc @golang/compiler @cherrymui

@prattmic prattmic added this to the Go1.25 milestone May 7, 2025
@prattmic prattmic added Soon This needs action soon. (recent regressions, service outages, unusual time-sensitive situations) Critical A critical problem that affects the availability or correctness of production systems built using Go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 7, 2025
@prattmic
Copy link
Member

prattmic commented May 7, 2025

Possibly this is an interaction between the dlopen symbol in https://github.com/ebitengine/purego/blob/9059adfae616e486aa4145d6f4d5fefaa1b47a61/dlfcn_stubs.s and //go:cgo_import_dynamic purego_dlopen dlopen "/usr/lib/libSystem.B.dylib" in https://github.com/ebitengine/purego/blob/9059adfae616e486aa4145d6f4d5fefaa1b47a61/dlfcn_darwin.go#L16?

@prattmic
Copy link
Member

prattmic commented May 7, 2025

Oh, it is probably between the assembly symbol and data symbol at https://github.com/ebitengine/purego/blob/9059adfae616e486aa4145d6f4d5fefaa1b47a61/dlfcn.go#L85-L86.

@TotallyGamerJet
Copy link

Oh, it is probably between the assembly symbol and data symbol at https://github.com/ebitengine/purego/blob/9059adfae616e486aa4145d6f4d5fefaa1b47a61/dlfcn.go#L85-L86.

I don't believe they have the same name though. One is purego.dlopen and the other is just dlopen since it doesn't have the special dot symbol. Changing the assembly name still fails to link.

Subject: [PATCH] rename assembly
---
Index: dlfcn.go
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/dlfcn.go b/dlfcn.go
--- a/dlfcn.go	(revision 2c36c15debde48b37b30e9ae9dea32483ef79922)
+++ b/dlfcn.go	(date 1746629059115)
@@ -82,18 +82,18 @@
 // sadly, I do not know of anyway to remove the assembly stubs entirely because //go:linkname doesn't
 // appear to work if you link directly to the C function on darwin arm64.
 
-//go:linkname dlopen dlopen
+//go:linkname dlopen go_dlopen
 var dlopen uintptr
 var dlopenABI0 = uintptr(unsafe.Pointer(&dlopen))
 
-//go:linkname dlsym dlsym
+//go:linkname dlsym go_dlsym
 var dlsym uintptr
 var dlsymABI0 = uintptr(unsafe.Pointer(&dlsym))
 
-//go:linkname dlclose dlclose
+//go:linkname dlclose go_dlclose
 var dlclose uintptr
 var dlcloseABI0 = uintptr(unsafe.Pointer(&dlclose))
 
-//go:linkname dlerror dlerror
+//go:linkname dlerror go_dlerror
 var dlerror uintptr
 var dlerrorABI0 = uintptr(unsafe.Pointer(&dlerror))
Index: dlfcn_stubs.s
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/dlfcn_stubs.s b/dlfcn_stubs.s
--- a/dlfcn_stubs.s	(revision 2c36c15debde48b37b30e9ae9dea32483ef79922)
+++ b/dlfcn_stubs.s	(date 1746629059143)
@@ -6,21 +6,21 @@
 #include "textflag.h"
 
 // func dlopen(path *byte, mode int) (ret uintptr)
-TEXT dlopen(SB), NOSPLIT|NOFRAME, $0-0
+TEXT go_dlopen(SB), NOSPLIT|NOFRAME, $0-0
 	JMP purego_dlopen(SB)
 	RET
 
 // func dlsym(handle uintptr, symbol *byte) (ret uintptr)
-TEXT dlsym(SB), NOSPLIT|NOFRAME, $0-0
+TEXT go_dlsym(SB), NOSPLIT|NOFRAME, $0-0
 	JMP purego_dlsym(SB)
 	RET
 
 // func dlerror() (ret *byte)
-TEXT dlerror(SB), NOSPLIT|NOFRAME, $0-0
+TEXT go_dlerror(SB), NOSPLIT|NOFRAME, $0-0
 	JMP purego_dlerror(SB)
 	RET
 
 // func dlclose(handle uintptr) (ret int)
-TEXT dlclose(SB), NOSPLIT|NOFRAME, $0-0
+TEXT go_dlclose(SB), NOSPLIT|NOFRAME, $0-0
 	JMP purego_dlclose(SB)
 	RET

@cherrymui
Copy link
Member

Yes, it is the assembly (TEXT) symbol and the data (BSS) symbol. The assembly function is short (6 bytes), smaller than the data symbol (uintptr, 8 bytes). Previously we just pick the one with content, i.e. the text symbol. Now we error out if the non-content symbol is larger, in the sense that if the code actually reads the variable as a uintptr, it will read to the next symbol. In this case, the code doesn't read the variable, just takes its address, which is fine.

For backward compatibility we probably want to still allow this case, at least if one side is TEXT symbol. There is no easy way in the linker to detect whether we read the content as a variable.

For a quick fix, you can change the variable definition from uintptr to uint8, like var dlopen uint8, then it is smaller.

@cherrymui
Copy link
Member

I don't believe they have the same name though. One is purego.dlopen and the other is just dlopen since it doesn't have the special dot symbol. Changing the assembly name still fails to link.

They do. Both sides don't have the dot. The name of the assembly symbol is the one at the TEXT line, i.e. dlopen. The variable's link name in the Go file is the last component of the go:linkname pragma, which is dlopen, not purego.dlopen.

Your patch changes both from dlopen to go_dlopen, so they still have the same name.

@cherrymui
Copy link
Member

cherrymui commented May 7, 2025

This patch to the linker may make it work. I haven't thoroughly tested it though.

diff --git a/src/cmd/link/internal/loader/loader.go b/src/cmd/link/internal/loader/loader.go
index 128173b8cf..52d48912c7 100644
--- a/src/cmd/link/internal/loader/loader.go
+++ b/src/cmd/link/internal/loader/loader.go
@@ -455,29 +455,45 @@ func (st *loadState) addSym(name string, ver int, r *oReader, li uint32, kind in
 	// If one is a DATA symbol (i.e. has content, DataSize != 0)
 	// and the other is BSS, the one with content wins.
 	// If both are BSS, the one with larger size wins.
-	// Specifically, the "overwrite" variable and the final result are
+	//
+	// For a special case, we allow a TEXT symbol overwrites a BSS symbol
+	// even if the BSS symbol has larger size. This is because there is
+	// code like below to take the address of a function
+	//
+	//	//go:linkname fn
+	//	var fn uintptr
+	//	var fnAddr = uintptr(unsafe.Pointer(&fn))
+	//
+	// TODO: maybe limit this case to just pointer sized variable?
+	//
+	// In summary, the "overwrite" variable and the final result are
 	//
 	// new sym       old sym       overwrite
 	// ---------------------------------------------
 	// DATA          DATA          true  => ERROR
 	// DATA lg/eq    BSS  sm/eq    true  => new wins
 	// DATA small    BSS  large    true  => ERROR
+	// TEXT          BSS           true  => new wins
 	// BSS  large    DATA small    true  => ERROR
 	// BSS  large    BSS  small    true  => new wins
 	// BSS  sm/eq    D/B  lg/eq    false => old wins
-	overwrite := r.DataSize(li) != 0 || oldsz < sz
+	// BSS           TEXT          false => old wins
+	overwrite := r.DataSize(li) != 0 || oldsz < sz ||
+		sym.AbiSymKindToSymKind[objabi.SymKind(osym.Type())].IsText()
 	if overwrite {
 		// new symbol overwrites old symbol.
 		oldtyp := sym.AbiSymKindToSymKind[objabi.SymKind(oldsym.Type())]
-		if !(oldtyp.IsData() && oldr.DataSize(oldli) == 0) || oldsz > sz {
-			log.Fatalf("duplicated definition of symbol %s, from %s and %s", name, r.unit.Lib.Pkg, oldr.unit.Lib.Pkg)
+		newtyp := sym.AbiSymKindToSymKind[objabi.SymKind(osym.Type())]
+		if !(oldtyp.IsData() && oldr.DataSize(oldli) == 0) || (oldsz > sz && !newtyp.IsText()) {
+			log.Fatalf("duplicated definition of symbol %s, from %s (type %s size %d) and %s (type %s size %d)", name, r.unit.Lib.Pkg, newtyp, sz, oldr.unit.Lib.Pkg, oldtyp, oldsz)
 		}
 		l.objSyms[oldi] = objSym{r.objidx, li}
 	} else {
 		// old symbol overwrites new symbol.
-		typ := sym.AbiSymKindToSymKind[objabi.SymKind(oldsym.Type())]
-		if !typ.IsData() { // only allow overwriting data symbol
-			log.Fatalf("duplicated definition of symbol %s, from %s and %s", name, r.unit.Lib.Pkg, oldr.unit.Lib.Pkg)
+		newtyp := sym.AbiSymKindToSymKind[objabi.SymKind(osym.Type())]
+		if !newtyp.IsData() { // only allow overwriting data symbol
+			oldtyp := sym.AbiSymKindToSymKind[objabi.SymKind(oldsym.Type())]
+			log.Fatalf("duplicated definition of symbol %s, from %s (type %s size %d) and %s (type %s size %d)", name, r.unit.Lib.Pkg, newtyp, sz, oldr.unit.Lib.Pkg, oldtyp, oldsz)
 		}
 	}
 	return oldi

@cherrymui cherrymui added release-blocker and removed Soon This needs action soon. (recent regressions, service outages, unusual time-sensitive situations) labels May 7, 2025
hajimehoshi added a commit to ebitengine/purego that referenced this issue May 7, 2025
hajimehoshi added a commit to ebitengine/purego that referenced this issue May 7, 2025
@djedward
Copy link
Author

djedward commented May 7, 2025

The patch appears to be working for the builds that were failing for us.

@denizkoele
Copy link

updating to github.com/ebitengine/purego v0.9.0-alpha.3.0.20250507171635-5047c08daa38 made it work, thank you!

@hajimehoshi
Copy link
Member

updating to github.com/ebitengine/purego v0.9.0-alpha.3.0.20250507171635-5047c08daa38 made it work, thank you!

This works, though I recommend the stable branch version:

go get github.com/ebitengine/purego@1638563e361522e5f63511d84c4541ae1c5fd704

github-merge-queue bot pushed a commit to jaegertracing/jaeger that referenced this issue May 9, 2025
## Which problem is this PR solving?
- Our release #7107 is blocked by the build error `link: duplicated
definition of symbol dlopen, from github.com/ebitengine/purego and
github.com/ebitengine/purego`
- Ongoing upstream issue golang/go#73617

## Description of the changes
- Pin to a pre-release of https://github.com/ebitengine/purego v0.8.3
that provides a patch

## How was this change tested?
- CI

---------

Signed-off-by: Yuri Shkuro <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Critical A critical problem that affects the availability or correctness of production systems built using Go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
Status: No status
Development

No branches or pull requests

9 participants