This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Keep orphan section names (.rodata.foo .text.foo) unchanged if !hasSectionsCommand
ClosedPublic

Authored by MaskRay on Feb 26 2020, 8:19 PM.

Details

Summary

This behavior matches GNU ld and seems reasonable.

// If a SECTIONS command is not specified
.text.* -> .text
.rodata.* -> .rodata
.init_array.* -> .init_array

A proposed Linux feature CONFIG_FG_KASLR may depend on the GNU ld behavior.

Reword a comment about -z keep-text-section-prefix.

Diff Detail

Event Timeline

MaskRay created this revision.Feb 26 2020, 8:19 PM
MaskRay edited the summary of this revision. (Show Details)Feb 26 2020, 8:21 PM
grimar added inline comments.Feb 27 2020, 12:32 AM
lld/ELF/Writer.cpp
136

2 questions regarding the current code.

  1. Seems we always rename COMMON to .bss. Does not look right for the non-linker script case?
  2. If (1) is true, what about doing in the following way?
if (script->hasSectionsCommand) {
  // CommonSection is identified as "COMMON" in linker scripts.
  // By default, it should go to .bss section.
  if (s->name == "COMMON")
    return ".bss";
  return s->name;
}

// When no SECTIONS is specified, emulate GNU ld's internal linker scripts...
<code>
lld/test/ELF/text-section-prefix.s
20

Do you need -z keep-text-section-prefix? Seems both ld.bfd and ld.gold produce the same output without this option.

I'm happy to make the change, although orphan section placement is documented to be linker's choice it will help people needing support in ld.bfd and lld if we have similar rules.

MaskRay marked 2 inline comments as done.Feb 27 2020, 9:07 AM
MaskRay added inline comments.
lld/ELF/Writer.cpp
136
  1. "COMMON" sections are created this way: auto *bss = make<BssSection>("COMMON", s->size, s->alignment); With a linker script without mentioning an input section description COMMON, GNU ld can still place COMMON in .bss The logic here is compatible with GNU ld.
  1. I feel that an early return pattern here does not improve readability.
lld/test/ELF/text-section-prefix.s
20

This is to emphasize that a SECTIONS command overrides -z keep-text-section-prefix.

kees added a comment.Feb 27 2020, 9:09 AM

.text.* -> .text

This is not accurate: ld.bfd will keep the .text.$foo names, but place them all after the .text (it does not merge them into .text). Currently, ld.lld seems to merge them into .text. FGKASLR depends on the non-merging behavior.

.text.* -> .text

This is not accurate: ld.bfd will keep the .text.$foo names, but place them all after the .text (it does not merge them into .text). Currently, ld.lld seems to merge them into .text. FGKASLR depends on the non-merging behavior.

I think the description is correct. I have a line // If a SECTIONS command is not specified in the code block.

Here is GNU ld's internal linker script:

.text           :
{
  *(.text.unlikely .text.*_unlikely .text.unlikely.*)
  *(.text.exit .text.exit.*)
  *(.text.startup .text.startup.*)
  *(.text.hot .text.hot.*)
  *(.text .stub .text.* .gnu.linkonce.t.*)
  /* .gnu.warning sections are handled specially by elf32.em.  */
  *(.gnu.warning)
}

(As you can see, -z keep-text-section-prefix does less than what GNU ld does. One issue with GNU ld's internal linker script is that -ffunction-sections (typical when building a libc) will cause the function exit to be reordered before others...)

kees added a comment.Mar 9 2020, 9:12 AM

.text.* -> .text

This is not accurate: ld.bfd will keep the .text.$foo names, but place them all after the .text (it does not merge them into .text). Currently, ld.lld seems to merge them into .text. FGKASLR depends on the non-merging behavior.

I think the description is correct. I have a line // If a SECTIONS command is not specified in the code block.

Here is GNU ld's internal linker script:

.text           :
{
  *(.text.unlikely .text.*_unlikely .text.unlikely.*)
  *(.text.exit .text.exit.*)
  *(.text.startup .text.startup.*)
  *(.text.hot .text.hot.*)
  *(.text .stub .text.* .gnu.linkonce.t.*)
  /* .gnu.warning sections are handled specially by elf32.em.  */
  *(.gnu.warning)
}

(As you can see, -z keep-text-section-prefix does less than what GNU ld does. One issue with GNU ld's internal linker script is that -ffunction-sections (typical when building a libc) will cause the function exit to be reordered before others...)

Maybe by default? However, when building the kernel and the linker script has an explicit .text output section without an input .text.* pattern, ld.bfd does not collect them into the output .text section (they are just adjacent).

grimar added inline comments.Mar 10 2020, 5:08 AM
lld/ELF/Writer.cpp
136
> I feel that an early return pattern here does not improve readability.

A little reordering converts it to the following piece of code without an additional nesting.
Isn't it a bit simpler?

  // CommonSection is identified as "COMMON" in linker scripts.
  // By default, it should go to .bss section.
  if (s->name == "COMMON")
    return ".bss";

  if (script->hasSectionsCommand)
    return s->name;

  // When no SECTIONS is specified, emulate GNU ld's internal linker scripts...
  ...
  return s->name;
}
lld/test/ELF/linkerscript/icf-output-sections.s
31

The comment is outdated.

MaskRay updated this revision to Diff 251942.Mar 22 2020, 9:53 PM
MaskRay marked 3 inline comments as done.

Rebase. Address comments

grimar accepted this revision.Mar 23 2020, 1:07 AM

LGTM

This revision is now accepted and ready to land.Mar 23 2020, 1:07 AM
MaskRay updated this revision to Diff 252092.Mar 23 2020, 10:24 AM

Fix a comment about CommonSection. CommonSection was removed long ago.

This revision was automatically updated to reflect the committed changes.