[PATCH] D43284: [ELF] Simplify handling of AT section attribute.

[Igor Kudrin via llvm-commits]

Hello Rafael,

Thanks for the comments, I'll update the patch.

I don't know whether there are real problems or not. The test is mostly artificial, intended to show the possible misbehavior. The main goal of the patch was to simplify the code while making the behavior more close to GNU ld's at the same time.

For me, the way more important are changes in https://reviews.llvm.org/D43228 because they directly affect the linker script used in our project.

Best Regards,
Igor Kudrin
C++ Developer, Access Softek, Inc.

________________________________________
From: Rafael Avila de Espindola <rafael.espindola at gmail.com>
Sent: Wednesday, February 14, 2018 10:19 PM
To: reviews+D43284+public+14d4acfa224c0e7f at reviews.llvm.org; Igor Kudrin; ruiu at google.com
Cc: Evgeny Leviant; George Rimar; emaste at freebsd.org; llvm-commits at lists.llvm.org; arichardson.kde at gmail.com; pshung807 at gmail.com; adhemerval.zanella at linaro.org
Subject: Re: [PATCH] D43284: [ELF] Simplify handling of AT section attribute.

Igor Kudrin via Phabricator <reviews at reviews.llvm.org> writes:

What was broken by the current behavior? Should this change be
backported to 6.0?

> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -821,8 +821,6 @@
>    p_align = std::max(p_align, Sec->Alignment);
>    if (p_type == PT_LOAD)
>      Sec->PtLoad = this;
> -  if (Sec->LMAExpr)
> -    ASectionHasLMA = true;
>  }
>
>  // The beginning and the ending of .rel[a].plt section are marked
> @@ -1654,6 +1652,7 @@
>    Load->add(Out::ElfHeader);
>    Load->add(Out::ProgramHeaders);
>
> +  bool FirstLoadSection = true;
>    for (OutputSection *Sec : OutputSections) {
>      if (!(Sec->Flags & SHF_ALLOC))
>        break;
> @@ -1666,14 +1665,15 @@
>      // different flags or is loaded at a discontiguous address using AT linker
>      // script command.
>      uint64_t NewFlags = computeFlags(Sec->getPhdrFlags());
> -    if ((Sec->LMAExpr && Load->ASectionHasLMA) ||
> +    if ((Sec->LMAExpr && !FirstLoadSection) ||

Instead of adding FirstLoadSection, you can use:

Load->LastSec != Out::ProgramHeaders.

Please also add a comment saying what we are doing: including the
headers in the first PT_LOAD even if it has an AT.

Cheers,
Rafael