[ELF2] - dont merge .data.rel.ro/.data.rel.ro.local into .data section.

[grimar]

Summary

This sections can be protected with relro after resolving relocations by dynamic linker.

[ruiu]

ELF/Writer.cpp

465–468 ↗

(On Diff #39895)

Is this equivalent to this?

if (S.startswith(".data.rel.ro")
  return S;

[grimar]

ELF/Writer.cpp

465–468 ↗

(On Diff #39895)

I am not sure it is. Thats depends on if something like ".data.rel.ro.1" can appear in .o files. I dont think I saw such things and dont know if that is possible. Probably someone can rely on this behavior in gold. My implementation here is similiar to what gold do and I would keep that for behavior consistency.

[• rafael]

I am taking a look at what exactly these sections are, but in general
sections can have suffixes because of
-ffunction-section/-fdata-section.

[grimar]

If the main aim to make code shorter then I would suggest next one.

if (S.startswith(".data.rel.ro")
  return ".data.rel.ro";

The only point to have this sections in output is to split the rw .data to read only part + smaller rw .data for Relro. There are probably no reasons to keep both .data.rel.ro and .data.rel.ro.local for that. Except behavior consistency with gold of-cource.

[• rafael]

The logic behind .data.rel.ro is simple: The compiler knows that any
data in it is read only once the dynamic linker is done.

For .data.rel.ro.local things are not as simple. It was created to
support prelinking. The idea was to

• Put in .data.rel.ro.local data that is ro once the dynamic linker is

done and whose relocations resolve to the same DSO.

• Have a prelink program assign addresses to the DSOs and resolve the

relocations in .data.rel.ro.local.

There are a few things that are bad with this

• There are ways of speeding up DSOs that don't compromise security.
• The linker knows if a reloattion will always refer to the same DSO

or not, so having the compiler pass that down seems redundant.

• The prelinker seems dead. It has removed from fedora after failing

to build for two releases:
http://pkgs.fedoraproject.org/cgit/prelink.git/commit/?id=eb43100a8331d91c801ee3dcdb0a0bb9babfdc1f

As for the patch, it would also have been incomplete, since it was not
putting the .local sections in a contiguous range.

My suggestion then is:

• Map ".data.rel.ro.*" to ".data.rel.ro". That is, fully ignore the .local part.
• Drop prelink support from llvm (I am writing the patch).

Cheers,
Rafael

[grimar]

Updated to suggested variant.

[ruiu]

LGTM

[• rafael]

LGTM

lld/trunk/ELF/Writer.cpp

StringRef Writer<ELFT>::getOutputSectionName(StringRef S) const {
   auto It = InputToOutputSection.find(S);
   if (It != std::end(InputToOutputSection))
     return It->second;
 
   if (S.startswith(".text."))
     return ".text";
   if (S.startswith(".rodata."))
     return ".rodata";
+  if (S.startswith(".data.rel.ro"))
+    return ".data.rel.ro";
   if (S.startswith(".data."))
     return ".data";
   if (S.startswith(".bss."))
     return ".bss";
   return S;
 }
 
 template <class ELFT>

lld/trunk/test/elf2/section-name.s

 .section .rodata.a,"a"
 .section .rodata,"a"
 .section .data.a,"aw"
 .section .data,"aw"
 .section .bss.a,"",@nobits
 .section .bss,"",@nobits
 .section .foo.a,"aw"
 .section .foo,"aw"
+.section .data.rel.ro,"aw",%progbits
+.section .data.rel.ro.a,"aw",%progbits
+.section .data.rel.ro.local,"aw",%progbits
+.section .data.rel.ro.local.a,"aw",%progbits
 
 // CHECK-NOT: Name: .rodata.a
 // CHECK:     Name: .rodata
 // CHECK-NOT: Name: .text.a
 // CHECK:     Name: .text
 // CHECK-NOT: Name: .data.a
 // CHECK:     Name: .data
 // CHECK:     Name: .foo.a
 // CHECK:     Name: .foo
+// CHECK-NOT: Name: .data.rel.ro.a
+// CHECK-NOT: Name: .data.rel.ro.local.a
+// CHECK:     Name: .data.rel.ro
 // CHECK-NOT: Name: .bss.a
 // CHECK:     Name: .bss