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

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: lld/ELF/Writer.cpp:136
   if (s->name == "COMMON")
     return ".bss";
 
----------------
MaskRay wrote:
> grimar wrote:
> > 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>
> > ```
> > 
> 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.
> 
> 2. I feel that an early return pattern here does not improve readability.
  > 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;
}
```


================
Comment at: lld/test/ELF/linkerscript/icf-output-sections.s:31
 
 ## .text.bar* are orphans that get assigned to .text.
 # RUN: echo 'SECTIONS { .text.foo : {*(.text.foo*)} }' > %t3.script
----------------
The comment is outdated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75225/new/

https://reviews.llvm.org/D75225