This is an archive of the discontinued LLVM Phabricator instance.

[gold-plugin] Disable section ordering for relocatable links
ClosedPublic

Authored by void on Jun 29 2018, 1:07 AM.

Details

Summary

Not all programs want section ordering when compiled with LTO.
In particular, the Linux kernel is very sensitive when it comes to linking, and
doesn't boot when each function is placed in its own sections.

Diff Detail

Repository
rL LLVM

Event Timeline

void created this revision.Jun 29 2018, 1:07 AM

Added Davide who made the change to make these the default (in r309056 and r309075) for thoughts. Should the default be true to be consistent with prior behavior? I've noted a bad interaction that I think will occur with the -ffunction-sections/-fdata-sections options below.

tools/gold/gold-plugin.cpp
841 ↗(On Diff #153441)

I think unconditionally writing these options will interact badly with -ffunction-sections and -fdata-sections. Back in r284140 I added support to clang to pass down the internal -function-sections and -fdata-sections options when those driver options are set. From what I can tell, those internal llvm options will be applied to Conf.Options in InitTargetOptionsFromCodeGenFlags(), which is just above here (can't see it in phab - please upload patches with context).

So I think by default you would be always disabling those despite the internal llvm options being set. Can you confirm? Looks like the test I added with that patch just looked for the driver passing the internal option, so wouldn't catch this change in behavior.

Likely the clang side would need to change to set this gold plugin option instead.

pcc requested changes to this revision.Jun 29 2018, 10:36 AM
pcc added a subscriber: pcc.

Whatever we do I think this should certainly not be the default. 99+% of programs benefit from function sections and data sections due to reduced code size.

Can you explain why enabling function sections and data sections causes the Linux kernel not to boot?

This revision now requires changes to proceed.Jun 29 2018, 10:36 AM
davide requested changes to this revision.Jun 29 2018, 1:06 PM

FWIW, I tried this on a lot of software in several context without having issues (including a kernel, the PS4 one).
If the kernel doesn't boot, I consider this a bug in the kernel.
Now, I really understand that this might be unfixable easily in the project, but I agree with Peter it shouldn't be the default.

void added a comment.Jun 29 2018, 1:07 PM

Can I explain why it doesn't boot? No. At least not the root cause of why it won't boot. That's just the behavior we see. As I mentioned, Linux is very sensitive to what the linker is doing. E.g., we can't use the gold linker in the final binary link phase. If you wish, I can change the default here. Though frankly if the user doesn't specify --segment-ordering-file, then I'm not sure why the default is for this to be true.

void updated this revision to Diff 153647.Jul 1 2018, 2:10 AM

I changed it so that the original behavior is not changed. You need to specify disable-section-ordering as a plugin option.

This version is ok with me, question below around whether we want 2 separate options here though.

Davide, did you consider changing the default for the clang driver options instead of just in gold/lld for LTO? It might also be nice to hook up -fno-function-sections and -fno-data-sections to this new gold option (and mirror it in lld), just as I had done for the affirmative versions of those options in tools::AddGoldPlugin (which is now essentially a no-op due to the default change). In which case we would want 2 separate options here (one for functions and one for data). Thoughts?

This version is ok with me, question below around whether we want 2 separate options here though.

Davide, did you consider changing the default for the clang driver options instead of just in gold/lld for LTO? It might also be nice to hook up -fno-function-sections and -fno-data-sections to this new gold option (and mirror it in lld), just as I had done for the affirmative versions of those options in tools::AddGoldPlugin (which is now essentially a no-op due to the default change). In which case we would want 2 separate options here (one for functions and one for data). Thoughts?

Yeah, I think it's reasonable, so, no objections from me, but I'll let Peter decide as he owns LTO.

pcc added a comment.Jul 2 2018, 11:40 AM

Though frankly if the user doesn't specify --segment-ordering-file, then I'm not sure why the default is for this to be true.

Because -function-sections and -data-sections allow for code size optimizations, specifically --gc-sections and --icf.

This version is ok with me, question below around whether we want 2 separate options here though.

Davide, did you consider changing the default for the clang driver options instead of just in gold/lld for LTO? It might also be nice to hook up -fno-function-sections and -fno-data-sections to this new gold option (and mirror it in lld), just as I had done for the affirmative versions of those options in tools::AddGoldPlugin (which is now essentially a no-op due to the default change). In which case we would want 2 separate options here (one for functions and one for data). Thoughts?

Yeah, I think it's reasonable, so, no objections from me, but I'll let Peter decide as he owns LTO.

I don't think we should do that because it means that there are three possibilities in the clang driver:

  • no flag: no function sections when compiling, function sections when linking
  • -ffunction-sections: function sections when compiling, function sections when linking
  • -fno-function-sections: no function sections when compiling, no function sections when linking

which would be a little confusing.

Also, without further information about the issue with the Linux kernel I don't see a reason why we should let users disable function sections because as far as I know it is just working around a bug. I would grudgingly accept it as a developer-only flag which allows developers to debug issues more easily, although to be honest if I were debugging such an issue I would do it by modifying the LTO code directly.

tools/gold/gold-plugin.cpp
841 ↗(On Diff #153647)

Instead of adding a new flag, can you just make it so that these flags are sensitive to the existing -function-sections and -data-sections flags? You can do that like this:

if (FunctionSections.getNumOccurrences() == 0)
  Conf.Options.FunctionSections = true;

With that you can disable function sections with -plugin-opt -function-sections=0.

void added a comment.Jul 7 2018, 4:19 PM
In D48756#1149891, @pcc wrote:

Though frankly if the user doesn't specify --segment-ordering-file, then I'm not sure why the default is for this to be true.

Because -function-sections and -data-sections allow for code size optimizations, specifically --gc-sections and --icf.

Then it should be dependent upon those options and not set true for every run. Note that given linkers don't normally create new sections for each function, running LTO on the program unexpectedly changes the resulting output. This isn't good, despite the fact that 99.99% of people may not notice a difference.

This version is ok with me, question below around whether we want 2 separate options here though.

Davide, did you consider changing the default for the clang driver options instead of just in gold/lld for LTO? It might also be nice to hook up -fno-function-sections and -fno-data-sections to this new gold option (and mirror it in lld), just as I had done for the affirmative versions of those options in tools::AddGoldPlugin (which is now essentially a no-op due to the default change). In which case we would want 2 separate options here (one for functions and one for data). Thoughts?

Yeah, I think it's reasonable, so, no objections from me, but I'll let Peter decide as he owns LTO.

I don't think we should do that because it means that there are three possibilities in the clang driver:

  • no flag: no function sections when compiling, function sections when linking
  • -ffunction-sections: function sections when compiling, function sections when linking
  • -fno-function-sections: no function sections when compiling, no function sections when linking

which would be a little confusing.

Also, without further information about the issue with the Linux kernel I don't see a reason why we should let users disable function sections because as far as I know it is just working around a bug. I would grudgingly accept it as a developer-only flag which allows developers to debug issues more easily, although to be honest if I were debugging such an issue I would do it by modifying the LTO code directly.

And I haven't been given a good reason why these options being on by default is necessary. Code size optimization isn't good enough if it doesn't work for all programs. As we all know, Linux is a special beast when it comes to compilation and linking. They've coupled many of their design decisions to how GCC, GAS, and the GNU linker operate. In this particular case, using function sections breaks the kernel. It's not a kernel bug because being sensitive to how things are linked isn't a bug. (You can argue that it's not as robust as it could be, but that's not the same as saying it's a bug.) Therefore, it's enough to say that "this doesn't work for all programs" to warrant adding an option to disable this feature. I chose to disable it by default because it won't reverse existing behavior, thus surprising existing users. However, I still doubt the need for it to be turned on by default; I'm just not going to argue the point further.

void updated this revision to Diff 154505.Jul 7 2018, 4:25 PM
void marked an inline comment as done.
pcc added a comment.Jul 11 2018, 3:26 PM

I think it's important to understand exactly how the Linux kernel is sensitive to being linked, so that once we understand the root cause, we can develop an appropriate fix. It may be that the root cause is a deliberate dependency on an implementation detail, but that doesn't necessarily mean that the right fix is to add a flag, it could mean that we can develop a more targeted fix that doesn't require a flag. Or it could just be a bug, in which case we can fix it uncontroversially without needing to add a flag. In general I think it's important to find a solution that avoids adding flags because that increases the maintenance burden of the program. Here's an excellent article I recently read that makes the point better than I can: http://neugierig.org/software/blog/2018/07/options.html

I want to help you get this fixed, so I had one idea about what the root cause might be. Normally the linker will strip the name of the function from the output section when -function-sections is enabled, so .text.foo becomes .text in the output file. What that means is that without any special flags, -function-sections shouldn't normally change the output in any observable way. One case in which it is observable is when creating a relocatable object file with the -r flag. I happen to know that Linux kernel modules (.ko files) are represented as relocatable object files, so my suspicion is that the Linux kernel loader doesn't like section names of the form .text.foo in .ko files. If that's the case, a better fix would be to disable function sections only when creating a relocatable object file. That seems fine to me because relocatable files cannot usually be optimized by the linker. (What it probably also means is that in a later change we will want to save the -ffunction-sections and -fdata-sections flags passed at compile time and use them at link time. That would make things more complicated if we had just added a flag, because now we need to decide what to do if the link-time flag conflicts with the compile-time flag.) You can check whether -r was passed by checking whether the LDPT_LINKER_OUTPUT tag passed to the plugin is LDPO_REL.

lebedev.ri retitled this revision from Add option for section ordering to [gold-plugin] Add option for section ordering.Jul 11 2018, 3:28 PM
void added a comment.Jul 11 2018, 4:12 PM
In D48756#1159339, @pcc wrote:

I think it's important to understand exactly how the Linux kernel is sensitive to being linked, so that once we understand the root cause, we can develop an appropriate fix. It may be that the root cause is a deliberate dependency on an implementation detail, but that doesn't necessarily mean that the right fix is to add a flag, it could mean that we can develop a more targeted fix that doesn't require a flag. Or it could just be a bug, in which case we can fix it uncontroversially without needing to add a flag. In general I think it's important to find a solution that avoids adding flags because that increases the maintenance burden of the program. Here's an excellent article I recently read that makes the point better than I can: http://neugierig.org/software/blog/2018/07/options.html

I'm not a fan of adding options either. However, the justification here is to fix the issue where the gold linker was making an assumption that didn't work for all programs. I also wanted the gold linker to replicate as much as possible "normal" linker behavior when LTO isn't used. In the non-LTO instance, we don't get function/data sections. Given these two things, we really need to supply a way for the programmer to turn this feature off when it doesn't suit their needs—either with a new option or using an existing one.

I want to help you get this fixed, so I had one idea about what the root cause might be. Normally the linker will strip the name of the function from the output section when -function-sections is enabled, so .text.foo becomes .text in the output file. What that means is that without any special flags, -function-sections shouldn't normally change the output in any observable way. One case in which it is observable is when creating a relocatable object file with the -r flag. I happen to know that Linux kernel modules (.ko files) are represented as relocatable object files, so my suspicion is that the Linux kernel loader doesn't like section names of the form .text.foo in .ko files. If that's the case, a better fix would be to disable function sections only when creating a relocatable object file. That seems fine to me because relocatable files cannot usually be optimized by the linker. (What it probably also means is that in a later change we will want to save the -ffunction-sections and -fdata-sections flags passed at compile time and use them at link time. That would make things more complicated if we had just added a flag, because now we need to decide what to do if the link-time flag conflicts with the compile-time flag.) You can check whether -r was passed by checking whether the LDPT_LINKER_OUTPUT tag passed to the plugin is LDPO_REL.

The -r flag is being used, so your analysis is probably correct. From looking at the code, the kernel turns on function/data sections only when the CONFIG_SPLIT_SECTIONS option is set. The issue with disabling function sections for .ko files is that the gold linker doesn't know that they're turned off for those files. That information is lost because the command line information is lost. Also, I believe I was seeing these function sections in non-.ko files (after the final link).

We have a hack to "fix" function sections for live patching when CONFIG_SPLIT_SECTIONS is enabled. However, it aborted when I tried it on a normal build.

pcc added a comment.Jul 11 2018, 4:42 PM
In D48756#1159339, @pcc wrote:

I think it's important to understand exactly how the Linux kernel is sensitive to being linked, so that once we understand the root cause, we can develop an appropriate fix. It may be that the root cause is a deliberate dependency on an implementation detail, but that doesn't necessarily mean that the right fix is to add a flag, it could mean that we can develop a more targeted fix that doesn't require a flag. Or it could just be a bug, in which case we can fix it uncontroversially without needing to add a flag. In general I think it's important to find a solution that avoids adding flags because that increases the maintenance burden of the program. Here's an excellent article I recently read that makes the point better than I can: http://neugierig.org/software/blog/2018/07/options.html

I'm not a fan of adding options either. However, the justification here is to fix the issue where the gold linker was making an assumption that didn't work for all programs. I also wanted the gold linker to replicate as much as possible "normal" linker behavior when LTO isn't used. In the non-LTO instance, we don't get function/data sections. Given these two things, we really need to supply a way for the programmer to turn this feature off when it doesn't suit their needs—either with a new option or using an existing one.

Existing is better, and as I mentioned this would ideally involve the -ffunction-sections and -fdata-sections flags passed at compile time.

I want to help you get this fixed, so I had one idea about what the root cause might be. Normally the linker will strip the name of the function from the output section when -function-sections is enabled, so .text.foo becomes .text in the output file. What that means is that without any special flags, -function-sections shouldn't normally change the output in any observable way. One case in which it is observable is when creating a relocatable object file with the -r flag. I happen to know that Linux kernel modules (.ko files) are represented as relocatable object files, so my suspicion is that the Linux kernel loader doesn't like section names of the form .text.foo in .ko files. If that's the case, a better fix would be to disable function sections only when creating a relocatable object file. That seems fine to me because relocatable files cannot usually be optimized by the linker. (What it probably also means is that in a later change we will want to save the -ffunction-sections and -fdata-sections flags passed at compile time and use them at link time. That would make things more complicated if we had just added a flag, because now we need to decide what to do if the link-time flag conflicts with the compile-time flag.) You can check whether -r was passed by checking whether the LDPT_LINKER_OUTPUT tag passed to the plugin is LDPO_REL.

The -r flag is being used, so your analysis is probably correct. From looking at the code, the kernel turns on function/data sections only when the CONFIG_SPLIT_SECTIONS option is set. The issue with disabling function sections for .ko files is that the gold linker doesn't know that they're turned off for those files. That information is lost because the command line information is lost.

It's certainly lost at compile time right now. Putting no-function-sections behind -r isn't as good as using the compile-time flags, but it seems like it would make things more likely to work in cases where the consumer of the output file cares about section names, so it seems like it would do until the flag gets hooked up properly.

Also, I believe I was seeing these function sections in non-.ko files (after the final link).

Can you double check which kind of files they were? It's possible that they were non-.ko relocatable files that might have been used to create the final image.

void added a comment.Jul 11 2018, 5:07 PM
In D48756#1159410, @pcc wrote:
In D48756#1159339, @pcc wrote:

I want to help you get this fixed, so I had one idea about what the root cause might be. Normally the linker will strip the name of the function from the output section when -function-sections is enabled, so .text.foo becomes .text in the output file. What that means is that without any special flags, -function-sections shouldn't normally change the output in any observable way. One case in which it is observable is when creating a relocatable object file with the -r flag. I happen to know that Linux kernel modules (.ko files) are represented as relocatable object files, so my suspicion is that the Linux kernel loader doesn't like section names of the form .text.foo in .ko files. If that's the case, a better fix would be to disable function sections only when creating a relocatable object file. That seems fine to me because relocatable files cannot usually be optimized by the linker. (What it probably also means is that in a later change we will want to save the -ffunction-sections and -fdata-sections flags passed at compile time and use them at link time. That would make things more complicated if we had just added a flag, because now we need to decide what to do if the link-time flag conflicts with the compile-time flag.) You can check whether -r was passed by checking whether the LDPT_LINKER_OUTPUT tag passed to the plugin is LDPO_REL.

The -r flag is being used, so your analysis is probably correct. From looking at the code, the kernel turns on function/data sections only when the CONFIG_SPLIT_SECTIONS option is set. The issue with disabling function sections for .ko files is that the gold linker doesn't know that they're turned off for those files. That information is lost because the command line information is lost.

It's certainly lost at compile time right now. Putting no-function-sections behind -r isn't as good as using the compile-time flags, but it seems like it would make things more likely to work in cases where the consumer of the output file cares about section names, so it seems like it would do until the flag gets hooked up properly.

Also, I believe I was seeing these function sections in non-.ko files (after the final link).

Can you double check which kind of files they were? It's possible that they were non-.ko relocatable files that might have been used to create the final image.

They're normal .o files. For example, net/ipv6/ndisc.c is compiled without those flags, but gets function sections during linking. (The linking uses the -r flag. Without it the link fails with a litany of unresolved references.)

pcc added a comment.Jul 11 2018, 5:15 PM
In D48756#1159410, @pcc wrote:
In D48756#1159339, @pcc wrote:

I want to help you get this fixed, so I had one idea about what the root cause might be. Normally the linker will strip the name of the function from the output section when -function-sections is enabled, so .text.foo becomes .text in the output file. What that means is that without any special flags, -function-sections shouldn't normally change the output in any observable way. One case in which it is observable is when creating a relocatable object file with the -r flag. I happen to know that Linux kernel modules (.ko files) are represented as relocatable object files, so my suspicion is that the Linux kernel loader doesn't like section names of the form .text.foo in .ko files. If that's the case, a better fix would be to disable function sections only when creating a relocatable object file. That seems fine to me because relocatable files cannot usually be optimized by the linker. (What it probably also means is that in a later change we will want to save the -ffunction-sections and -fdata-sections flags passed at compile time and use them at link time. That would make things more complicated if we had just added a flag, because now we need to decide what to do if the link-time flag conflicts with the compile-time flag.) You can check whether -r was passed by checking whether the LDPT_LINKER_OUTPUT tag passed to the plugin is LDPO_REL.

The -r flag is being used, so your analysis is probably correct. From looking at the code, the kernel turns on function/data sections only when the CONFIG_SPLIT_SECTIONS option is set. The issue with disabling function sections for .ko files is that the gold linker doesn't know that they're turned off for those files. That information is lost because the command line information is lost.

It's certainly lost at compile time right now. Putting no-function-sections behind -r isn't as good as using the compile-time flags, but it seems like it would make things more likely to work in cases where the consumer of the output file cares about section names, so it seems like it would do until the flag gets hooked up properly.

Also, I believe I was seeing these function sections in non-.ko files (after the final link).

Can you double check which kind of files they were? It's possible that they were non-.ko relocatable files that might have been used to create the final image.

They're normal .o files. For example, net/ipv6/ndisc.c is compiled without those flags, but gets function sections during linking. (The linking uses the -r flag. Without it the link fails with a litany of unresolved references.)

Okay. So it sounds like making no-function-sections conditional on -r is the way to go for now then, provided that it actually lets the kernel boot.

void updated this revision to Diff 155250.Jul 12 2018, 12:17 PM

Is this the change you were thinking of?

There appears to be two unrelated failures in ToT:

FAIL: LLVM :: tools/gold/X86/comdat.ll (25489 of 26550)
******************** TEST 'LLVM :: tools/gold/X86/comdat.ll' FAILED ********************
Script:
--
: 'RUN: at line 1';   /usr/local/google/home/morbo/llvm/llvm.obj/bin/llvm-as /usr/local/google/home/morbo/llvm/llvm.src/test/tools/gold/X86/comdat.ll -o /usr/local/google/home/morbo/llvm/llvm.obj/test/tools/gold/X86/Output/comdat.ll.tmp1.o
: 'RUN: at line 2';   /usr/local/google/home/morbo/llvm/llvm.obj/bin/llvm-as /usr/local/google/home/morbo/llvm/llvm.src/test/tools/gold/X86/Inputs/comdat.ll -o /usr/local/google/home/morbo/llvm/llvm.obj/test/tools/gold/X86/Output/comdat.ll.tmp2.o
: 'RUN: at line 3';   /usr/bin/ld.gold -shared -o /usr/local/google/home/morbo/llvm/llvm.obj/test/tools/gold/X86/Output/comdat.ll.tmp3.o -plugin /usr/local/google/home/morbo/llvm/llvm.obj/./lib/LLVMgold.so /usr/local/google/home/morbo/llvm/llvm.obj/test/tools/gold/X86/Output/comdat.ll.tmp1.o /usr/local/google/home/morbo/llvm/llvm.obj/test/tools/gold/X86/Output/comdat.ll.tmp2.o   -m elf_x86_64   -plugin-opt=save-temps
: 'RUN: at line 6';   /usr/local/google/home/morbo/llvm/llvm.obj/bin/FileCheck --check-prefix=RES /usr/local/google/home/morbo/llvm/llvm.src/test/tools/gold/X86/comdat.ll < /usr/local/google/home/morbo/llvm/llvm.obj/test/tools/gold/X86/Output/comdat.ll.tmp3.o.resolution.txt
: 'RUN: at line 7';   /usr/local/google/home/morbo/llvm/llvm.obj/bin/llvm-readobj -t /usr/local/google/home/morbo/llvm/llvm.obj/test/tools/gold/X86/Output/comdat.ll.tmp3.o | /usr/local/google/home/morbo/llvm/llvm.obj/bin/FileCheck --check-prefix=OBJ /usr/local/google/home/morbo/llvm/llvm.src/test/tools/gold/X86/comdat.ll
--
Exit Code: 1

Command Output (stderr):
--
/usr/local/google/home/morbo/llvm/llvm.src/test/tools/gold/X86/comdat.ll:63:13: error: expected string not found in input
; OBJ-NEXT: Other [
            ^
<stdin>:233:8: note: scanning from here
 Type: Function (0x2)
       ^
<stdin>:234:2: note: possible intended match here
 Other: 0
 ^

--

********************
FAIL: LLVM :: tools/gold/X86/visibility.ll (25549 of 26550)
******************** TEST 'LLVM :: tools/gold/X86/visibility.ll' FAILED ********************
Script:
--
: 'RUN: at line 1';   /usr/local/google/home/morbo/llvm/llvm.obj/bin/llvm-as /usr/local/google/home/morbo/llvm/llvm.src/test/tools/gold/X86/visibility.ll -o /usr/local/google/home/morbo/llvm/llvm.obj/test/tools/gold/X86/Output/visibility.ll.tmp.o
: 'RUN: at line 2';   /usr/local/google/home/morbo/llvm/llvm.obj/bin/llvm-as /usr/local/google/home/morbo/llvm/llvm.src/test/tools/gold/X86/Inputs/visibility.ll -o /usr/local/google/home/morbo/llvm/llvm.obj/test/tools/gold/X86/Output/visibility.ll.tmp2.o
: 'RUN: at line 4';   /usr/bin/ld.gold -plugin /usr/local/google/home/morbo/llvm/llvm.obj/./lib/LLVMgold.so     -m elf_x86_64     --plugin-opt=save-temps     -shared /usr/local/google/home/morbo/llvm/llvm.obj/test/tools/gold/X86/Output/visibility.ll.tmp.o /usr/local/google/home/morbo/llvm/llvm.obj/test/tools/gold/X86/Output/visibility.ll.tmp2.o -o /usr/local/google/home/morbo/llvm/llvm.obj/test/tools/gold/X86/Output/visibility.ll.tmp.so
: 'RUN: at line 8';   /usr/local/google/home/morbo/llvm/llvm.obj/bin/llvm-readobj -t /usr/local/google/home/morbo/llvm/llvm.obj/test/tools/gold/X86/Output/visibility.ll.tmp.so | /usr/local/google/home/morbo/llvm/llvm.obj/bin/FileCheck /usr/local/google/home/morbo/llvm/llvm.src/test/tools/gold/X86/visibility.ll
: 'RUN: at line 9';   /usr/local/google/home/morbo/llvm/llvm.obj/bin/llvm-dis /usr/local/google/home/morbo/llvm/llvm.obj/test/tools/gold/X86/Output/visibility.ll.tmp.so.0.2.internalize.bc -o - | /usr/local/google/home/morbo/llvm/llvm.obj/bin/FileCheck --check-prefix=IR /usr/local/google/home/morbo/llvm/llvm.src/test/tools/gold/X86/visibility.ll
--
Exit Code: 1

Command Output (stderr):
--
/usr/local/google/home/morbo/llvm/llvm.src/test/tools/gold/X86/visibility.ll:14:15: error: expected string not found in input
; CHECK-NEXT: Binding: Global
              ^
<stdin>:67:8: note: scanning from here
 Size: 1
       ^
<stdin>:68:2: note: possible intended match here
 Binding: Weak (0x2)
 ^

--

I'd like Peter to comment on the code changes, but note that the two failures are gold issues that have been fixed in binutils (https://bugs.llvm.org/show_bug.cgi?id=36166).

pcc accepted this revision.Jul 12 2018, 12:32 PM

LGTM

Is this the change you were thinking of?

Yes, pretty much.

tools/gold/gold-plugin.cpp
118 ↗(On Diff #155250)

I'd just have a single variable SplitSections.

void updated this revision to Diff 155259.Jul 12 2018, 1:35 PM
void retitled this revision from [gold-plugin] Add option for section ordering to [gold-plugin] Disable section ordering for relocatable links.
This revision was not accepted when it landed; it landed in state Needs Review.Jul 12 2018, 1:41 PM
This revision was automatically updated to reflect the committed changes.