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 · 20 comments

Comments

Projects
None yet
4 participants
@ianlancetaylor
Contributor

ianlancetaylor commented May 25, 2017

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

This comment has been minimized.

Contributor

cherrymui commented May 25, 2017

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

This comment has been minimized.

Contributor

ianlancetaylor commented May 25, 2017

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

This comment has been minimized.

Contributor

cherrymui commented Jun 8, 2017

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

This comment has been minimized.

Contributor

ianlancetaylor commented Jun 8, 2017

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

This comment has been minimized.

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

This comment has been minimized.

Contributor

cherrymui commented Jun 8, 2017

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

This comment has been minimized.

Contributor

ianlancetaylor commented Jun 8, 2017

@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

This comment has been minimized.

Contributor

cherrymui commented Jun 8, 2017

Thanks.

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

@laboger

This comment has been minimized.

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

This comment has been minimized.

Contributor

ianlancetaylor commented Jun 8, 2017

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

This comment has been minimized.

Contributor

ianlancetaylor commented Jun 8, 2017

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

This comment has been minimized.

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

This comment has been minimized.

Contributor

ianlancetaylor commented Jun 9, 2017

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

@laboger

This comment has been minimized.

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

This comment has been minimized.

Contributor

ianlancetaylor commented Jun 9, 2017

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

This comment has been minimized.

Contributor

ianlancetaylor commented Jun 9, 2017

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

This comment has been minimized.

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

This comment has been minimized.

Contributor

ianlancetaylor commented Jun 13, 2017

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

This comment has been minimized.

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

This comment has been minimized.

Contributor

ianlancetaylor commented Jun 14, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment