Skip to content

cmd/go: TestWindowsDefaultBuildmodIsPIE/cgo fails on windows-arm64-aws #46510

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
dmitshur opened this issue Jun 2, 2021 · 7 comments
Closed

cmd/go: TestWindowsDefaultBuildmodIsPIE/cgo fails on windows-arm64-aws #46510

dmitshur opened this issue Jun 2, 2021 · 7 comments
Labels
arch-arm64 FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Windows
Milestone
Go1.17

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Jun 2, 2021 

--- FAIL: TestWindowsDefaultBuildmodIsPIE (0.00s)
    --- FAIL: TestWindowsDefaultBuildmodIsPIE/cgo (9.23s)
        go_test.go:2129: running testgo [build -o C:\Users\gopher\AppData\Local\Temp\cmd-go-test-577963624\gotest1567665900\main.exe C:\Users\gopher\AppData\Local\Temp\cmd-go-test-577963624\gotest1567665900\main.go]
        go_test.go:2188: .edata section is not present
go test proxy running at GOPROXY=http://127.0.0.1:52061/mod
FAIL
FAIL	cmd/go	362.581s

Source: https://storage.googleapis.com/go-build-log/152255c1/windows-arm64-aws_1dc534d5.log from CL 323992.

This needs investigation.
@dmitshur dmitshur added OS-Windows NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. arch-arm64 labels Jun 2, 2021
@dmitshur dmitshur added this to the Go1.17 milestone Jun 2, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/323993 mentions this issue: cmd/go: skip TestWindowsDefaultBuildmodIsPIE/cgo on windows/arm64

@bcmills
Copy link
Contributor

bcmills commented Jun 2, 2021 •
edited
Loading

This test was added in CL 230217 (CC @alexbrainman @ianlancetaylor) for #35192 (CC @zx2c4 @aarzilli @networkimprov).

@alexbrainman
Copy link
Member

The code that fails

go/src/cmd/go/go_test.go

Lines 2181 to 2189 in 18e666b

// Test that only one symbol is exported (#40795).
// PIE binaries don´t require .edata section but unfortunately
// binutils doesn´t generate a .reloc section unless there is
// at least one symbol exported.
// See https://sourceware.org/bugzilla/show_bug.cgi?id=19011
section := f.Section(".edata")
if section == nil {
t.Fatalf(".edata section is not present")
}

was added in

https://go-review.googlesource.com/c/go/+/264459

The code assumes that exported symbols are stored in .edata PE section. And that is a reasonable assumption for executable built with Mingw GCC.

Perhaps that assumption is incorrect for whatever C compiler is used on windows-amd64 builder. I would try to build executable as per test and look at executable PE sections.

/cc @qmuntal and @zx2c4 in case they have suggestions here

Alex

@qmuntal
Copy link
Member

qmuntal commented Jun 3, 2021 •
edited
Loading

Assumption: windows-arm64-aws uses a llvm toolchain.

I don't really know much about llvm/clang, but looking at the lld source code it seems that the exported symbols are also stored in .edata.

My guess is that:

  • The cgo toolchain is missing some parameters when invoking clang
  • Or lld (clang's linker) is not seeing any exported symbol, thus it is not creating the .edata section.

@dmitshur
Copy link
Contributor Author

dmitshur commented Jun 4, 2021 •
edited
Loading

See here for more information about the C toolchain windows-arm64-aws uses.

@alexbrainman
Copy link
Member

See here for more information about the C toolchain windows-arm64-aws uses.

Thanks for the link, but I don't have and don't use windows-arm64. So I don't plan working on it.

Alex

@dmitshur dmitshur mentioned this issue Jun 12, 2021
Open
@dmitshur
Copy link
Contributor Author

TestWindowsDefaultBuildmodIsPIE/cgo no longer fails on windows-arm64-aws as of CL 327549, so closing. (CC @zx2c4.)

There's also #46719 which is still open and can be used if there's something more to do.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jun 14, 2021
@golang golang locked and limited conversation to collaborators Jun 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
arch-arm64 FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Windows
Projects
None yet
Milestone
Go1.17
Development

No branches or pull requests
5 participants
@dmitshur @qmuntal @bcmills @gopherbot @alexbrainman