[ELF] - Stop naming relocation sections with first input section name.

[grimar]

Summary

It was reported (https://reviews.llvm.org/D38724#902841) that when we use
-ffunction-sections --emit-relocs build, REL[A] output section receives the name of first
input section, like .rela.text.first_function_in_text rather than .rela.text.

It is probably not really an issue as sh_info still points to correct target section, but
it does not look clean in output and allows internal section name to leak there,
what at least looks confusing and is not consistent with ld.bfd.

Patch changes this behavior so that target output section name is used as a base.

[grimar]

• Better version.

[ruiu]

ELF/Writer.cpp

101 ↗

(On Diff #120072)

Is there an "abnormal" situation in which this code is used even if --emit-relocs is not given?

104–110 ↗

(On Diff #120072)

Is this the same as this?

if (Name.startswith(".rel."))
  return Saver.save(".rel." + getOutputSectionName(Name.substr(4)));
if (Name.startswith(".rela."))
  return Saver.save(".rela." + getOutputSectionName(Name.substr(5)));

[grimar]

ELF/Writer.cpp

101 ↗

(On Diff #120072)

Yes. Since this code called for synthetic sections, it executes for ".rela.dyn" for example.
It looks to be safe, as it returns the same result as original code for that case, so does
not seem we need additional checks here.

104–110 ↗

(On Diff #120072)

That was something close to what I did in the first diff. I do not think we want to do that.
getOutputSectionName sometimes returns value from string saver, or for "COMMON" case it returns
".bss". So it is not consistent and not always returns the part of the same string.

I am not sure how much is possible to create .rela.COMMON or .rela.zdebug_ section, but I think we should not
write code that assumes it is impossible, as it is too complicated assumption which needs additional testing probably.
I believe my version is much more straightforward and the fact it rarely allocates the string is not a great issue.

[ruiu]

ELF/Writer.cpp

104–110 ↗

(On Diff #120072)

I just tried to unroll your loop, so there shouldn't be any semantic differences. What am I missing?

[grimar]

ELF/Writer.cpp

104–110 ↗

(On Diff #120072)

I am sorry, I think I read your code wrong at first. Yes, that should be equal and your way looks better for me.

[grimar]

• Used code suggested.

[ruiu]

LGTM

ELF/Writer.cpp

101–103 ↗

(On Diff #120389)

This is for --emit-relocs. If .text.foo is emitted as .text, we want to emit .rela.text.foo as .rel.text for consistency (this is not technically required, but not doing it is odd). This code guarantees that.

lld/trunk/ELF/Writer.cpp

StringRef elf::getOutputSectionName(StringRef Name) {
   // ".zdebug_" is a prefix for ZLIB-compressed sections.
   // Because we decompressed input sections, we want to remove 'z'.
   if (Name.startswith(".zdebug_"))
     return Saver.save("." + Name.substr(2));
 
   if (Config->Relocatable)
     return Name;
 
+  // This is for --emit-relocs. If .text.foo is emitted as .text, we want to
+  // emit .rela.text.foo as .rel.text for consistency (this is not technically
+  // required, but not doing it is odd). This code guarantees that.
+  if (Name.startswith(".rel."))
+    return Saver.save(".rel" + getOutputSectionName(Name.substr(4)));
+  if (Name.startswith(".rela."))
+    return Saver.save(".rela" + getOutputSectionName(Name.substr(5)));
+
   for (StringRef V :
        {".text.", ".rodata.", ".data.rel.ro.", ".data.", ".bss.rel.ro.",
         ".bss.", ".init_array.", ".fini_array.", ".ctors.", ".dtors.", ".tbss.",
         ".gcc_except_table.", ".tdata.", ".ARM.exidx.", ".ARM.extab."}) {
     StringRef Prefix = V.drop_back();
     if (Name.startswith(V) || Name == Prefix)
       return Prefix;
   }

lld/trunk/test/ELF/emit-relocs-merge.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 # RUN: ld.lld --emit-relocs %t.o -o %t.so -shared
 # RUN: llvm-readobj -r %t.so | FileCheck %s
 
 # CHECK:       Relocations [
 # CHECK-NEXT:    Section ({{.*}}) .rela.dyn {
 # CHECK-NEXT:     0x1000 R_X86_64_64 zed 0x0
 # CHECK-NEXT:     0x1008 R_X86_64_64 zed 0x0
 # CHECK-NEXT:   }
-# CHECK-NEXT:   Section ({{.*}}) .rela.data.foo {
+# CHECK-NEXT:   Section ({{.*}}) .rela.data {
 # CHECK-NEXT:     0x1000 R_X86_64_64 zed 0x0
 # CHECK-NEXT:     0x1008 R_X86_64_64 zed 0x0
 # CHECK-NEXT:   }
 # CHECK-NEXT: ]
 
 .section        .data.foo,"aw",%progbits
 .quad zed
 .section        .data.bar,"aw",%progbits
 .quad zed

lld/trunk/test/ELF/emit-relocs.s

 # CHECK-NEXT:     Size: 0
 # CHECK-NEXT:     Binding: Global
 # CHECK-NEXT:     Type: Function
 # CHECK-NEXT:     Other: 0
 # CHECK-NEXT:     Section: .text
 # CHECK-NEXT:   }
 # CHECK-NEXT: ]
 
-.section .text,"ax",@progbits,unique,0
+.section .text.fn,"ax",@progbits,unique,0
 .globl fn
 .type fn,@function
 fn:
  nop
 
 bar:
   movl $bar, %edx
   callq fn@PLT
   nop
 
-.section .text,"ax",@progbits,unique,1
+.section .text.fn2,"ax",@progbits,unique,1
 .globl fn2
 .type fn2,@function
 fn2:
  nop
 
 foo:
   movl $foo, %edx
   callq fn2@PLT
   nop