[ELF] Error if -Ttext-segment is specified

[MaskRay]

Summary

In GNU ld, -Ttext sets the address of the .text section and -Ttext-segment sets the address of the text segment (RX).

gold only supports the -Ttext-segment semantic and treats -Ttext as an alias for -Ttext-segment.

lld only supports the -Ttext semantic and treats -Ttext-segment as an
alias for -Ttext. The text segment will be assigned to an address less
than the specified -Ttext-segment value.

This patch drops the -Ttext-segment alias.

The text segment is traditionally the first segment. Users who specify
-Ttext-segment may actually want to specify --image-base, the lld way to
express this. Unfortunately currently this is supported by GNU ld's
COFF port but not by its ELF port. gold does not support this option.
With -z separate-code, the behavior of GNU ld -Ttext-segment is weird (see https://sourceware.org/bugzilla/show_bug.cgi?id=25207)

rL289827 introduced the alias for linking qemu's non-pie user mode
binaries. As explained previously, this actually assigns the text
segment to an address less than 0x60000000. I feel that a better fix is
on the qemu side:
https://lists.nongnu.org/archive/html/qemu-devel/2019-11/msg02480.html

[ruiu]

lld/ELF/Driver.cpp

1015–1016

Could you expand the comment so that you explain a reasoning just as you explained it in the commit message?

[MaskRay]

Expand a comment

[ruiu]

LGTM

lld/ELF/Options.td

62

I'd remove HelpText so that it is recognized but doesn't appear in the --help message, unless you actually try to use the option. We have other unsupported options, and showing only this one seems a bit odd to me.

[MaskRay]

Remove HelpText from Ttext_segment.

[grimar]

LGTM with a test.

lld/ELF/Driver.cpp

1020

-Ttext-segment->--Ttext-segment?
(here and in the comment)

I'd also suggest to add a test for this error. At least testing errors is useful for people who wants to search for a test for a particular option to see if it is supported and how.

[MaskRay]

lld/ELF/Driver.cpp

1020

-Ttext-segment is mix cased and documented in GNU ld as a one-dash option. Options that are documented as double-dash ones are exclusively lower case only. So I think we probably should just use -Ttext-segment.

[MaskRay]

Forgot to git add a test...

[troyj]

Unfortunately currently this is supported by GNU ld's COFF port but not by its ELF port. gold does not support this option.

Just picked up this commit downstream today and likely will be reverting it locally. I think it was a bad decision to remove an option like this when the expected replacement isn't supported by the other linkers yet. Some of us have environments where not everything is aware of the different nuances between bfd/gold/lld and so scripts supply common options to all three, which generally tends to work for most common options. As such, switching to --image-base isn't an option until the other linkers also support it.

[MaskRay]

In D70468#1857300, @troyj wrote:

Unfortunately currently this is supported by GNU ld's COFF port but not by its ELF port. gold does not support this option.

Just picked up this commit downstream today and likely will be reverting it locally. I think it was a bad decision to remove an option like this when the expected replacement isn't supported by the other linkers yet. Some of us have environments where not everything is aware of the different nuances between bfd/gold/lld and so scripts supply common options to all three, which generally tends to work for most common options. As such, switching to --image-base isn't an option until the other linkers also support it.

@troyj I think it was a good decision. As the description says, -Ttext and -Ttext-segment are different. gold incorrectly treats them the same. lld incorrectly did that too, with a different interpretation. D70468 made it clear that -Ttext-segment is not supported. If you are happy with the previous behavior, s/-Ttext-segment/-Ttext/

[troyj]

I appreciate your point of view, but it's not very pragmatic given how "baked in" an old option like this is across existing environments. I have reverted downstream and we will continue to support the option ourselves until it can be phased out with less of an impact.

[MaskRay]

In D70468#1857432, @troyj wrote:

I appreciate your point of view, but it's not very pragmatic given how "baked in" an old option like this is across existing environments. I have reverted downstream and we will continue to support the option ourselves until it can be phased out with less of an impact.

I appreciate your feedback. In some cases there are still subtle differences among linkers. For this -Ttext-segment we faced a decision whether we should silently interpret an option incorrectly or leave it as an hard error. We chose the latter. I know the documentation says lld aims to be a drop-in placement of GNU linkers, but in my opinion in some cases we really need downstream users to do some necessary refactorings to adopt lld. This is like adopting clang. If a project enables -Werror and clang happens to report more diagnostics. That project may have to do some adaptations. Clang developers will assuredly consider the positive rate when deciding whether an option should be enabled by default. This may be more so for closed source software.

I remember that Peter Smith once said we probably need to consider a deprecation mechanism. For some more common options, if we eventually want to make some semantic changes, we should to consider that. For this -Ttext-segment, however, I don't think making it warning first then upgrading it to an error in a later release is worth the effort.

[troyj]

For this -Ttext-segment we faced a decision whether we should silently interpret an option incorrectly or leave it as an hard error.

Understood, but since the incorrect interpretation was silently "working," it is necessary for us to remove this change until the option is no longer used anywhere.

I know the documentation says lld aims to be a drop-in placement of GNU linkers, but in my opinion in some cases we really need downstream users to do some necessary refactorings to adopt lld. This is like adopting clang.

It's not quite the same. We have adopted Clang, but Clang (and GNU) provide a -fuse-ld option, which means that the choice of linker is actually exposed to the user. We prefer to use lld as our default because we can more easily modify its source as necessary (such as in this case), but in practice the reason that our users sometimes need to select a specific linker is to workaround bugs in bfd/gold/lld. Normally the bugs present themselves as relocation errors that appear with only one of the three linkers. Therefore, the users need to be able to swap between the different linkers and any option that is not supported by all linkers requires additional effort -- either educating the user to be aware and change their options, or having scripts that automatically detect the active linker and hide the differences. Either takes time to implement and needs to be implemented before we leave the hard error in place.

I remember that Peter Smith once said we probably need to consider a deprecation mechanism

Yes, that would have been appreciated here.

[MaskRay]

In D70468#1857643, @troyj wrote:

For this -Ttext-segment we faced a decision whether we should silently interpret an option incorrectly or leave it as an hard error.

Understood, but since the incorrect interpretation was silently "working," it is necessary for us to remove this change until the option is no longer used anywhere.

It was "working" for your use case but "incorrect" in some others (qemu -Ttext-segment (Richard Henderson did more refactoring to qemu's configure and this kind of stuff is completed deleted now), an internal ld.so like program I debugged).

I know the documentation says lld aims to be a drop-in placement of GNU linkers, but in my opinion in some cases we really need downstream users to do some necessary refactorings to adopt lld. This is like adopting clang.

It's not quite the same. We have adopted Clang, but Clang (and GNU) provide a -fuse-ld option, which means that the choice of linker is actually exposed to the user. We prefer to use lld as our default because we can more easily modify its source as necessary (such as in this case), but in practice the reason that our users sometimes need to select a specific linker is to workaround bugs in bfd/gold/lld. Normally the bugs present themselves as relocation errors that appear with only one of the three linkers. Therefore, the users need to be able to swap between the different linkers and any option that is not supported by all linkers requires additional effort -- either educating the user to be aware and change their options, or having scripts that automatically detect the active linker and hide the differences. Either takes time to implement and needs to be implemented before we leave the hard error in place.

For your amusement, I filed a feature request/bug report regarding GNU ld's less rigid behavior about R_PPC_REL16_LO https://sourceware.org/bugzilla/show_bug.cgi?id=25500
There is a possibility that it will be closed as a wontfix. I am glad that lld has been rigid since its early days. If we receive feature requests about special casing R_PPC_REL16_LO and silencing such reasonable diagnostics, honestly I will consider them unreasonable.

It looks like you already modify lld source, so I agree this change is a bit inconvenient to you but it is not insurmountable. Many builds also enable -Wl,--fatal-warnings. Even linker warnings will not make them happy.

I remember that Peter Smith once said we probably need to consider a deprecation mechanism

Yes, that would have been appreciated here.

lld/ELF/Driver.cpp

static void readConfigs(opt::InputArgList &args) {
   if (config->ltoPartitions == 0)
     error("--lto-partitions: number of threads must be > 0");
   if (config->thinLTOJobs == 0)
     error("--thinlto-jobs: number of threads must be > 0");
 
   if (config->splitStackAdjustSize < 0)
     error("--split-stack-adjust-size: size must be >= 0");
 
+  // The text segment is traditionally the first segment, whose address equals
+  // the base address. However, lld places the R PT_LOAD first. -Ttext-segment
+  // is an old-fashioned option that does not play well with lld's layout.
+  // Suggest --image-base as a likely alternative.
+  if (args.hasArg(OPT_Ttext_segment))
+    error("-Ttext-segment is not supported. Use --image-base if you "
+          "intend to set the base address");
+
   // Parse ELF{32,64}{LE,BE} and CPU type.
   if (auto *arg = args.getLastArg(OPT_m)) {
     StringRef s = arg->getValue();
     std::tie(config->ekind, config->emachine, config->osabi) =
         parseEmulation(s);
     config->mipsN32Abi =
         (s.startswith("elf32btsmipn32") || s.startswith("elf32ltsmipn32"));
     config->emulation = s;

lld/ELF/Options.td

 def O: JoinedOrSeparate<["-"], "O">, HelpText<"Optimize output file size">;
 
 defm Tbss: Eq<"Tbss", "Same as --section-start with .bss as the sectionname">;
 
 defm Tdata: Eq<"Tdata", "Same as --section-start with .data as the sectionname">;
 
 defm Ttext: Eq<"Ttext", "Same as --section-start with .text as the sectionname">;
 
+def Ttext_segment: Separate<["-", "--"], "Ttext-segment">;
+
 defm allow_multiple_definition: B<"allow-multiple-definition",
     "Allow multiple definitions",
     "Do not allow multiple definitions (default)">;
 
 defm allow_shlib_undefined: B<"allow-shlib-undefined",
     "Allow unresolved references in shared libraries (default when linking a shared library)",
     "Do not allow unresolved references in shared libraries (default when linking an executable)">;
 

 def: JoinedOrSeparate<["-"], "R">, Alias<rpath>, HelpText<"Alias for --rpath">;
 def: JoinedOrSeparate<["-"], "T">, Alias<script>, HelpText<"Alias for --script">;
 def: F<"Bshareable">, Alias<shared>, HelpText<"Alias for --shared">;
 def: JoinedOrSeparate<["-"], "h">, Alias<soname>, HelpText<"Alias for --soname">;
 def: Flag<["-"], "(">, Alias<start_group>, HelpText<"Alias for --start-group">;
 def: Flag<["-"], "s">, Alias<strip_all>, HelpText<"Alias for --strip-all">;
 def: Flag<["-"], "S">, Alias<strip_debug>, HelpText<"Alias for --strip-debug">;
 def: Flag<["-"], "t">, Alias<trace>, HelpText<"Alias for --trace">;
+def: Joined<["-", "--"], "Ttext-segment=">, Alias<Ttext_segment>;
 def: JoinedOrSeparate<["-"], "y">, Alias<trace_symbol>, HelpText<"Alias for --trace-symbol">;
-def: Separate<["-", "--"], "Ttext-segment">, Alias<Ttext>, HelpText<"Alias for --Ttext">;
-def: Joined<["-", "--"], "Ttext-segment=">, Alias<Ttext>, HelpText<"Alias for --Ttext">;
 def: JoinedOrSeparate<["-"], "u">, Alias<undefined>, HelpText<"Alias for --undefined">;
 def: Flag<["-"], "V">, Alias<version>, HelpText<"Alias for --version">;
 
 // LTO-related options.
 def lto_aa_pipeline: J<"lto-aa-pipeline=">,
   HelpText<"AA pipeline to run during LTO. Used in conjunction with -lto-newpm-passes">;
 def lto_debug_pass_manager: F<"lto-debug-pass-manager">,
   HelpText<"Debug new pass manager">;

lld/test/ELF/sectionstart.s

 # RUN: ld.lld %t.o --section-start .text=100000 \
 # RUN:   --section-start .data=110000 --section-start .bss=0x200000 -o %t1
 # RUN: llvm-objdump -section-headers %t1 | FileCheck %s
 
 ## Use -Ttext, -Tdata, -Tbss as replacement for --section-start:
 # RUN: ld.lld %t.o -Ttext=0x100000 -Tdata=0x110000 -Tbss=0x200000 -o %t4
 # RUN: llvm-objdump -section-headers %t4 | FileCheck %s
 
-## Check Ttext-segment X and Ttext-segment=X forms.
-# RUN: ld.lld %t.o -Ttext-segment=0x100000 -Tdata=0x110000 -Tbss=0x200000 -o %t4
-# RUN: llvm-objdump -section-headers %t4 | FileCheck %s
-# RUN: ld.lld %t.o -Ttext-segment 0x100000 -Tdata=0x110000 -Tbss=0x200000 -o %t4
-# RUN: llvm-objdump -section-headers %t4 | FileCheck %s
-
 ## The same, but dropped "0x" prefix.
 # RUN: ld.lld %t.o -Ttext=100000 -Tdata=110000 -Tbss=200000 -o %t5
 # RUN: llvm-objdump -section-headers %t5 | FileCheck %s
 
 ## Check form without assignment:
 # RUN: ld.lld %t.o -Ttext 0x100000 -Tdata 0x110000 -Tbss 0x200000 -o %t4
 # RUN: llvm-objdump -section-headers %t4 | FileCheck %s
 

lld/test/ELF/ttext-segment.s

+# REQUIRES: x86
+## Check that we emit an error for -Ttext-segment.
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 /dev/null -o %t.o
+# RUN: not ld.lld %t.o -Ttext-segment=0x100000 -o /dev/null 2>&1 | FileCheck %s
+# RUN: not ld.lld %t.o -Ttext-segment 0x100000 -o /dev/null 2>&1 | FileCheck %s
+
+# CHECK: error: -Ttext-segment is not supported. Use --image-base if you intend to set the base address