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/go, misc/cgo: clang does not generate .edata sections #46719

Open
zx2c4 opened this issue Jun 12, 2021 · 7 comments
Open

cmd/go, misc/cgo: clang does not generate .edata sections #46719

zx2c4 opened this issue Jun 12, 2021 · 7 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Milestone
Backlog

Comments

@zx2c4
Copy link
Contributor

zx2c4 commented Jun 12, 2021

We're currently using the .edata section to check whether cgo generated a PE binary with the expected symbols. While binutils generates this section, clang's LLD does not. It still exports the symbols, but just doesn't have the .edata section. This issue is to track the (eventual) refactoring of those tests.
@gopherbot
Copy link

Change https://golang.org/cl/327549 mentions this issue: cmd/go, misc/cgo: skip test if no .edata

@zx2c4
Copy link
Contributor Author

zx2c4 commented Jun 12, 2021

For now, CL 327549 just skips them.

@dmitshur
Copy link
Contributor

Is this related to issue #46510?

gopherbot pushed a commit that referenced this issue Jun 13, 2021
@zx2c4
cmd/go, misc/cgo: skip test if no .edata
24cff0f 
Clang does not produce binaries with an .edata section, even when it
exports symbols properly, so just skip this binutils-specific test for
that case. Later we can rewrite these tests entirely to do something
more robust.

Updates #46719.

Change-Id: I864b3c2d91e66800c55454ae11d4ab1623693d14
Reviewed-on: https://go-review.googlesource.com/c/go/+/327549
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@cagedmantis cagedmantis added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows labels Jun 14, 2021
@cagedmantis cagedmantis added this to the Backlog milestone Jun 14, 2021
@cagedmantis
Copy link
Contributor

/cc @ianlancetaylor

@ianlancetaylor
Copy link
Contributor

To clarify, this is only related to tests.

@zx2c4
Copy link
Contributor Author

zx2c4 commented Jun 14, 2021

Is this related to issue #46510?

Yes, same thing basically. There's one other test that uses .edata in addition to the one mentioned in that issue. Both changed in 24cff0f

@dmitshur dmitshur mentioned this issue Jun 14, 2021
Closed
@gopherbot
Copy link

Change https://golang.org/cl/328049 mentions this issue: cmd/go, misc/cgo: restore .edata section checking on amd64 and 386

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@zx2c4 @cagedmantis @dmitshur @ianlancetaylor @gopherbot