[ELF] Ignore --no-relax for RISC-V

[MaskRay]

Summary

In GNU ld, --no-relax can disable x86-64 GOTPCRELX relaxation.
It is not useful, so we don't implement it.

For RISC-V, --no-relax disables linker relaxations which have larger
impact.
Linux kernel specifies --no-relax when CONFIG_DYNAMIC_FTRACE is specified
(since http://git.kernel.org/linus/a1d2a6b4cee858a2f27eebce731fbf1dfd72cb4e ).
LLD has not implemented the relaxations, so this option is a no-op.

[grimar]

Can --no-relax just be an ignored option for now? Seems it is only valueable for RISC-V.

[MaskRay]

In D81359#2079188, @grimar wrote:

Can --no-relax just be an ignored option for now? Seems it is only valueable for RISC-V.

The implementation for x86-64 seems simple enough that we can implement it as well.

ppc64 has a similar --no-toc-optimize and once for a while I used it to bypass side effects caused by instruction rewriting.

[grimar]

In D81359#2079839, @MaskRay wrote:

In D81359#2079188, @grimar wrote:

Can --no-relax just be an ignored option for now? Seems it is only valueable for RISC-V.

The implementation for x86-64 seems simple enough that we can implement it as well.

But:

1. No implementing it, but ignoring is a single line in options.td I think.
2. --no-relax name is probably itself confusing because of TLS relaxations that are not affected.
3. It still introduce some code but there is no evidence it is usefull.

[MaskRay]

Remove x86-64 change

[MaskRay]

Add an x86-64 test

[grimar]

lld/ELF/Config.h

188

You do not need this.

lld/ELF/Driver.cpp

977

And this.

lld/ELF/Options.td

340

And this too I think.

631

You just need to list the option(s) here I believe.

[MaskRay]

lld/ELF/Config.h

188

The variable definitions and --help will sooner or later be needed. I don't think it is too troublesome to keep them.

[MaskRay]

lld/ELF/Options.td

631

Adding here will just lead to a future change which deletes the line and adds the help above.

[grimar]

lld/ELF/Config.h

188

In the past we usually never added a code for "sooner or later" when there is a chance that it never be needed. For example, I believe we do not implement all possible relaxations for i386 and nobody seems to care about them. I wouldn't be surprised to see the same situation for RISC-V, though I know almost nothing about it.

Or may be something else might change, as as far I understand this is only used in linux kernel for RISC-V. It sounds strange to me that this is the only target that needs it (it seems). Why do other targets don't need to stop relaxing? Is it a chance this will be solved differently? I.e. a with a compiler flag may be.

In case this will be needed one day (after implementing problematic RISC-V relaxations I guess), the help text might want to explicitly say that only RISC-V is affected.

I'll leave it to others to decide. Perhaps @ruiu might have something to say.

[MaskRay]

In the past we usually never added a code for "sooner or later" when there is a chance that it never be needed. For example, I believe we do not implement all possible relaxations for i386 and nobody seems to care about them. I wouldn't be surprised to see the same situation for RISC-V, though I know almost nothing about it.

In the first revision I added support for x86 as well. It is just a two line change. config->relax it totally justified. (I still don't think it is so useless that we should not implement it. The implementation is trivial.)

Or may be something else might change, as as far I understand this is only used in linux kernel for RISC-V. It sounds strange to me that this is the only target that needs it (it seems). Why do other targets don't need to stop relaxing? Is it a chance this will be solved differently? I.e. a with a compiler flag may be.

Linker relaxation is more powerful/troublesome on RISC-V: it can rewrite two instructions into one and shrink sections. Many more projects may request --relax or --no-relax if they rely on stable code sequences.

[MaskRay]

I have a use case for x86 R_X86_64_[REX_]GOTPCRELX when debugging a glibc static-pie issue.

GNU ld --no-relax (loads an address from GOT):

// elf_get_dynamic_info called by dl-reloc-static-pie.c
 316│ 176           if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_FILES, 0)
 317│    0x00007fefd511896e <+462>:   mov    0x695b(%rip),%rcx        # 0x7fefd511f2d0
###### %rcx is 0 => null pointer dereference
 318│    0x00007fefd5118975 <+469>:   testb  $0x40,(%rcx)

--relax (PC-relative):

316│ 176           if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_FILES, 0)
317│    0x00007f27dd56cdbe <+462>:   lea    0x6f87b(%rip),%rcx        # 0x7f27dd5dc640 <_dl_debug_mask>
318│    0x00007f27dd56cdc5 <+469>:   testb  $0x40,(%rcx)

In a static pie program, it is a bug accessing GOT entry before the program relocates itself, as the GOT entry is likely 0 initially. lea is safe to use. If LLD has --relax --no-relax, it can be easier to verify that the behavior matches GNU ld. Honestly this is still a small use case and I managed to figure out the bug even if LLD does not support --no-relax.

[grimar]

My main objection against this is that it introduces a config->relax which is not used and hence not needed.
I feel really uncomfortable to accept such way to do things as all last years of develepment we tried to avoid adding an unnecessary code to LLD and
I find it to be a very reasonable approach, I remember how I tried to do this, was stopped on reviews and as a result it actually helped to avoid a dead/unneded code few times.

GNU man says (https://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_node/ld_3.html):
An option with machine dependent effects. This option is only supported on a few targets. See section ld and the H8/300. See section ld and the Intel 960 family.
On some platforms, the `--relax' option performs global optimizations that become possible when the linker resolves addressing in the program, such as relaxing
address modes and synthesizing new instructions in the output object file. On platforms where this is not supported, `--relax' is accepted, but ignored.

I'd be OK just to add it to the "Options listed below are silently ignored for now for compatibility." list to ignore it (+ to docs/ld.lld.1),
as currently no platforms are supporting it in LLD.

[grimar]

In D81359#2089918, @MaskRay wrote:

I have a use case for x86 R_X86_64_[REX_]GOTPCRELX when debugging a glibc static-pie issue.

GNU ld --no-relax (loads an address from GOT):

// elf_get_dynamic_info called by dl-reloc-static-pie.c
 316│ 176           if (__builtin_expect (GLRO(dl_debug_mask) & DL_DEBUG_FILES, 0)
 317│    0x00007fefd511896e <+462>:   mov    0x695b(%rip),%rcx        # 0x7fefd511f2d0
###### %rcx is 0 => null pointer dereference
 318│    0x00007fefd5118975 <+469>:   testb  $0x40,(%rcx)

And this I think shows that supporting "--no-relax" for x86_64 can cause new bugs to appear. It is easy to avoid them by ignoring this option.

[MaskRay]

In D81359#2092282, @grimar wrote:

My main objection against this is that it introduces a config->relax which is not used and hence not needed.
I feel really uncomfortable to accept such way to do things as all last years of develepment we tried to avoid adding an unnecessary code to LLD and
I find it to be a very reasonable approach, I remember how I tried to do this, was stopped on reviews and as a result it actually helped to avoid a dead/unneded code few times.

GNU man says (https://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_node/ld_3.html):
An option with machine dependent effects. This option is only supported on a few targets. See section ld and the H8/300. See section ld and the Intel 960 family.
On some platforms, the `--relax' option performs global optimizations that become possible when the linker resolves addressing in the program, such as relaxing
address modes and synthesizing new instructions in the output object file. On platforms where this is not supported, `--relax' is accepted, but ignored.

I'd be OK just to add it to the "Options listed below are silently ignored for now for compatibility." list to ignore it (+ to docs/ld.lld.1),
as currently no platforms are supporting it in LLD.

My argument is whether we can go back to my first revision (https://reviews.llvm.org/D81359?id=269101) which did something potentially useful for x86-64. I have provided one use case: it can help compare --no-relax and --relax output. Then, there is no concern that config->relax is added for no use.

[MaskRay]

In the absence of a third opinion, I give up and make --no-relax an ignored option.

[grimar]

LGTM. Thanks..

lld/ELF/Config.h

struct Configuration {
   bool oFormatBinary;
   bool omagic;
   bool optimizeBBJumps;
   bool optRemarksWithHotness;
   bool picThunk;
   bool pie;
   bool printGcSections;
   bool printIcfSections;
+  bool relax;
   bool relocatable;
   bool relrPackDynRelocs;
   bool saveTemps;
   llvm::Optional<uint32_t> shuffleSectionSeed;
   bool singleRoRx;
   bool shared;
   bool symbolic;
   bool isStatic = false;

lld/ELF/Driver.cpp

static void readConfigs(opt::InputArgList &args) {
   config->pie = args.hasFlag(OPT_pie, OPT_no_pie, false);
   config->printIcfSections =
       args.hasFlag(OPT_print_icf_sections, OPT_no_print_icf_sections, false);
   config->printGcSections =
       args.hasFlag(OPT_print_gc_sections, OPT_no_print_gc_sections, false);
   config->printArchiveStats = args.getLastArgValue(OPT_print_archive_stats);
   config->printSymbolOrder =
       args.getLastArgValue(OPT_print_symbol_order);
+  config->relax = args.hasFlag(OPT_relax, OPT_no_relax, true);
   config->rpath = getRpath(args);
   config->relocatable = args.hasArg(OPT_relocatable);
   config->saveTemps = args.hasArg(OPT_save_temps);
   if (args.hasArg(OPT_shuffle_sections))
     config->shuffleSectionSeed = args::getInteger(args, OPT_shuffle_sections, 0);
   config->searchPaths = args::getStrings(args, OPT_library_path);
   config->sectionStartMap = getSectionStartMap(args);
   config->shared = args.hasArg(OPT_shared);

lld/ELF/Options.td

def pop_state: F<"pop-state">,
   HelpText<"Undo the effect of -push-state">;
 
 def push_state: F<"push-state">,
   HelpText<"Save the current state of -as-needed, -static and -whole-archive">;
 
 def print_map: F<"print-map">,
   HelpText<"Print a link map to the standard output">;
 
+defm relax: BB<"relax",
+  "Enable target-specific relaxations (default)",
+  "Disable target-specific relaxations">;
+
 defm reproduce: Eq<"reproduce", "Write a tar file containing input files and command line options to reproduce link">;
 
 defm rosegment: BB<"rosegment",
   "Put read-only non-executable sections in their own segment (default)",
   "Do not put read-only non-executable sections in their own segment">;
 
 defm rpath: Eq<"rpath", "Add a DT_RUNPATH to the output">;
 

 // Ignore GCC collect2 LTO plugin related options. Note that we don't support
 // GCC LTO, but GCC collect2 passes these options even in non-LTO mode.
 def: J<"plugin-opt=-fresolution=">;
 def: J<"plugin-opt=-pass-through=">;
 // This may be either an unhandled LLVMgold.so feature or GCC passed
 // -plugin-opt=path/to/{liblto_plugin.so,lto-wrapper}
 def plugin_opt_eq : J<"plugin-opt=">;
 
 // Options listed below are silently ignored for now for compatibility.
 def: F<"detect-odr-violations">;
 def: Flag<["-"], "g">;
 def: F<"long-plt">;
 def: F<"no-add-needed">;
 def: F<"no-copy-dt-needed-entries">;
 def: F<"no-ctors-in-init-array">;
 def: F<"no-keep-memory">;
 def: F<"no-pipeline-knowledge">;

lld/docs/ld.lld.1

 .It Fl -no-gnu-unique
 Disable STB_GNU_UNIQUE symbol binding.
 .It Fl -no-merge-exidx-entries
 Disable merging .ARM.exidx entries.
 .It Fl -no-nmagic
 Page align sections.
 .It Fl -no-omagic
 Do not set the text data sections to be writable, page align sections.
+.It Fl -no-relax
+Disable target-specific relaxations. This is currently a no-op.
 .It Fl -no-rosegment
 Do not put read-only non-executable sections in their own segment.
 .It Fl -no-undefined-version
 Report version scripts that refer undefined symbols.
 .It Fl -no-undefined
 Report unresolved symbols even if the linker is creating a shared library.
 .It Fl -no-warn-symbol-ordering
 Do not warn about problems with the symbol ordering file or call graph profile.

lld/test/ELF/x86-64-gotpc-relax.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -relax-relocations -triple=x86_64-unknown-linux %s -o %t.o
 # RUN: ld.lld %t.o -o %t1
 # RUN: llvm-readobj -r %t1 | FileCheck --check-prefix=RELOC %s
 # RUN: llvm-objdump -d %t1 | FileCheck --check-prefix=DISASM %s
 
+## Accept but ignore --relax and --no-relax.
+# RUN: ld.lld --relax %t.o -o %t2
+# RUN: cmp %t1 %t2
+# RUN: ld.lld --no-relax %t.o -o %t3
+# RUN: cmp %t1 %t3
+
 ## There is no relocations.
 # RELOC:    Relocations [
 # RELOC:    ]
 
 # 0x201173 + 7 - 10 = 0x201170
 # 0x20117a + 7 - 17 = 0x201170
 # 0x201181 + 7 - 23 = 0x201171
 # 0x201188 + 7 - 30 = 0x201171