[go] nil pointer deref in arm64.gensymlate fix

[Des Kramer (Gerrit)]

Des Kramer has uploaded the change for review

Commit message

nil pointer deref in arm64.gensymlate fix

Change-Id: I81020b66d570ee3f04205f61933a58de1f6ca283

Change diff

diff --git a/src/cmd/link/internal/arm64/asm.go b/src/cmd/link/internal/arm64/asm.go
index e615bf5..a620598 100644
--- a/src/cmd/link/internal/arm64/asm.go
+++ b/src/cmd/link/internal/arm64/asm.go
@@ -1279,11 +1279,15 @@
 	// addLabelSyms adds "label" symbols at s+limit, s+2*limit, etc.
 	addLabelSyms := func(s loader.Sym, limit, sz int64) {
 		v := ldr.SymValue(s)
+		sect := ldr.SymSect(s)
+		if sect == nil {
+			return // symbol not yet assigned to a section; skip label generation
+		}
 		for off := limit; off < sz; off += limit {
 			p := ldr.LookupOrCreateSym(offsetLabelName(ldr, s, off), ldr.SymVersion(s))
 			ldr.SetAttrReachable(p, true)
 			ldr.SetSymValue(p, v+off)
-			ldr.SetSymSect(p, ldr.SymSect(s))
+			ldr.SetSymSect(p, sect)
 			if ctxt.IsDarwin() {
 				ld.AddMachoSym(ldr, p)
 			} else if ctxt.IsWindows() {
diff --git a/src/cmd/link/link_test.go b/src/cmd/link/link_test.go
index 19607c3..5ca04d4 100644
--- a/src/cmd/link/link_test.go
+++ b/src/cmd/link/link_test.go
@@ -1368,6 +1368,59 @@
 	}
 }
 
+// testGensymlateCGOSrc is a CGO program with a data symbol larger than
+// machoRelocLimit (8 MB = 1<<23). On darwin/arm64 with external linking,
+// gensymlate iterates over such symbols to insert label symbols so that
+// Mach-O relocations can use small addends. Previously, symbols whose section
+// had not yet been assigned (SymSect == nil) caused a nil dereference in
+// SetSymSect. This source is used by TestGensymlateLargeDataCGO.
+const testGensymlateCGOSrc = `
+package main
+
+// #include <stddef.h>
+import "C"
+
+// dataSym is a data symbol exceeding machoRelocLimit (1<<23 bytes).
+var dataSym = [9 << 20]byte{1: 1}
+
+func main() {
+	// Access the symbol so the linker keeps it reachable.
+	println(dataSym[1])
+}
+`
+
+func TestGensymlateLargeDataCGO(t *testing.T) {
+	// Regression test for the nil-dereference crash in gensymlate when a data
+	// symbol larger than machoRelocLimit has no output section assigned yet
+	// (SymSect == nil) at the point gensymlate runs during external linking.
+	//
+	// The crash only fires on darwin/arm64 with CGO (which forces external
+	// linking). A data symbol must exceed 8 MB (machoRelocLimit = 1<<23).
+	if runtime.GOOS != "darwin" || runtime.GOARCH != "arm64" {
+		t.Skip("test only applies to darwin/arm64")
+	}
+	testenv.MustHaveGoBuild(t)
+	testenv.MustHaveCGO(t)
+	if testing.Short() {
+		t.Skip("skipping in -short mode: requires building a >8MB binary")
+	}
+
+	t.Parallel()
+
+	tmpdir := t.TempDir()
+	src := filepath.Join(tmpdir, "x.go")
+	if err := os.WriteFile(src, []byte(testGensymlateCGOSrc), 0666); err != nil {
+		t.Fatalf("failed to write source file: %v", err)
+	}
+
+	// CGO implies external linking on darwin, which is what triggers gensymlate.
+	cmd := goCmd(t, "build", "-o", filepath.Join(tmpdir, "x.exe"), src)
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		t.Fatalf("build failed (gensymlate nil-section crash may have recurred): %v\n%s", err, out)
+	}
+}
+
 func TestUnlinkableObj(t *testing.T) {
 	// Test that the linker emits an error with unlinkable object.
 	testenv.MustHaveGoBuild(t)

Change information

Files:

• M src/cmd/link/internal/arm64/asm.go
• M src/cmd/link/link_test.go

Change size: M

Delta: 2 files changed, 58 insertions(+), 1 deletion(-)

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement
• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: newchange

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I81020b66d570ee3f04205f61933a58de1f6ca283

Gerrit-Change-Number: 749880

Gerrit-PatchSet: 1

Gerrit-Owner: Des Kramer <destk...@gmail.com>

[Gopher Robot (Gerrit)]

Message from Gopher Robot

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
A maintainer will review your change and provide feedback. See
https://go.dev/doc/contribute#review for more info and tips to get your
patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://go.dev/s/release
for more details.

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement
• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I81020b66d570ee3f04205f61933a58de1f6ca283

Gerrit-Change-Number: 749880

Gerrit-PatchSet: 1

Gerrit-Owner: Des Kramer <destk...@gmail.com>

Gerrit-CC: Gopher Robot <go...@golang.org>

Gerrit-Comment-Date: Fri, 27 Feb 2026 15:23:04 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: No

[Cherry Mui (Gerrit)]

Attention needed from Des Kramer, Ian Lance Taylor and Russ Cox

Cherry Mui added 1 comment

File src/cmd/link/internal/arm64/asm.go

Line 1284, Patchset 1 (Latest): return // symbol not yet assigned to a section; skip label generation

Cherry Mui . unresolved

Thanks for the CL. I think we want to understand why we have reachable symbols without a section at this point, instead of simply skipping. I have a hunch. Feel free to look into this. Or I might be able to look into that later today. Thanks.

Open in Gerrit

Related details

Attention is currently required from:

• Des Kramer
• Ian Lance Taylor
• Russ Cox

Submit Requirements:

• Code-Review

• No-Unresolved-Comments

• Review-Enforcement
• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I81020b66d570ee3f04205f61933a58de1f6ca283

Gerrit-Change-Number: 749880

Gerrit-PatchSet: 1

Gerrit-Owner: Des Kramer <destk...@gmail.com>

Gerrit-Reviewer: Cherry Mui <cher...@google.com>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>

Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-CC: Gopher Robot <go...@golang.org>

Gerrit-Attention: Des Kramer <destk...@gmail.com>

Gerrit-Attention: Russ Cox <r...@golang.org>

Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>

Gerrit-Comment-Date: Fri, 27 Feb 2026 16:23:20 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Cherry Mui (Gerrit)]

Attention needed from Des Kramer, Ian Lance Taylor and Russ Cox

Cherry Mui added 1 comment

File src/cmd/link/link_test.go

Line 1378, Patchset 1 (Latest):package main

// #include <stddef.h>
import "C"



// dataSym is a data symbol exceeding machoRelocLimit (1<<23 bytes).


var dataSym = [9 << 20]byte{1: 1}



func main() {


// Access the symbol so the linker keeps it reachable.


println(dataSym[1])
}

Cherry Mui . unresolved

This doesn't actually reproduce the failure.

Gerrit-Comment-Date: Fri, 27 Feb 2026 16:26:13 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No