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

cmd/link: may need a command line option to set section split size #20492

Open
ianlancetaylor opened this issue May 25, 2017 · 21 comments
Open
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@ianlancetaylor
Copy link
Contributor

On ppc64x in external link mode the linker splits the text section into separate sections of size up to 0x1c00000 bytes. This gives the C linker space to insert stubs for out-of-range bl instructions. The C linker works in terms of a stub group size: it adds sections to a stub group until it reaches the group size, and then creates a new stub group. The default stub group size is, not coincidentally, 0x1c00000.

However, 1) it is possible for the linker to fail to insert a stub if there are too many stubs, in which case it can retry with a smaller stub group size; 2) the linker has a command line option to set the stub group size, and it may be set to be smaller. Either way, when the Go linker produces a section that is larger than the stub group size, the linker emits a warning (at least, the gold linker does), and in some cases the link may conceivably fail.

In order to accommodate these cases I think that either we need to have a command line option to set the size at which we split the text section, or we need to simply reduce the size to something more likely to work.

CC @laboger

@ianlancetaylor ianlancetaylor added this to the Go1.10 milestone May 25, 2017
@cherrymui
Copy link
Member

I thought the plan is to implement trampoline insertion for PPC64 external linking in the Go linker, as we do for ARM internal/external linking, and PPC64 internal linking. Then we don't need to do the split. Right?

@ianlancetaylor
Copy link
Contributor Author

If you mean that there will never be 24-bit PC-relative relocations from the Go object to symbols that are not defined in the Go object--if all the references will use 64-bit relocations--then, yes, that sounds good.

However, that doesn't help with the fact that the external linker will emit a warning if it sees a single section that is too large, even if that section doesn't happen to have any 24-bit PC-relative relocations. We still need to split the sections in order to avoid that warning. Or perhaps we should convince the linker developers to add an option to disable the warning....

@cherrymui
Copy link
Member

By 24-bit PC-relative relocations, did you mean relocations for direct CALL instruction? Is there any other?

I think Go to C calls are made through function pointers, right? We probably already use 64-bit relocations for addresses of functions.

@ianlancetaylor
Copy link
Contributor Author

Yes, calls from Go to C are made by passing the C function pointer to runtime.cgocall and calls from C to Go are made by passing the address of the Go function to crosscall2. So you are probably correct that there are no 24-bit PC-relative relocs from the Go object to external symbols.

@laboger
Copy link
Contributor

laboger commented Jun 8, 2017

Sorry I didn't see this before.

I have no problem with reducing the maximum size for a text section or adding an option to set it.

I am somewhat concerned about the removal of text section splitting because I don't really know for sure if there is still a situation where one huge text section would cause a problem with the linker in addition to the warning above. What if the linker has to create a plt stub but the offset to the plt stub is too far? I'm not 100% sure that if we add trampolines for everything, that won't happen in some odd case.

If we don't remove the code to splt text sections then I need an alternate solution to the latest (hot) problem with huge programs, because the default buildmode for executables with external linking is not initializing and maintaining r2 and when the offset to a call gets really large, the plt_branch stub generated by the linker depends on r2. I don't know if you have any suggestions on how to solve that.

@cherrymui
Copy link
Member

Dumb question: how does the external linker decide what type of stub to generate? Is it possible to tell it not to generate stub that depends R2, maybe -fno-pic or something like? (I asked without reading the external linker code, so I might go really off (sorry).)

@ianlancetaylor
Copy link
Contributor Author

@laboger I guess the issue is that if we do split into sections, then we may have call relocs between the sections, and then it is possible that the linker will decide that it must use a PLT reloc, and then r2 will not be set correctly. I suppose the fix for that is to never have call relocs between sections: for the Go linker to always insert a stub for any cross-section call. I admit it is somewhat tedious to do both section splitting and stub insertion. But since you have the code for stub insertion, perhaps it is not too hard to simply assume that any cross-section call requires a stub.

@cherrymui I think the linker will use a PLT stub for any symbol that already has a PLT entry. I'm not precisely sure which symbols in the Go section will wind up with PLT entries, though. Perhaps just symbols referenced by non-Go code?

@cherrymui
Copy link
Member

Thanks.

@ianlancetaylor's solution with both split sections and stub insertion SGTM.

@laboger
Copy link
Contributor

laboger commented Jun 8, 2017

@cherrymui I have found mention of the relocation type R_PPC64_REL24_NOTOC in the documentation, which sounds like what should be used in this case. Actually I think all calls from Go code where R2 is not initialized should be using that although there hasn't been a problem before now. Unfortunately this is a relatively new relocation type and most existing distros won't have support for that in their ld. I actually tried it but the binutils on the system I was working on didn't recognize it. It's not in binutils 2.24.

I can change my fix to do the cross breed solution, but what about this issue? Do you have a lower value that you'd like to use for the text size limit?

@ianlancetaylor
Copy link
Contributor Author

I do not have a good lower value. I noticed the issue when someone reported the unexpected linker warning, and it turned out that they were using -Wl,--stub-group-size=0x1800000 as part of their CC environment variable.

@ianlancetaylor
Copy link
Contributor Author

By the way, I don't see any support for R_PPC64_REL32_NOTOC in the GNU binutils or gold at all. The value is defined, but apparently never used.

@laboger
Copy link
Contributor

laboger commented Jun 9, 2017

I've never seen this warning with ld, and I see the option for the stub-group-size with the gold linker but not ld.

Rather than adding an option to modifiy the minimum size, what if they were required to set extldflags for stub-group-size and then have build.go recognize that setting and use it to indicate the minimum text size? Seems like they need to be in sync between the gold option and the size used for splitting sections anyway.

@ianlancetaylor
Copy link
Contributor Author

I think I like that idea but I'm not sure which build.go you are referring to.

@laboger
Copy link
Contributor

laboger commented Jun 9, 2017

I meant cmd/go/internal/work/build.go when it parses extldflags.

I see in the gold man pages for the stub-group-size option that it applies to ARM as well, so isn't this a problem there too?

@ianlancetaylor
Copy link
Contributor Author

I don't know if this is a problem on ARM. The warning that the gold linker emits on PPC64 does not exist on ARM. I don't know if that is an omission or if it is because it works differently.

@ianlancetaylor
Copy link
Contributor Author

As far as -extldflags goes, seems to me we can look through it in the linker. That is, have the linker pick out the stub group size from there, rather than adding a new rather-specific linker option.

@laboger
Copy link
Contributor

laboger commented Jun 13, 2017

That sounds fine to me. I don't see this option in the ld man pages, so it sounds like this is only a valid option with gold?

@ianlancetaylor
Copy link
Contributor Author

The GNU ld man pages are not reliable. The option is in fact supported by GNU ld, and I see it in the --help output (for a PPC64 GNU ld).

In fact, GNU ld also has the warning. I see it in bfd/elf64-ppc.c.

@laboger
Copy link
Contributor

laboger commented Jun 14, 2017

Ah OK, I didn't look hard enough.
So to summarize what we are saying is that the size where text sections are split will remain as the current default unless someone uses this linker option with -extldflags and then that value will be used as the max section size when splitting. No other option will be added to affect this value.

One other thought, is there any checking of the values being used here? The size shouldn't be larger than our current default, and also wondering if there is a minimum value.

@ianlancetaylor
Copy link
Contributor Author

So to summarize what we are saying is that the size where text sections are split will remain as the current default unless someone uses this linker option with -extldflags and then that value will be used as the max section size when splitting. No other option will be added to affect this value.

SGTM

One other thought, is there any checking of the values being used here? The size shouldn't be larger than our current default, and also wondering if there is a minimum value.

I don't see any checking in the linker code.

A value of 1 means to use the default.

@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017
@ianlancetaylor ianlancetaylor added help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jun 23, 2018
@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Unplanned Jun 23, 2018
@gopherbot
Copy link

Change https://golang.org/cl/241073 mentions this issue: [dev.link] cmd/link: add PPC64 debugging option to encourage text section splits

gopherbot pushed a commit that referenced this issue Aug 10, 2020
…tion splits

Add a new debugging command line option (-debugppc64textsize=N) that
forces the start of a new text section after ".text" hits N bytes as
opposed to the architected limit of 2^26. This is intended to enable
testing of the linker code paths that handle multiple .text sections
on PPC64 without resorting to building giant applications.

Updates #20492.

Change-Id: I74ab7fd1e412e9124de5bd0d8d248c5e73225ae3
Reviewed-on: https://go-review.googlesource.com/c/go/+/241073
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
@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
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants