[lld][ELF]Add option to make .dynamic read only

[jakehehrlich]

Summary

The .dynamic section of an ELF almost doesn't need to be written to with the exception of the DT_DEBUG entry. For several reasons having a read only .dynamic section would be useful. This change adds the -z keyword "rodynamic" which forces .dynamic to be read-only. In this case DT_DEBUG will not be emited.

[ruiu]

I have a couple of questions.

• Why can't .dynamic be read-only for all output?
• .dynamic is already covered by PT_GNU_RELRO segments. Does it make sense to make it read-only from beginning?

It seems that you are proposing a new command line option that even GNU linkers don't have, so this patch needs extra attention. Are you adding that option to GNU linkers too?

[ruiu]

On second thought, it isn't good to discuss new linker command line option in a code review thread, as most people do not read every review threads. Please start a thread in llvm-dev to propose a new option with an explanation why you want it.

[emaste]

Does it make sense to make it read-only from beginning?

For me I think having .dynamic relro is sufficient, and might actually like to have MIPS use the conventional DT_DEBUG scheme.

[pcc]

In D33251#756693, @emaste wrote:

Does it make sense to make it read-only from beginning?

For me I think having .dynamic relro is sufficient, and might actually like to have MIPS use the conventional DT_DEBUG scheme.

Agreed. It seems that this scheme exists only for historical reasons: looking through binutils history, DT_MIPS_RLD_MAP was introduced in 1994 [0], while PT_GNU_RELRO was introduced 10 years later [1].

[0] https://github.com/bminor/binutils-gdb/commit/4c040a649b610ab1a0b5aa30c01148bffa9e2f4b
[1] https://github.com/bminor/binutils-gdb/commit/8c37241be3b1baf394090269b4b67babceb83d61

[jakehehrlich]

After some discussion on llvm-dev it was decided that it would be best to not mess with DT_DEBUG_INDIRECT.

[ruiu]

I'm not convinced that we need this, so please don't submit.

If this is for your OS's vDSO which needs to be read by an in-kernel ELF loader which doesn't support the entire ELF spec, this patch is to work around your loader's limitation. If you don't want to implement a complete ELF loader, why do you want to "fix" the problem in the right way? It is odd that you do hack at one place and do the right thing at the other place. Your loader can ignore RW bit and map .dynamic sections to read-only segments.

[ruiu]

ELF/SyntheticSections.cpp

1008 ↗

(On Diff #100014)

... on MIPS and on Fuchsia OS which passes -z rodynamic.

1011 ↗

(On Diff #100014)

Probably it is better to not add this comment as it is not informative but could be confusing (e.g. why users may want it on other systems?)

1057 ↗

(On Diff #100014)

Add a space after // and a full stop at end of the line.

But you want to extend the comment to explain the background. I'd write something like:

DT_DEBUG points to a location containing some debug info so that debuggers can find debug info at runtime. We need it for each process, so we don't write it for DSOs.

DT_DEBUG is the only .dynamic entry to which the loader is expected to write. Some systems, such as Fuchsia, chose to make .dynamic sections completely read-only (and provides alternative mean to find debug info). If the target is such system, we don't write DT_DEBUG.

[jakehehrlich]

I made the changes Rui recommended.

[ruiu]

ELF/Config.h

147 ↗

(On Diff #100329)

These variables are named after their corresponding command line options, so ZRodynamic.

ELF/SyntheticSections.cpp

1060 ↗

(On Diff #100329)

You want to break the paragraph by adding a blank // line.

1061–1062 ↗

(On Diff #100329)

Let's avoid writing comments too abstractly. "Some systems (currently only Fuchsia)" and instead of "Such systemS", "Fuchsia".

[jakehehrlich]

Fixed a few more things.

[ruiu]

ELF/Config.h

147–148 ↗

(On Diff #100335)

Sort.

ELF/Driver.cpp

690–691 ↗

(On Diff #100335)

Sort

ELF/SyntheticSections.cpp

1064 ↗

(On Diff #100335)

What I wanted is this

// blah blah blah ...
// blah blah blah ...
// 
// blah blah blah ...
// blah blah blah ...

and not

// blah blah blah ...
// blah blah blah ...
// blah blah blah ...
// blah blah blah ...
//

(Please look around to find a local convention and follow it.)

[jakehehrlich]

Sorry about these last fixes. I shouldn't have made those mistakes. Should be fixed.

[ruiu]

test/ELF/rodynamic.s

2 ↗

(On Diff #100347)

This doesn't test the feature you've just added, since -shared is passed. You want to remove that flag.

8–13 ↗

(On Diff #100347)

This checks the presence of .dynamic section, which is not what you actually want to do. You want to use llvm-readobj like this.

RUN: ld.lld -o %t1 %t.o
RUN: llvm-readobj -dynamic-table %t1 | FileCheck -check-prefix=DEFAULT %s
DEFAULT: DEBUG

RUN: ld.lld -o %t2 %t.o -z rodynamic
RUN: llvm-readobj -dynamic-table %t2 | FileCheck %s
CHECK-NOT: DEBUG

[jakehehrlich]

Right! I forgot to check that DEBUG wasn't being emitted in the test. I also forgot to check the default behavior. The other part of the test ensures that the .dynamic section is in fact read-only and that in the default case it is read-write. I had to produce a shared library to make the executable have a .dynamic section as well so I added a simple file to Inputs.

[ruiu]

LGTM with these changes.

test/ELF/rodynamic.s

3 ↗

(On Diff #100355)

Break right here

7 ↗

(On Diff #100355)

and here to make it more comfortable to read. (It is a good practice to separate code blocks with blank lines so that you don't need to read an entire block of code without breathing.)

[jakehehrlich]

Made the final fix.

lld/trunk/ELF/Config.h

struct Configuration {
   bool ZCombreloc;
   bool ZExecstack;
   bool ZNocopyreloc;
   bool ZNodelete;
   bool ZNodlopen;
   bool ZNow;
   bool ZOrigin;
   bool ZRelro;
+  bool ZRodynamic;
   bool ZText;
   bool ExitEarly;
   bool ZWxneeded;
   DiscardPolicy Discard;
   SortSectionPolicy SortSection;
   StripPolicy Strip;
   UnresolvedPolicy UnresolvedSymbols;
   Target2Policy Target2;

lld/trunk/ELF/Driver.cpp

void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   Config->ZCombreloc = !hasZOption(Args, "nocombreloc");
   Config->ZExecstack = hasZOption(Args, "execstack");
   Config->ZNocopyreloc = hasZOption(Args, "nocopyreloc");
   Config->ZNodelete = hasZOption(Args, "nodelete");
   Config->ZNodlopen = hasZOption(Args, "nodlopen");
   Config->ZNow = hasZOption(Args, "now");
   Config->ZOrigin = hasZOption(Args, "origin");
   Config->ZRelro = !hasZOption(Args, "norelro");
+  Config->ZRodynamic = hasZOption(Args, "rodynamic");
   Config->ZStackSize = getZOptionValue(Args, "stack-size", 0);
   Config->ZText = !hasZOption(Args, "notext");
   Config->ZWxneeded = hasZOption(Args, "wxneeded");
 
   if (Config->LTOO > 3)
     error("invalid optimization level for LTO: " + getString(Args, OPT_lto_O));
   if (Config->LTOPartitions == 0)
     error("--lto-partitions: number of threads must be > 0");

lld/trunk/ELF/SyntheticSections.cpp

 static unsigned getVerDefNum() { return Config->VersionDefinitions.size() + 1; }
 
 template <class ELFT>
 DynamicSection<ELFT>::DynamicSection()
     : SyntheticSection(SHF_ALLOC | SHF_WRITE, SHT_DYNAMIC, Config->Wordsize,
                        ".dynamic") {
   this->Entsize = ELFT::Is64Bits ? 16 : 8;
 
-  // .dynamic section is not writable on MIPS.
+  // .dynamic section is not writable on MIPS and on Fuchsia OS
+  // which passes -z rodynamic.
   // See "Special Section" in Chapter 4 in the following document:
   // ftp://www.linux-mips.org/pub/linux/mips/doc/ABI/mipsabi.pdf
-  if (Config->EMachine == EM_MIPS)
+  if (Config->EMachine == EM_MIPS || Config->ZRodynamic)
     this->Flags = SHF_ALLOC;
 
   addEntries();
 }
 
 // There are some dynamic entries that don't depend on other sections.
 // Such entries can be set early.
 template <class ELFT> void DynamicSection<ELFT>::addEntries() {
if (Config->ZOrigin) {
     DtFlags1 |= DF_1_ORIGIN;
   }
 
   if (DtFlags)
     add({DT_FLAGS, DtFlags});
   if (DtFlags1)
     add({DT_FLAGS_1, DtFlags1});
 
-  if (!Config->Shared && !Config->Relocatable)
+  // DT_DEBUG is a pointer to debug informaion used by debuggers at runtime. We
+  // need it for each process, so we don't write it for DSOs. The loader writes
+  // the pointer into this entry.
+  //
+  // DT_DEBUG is the only .dynamic entry that needs to be written to. Some
+  // systems (currently only Fuchsia OS) provide other means to give the
+  // debugger this information. Such systems may choose make .dynamic read-only.
+  // If the target is such a system (used -z rodynamic) don't write DT_DEBUG.
+  if (!Config->Shared && !Config->Relocatable && !Config->ZRodynamic)
     add({DT_DEBUG, (uint64_t)0});
 }
 
 // Add remaining entries to complete .dynamic contents.
 template <class ELFT> void DynamicSection<ELFT>::finalizeContents() {
   if (this->Size)
     return; // Already finalized.
 

lld/trunk/test/ELF/Inputs/rodynamic.s

+.global foo
+.type foo, @function
+foo:
+  ret

lld/trunk/test/ELF/rodynamic.s

+# RUN: llvm-mc %s -o %t.o -filetype=obj -triple=x86_64-pc-linux
+# RUN: llvm-mc %p/Inputs/rodynamic.s -o %t.so.o -filetype=obj -triple=x86_64-pc-linux
+
+# RUN: ld.lld -shared %t.so.o -o %t.so
+# RUN: ld.lld %t.o %t.so -o %t.exe
+# RUN: llvm-readobj -dynamic-table %t.exe | FileCheck -check-prefix=DEFDEBUG %s
+# RUN: llvm-readobj -sections %t.exe | FileCheck -check-prefix=DEFSEC %s
+
+# RUN: ld.lld -shared -z rodynamic %t.so.o -o %t.so
+# RUN: ld.lld -z rodynamic %t.o %t.so -o %t.exe
+# RUN: llvm-readobj -dynamic-table %t.exe | FileCheck -check-prefix=RODEBUG %s
+# RUN: llvm-readobj -sections %t.exe | FileCheck -check-prefix=ROSEC %s
+
+.globl _start
+_start:
+  call foo
+
+# DEFDEBUG: DEBUG
+
+# DEFSEC:      Section {
+# DEFSEC:        Name: .dynamic
+# DEFSEC-NEXT:   Type: SHT_DYNAMIC
+# DEFSEC-NEXT:   Flags [
+# DEFSEC-NEXT:     SHF_ALLOC
+# DEFSEC-NEXT:     SHF_WRITE
+# DEFSEC-NEXT:   ]
+
+# RODEBUG-NOT: DEBUG
+
+# ROSEC:      Section {
+# ROSEC:        Name: .dynamic
+# ROSEC-NEXT:   Type: SHT_DYNAMIC
+# ROSEC-NEXT:   Flags [
+# ROSEC-NEXT:     SHF_ALLOC
+# ROSEC-NEXT:   ]