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

[Rafael Avila de Espindola via llvm-commits]

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