Skip to content
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

cmd/link/internal/ld,cmd/nm: tests fail on android/arm64 #58807

Open
bcmills opened this issue Mar 1, 2023 · 9 comments
Open

cmd/link/internal/ld,cmd/nm: tests fail on android/arm64 #58807

bcmills opened this issue Mar 1, 2023 · 9 comments
Labels
arch-arm64 compiler/runtime Issues related to the Go compiler and/or runtime. mobile Android, iOS, and x/mobile NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Android
Milestone
Backlog

Comments

@bcmills
Copy link
Contributor

bcmills commented Mar 1, 2023

In CL 472096, I am experimenting with enabling tests that use go build on android, because I don't see a good reason not to. Unfortunately, several cmd/link/internal/ld tests fail when run on android/arm64:

--- FAIL: TestUndefinedRelocErrors (0.15s)
    ld_test.go:68: extra errors: function main is undeclared in the main package (x2)
--- FAIL: TestRuntimeTypeAttrExternal (0.94s)
    dwarf_test.go:987: DWARF type offset was 0x2b20+0x80fc0, but test program said 0x5ebd4d1ae0
--- FAIL: TestRuntimeTypeAttrInternal (0.19s)
    dwarf_test.go:987: DWARF type offset was 0x2b20+0xa0000, but test program said 0x5c2d85bb20
FAIL
FAIL	cmd/link/internal/ld	10.733s

(https://storage.googleapis.com/go-build-log/c86ce876/android-arm64-corellium_04a8c6e6.log)

(attn @golang/android @golang/compiler)
@bcmills bcmills added OS-Android NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. mobile Android, iOS, and x/mobile arch-arm64 compiler/runtime Issues related to the Go compiler and/or runtime. labels Mar 1, 2023
@bcmills bcmills added this to the Backlog milestone Mar 1, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/472455 mentions this issue: cmd/link: skip tests that fail on android/arm64

@bcmills
Copy link
Contributor Author

bcmills commented Mar 2, 2023

There are some similar failures (involving bad DWARF addresses) for cmd/nm as well:

--- FAIL: TestExternalLinkerCgoExec (1.45s)
    nm_test.go:187: want 0x63a4ba4930 address for main.main symbol, but have 0xb7930
    nm_test.go:187: want 0x63a4c5cbb4 address for main.testdata symbol, but have 0x16fbb4
    nm_test.go:187: want 0x63a4ba4970 address for main.testfunc symbol, but have 0xb7970
    nm_test.go:201: want R type for runtime.epclntab symbol, but have D
--- FAIL: TestGoExec (0.25s)
    nm_test.go:187: want 0x645ec493d0 address for main.main symbol, but have 0x8c3d0
    nm_test.go:187: want 0x645ed4a894 address for main.testdata symbol, but have 0x18d894
    nm_test.go:187: want 0x645ec49410 address for main.testfunc symbol, but have 0x8c410
    nm_test.go:201: want R type for runtime.epclntab symbol, but have D
FAIL
FAIL	cmd/nm	2.505s

@bcmills bcmills changed the title cmd/link/internal/ld: tests fail on android/arm64 cmd/link/internal/ld,cmd/nm: tests fail on android/arm64 Mar 2, 2023
@cherrymui
Copy link
Member

These are because we build PIE binaries on Android/ARM64, and these tests don' work for PIE. https://cs.opensource.google/go/go/+/master:src/cmd/nm/nm_test.go;l=155-175 is to skip the test for PIE platforms. We need to add android there as well.

@cherrymui
Copy link
Member

Same for the linker DWARF test (see the skip for darwin/arm64).

@bcmills
Copy link
Contributor Author

bcmills commented Mar 2, 2023

Maybe there should be a function in internal/platform that reports whether binaries are PIE by default? I don't like that we're duplicating OS/arch assumptions in so many different places across the codebase.

@cherrymui
Copy link
Member

SGTM.

@mknyszek mknyszek assigned cherrymui and unassigned cherrymui Mar 8, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/475076 mentions this issue: cmd/nm: skip failing checks on android/arm64

gopherbot pushed a commit that referenced this issue Mar 10, 2023
@gopherbot
all: skip tests that fail on android/arm64
d12fe60 
Many of the tests skipped platforms that build PIE binaries by
default, but (still) lack a central function to report which platforms
those are.

Some of the tests assumed (but did not check for) internal linking
support, or invoked `go tool link` directly without properly
configuring the external linker.

A few of the tests seem to be triggering latent bugs in the linker.

For #58806.
For #58807.
For #58794.

Change-Id: Ie4d06b1597f404590ad2abf978d4c363647407ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/472455
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/475457 mentions this issue: internal/platform: add a function to report whether default builds are PIE

gopherbot pushed a commit that referenced this issue Mar 15, 2023
@gopherbot
internal/platform: add a function to report whether default builds ar…
bbf795e 
…e PIE

This consolidates a condition that was previously repeated (in
different approximations) in several different places in the code.

For #58807.

Change-Id: Idd308759f6262b1f5c61f79022965612319edf94
Reviewed-on: https://go-review.googlesource.com/c/go/+/475457
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
@bcmills
Copy link
Contributor Author

bcmills commented Mar 16, 2023

The offset errors are fixed with the updated PIE skips. There are still two TODOs for this issue for the runtime.epclntab symbol type and the duplicated function main is undeclared in the main package error, so leaving this issue open for those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
arch-arm64 compiler/runtime Issues related to the Go compiler and/or runtime. mobile Android, iOS, and x/mobile NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Android
Projects
Go Compiler / Runtime
Status: Todo
Milestone
Backlog
Development

No branches or pull requests
4 participants
@mknyszek @bcmills @gopherbot @cherrymui