[ELF] - Disable emiting multiple output sections when merging is disabled.

[grimar]

Summary

When -O0 is specified, we do not do section merging.
Though before this patch several sections were generated instead
of single, what is useless I think.

This is important for current testing of FreeBSD bootstrap.
Particulaty for EFI loader. Since allows to get closer to output of
gnu linkers when needed.

[ruiu]

I think I do not understand what this patch is trying to address. Did you mean that FreeBSD EFI loader passes -O0 to linker?

[grimar]

In D27041#604490, @ruiu wrote:

I think I do not understand what this patch is trying to address. Did you mean that FreeBSD EFI loader passes -O0 to linker?

No it doesn't. My point is next:

For linkerscript we take "M" and "S" flags in account:

uintX_t Flags = C->Flags & (SHF_MERGE | SHF_STRINGS);

That is done because currently our implementation requires one output section per "kind",
what is different from what gnu linkers do.

With -O0 sections metging is disabled:

bool elf::ObjectFile<ELFT>::shouldMerge(const Elf_Shdr &Sec) {
  if (Config->Optimize == 0)
    return false;

In that case there is no point to create more output sections, we can combine them as regular into single section.
Currently that is the only way to get output similar to ld/gold. And currently that way broken.

Returning o EFI loader. Change above is useful for investigation of broken files, including the loader.
Currently LLD output for loader is:

[ 4] .data             PROGBITS         0000000000058000  00059000
      0000000000006355  0000000000000001 AMS       0     0     1
 [ 5] .got              PROGBITS         0000000000068520  0005f520
      00000000000004a0  0000000000000000  WA       0     0     8
 [ 6] .data             PROGBITS         000000000005e360  00055360
      000000000000a142  0000000000000000  WA       0     0     16
 [ 7] .data             PROGBITS         00000000000684b0  0005f4b0
      0000000000000051  0000000000000001 AMS       0     0     16
 [ 8] .data             PROGBITS         0000000000068502  0005f502
      0000000000000016  0000000000000002 AMS       0     0     2
 [ 9] .data             PROGBITS         0000000000068518  0005f518

.got inside .data is fixed by D27040, but there are still multiple .data sections.
That might be a problem and might be not. It is unclear now if EFI firmware
accepts file with multiple data or if we can have any other problems at other points like
if objcopy fine to convert this to EFI PE format.

So I believe the behavior of this patch is:

1. Fixes bug in output (no need to produce more sections that we can and take in account flags that are unused).
2. Might be helpfull for investigation such issues like above in FreeBSD at least.

[grimar]

To be more clear currently lld linked loader just hangs under QEMU. After D27040 situation changes, so now QEMU crashes when trying to use it.
It is unclear if this patch changes anything yet, just adds a one more button to trigger during investigation.

[ruiu]

ELF/LinkerScript.cpp

280

I don't think this is the right place to fix it.

If shouldMerge() returns false, we should override Flags so that it doesn't have SHF_MERGE|SHF_STRINGS.

Then naturally they'll be linked as if they were not mergeable.

[grimar]

ELF/LinkerScript.cpp

280

Ok, that sounds reasonable.

[grimar]

• Addressed review comments.

[ruiu]

ELF/InputSection.cpp

60–62 ↗

(On Diff #79289)

We shouldn't implement the same logic again here. You want to change this line https://github.com/llvm-mirror/lld/blob/d93aa5ab9ac56831d625a0233a0e17f482822d07/ELF/InputSection.cpp#L208to turn off SHF_MERGE and SHF_STRINGS flags unconditionally.

[grimar]

ELF/InputSection.cpp

60–62 ↗

(On Diff #79289)

That will not work unfortunately.
Next code is involved here and not that line:

if (shouldMerge(Sec))
  return make<MergeInputSection<ELFT>>(this, &Sec, Name);
return make<InputSection<ELFT>>(this, &Sec, Name);

calls:

template <class ELFT>
InputSection<ELFT>::InputSection(elf::ObjectFile<ELFT> *F,
                                 const Elf_Shdr *Header, StringRef Name)
    : InputSectionBase<ELFT>(F, Header, Name, Base::Regular) {}

So the suggested change will not take any affect on .data

[ruiu]

ELF/InputSection.cpp

60–62 ↗

(On Diff #79289)

But implementing the same logic here is not right.

91 ↗

(On Diff #79289)

How about this. You could add code here to drop the flag.

// If it is not a mergeable section, overwrite the flag so that the flag
// is consistent with the class. This inconsistency could occur when
// string merging is disabled using -O0 flag.
if (!isa<MergeInputSection<ELFT>>(this))
  Flags &= ~(SHF_MERGE | SHF_STRINGS);

[grimar]

• Addressed comments.

ELF/InputSection.cpp

91 ↗

(On Diff #79289)

This works for me with minor modification, thanks.

[ruiu]

LGTM

ELF/LinkerScript.cpp

static SectionKey<ELFT::Is64Bits> createKey(InputSectionBase<ELFT> *C,
   // of SHF_MERGE sections, but our implementation requires one output section
   // per "kind" (string or not, which size/aligment).
   // Fortunately, creating symbols in the middle of a merge section is not
   // supported by bfd or gold, so we can just create multiple section in that
   // case.
   typedef typename ELFT::uint uintX_t;
   uintX_t Flags = C->Flags & (SHF_MERGE | SHF_STRINGS);
 
+  // We do not do merge of sections in case when optimization is disabled,
+  // drop the flags in that case to stop produce multiple output sections.
+  if (Config->Optimize == 0)
+    Flags = 0;
+
   uintX_t Alignment = 0;
   if (isa<MergeInputSection<ELFT>>(C))
     Alignment = std::max<uintX_t>(C->Alignment, C->Entsize);
 
   return SectionKey<ELFT::Is64Bits>{OutsecName, /*Type*/ 0, Flags, Alignment};
 }
 
 template <class ELFT>

test/ELF/no-merge.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: echo "SECTIONS { .data : {*(.data.*)} }" > %t0.script
+# RUN: ld.lld %t.o -o %t0.out --script %t0.script
+# RUN: llvm-objdump -s %t0.out | FileCheck %s --check-prefix=OPT
+# OPT:      Contents of section .data:
+# OPT-NEXT:   0000 01
+# OPT-NEXT: Contents of section .data:
+# OPT-NEXT:   0001 6100
+# OPT-NEXT: Contents of section .data:
+# OPT-NEXT:   0003 03
+
+# RUN: ld.lld -O0 %t.o -o %t1.out --script %t0.script
+# RUN: llvm-objdump -s %t1.out | FileCheck %s --check-prefix=NOOPT
+# NOOPT:      Contents of section .data:
+# NOOPT-NEXT:   0000 01610003
+
+.section .data.aw,"aw",@progbits
+.byte 1
+
+.section .data.ams,"aMS",@progbits,1
+.asciz "a"
+
+.section .data.am,"aM",@progbits,1
+.byte 3