[PATCH] D44376: [ELF] - Drop special flags for empty output sections.

[Rafael Ávila de Espíndola via Phabricator via llvm-commits]

rafael added a comment.

George Rimar via Phabricator via llvm-commits
<llvm-commits at lists.llvm.org> writes:

>   void LinkerScript::adjustSectionsBeforeSorting() {
> 
> @@ -839,32 +839,34 @@
> 
>   // the previous sections. Only a few flags are needed to keep the impact low.
>   uint64_t Flags = SHF_ALLOC;
>     
> 
> - for (BaseCommand *&Cmd : SectionCommands) { +  for (BaseCommand *Cmd : SectionCommands) { auto *Sec = dyn_cast<OutputSection>(Cmd); if (!Sec) continue;
> 
>   // A live output section means that some input section was added to it. It
> - // might have been removed (gc, or empty synthetic section), but we at least +    // might have been removed (if it was empty synthetic section), but we at least // know the flags. if (Sec->Live)
> - Flags = Sec->Flags & (SHF_ALLOC | SHF_WRITE | SHF_EXECINSTR);
> - else
> - Sec->Flags = Flags; -
> - if (isDiscardable(*Sec)) {
> - Sec->Live = false;
> - Cmd = nullptr;
> - } +      Flags = Sec->Flags; + +    // We do not want to keep any special flags for output section in case it is empty. +    bool IsEmpty = getInputSections(Sec).empty(); +    if (IsEmpty) +      Sec->Flags = Flags & (SHF_ALLOC | SHF_WRITE | SHF_EXECINSTR); + +    Sec->Live = !IsEmpty || !isDiscardable(*Sec);

I would keep this as

  if (IsEmpty && isDiscardable(*Sec)) {
    Sec->Live = false;
    Cmd = nullptr;
  }

>   }
>     
>   // It is common practice to use very generic linker scripts. So for any
>   // given run some of the output sections in the script will be empty.
>   // We could create corresponding empty output sections, but that would
>   // clutter the output.
>   // We instead remove trivially empty sections. The bfd linker seems even
>   // more aggressive at removing them.
> 
> - llvm::erase_if(SectionCommands, [&](BaseCommand *Base) { return !Base; }); +  llvm::erase_if(SectionCommands, [&](BaseCommand *Base) { +    return isa<OutputSection>(Base) && !cast<OutputSection>(Base)->Live; +  }); }

And this as

  llvm::erase_if(SectionCommands, [&](BaseCommand *Base) { return !Base; });

LGTM with that.

Cheers,
Rafael


https://reviews.llvm.org/D44376