Skip to content

[WIP] internal/build: fix linkname support main.sym#2155

Draft
visualfc wants to merge 2 commits into
xgo-dev:mainfrom
visualfc:main_pkgpath
Draft

[WIP] internal/build: fix linkname support main.sym#2155
visualfc wants to merge 2 commits into
xgo-dev:mainfrom
visualfc:main_pkgpath

Conversation

@visualfc

@visualfc visualfc commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

fix #2156

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@visualfc
visualfc marked this pull request as draft July 22, 2026 12:57
@visualfc
visualfc force-pushed the main_pkgpath branch 4 times, most recently from 7ce9de5 to 3fe2fc7 Compare July 23, 2026 04:25
@visualfc
visualfc marked this pull request as ready for review July 23, 2026 14:12
@visualfc visualfc changed the title internal/build: fix linkname support main.sym [WIP] internal/build: fix linkname support main.sym Jul 23, 2026

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review summary

Scoped review of the 6 source files in this PR (the other ~174 changed files are regenerated golden test-data). The core change — emitting main-package symbols under the main. prefix and running all init functions in the library constructor — is coherent and the library ctor init ordering correctly mirrors defineEntryFunction (pyInit, rtInit, abiInit, runtimeStub, mainInit) while correctly excluding mainMain/pyFinalize.

Main findings are inline. One cross-cutting note below.

Maintainability — duplicated main-prefix rule. The rule "if the package name is main, the path/prefix is main, else use the import path" now appears in three places: ssa/abi/abi.go PathOf, internal/build/main_module.go genMainModule (the pkgPath block), and internal/build/build.go linkMainPkg. A closely related but different rule already exists in ssa/abitype.go reflectPkgPath (keyed on Path() == "command-line-arguments"). Four near-duplicate implementations of the same concept invite drift. Consider centralizing on the existing abi.PathOf helper and calling it from the build package — this would also remove the raw pkg.Types.Name() dereferences flagged inline.

No security or performance regressions found (build-time codegen over bounded inputs).

Findings without inline locations

  • internal/build/main_module.go:140: This doc comment ("arranges for the LLGo runtime to be initialized") now understates the behavior: after the variadic refactor the ctor invokes multiple inits (Python init, runtime init, abitypes init, runtime stub, package init), not just the runtime. Consider updating the comment, and noting that the argument order must mirror defineEntryFunction's init sequence since the two lists are now maintained independently and can silently diverge.

Comment thread internal/build/build.go
@@ -1132,6 +1135,9 @@ func linkMainPkg(ctx *context, pkg *packages.Package, pkgs []*aPackage, outputPa
methodByName := make(map[string]none)
allPkgs := []*packages.Package{pkg}
for _, v := range pkgs {
if v.PkgPath != pkg.PkgPath && v.Types.Name() == "main" {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v.Types.Name() is dereferenced without a nil check, unlike the guard added a few lines up in filterTestPackages (if pkg.Types != nil && pkg.Types.Name() == "main", line 692). Elsewhere in this file p.Types is defensively nil-checked before use. A package with Types == nil (e.g. an ill-typed or synthetically-loaded dep) reaching this loop would panic. Recommend v.Types != nil && v.Types.Name() == "main" for consistency and crash-safety.

mainInit := declareNoArgFunc(mainPkg, pkg.PkgPath+".init")
mainMain := declareNoArgFunc(mainPkg, pkg.PkgPath+".main")
var pkgPath string
if pkg.Types.Name() == "main" {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pkg.Types.Name() is dereferenced without a nil guard here, whereas filterTestPackages guards the same access with pkg.Types != nil. If pkg.Types can ever be nil at this point, this panics. Add the nil check (or centralize via abi.PathOf, which already special-cases main) so the three new Name()=="main" sites behave consistently.

Comment thread internal/header/header.go Outdated
@visualfc
visualfc marked this pull request as draft July 24, 2026 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

go:linkname unsupport main.sym

1 participant