Error instead of allocating a header bellow address 0

[• espindola]

Summary

With the current code if the script has a PHDRS we always obey and try to allocate a header. This can cause Min - HeaderSize to underflow.

It looks like bfd actually prints an error for this case. With this patch we do the same.

Found while looking at pr36515

[ruiu]

If you can detect an underflow (or actually an overflow towards negative infinity), how hard is it to report it as an error, instead of not emitting a header without any message?

[jhenderson]

test/ELF/linkerscript/header-phdr2.s

4–5

I think it's been mentioned before that it's better to use multiple echoes and ">>" appending, to get line breaks in the script, making it easier to debug with.

[grimar]

This looks fine for me.

[• espindola]

This now matches bfd and produces an error.

[grimar]

ELF/LinkerScript.cpp

898

Doesn't seem you need else if here, it could be:

if (Script->AllocateHeaders)
  error("Could not allocate headers");

Out::ElfHeader->PtLoad = nullptr;
Out::ProgramHeaders->PtLoad = nullptr;
...

ELF/LinkerScript.h

277

I think you can avoid introducing this flag and
just iterate over Script->PhdrsCommands to find
Cmd.HasPhdrs || Cmd.HasFilehdr instead ?

[• espindola]

No else after return.

Use any_of instead of a new member variable.

[grimar]

This LGTM with few nits.

ELF/LinkerScript.cpp

890

Allocateheaders -> AllocateHeaders ?
Or may be HasExplicitHeaders would be better.

903

Could -> could.

[• espindola]

r326441

[• espindola]

326441

ELF/LinkerScript.h

public:
   // SECTIONS command list.
   std::vector<BaseCommand *> SectionCommands;
 
   // PHDRS command list.
   std::vector<PhdrsCommand> PhdrsCommands;
 
   bool HasSectionsCommand = false;
   bool ErrorOnMissingSection = false;
+  bool AllocateHeaders = false;
 
   // List of section patterns specified with KEEP commands. They will
   // be kept even if they are unused and --gc-sections is specified.
   std::vector<InputSectionDescription *> KeptSections;
 
   // A map from memory region name to a memory region descriptor.
   llvm::MapVector<llvm::StringRef, MemoryRegion *> MemoryRegions;
 

ELF/LinkerScript.cpp

 
 static OutputSection *findFirstSection(PhdrEntry *Load) {
   for (OutputSection *Sec : OutputSections)
     if (Sec->PtLoad == Load)
       return Sec;
   return nullptr;
 }
 
+static uint64_t computeBase(uint64_t Min) {
+  // If there is no SECTIONS or if the linkerscript is explicit about program
+  // headers, do our best to allocate them.
+  if (!Script->HasSectionsCommand || Script->AllocateHeaders)
+    return 0;
+  // Otherwise only allocate program headers if that would not add a page.
+  return alignDown(Min, Config->MaxPageSize);
+}
+
 // Try to find an address for the file and program headers output sections,
 // which were unconditionally added to the first PT_LOAD segment earlier.
 //
 // When using the default layout, we check if the headers fit below the first
 // allocated section. When using a linker script, we also check if the headers
 // are covered by the output section. This allows omitting the headers by not
 // leaving enough space for them in the linker script; this pattern is common
 // in embedded systems.
 //
 // If there isn't enough space for these sections, we'll remove them from the
 // PT_LOAD segment, and we'll also remove the PT_PHDR segment.
 void LinkerScript::allocateHeaders(std::vector<PhdrEntry *> &Phdrs) {
   uint64_t Min = std::numeric_limits<uint64_t>::max();
   for (OutputSection *Sec : OutputSections)
     if (Sec->Flags & SHF_ALLOC)
       Min = std::min<uint64_t>(Min, Sec->Addr);
 
   auto It = llvm::find_if(
       Phdrs, [](const PhdrEntry *E) { return E->p_type == PT_LOAD; });
   if (It == Phdrs.end())
     return;
   PhdrEntry *FirstPTLoad = *It;
 
   uint64_t HeaderSize = getHeaderSize();
-  // When linker script with SECTIONS is being used, don't output headers
-  // unless there's a space for them.
-  uint64_t Base = HasSectionsCommand ? alignDown(Min, Config->MaxPageSize) : 0;
-  if (HeaderSize <= Min - Base || Script->hasPhdrsCommands()) {
+  if (HeaderSize <= Min - computeBase(Min)) {
     Min = alignDown(Min - HeaderSize, Config->MaxPageSize);
     Out::ElfHeader->Addr = Min;
     Out::ProgramHeaders->Addr = Min + Out::ElfHeader->Size;
     return;
+  } else if (Script->AllocateHeaders) {
+    // Error if we were explicitly asked to allocate headers.
+    error("Could not allocate headers");
   }
 
   Out::ElfHeader->PtLoad = nullptr;
   Out::ProgramHeaders->PtLoad = nullptr;
   FirstPTLoad->FirstSec = findFirstSection(FirstPTLoad);
 
   llvm::erase_if(Phdrs,
                  [](const PhdrEntry *E) { return E->p_type == PT_PHDR; });
 }
 
 LinkerScript::AddressState::AddressState() {
   for (auto &MRI : Script->MemoryRegions) {
     MemoryRegion *MR = MRI.second;

ELF/ScriptParser.cpp

while (!errorCount() && !consume(";")) {
         Cmd.HasPhdrs = true;
       else if (consume("AT"))
         Cmd.LMAExpr = readParenExpr();
       else if (consume("FLAGS"))
         Cmd.Flags = readParenExpr()().getValue();
       else
         setError("unexpected header attribute: " + next());
     }
+    if (Cmd.HasPhdrs || Cmd.HasFilehdr)
+      Script->AllocateHeaders = true;
 
     Script->PhdrsCommands.push_back(Cmd);
   }
 }
 
 void ScriptParser::readRegionAlias() {
   expect("(");
   StringRef Alias = unquote(next());

test/ELF/linkerscript/header-phdr2.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: echo "PHDRS { foobar PT_LOAD FILEHDR PHDRS; }"  > %t.script
+# RUN: echo "SECTIONS { .text : { *(.text) } : foobar }" >> %t.script
+# RUN: not ld.lld --script %t.script %t.o -o %t 2>&1 | FileCheck %s
+
+# CHECK: Could not allocate headers
+
+        .global _start
+_start:
+        retq