Skip to content

cmd/link: incorrect GC bitmap when global's type is in another shared object [1.14 backport] #39955

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.

Sign up for GitHub

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

Jump to bottom
Closed
gopherbot opened this issue Jun 30, 2020 · 6 comments
Closed

cmd/link: incorrect GC bitmap when global's type is in another shared object [1.14 backport] #39955

gopherbot opened this issue Jun 30, 2020 · 6 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone
Go1.14.9

Comments

@gopherbot
Copy link
Contributor

@aclements requested issue #39927 to be considered for backport to the next 1.14 minor release.

@gopherbot, please backport to Go 1.14 and 1.15.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Jun 30, 2020
@gopherbot gopherbot mentioned this issue Jun 30, 2020
Closed
@gopherbot gopherbot added this to the Go1.14.5 milestone Jun 30, 2020
@dmitshur
Copy link
Member

@aclements Can you please include a short rationale about why the backport might be needed? (Per MinorReleases.) Thanks.

@gopherbot
Copy link
Contributor Author

Change https://golang.org/cl/240511 mentions this issue: [release-branch.go1.14] cmd/link: fix GC data reading from shared library (attempt 2)

@aclements
Copy link
Member

Rationale for backport: This causes the garbage collector to incorrectly free reachable objects when using shared build mode, leading to memory corruption and crashes. There is no workaround (other than not using shared build mode).

@dmitshur
Copy link
Member

dmitshur commented Jul 10, 2020 •
edited
Loading

Backport approved for Go 1.14 (this issue) and 1.13 (#39956) based on that rationale.

@dmitshur dmitshur removed the CherryPickCandidate Used during the release process for point releases label Jul 10, 2020
@dmitshur dmitshur mentioned this issue Jul 10, 2020
Closed
@dmitshur dmitshur added the CherryPickApproved Used during the release process for point releases label Jul 10, 2020
@gopherbot

This comment has been minimized.

Sign in to view
@andybons andybons modified the milestones: Go1.14.5, Go1.14.6, Go1.14.7 Jul 14, 2020
@toothrot toothrot modified the milestones: Go1.14.7, Go1.14.8 Aug 6, 2020
@gopherbot
Copy link
Contributor Author

Closed by merging 73268be to release-branch.go1.14.

gopherbot pushed a commit that referenced this issue Aug 21, 2020
@cherrymui @dmitshur
[release-branch.go1.14] cmd/link: fix GC data reading from shared lib…
73268be 
…rary (attempt 2)

This is a backport of CL 240621. This is not a clean cherry-pick,
as Go 1.15 switches to the new linker while it is still the old
linker here. Backporting is straightforward, though.

When linking against a Go shared library, when a global variable
in the main module has a type defined in the shared library, the
linker needs to pull the GC data from the shared library to build
the GC program for the global variable. Currently, this fails
silently, as the shared library file is closed too early and the
read failed (with no error check), causing a zero GC map emitted
for the variable, which in turn causes the runtime to treat the
variable as pointerless.

For now, fix this by keeping the file open. In the future we may
want to use mmap to read from the shared library instead.

Also add error checking. And fix a (mostly harmless) mistake in
size caluculation.

Also remove an erroneous condition for ARM64. ARM64 has a special
case to get the addend from the relocation on the gcdata field.
But that doesn't actually work. And it's no longer necessary to
have any special case, since the addend is now applied directly
to the gcdata field on ARM64, like on all the other platforms.

Fixes #39955.
Updates #39927.

Change-Id: I01c82422b9f67e872d833336885935bc509bc91b
Reviewed-on: https://go-review.googlesource.com/c/go/+/240621
Run-TryBot: Cherry Zhang <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
(cherry picked from commit 7799756)
Reviewed-on: https://go-review.googlesource.com/c/go/+/240511
Reviewed-by: Austin Clements <[email protected]>
@dmitshur dmitshur modified the milestones: Go1.14.8, Go1.14.9 Sep 1, 2020
@golang golang locked and limited conversation to collaborators Sep 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Milestone
Go1.14.9
Development

No branches or pull requests
5 participants
@toothrot @andybons @dmitshur @aclements @gopherbot