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.

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: making sense of HeadType #22270

Open
crawshaw opened this issue Oct 14, 2017 · 3 comments
Open

cmd/link: making sense of HeadType #22270

crawshaw opened this issue Oct 14, 2017 · 3 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unplanned

Comments

@crawshaw
Copy link
Member

The linker makes extensive use of HeadType. It does not make much sense to me.

When used as a value, it is very close to meaning GOOS. One exception is both "android" and "linux" are represented as "Hlinux", but besides that they are equivalent.

When used as a flag to the linker, -H, it is the value of GOOS, plus the potential value "windowsgui", which is treated as "windows" except it sets a global linker variable windowsgui to true. The linker is also receiving the GOOS environment variable used by the compiler (and all other tools) to make their OS decision.

Proposal:

  • Leave the -H flag alone, as other build systems may depend on it.
  • Check when the linker starts that GOOS and -H match.
  • Convert any OS-specific conditions in the linker to using GOOS.
  • Convert the HeadType enumeration into: 
    const (
      Unknown HeadType = iota
      ELF
      MachO
      Plan9
      PE
      PEGUI
)
  • Replace the IsELF global variable with ctxt.HeadType == objabi.ELF instead.
  • Replace windowsgui with ctxt.HeadType == objabi.PEGUI.

Thoughts?

cc @ianlancetaylor
@rsc
Copy link
Contributor

rsc commented Oct 14, 2017

SGTM.

@alexbrainman
Copy link
Member

PE
PEGUI

I think you want PE only. PEGUI is not much different from PE. There are 2 places in cmd/link/internal/ld where that difference is important. On the other hand there are many places where Hwindows is used, so you would end up with code like (Headtype == PE || Headtype == PEGUI) instead of Headtype == PE everywhere. And if you forget the || Headtype == PEGUI part here or there, you won't get a broken test, because we don't have many tests that check PEGUI.

And PEGUI will be exported, unlike current windowsgui variable.

We already had PE and PEGUI split before, so you would be effectively revert CL 38761.

Alex

@ianlancetaylor
Copy link
Contributor

As far as I know the notion of "head type" precedes the notion of GOOS.

Like Alex, I don't think we need PEGUI. The variable windowsgui (which should of course be in the Link struct) seems like the right approach to me. It's just a small variant of Windows, not worth calling out in such a visible way.

There's really no reason to call your new suggested enum HeadType but I suppose we can do that to puzzle future readers.

@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 29, 2018
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Mar 29, 2018
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@rsc @crawshaw @ianlancetaylor @gopherbot @alexbrainman