[ELF] - Linkerscript: implemented ALIGN modificatior of output sections.

[grimar]

Summary

Output section description can contain ALIGN modificator:
https://sourceware.org/binutils/docs/ld/Output-Section-Description.html#Output-Section-Description

Patch implements it.

[davide]

FWIW, this is very important because both FreeBSD and Linux rely on it (kernel).

[davide]

I think this one is OK. Please wait for Rui for final sign-off.

[ruiu]

ALIGN shouldn't be implemented as a special feature that is allowed only after ":" in SECTIONS subcommands. It is a generic function as described in https://sourceware.org/binutils/docs/ld/Builtin-Functions.html#Builtin-Functions.

If you support an expression after ":" in the context and the ALIGN built-in function, this feature is going to naturally be supported.

[grimar]

In D22674#493846, @ruiu wrote:

ALIGN shouldn't be implemented as a special feature that is allowed only after ":" in SECTIONS subcommands. It is a generic function as described in https://sourceware.org/binutils/docs/ld/Builtin-Functions.html#Builtin-Functions.

If you support an expression after ":" in the context and the ALIGN built-in function, this feature is going to naturally be supported.

I think we have different typese of ALIGN here. One is a output section description modificator.
3.6.1 Output Section Description explicitly says that ALIGN is a part of description.
(https://sourceware.org/binutils/docs/ld/Output-Section-Description.html#Output-Section-Description).
That is what this patch implements.

The other ALIGN is a generic function, as you mentioned.

I think location counter value in output secrion description is "global", but
in expressions inside declaration is "local" (relative to OutSec start address).
So ALIGN in declarations allows to use global location counter and layout the section
properly.

Because of above I think patch implements it in a correct way.

[grimar]

So, the full description of an output section looks like this:
(https://sourceware.org/binutils/docs/ld/Output-Section-Description.html#Output-Section-Description)

section [address] [(type)] :
  [AT(lma)]
  [ALIGN(section_align) | ALIGN_WITH_INPUT]
  [SUBALIGN(subsection_align)]
  [constraint]
  {
    output-section-command
    output-section-command
    ...
  } [>region] [AT>lma_region] [:phdr :phdr ...] [=fillexp] [,]

There are just a few things that can be between ":" and "{". One of them is ALIGN.
ALIGN also can be meet as regular command after "{", but it will have different meaning,
it will work with localcounter that is not absolete address, but offset from start of the section.
This way I think it is unnatural to treat everything between ":" and "{" as possible expression,
as the only possible expression is ALIGN. But that is not expression, but output section modificator.
So I believe logic of the patch is fine, I just plan to rebase it after recent changes soon.

[grimar]

• Rebased.

[ruiu]

Instead of adding new code to the function, can you create a new function that processes only ALIGN option?

ELF/LinkerScript.cpp

664–665 ↗

(On Diff #65312)

if (skip("ALIGN"))
  readAlign(Cmd);

[grimar]

• Rebased, addressed review comments.

[grimar]

In D22674#495326, @ruiu wrote:

Instead of adding new code to the function, can you create a new function that processes only ALIGN option?

I moved the handling to a assignAddresses() method, IMO more appropriate for that, and so handling is just 2 lines now.
I am not sure if moving to new function worth that now, when it is just 2 lines ? I guess no.
And the reason I moved it to assignAddresses() is that I found that location counter can be used in
ALIGN, like:
.aaa : ALIGN(4096 + . * 100) { ... }
Though I never saw this in real linkerscripts, it still looks as a more correct place, since we know the value
of location counter only in that place. What do you think ?

ELF/LinkerScript.cpp

664–665 ↗

(On Diff #65312)

Done.

[ruiu]

LGTM

[• Octav]

Does llvm ld support the ALIGN_WITH_INPUT option?

[MaskRay]

In D22674#1617615, @Octav wrote:

Does llvm ld support the ALIGN_WITH_INPUT option?

@Octav It doesn't. You can verify that the token "ALIGN_WITH_INPUT" is not mentioned in lld/ELF. What would be helpful is to give your minimized linker script which uses ALIGN_WITH_INPUT.

lld/trunk/ELF/LinkerScript.h

 enum class ConstraintKind { NoConstraint, ReadOnly, ReadWrite };
 
 struct OutputSectionCommand : BaseCommand {
   OutputSectionCommand(StringRef Name)
       : BaseCommand(OutputSectionKind), Name(Name) {}
   static bool classof(const BaseCommand *C);
   StringRef Name;
   Expr AddrExpr;
+  Expr AlignExpr;
   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) {}

lld/trunk/ELF/LinkerScript.cpp

for (const std::unique_ptr<BaseCommand> &Base : Opt.Commands) {
     auto *Cmd = cast<OutputSectionCommand>(Base.get());
     for (OutputSectionBase<ELFT> *Sec : Sections) {
       if (Sec->getName() != Cmd->Name)
         continue;
 
       if (Cmd->AddrExpr)
         Dot = Cmd->AddrExpr(Dot);
 
+      if (Cmd->AlignExpr)
+        Sec->updateAlignment(Cmd->AlignExpr(Dot));
+
       if ((Sec->getFlags() & SHF_TLS) && Sec->getType() == SHT_NOBITS) {
         uintX_t TVA = Dot + ThreadBssOffset;
         TVA = alignTo(TVA, Sec->getAlignment());
         Sec->setVA(TVA);
         ThreadBssOffset = TVA - Dot + Sec->getSize();
         continue;
       }
 
private:
   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);
 
   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) {
   Cmd->Commands.emplace_back(InCmd);
   while (!Error && !skip(")")) {
     Opt.KeptSections.push_back(peek());
     InCmd->Patterns.push_back(next());
   }
   expect(")");
 }
 
+void ScriptParser::readAlign(OutputSectionCommand *Cmd) {
+  expect("(");
+  Cmd->AlignExpr = 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("ALIGN"))
+    readAlign(Cmd);
+
   // Parse constraints.
   if (skip("ONLY_IF_RO"))
     Cmd->Constraint = ConstraintKind::ReadOnly;
   if (skip("ONLY_IF_RW"))
     Cmd->Constraint = ConstraintKind::ReadWrite;
   expect("{");
 
   while (!Error && !skip("}")) {

lld/trunk/test/ELF/linkerscript/linkerscript-align.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
 
+## Check that ALIGN command workable using location counter
 # RUN: echo "SECTIONS { \
 # RUN:  . = 0x10000; \
 # RUN:  .aaa : \
 # RUN:  { \
 # RUN:   *(.aaa) \
 # RUN:  } \
 # RUN:  . = ALIGN(4096); \
 # RUN:  .bbb : \

 # RUN: llvm-objdump -section-headers %t1 | FileCheck %s
 # CHECK:      Sections:
 # CHECK-NEXT: Idx Name          Size      Address          Type
 # CHECK-NEXT:   0               00000000 0000000000000000
 # CHECK-NEXT:   1 .aaa          00000008 0000000000010000 DATA
 # CHECK-NEXT:   2 .bbb          00000008 0000000000011000 DATA
 # CHECK-NEXT:   3 .ccc          00000008 0000000000014000 DATA
 
+## Check output sections ALIGN modificator
+# RUN: echo "SECTIONS { \
+# RUN:  . = 0x10000; \
+# RUN:  .aaa : \
+# RUN:  { \
+# RUN:   *(.aaa) \
+# RUN:  } \
+# RUN:  .bbb : ALIGN(4096) \
+# RUN:  { \
+# RUN:   *(.bbb) \
+# RUN:  } \
+# RUN:  .ccc : ALIGN(4096 * 4) \
+# RUN:  { \
+# RUN:   *(.ccc) \
+# RUN:  } \
+# RUN: }" > %t2.script
+# RUN: ld.lld -o %t2 --script %t2.script %t
+# RUN: llvm-objdump -section-headers %t1 | FileCheck %s
+
 .global _start
 _start:
  nop
 
 .section .aaa, "a"
 .quad 0
 
 .section .bbb, "a"
 .quad 0
 
 .section .ccc, "a"
 .quad 0