[ELF] - linkerscript AT keyword (in output section description) implemented.

[grimar]

Summary

The linker will normally set the LMA equal to the VMA.
You can change that by using the AT keyword.
The expression lma that follows the AT keyword specifies
the load address of the section.

Patch implements this keyword.

[ruiu]

ELF/LinkerScript.cpp

378

Ah, so PA stands for Physical Address, but is it a common name? At least it doesn't feel "physical" to me. Why don't you name this getAddr?

Also I'd rename Name -> SectionName.

458

readAt for consistency.

719

Use setError.

739–741

No need to reuse a variable.

if (peek() == "AT")
 readAt(OutSec);

ELF/LinkerScript.h

135

What does PA stand for?

153

StringMap copies keys. Do you want to use DenseMap instead?

ELF/Writer.cpp

988–992

Why we want to create a new load segment for sections with AT commands?

[grimar]

• Addressed review commens.

ELF/LinkerScript.cpp

378

I took "physical address" term from description of p_paddr field:
https://docs.oracle.com/cd/E19683-01/816-1386/chapter6-83432/index.html
I did not name getAddr() because it seems confusing to me, when I hear "address" I usually think about VMA, since LMA is not
really common.
But I think we can call it getLma(), as LMA is a term used for AT expressions:
https://www.sourceware.org/binutils/docs-2.12/ld.info/Output-Section-LMA.html

458

Done.

719

Done.

739–741

Done.

ELF/LinkerScript.h

153

Yes, I know it do. I see no problem here. StringMap can be a bit faster for lookup, but this place should not be
bottleneck though, so I think there are no reasons to use it or not to do that here,
because amount of keys should be little either.
I used it mostly to be consistent with Filler field. May be we should combine them into one variable ?

struct OutputSectionData {
  std::vector<uint8_t> Filler;
  std::vector<StringRef> PhysAddrExpr;
};

llvm::StringMap<OutputSectionData> OutSections;

ELF/Writer.cpp

988–992

Section header has only one address field sh_addr which is VMA:
https://docs.oracle.com/cd/E19455-01/806-3773/elf-2/index.html
So there is not way to set LMA for section, only for load segment itself I believe.
And that means that for section that explicitly requested LMA new load has
to be created.

Both gold and bfd do the same.

AT commands changes p_paddr of PT_LOAD header in binutils:
https://sourceware.org/ml/binutils/2011-06/msg00262.html

[ruiu]

I wonder if you are implementing this feature for some specific product. For example, are you trying to link some kernel or something? Or are you doing this for the sake of completeness? It seems to me that AT command is a tricky option that you want to use only when you are dealing with embedded systems or something.

ELF/LinkerScript.cpp

378

Can you name this getLoadAddr and add comments what the load address is?

ELF/Writer.cpp

988–992

Then please explain why we need to do this instead of what we are doing here in the comment.

// Segments are contiguous memory regions that has the same attributes
// (e.g. executable or writable). There is one phdr for each segment. Therefore,
// we need to create a new phdr when the next section has different flags or
// is loaded at a discontiguous address using AT linker script command.

[grimar]

In D19272#407140, @ruiu wrote:

I wonder if you are implementing this feature for some specific product. For example, are you trying to link some kernel or something? Or are you doing this for the sake of completeness? It seems to me that AT command is a tricky option that you want to use only when you are dealing with embedded systems or something.

I am using freebsd script as reference (https://svnweb.freebsd.org/base/head/sys/conf/ldscript.amd64?view=markup), assuming that it contains some
subset of features that is reasonable to implement. It uses AT. It definitely looks tricky feature, but it was easy to implement, so I did it because
assumed that supporting it is better than ignoring or something.
Though it would be interesting to know how much is it important to have and can it be replaced with something/ignored or not (I don't know answer on this question).

[grimar]

• Addressed review comments.

ELF/LinkerScript.cpp

378

Done.

ELF/Writer.cpp

988–992

Done.

[emaste]

Note that AT was introduced to the FreeBSD kernel linker script only recently, https://svnweb.freebsd.org/base/head/sys/conf/ldscript.amd64?r1=284869&r2=284870&

For testing I'll revert that commit but we will need it to be supported.

[grimar]

• Rebased.

[evgeny777]

ELF/LinkerScript.cpp

29

Can you reuse readSectionsCommandExpr() ?

ELF/Writer.cpp

13

If your patch is assigning LMA for *sections* then it should be done in LinkerScript::assignAddresses, I think

test/ELF/linkerscript-at.s

66 ↗

(On Diff #65049)

I have feeling that section alignment is not done for LMA

[grimar]

ELF/Writer.cpp

13

Section header has only one address field sh_addr which is VMA:
https://docs.oracle.com/cd/E19455-01/806-3773/elf-2/index.html
So there is not way to set LMA for section, only for load segment itself I believe.
And that means that for section that explicitly requested LMA new load has
to be created.

Both gold and bfd do the same.

[evgeny777]

ELF/Writer.cpp

13

Well, OutputSectionBase contains not only header, but PageAlign, SectionIndex and Name.
May be adding some extra field like PhysicalAddress to OutputSectionBase is not that bad?

[grimar]

test/ELF/linkerscript-at.s

66 ↗

(On Diff #65049)

I guess you meant VMA.

As far I understand the feature, AT just sets the physical address field of PT_LOAD. VMA is different and should be controlled
by location counter or something else. Setting PA gives ability to control LMA for sections.
I think the output after applying my patch is equal to what gnu ld do.
gold also touches the file offsets of the sections, gnu ld does not, this patch also does not.

Would be interesting to hear opinion from reviewers

[grimar]

• Rebased.

[emaste]

FYI the linker script parser now handles all of the FreeBSD kernel linker script except for AT. Now the only kernel linker script workaround in my test FreeBSD tree is:

diff --git a/sys/conf/ldscript.amd64 b/sys/conf/ldscript.amd64
index d445857..3efca9a 100644
--- a/sys/conf/ldscript.amd64
+++ b/sys/conf/ldscript.amd64
@@ -8,12 +8,7 @@ SECTIONS
   /* Read-only sections, merged into text segment: */
   kernphys = CONSTANT (MAXPAGESIZE);
   . = kernbase + kernphys + SIZEOF_HEADERS;
-  /*
-   * Use the AT keyword in order to set the right LMA that contains
-   * the physical address where the section should be loaded. This is
-   * needed for the Xen loader which honours the LMA.
-   */
-  .interp         : AT (kernphys + SIZEOF_HEADERS) { *(.interp) }
+  .interp         : { *(.interp) }
   .hash           : { *(.hash) }
   .gnu.hash       : { *(.gnu.hash) }
   .dynsym         : { *(.dynsym) }

[grimar]

In D19272#512904, @emaste wrote:

FYI the linker script parser now handles all of the FreeBSD kernel linker script except for AT. Now the only kernel linker script workaround in my test FreeBSD tree is:

diff --git a/sys/conf/ldscript.amd64 b/sys/conf/ldscript.amd64
index d445857..3efca9a 100644
--- a/sys/conf/ldscript.amd64
+++ b/sys/conf/ldscript.amd64
@@ -8,12 +8,7 @@ SECTIONS
   /* Read-only sections, merged into text segment: */
   kernphys = CONSTANT (MAXPAGESIZE);
   . = kernbase + kernphys + SIZEOF_HEADERS;
-  /*
-   * Use the AT keyword in order to set the right LMA that contains
-   * the physical address where the section should be loaded. This is
-   * needed for the Xen loader which honours the LMA.
-   */
-  .interp         : AT (kernphys + SIZEOF_HEADERS) { *(.interp) }
+  .interp         : { *(.interp) }
   .hash           : { *(.hash) }
   .gnu.hash       : { *(.gnu.hash) }
   .dynsym         : { *(.dynsym) }

Yes, I know. I am testing it with the same "change". I think 10.3 release sources head does not contain AT and I using them.

I am not sure how to test properly that AT implementation is correct ? Since it is not important to have for FreeBSD.

[davide]

In D19272#513530, @grimar wrote:

In D19272#512904, @emaste wrote:

FYI the linker script parser now handles all of the FreeBSD kernel linker script except for AT. Now the only kernel linker script workaround in my test FreeBSD tree is:

diff --git a/sys/conf/ldscript.amd64 b/sys/conf/ldscript.amd64
index d445857..3efca9a 100644
--- a/sys/conf/ldscript.amd64
+++ b/sys/conf/ldscript.amd64
@@ -8,12 +8,7 @@ SECTIONS
   /* Read-only sections, merged into text segment: */
   kernphys = CONSTANT (MAXPAGESIZE);
   . = kernbase + kernphys + SIZEOF_HEADERS;
-  /*
-   * Use the AT keyword in order to set the right LMA that contains
-   * the physical address where the section should be loaded. This is
-   * needed for the Xen loader which honours the LMA.
-   */
-  .interp         : AT (kernphys + SIZEOF_HEADERS) { *(.interp) }
+  .interp         : { *(.interp) }
   .hash           : { *(.hash) }
   .gnu.hash       : { *(.gnu.hash) }
   .dynsym         : { *(.dynsym) }

Yes, I know. I am testing it with the same "change". I think 10.3 release sources head does not contain AT and I using them.

I am not sure how to test properly that AT implementation is correct ? Since it is not important to have for FreeBSD.

As far as I can tell the Xen loader in FreeBSD loader uses it. Eventually we might want to try (or ask somebody to try).
I think that getting X86-64 (and maybe other tier1 or tier2-almost-1 arches) to run is an higher priority (at least it is for me:))

[davide]

In D19272#513746, @davide wrote:

In D19272#513530, @grimar wrote:

In D19272#512904, @emaste wrote:

FYI the linker script parser now handles all of the FreeBSD kernel linker script except for AT. Now the only kernel linker script workaround in my test FreeBSD tree is:

diff --git a/sys/conf/ldscript.amd64 b/sys/conf/ldscript.amd64
index d445857..3efca9a 100644
--- a/sys/conf/ldscript.amd64
+++ b/sys/conf/ldscript.amd64
@@ -8,12 +8,7 @@ SECTIONS
   /* Read-only sections, merged into text segment: */
   kernphys = CONSTANT (MAXPAGESIZE);
   . = kernbase + kernphys + SIZEOF_HEADERS;
-  /*
-   * Use the AT keyword in order to set the right LMA that contains
-   * the physical address where the section should be loaded. This is
-   * needed for the Xen loader which honours the LMA.
-   */
-  .interp         : AT (kernphys + SIZEOF_HEADERS) { *(.interp) }
+  .interp         : { *(.interp) }
   .hash           : { *(.hash) }
   .gnu.hash       : { *(.gnu.hash) }
   .dynsym         : { *(.dynsym) }

Yes, I know. I am testing it with the same "change". I think 10.3 release sources head does not contain AT and I using them.

I am not sure how to test properly that AT implementation is correct ? Since it is not important to have for FreeBSD.

As far as I can tell the Xen loader in FreeBSD loader uses it. Eventually we might want to try (or ask somebody to try).
I think that getting X86-64 (and maybe other tier1 or tier2-almost-1 arches) to run is an higher priority (at least it is for me:))

But, to clarify, it's important :) (or at least there are some people using Xen which care about it =))

[phosek]

We'd like to use LLD for linking LK kernel and the AT keyword support is currently the only thing that's stopping us, so landing this change would really help us as well.

[phosek]

In D19272#407140, @ruiu wrote:

I wonder if you are implementing this feature for some specific product. For example, are you trying to link some kernel or something? Or are you doing this for the sake of completeness? It seems to me that AT command is a tricky option that you want to use only when you are dealing with embedded systems or something.

IMHO that's the case not only for AT command but for linker scripts in general, vast majority of linker script use cases would be kernels and embedded systems.

[grimar]

In D19272#514579, @phosek wrote:

We'd like to use LLD for linking LK kernel and the AT keyword support is currently the only thing that's stopping us, so landing this change would really help us as well.

Did you try to link with this patch applied ? I am curios whether this patch is enough to support AT.

[ruiu]

This change needs a test.

ELF/LinkerScript.cpp

334

Small but noticeable duplicate code. Please define hasLma using getLma.

return getLma(Name) != -1;

ELF/Writer.cpp

993

I don't think this will create a valid PHDR if there's a section with an AT command followed by another section without AT.

.foo 0x1000 : AT(0) { ... }
.bar 0x2000 { ... }

For example, if .bar's flags are the same as .foo's, then this code would put both .foo and .bar into the same segment.

1158

Remove this comment. I think it is too terse and obvious.

[grimar]

• Restored lost testcase.
• Rebased, addressed review comments.

ELF/LinkerScript.cpp

334

Not sure I could do that. getLma had VA argument to use as a Dot and returned
the result of expression. It would be not correct to compare expression result with -1,
also it would introduce excessive computation of expression (first for hasLma() and then for getLma()).

I reimplemented this.

ELF/Writer.cpp

993

It is expected behavior. AT starts new segment but other sections can be put inside it as well.
It just like ADDR() for LMA.

Using next script ld will create 2 loads:

SECTIONS { 
  . = 0x1000; 
  .aaa : AT(0x2000) 
  { 
   *(.aaa) 
  } 
  .bbb : 
  { 
   *(.bbb) 
  } 
  .ccc : AT(0x3000) 
  { 
   *(.ccc) 
  } 
  .text : { *(.text*) }
}

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  LOAD           0x0000000000000000 0x0000000000000000 0x0000000000001000
                 0x0000000000001010 0x0000000000001010  R      200000
  LOAD           0x0000000000001010 0x0000000000001010 0x0000000000003000
                 0x0000000000000009 0x0000000000000009  R E    200000

 Section to Segment mapping:
  Segment Sections...
   00     .aaa .bbb 
   01     .ccc .text

1158

Done.

[ruiu]

LGTM

ELF/Writer.cpp

1159–1160

if (Expr LmaExpr = Script<ELFT>::X->getLma(First->getName())

1161

I have a concern that the stuff for the linker script is leaking here. Expr is for the linker script, and I wanted to keep Writer being agnostic about it. From that point of view, this is not very clean. However, being said that, I have no good alternative suggestion, so I'm OK with this.

ELF/LinkerScript.h

 
 struct OutputSectionCommand : BaseCommand {
   OutputSectionCommand(StringRef Name)
       : BaseCommand(OutputSectionKind), Name(Name) {}
   static bool classof(const BaseCommand *C);
   StringRef Name;
   Expr AddrExpr;
   Expr AlignExpr;
+  Expr LmaExpr;
   std::vector<std::unique_ptr<BaseCommand>> Commands;
   std::vector<StringRef> Phdrs;
   std::vector<uint8_t> Filler;
   ConstraintKind Constraint = ConstraintKind::NoConstraint;
 };
 
 struct InputSectionDescription : BaseCommand {
   InputSectionDescription() : BaseCommand(InputSectionKind) {}

 public:
   std::vector<OutputSectionBase<ELFT> *>
   createSections(OutputSectionFactory<ELFT> &Factory);
 
   std::vector<PhdrEntry<ELFT>>
   createPhdrs(ArrayRef<OutputSectionBase<ELFT> *> S);
 
   ArrayRef<uint8_t> getFiller(StringRef Name);
+  uintX_t getLma(StringRef Name, uintX_t VA);
+  bool hasLma(StringRef Name);
   bool shouldKeep(InputSectionBase<ELFT> *S);
   void assignAddresses(ArrayRef<OutputSectionBase<ELFT> *> S);
   int compareSections(StringRef A, StringRef B);
   void addScriptedSymbols();
   bool hasPhdrsCommands();
 
 private:
   std::vector<std::pair<StringRef, const InputSectionDescription *>>
   getSectionMap();
 
   std::vector<InputSectionBase<ELFT> *>
   getInputSections(const InputSectionDescription *);
 
   // "ScriptConfig" is a bit too long, so define a short name for it.
   ScriptConfiguration &Opt = *ScriptConfig;
 
   std::vector<OutputSectionBase<ELFT> *>
   filter(std::vector<OutputSectionBase<ELFT> *> &Sections);
 
   int getSectionIndex(StringRef Name);
   std::vector<size_t> getPhdrIndices(StringRef SectionName);
   size_t getPhdrIndex(StringRef PhdrName);
   void dispatchAssignment(SymbolAssignment *Cmd);
 
   uintX_t Dot;
 };
 
 // Variable template is a C++14 feature, so we can't template
 // a global variable. Use a struct to workaround.
 template <class ELFT> struct Script { static LinkerScript<ELFT> *X; };
 template <class ELFT> LinkerScript<ELFT> *Script<ELFT>::X;
 
 } // namespace elf
 } // namespace lld
 
 #endif

ELF/LinkerScript.cpp

 #include "Config.h"
 #include "Driver.h"
 #include "InputSection.h"
 #include "OutputSections.h"
 #include "ScriptParser.h"
 #include "Strings.h"
 #include "Symbols.h"
 #include "SymbolTable.h"
 #include "Target.h"
 #include "Writer.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/ELF.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/StringSaver.h"
 

 ArrayRef<uint8_t> LinkerScript<ELFT>::getFiller(StringRef Name) {
   for (const std::unique_ptr<BaseCommand> &Base : Opt.Commands)
     if (auto *Cmd = dyn_cast<OutputSectionCommand>(Base.get()))
       if (Cmd->Name == Name)
         return Cmd->Filler;
   return {};
 }
 
+template <class ELFT> bool LinkerScript<ELFT>::hasLma(StringRef Name) {
+  for (const std::unique_ptr<BaseCommand> &Base : Opt.Commands)
+    if (auto *Cmd = dyn_cast<OutputSectionCommand>(Base.get()))
+      if (Cmd->Name == Name)
+        return (bool)Cmd->LmaExpr;
+  return nullptr;
+}
+
+template <class ELFT>
+typename ELFT::uint LinkerScript<ELFT>::getLma(StringRef Name, uintX_t VA) {
+  for (const std::unique_ptr<BaseCommand> &Base : Opt.Commands)
+    if (auto *Cmd = dyn_cast<OutputSectionCommand>(Base.get()))
+      if (Cmd->Name == Name && Cmd->LmaExpr)
+        return Cmd->LmaExpr(VA);
+  llvm_unreachable("no LMA for section");
+  return (uintX_t)-1;
+}
+
 // Returns the index of the given section name in linker script
 // SECTIONS commands. Sections are laid out as the same order as they
 // were in the script. If a given name did not appear in the script,
 // it returns INT_MAX, so that it will be laid out at end of file.
 template <class ELFT> int LinkerScript<ELFT>::getSectionIndex(StringRef Name) {
   int I = 0;
   for (std::unique_ptr<BaseCommand> &Base : Opt.Commands) {
     if (auto *Cmd = dyn_cast<OutputSectionCommand>(Base.get()))

 int LinkerScript<ELFT>::compareSections(StringRef A, StringRef B) {
   int I = getSectionIndex(A);
   int J = getSectionIndex(B);
   if (I == INT_MAX && J == INT_MAX)
     return 0;
   return I < J ? -1 : 1;
 }
 
 template <class ELFT> void LinkerScript<ELFT>::addScriptedSymbols() {
   for (const std::unique_ptr<BaseCommand> &Base : Opt.Commands) {
     auto *Cmd = dyn_cast<SymbolAssignment>(Base.get());
     if (!Cmd || Cmd->Name == ".")
       continue;
 
     SymbolBody *B = Symtab<ELFT>::X->find(Cmd->Name);
     // The semantic of PROVIDE is that of introducing a symbol only if
     // it's not defined and there's at least a reference to it.
private:
   void readOutput();
   void readOutputArch();
   void readOutputFormat();
   void readPhdrs();
   void readSearchDir();
   void readSections();
 
   SymbolAssignment *readAssignment(StringRef Name);
   void readOutputSectionDescription(StringRef OutSec);
   std::vector<StringRef> readOutputSectionPhdrs();
   unsigned readPhdrType();
   void readProvide(bool Hidden);
   void readAlign(OutputSectionCommand *Cmd);
+  void readAt(OutputSectionCommand *Cmd);
 
   Expr readExpr();
   Expr readExpr1(Expr Lhs, int MinPrec);
   Expr readPrimary();
   Expr readTernary(Expr Cond);
   Expr combine(StringRef Op, Expr Lhs, Expr Rhs);
 
   const static StringMap<Handler> Cmd;
void ScriptParser::readKeep(OutputSectionCommand *Cmd) {
   Opt.KeptSections.insert(Opt.KeptSections.end(), InCmd->Patterns.begin(),
                           InCmd->Patterns.end());
   Cmd->Commands.push_back(std::move(InCmd));
   expect(")");
 }
 
 void ScriptParser::readAlign(OutputSectionCommand *Cmd) {
   expect("(");
   Cmd->AlignExpr = readExpr();
   expect(")");
 }
 
+void ScriptParser::readAt(OutputSectionCommand *Cmd) {
+  expect("(");
+  Cmd->LmaExpr = readExpr();
+  expect(")");
+}
+
 void ScriptParser::readOutputSectionDescription(StringRef OutSec) {
   OutputSectionCommand *Cmd = new OutputSectionCommand(OutSec);
   Opt.Commands.emplace_back(Cmd);
 
   // Read an address expression.
   // https://sourceware.org/binutils/docs/ld/Output-Section-Address.html#Output-Section-Address
   if (peek() != ":")
     Cmd->AddrExpr = readExpr();
 
   expect(":");
 
+  if (skip("AT"))
+    readAt(Cmd);
+
   if (skip("ALIGN"))
     readAlign(Cmd);
 
   // Parse constraints.
   if (skip("ONLY_IF_RO"))
     Cmd->Constraint = ConstraintKind::ReadOnly;
   if (skip("ONLY_IF_RW"))
     Cmd->Constraint = ConstraintKind::ReadWrite;

ELF/Writer.cpp

 //===- Writer.cpp ---------------------------------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "Writer.h"
 #include "Config.h"
 #include "LinkerScript.h"
 #include "OutputSections.h"
 #include "Relocations.h"
 #include "Strings.h"
 #include "SymbolTable.h"
 #include "Target.h"
 
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/FileOutputBuffer.h"
for (OutputSectionBase<ELFT> *Sec : OutputSections) {
     // and put all TLS sections inside for futher use when
     // assign addresses.
     if (Sec->getFlags() & SHF_TLS)
       TlsHdr.add(Sec);
 
     if (!needsPtLoad(Sec))
       continue;
 
-    // If flags changed then we want new load segment.
+    // Segments are contiguous memory regions that has the same attributes
+    // (e.g. executable or writable). There is one phdr for each segment. Therefore,
+    // we need to create a new phdr when the next section has different flags or
+    // is loaded at a discontiguous address using AT linker script command.
     uintX_t NewFlags = toPhdrFlags(Sec->getFlags());
-    if (Flags != NewFlags) {
+    if (Script<ELFT>::X->hasLma(Sec->getName()) || Flags != NewFlags) {
       Load = AddHdr(PT_LOAD, NewFlags);
       Flags = NewFlags;
     }
 
     Load->add(Sec);
 
     if (isRelroSection(Sec))
       RelRo.add(Sec);
if (First) {
       H.p_memsz = Last->getVA() + Last->getSize() - First->getVA();
       H.p_offset = First->getFileOff();
       H.p_vaddr = First->getVA();
     }
     if (H.p_type == PT_LOAD)
       H.p_align = Target->PageSize;
     else if (H.p_type == PT_GNU_RELRO)
       H.p_align = 1;
+
+    // Now set the segment's physical address.
+    if (H.p_type == PT_LOAD && First &&
+        Script<ELFT>::X->hasLma(First->getName()))
+      H.p_paddr = Script<ELFT>::X->getLma(First->getName(), H.p_vaddr);
+    else
       H.p_paddr = H.p_vaddr;
 
     // The TLS pointer goes after PT_TLS. At least glibc will align it,
     // so round up the size to make sure the offsets are correct.
     if (H.p_type == PT_TLS) {
       Out<ELFT>::TlsPhdr = &H;
       H.p_memsz = alignTo(H.p_memsz, H.p_align);
     }
   }