[BOLT] Set IsRelro section attribute based on PT_GNU_RELRO segment

[Amir]

Summary

Handle PT_GNU_RELRO segment in accordance with Linux Standard Base spec
chapter 12:

PT_GNU_RELRO
The array element specifies the location and size of a segment which may
be made *read-only* after relocations have been processed.

Perform a readelf-style mapping check between this segment and sections,
set IsRelro section attribute.

[Amir]

Add TLS section check

[maksfb]

I think we need a new type for RELRO sections. We rely on the contents of regular read-only sections to match that in the file. While for RELRO, the contents will depend on the presence of runtime relocations against any region and the type of relocations.

Update gotpcrelx.s test as we start recognizing jmp *foo@GOTPCREL(%rip) as fixed indirect jump and convert it to a tail call with X86::JMP32m opcode.

If we recognize it as a fixed jump, why does the test case still have an indirect jump?

[Amir]

In D152944#4428996, @maksfb wrote:

I think we need a new type for RELRO sections. We rely on the contents of regular read-only sections to match that in the file. While for RELRO, the contents will depend on the presence of runtime relocations against any region and the type of relocations.

Makes sense.

Update gotpcrelx.s test as we start recognizing jmp *foo@GOTPCREL(%rip) as fixed indirect jump and convert it to a tail call with X86::JMP32m opcode.

If we recognize it as a fixed jump, why does the test case still have an indirect jump?

We keep mem-indirect jumps indirect: https://github.com/llvm/llvm-project/blob/7a709689bc1755a569864a97d93b96d22f988eb4/bolt/lib/Target/X86/X86MCPlusBuilder.cpp#L1570

[Amir]

Add BinarySection flag

[maksfb]

bolt/lib/Rewrite/RewriteInstance.cpp

425

We call this function only when Phdr.p_type == ELF::PT_GNU_RELRO. Do you plan to use it for more checks?

474

Rename.

496

We check the validity of program_headers() in discoverStorage(). Let's use cantFail() here.

1837–1840

Let's call the new function from here.

[Amir]

Address review comments

[Amir]

clang-format

[maksfb]

bolt/lib/Rewrite/RewriteInstance.cpp

452

The function currently always returns Error::success(). Thinking about it, it will be an error condition when only part of the section is in RELRO segment. Let's add such check.

463–464

Use BinarySection(*BC, SecRef).isTBSS(). Or better yet move the search for the matching BinarySection here and call isTBSS().

bolt/test/X86/pt_gnu_relro.s

5

[Amir]

Check for partial overlaps, address comments

[Amir]

clang-format

[maksfb]

bolt/lib/Rewrite/RewriteInstance.cpp

476

I realized TBSS sections are specifically excluded from isAllocatable(). We are not interested in other non-allocatable sections either, right?

[Amir]

bolt/lib/Rewrite/RewriteInstance.cpp

476

I realized I may have misunderstood the special treatment of TBSS in readelf segment->section mapping:
https://github.com/llvm/llvm-project/blob/e28bb6c3c797e06b2746e5b6dcba6fbb5e20a61b/llvm/tools/llvm-readobj/ELFDumper.cpp#L4498.

It basically says that TBSS section is treated as an empty section when it's in non-TLS segment. Let me adjust the check.

[Amir]

Gracefully handle partial overlaps, report a warning and proceed

[Amir]

Handle multiple PT_GNU_RELRO segments

[Amir]

Remove error handling as unneeded

[maksfb]

LGTM with a couple of nits.

bolt/lib/Rewrite/RewriteInstance.cpp

474

nit: for consistency in messages, let's use BinarySection->getName().

479

ditto

[Amir]

Use BinarySection->getName

[Amir]

Put warnings under opts::Verbosity >= 1

bolt/include/bolt/Core/BinarySection.h

class BinarySection {
   const SectionRef Section;    // SectionRef for input binary sections.
   StringRef Contents;          // Input section contents
   const uint64_t Address;      // Address of section in input binary (may be 0)
   const uint64_t Size;         // Input section size
   uint64_t InputFileOffset{0}; // Offset in the input binary
   unsigned Alignment;          // alignment in bytes (must be > 0)
   unsigned ELFType;            // ELF section type
   unsigned ELFFlags;           // ELF section flags
+  bool IsRelro{false};         // GNU RELRO section (read-only after relocation)
 
   // Relocations associated with this section. Relocation offsets are
   // wrt. to the original section address and size.
   using RelocationSetType = std::multiset<Relocation, std::less<>>;
   RelocationSetType Relocations;
 
   // Dynamic relocations associated with this section. Relocation offsets are
   // from the original section address.
if (isELF()) {
       return (ELFFlags & ELF::SHF_ALLOC) && !isTBSS();
     } else {
       // On non-ELF assume all sections are allocatable.
       return true;
     }
   }
   bool isReordered() const { return IsReordered; }
   bool isAnonymous() const { return IsAnonymous; }
+  bool isRelro() const { return IsRelro; }
+  void setRelro() { IsRelro = true; }
   unsigned getELFType() const { return ELFType; }
   unsigned getELFFlags() const { return ELFFlags; }
 
   uint8_t *getData() {
     return reinterpret_cast<uint8_t *>(
         const_cast<char *>(getContents().data()));
   }
   const uint8_t *getData() const {

bolt/include/bolt/Rewrite/RewriteInstance.h

private:
 
   /// Return address of a function or moved data in the new binary
   /// corresponding to \p OldAddress address in the original binary.
   uint64_t getNewFunctionOrDataAddress(uint64_t OldAddress);
 
   /// Return value for the symbol \p Name in the output.
   uint64_t getNewValueForSymbol(const StringRef Name);
 
+  /// Check for PT_GNU_RELRO segment presence, mark covered sections as
+  /// (dynamically) read-only (written once), as specified in LSB Chapter 12:
+  /// "segment which may be made read-only after relocations have been
+  /// processed".
+  void markGnuRelroSections();
+
   /// Detect addresses and offsets available in the binary for allocating
   /// new sections.
   Error discoverStorage();
 
   /// Adjust function sizes and set proper maximum size values after the whole
   /// symbol table has been processed.
   void adjustFunctionBoundaries();
 

bolt/lib/Rewrite/RewriteInstance.cpp

 #include "bolt/Rewrite/BinaryPassManager.h"
 #include "bolt/Rewrite/DWARFRewriter.h"
 #include "bolt/Rewrite/ExecutableFileMemoryManager.h"
 #include "bolt/Rewrite/JITLinkLinker.h"
 #include "bolt/RuntimeLibs/HugifyRuntimeLibrary.h"
 #include "bolt/RuntimeLibs/InstrumentationRuntimeLibrary.h"
 #include "bolt/Utils/CommandLineOpts.h"
 #include "bolt/Utils/Utils.h"
+#include "llvm/ADT/AddressRanges.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/DebugInfo/DWARF/DWARFDebugFrame.h"
 #include "llvm/IR/Function.h"
 #include "llvm/MC/MCAsmBackend.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCAsmLayout.h"
 #include "llvm/MC/MCDisassembler/MCDisassembler.h"
if (BF.isPseudo())
     return false;
 
   if (opts::processAllFunctions())
     return true;
 
   return !BF.isIgnored();
 }
 
+// Return if a section stored in the image falls into a segment address space.
+// If not, Set \p Overlap to true if there's a partial overlap.
+template <class ELFT>
+static bool checkOffsets(const typename ELFT::Phdr &Phdr,
+                         const typename ELFT::Shdr &Sec, bool &Overlap) {
+  // SHT_NOBITS sections don't need to have an offset inside the segment.
+  if (Sec.sh_type == ELF::SHT_NOBITS)
+    return true;
+
+  // Only non-empty sections can be at the end of a segment.
+  uint64_t SectionSize = Sec.sh_size ? Sec.sh_size : 1;
+  AddressRange SectionAddressRange(Sec.sh_offset, Sec.sh_offset + SectionSize);
+  AddressRange SegmentAddressRange(Phdr.p_offset,
+                                   Phdr.p_offset + Phdr.p_filesz);
+  if (SegmentAddressRange.contains(SectionAddressRange))
+    return true;
+
+  Overlap = SegmentAddressRange.intersects(SectionAddressRange);
+  return false;
+}
+
+// Check that an allocatable section belongs to a virtual address
+// space of a segment.
+template <class ELFT>
+static bool checkVMA(const typename ELFT::Phdr &Phdr,
+                     const typename ELFT::Shdr &Sec, bool &Overlap) {
+  // Only non-empty sections can be at the end of a segment.
+  uint64_t SectionSize = Sec.sh_size ? Sec.sh_size : 1;
+  AddressRange SectionAddressRange(Sec.sh_addr, Sec.sh_addr + SectionSize);
+  AddressRange SegmentAddressRange(Phdr.p_vaddr, Phdr.p_vaddr + Phdr.p_memsz);
+
+  if (SegmentAddressRange.contains(SectionAddressRange))
+    return true;
+  Overlap = SegmentAddressRange.intersects(SectionAddressRange);
+  return false;
+}
+
+void RewriteInstance::markGnuRelroSections() {
+  using ELFT = ELF64LE;
+  using ELFShdrTy = typename ELFObjectFile<ELFT>::Elf_Shdr;
+  auto ELF64LEFile = cast<ELF64LEObjectFile>(InputFile);
+  const ELFFile<ELFT> &Obj = ELF64LEFile->getELFFile();
+
+  auto handleSection = [&](const ELFT::Phdr &Phdr, SectionRef SecRef) {
+    BinarySection *BinarySection = BC->getSectionForSectionRef(SecRef);
+    // If the section is non-allocatable, ignore it for GNU_RELRO purposes:
+    // it can't be made read-only after runtime relocations processing.
+    if (!BinarySection || !BinarySection->isAllocatable())
+      return;
+    const ELFShdrTy *Sec = cantFail(Obj.getSection(SecRef.getIndex()));
+    bool ImageOverlap{false}, VMAOverlap{false};
+    bool ImageContains = checkOffsets<ELFT>(Phdr, *Sec, ImageOverlap);
+    bool VMAContains = checkVMA<ELFT>(Phdr, *Sec, VMAOverlap);
+    if (ImageOverlap) {
+      if (opts::Verbosity >= 1)
+        errs() << "BOLT-WARNING: GNU_RELRO segment has partial file offset "
+               << "overlap with section " << BinarySection->getName() << '\n';
+      return;
+    }
+    if (VMAOverlap) {
+      if (opts::Verbosity >= 1)
+        errs() << "BOLT-WARNING: GNU_RELRO segment has partial VMA overlap "
+               << "with section " << BinarySection->getName() << '\n';
+      return;
+    }
+    if (!ImageContains || !VMAContains)
+      return;
+    BinarySection->setRelro();
+    if (opts::Verbosity >= 1)
+      outs() << "BOLT-INFO: marking " << BinarySection->getName()
+             << " as GNU_RELRO\n";
+  };
+
+  for (const ELFT::Phdr &Phdr : cantFail(Obj.program_headers()))
+    if (Phdr.p_type == ELF::PT_GNU_RELRO)
+      for (SectionRef SecRef : InputFile->sections())
+        handleSection(Phdr, SecRef);
+}
+
 Error RewriteInstance::discoverStorage() {
   NamedRegionTimer T("discoverStorage", "discover storage", TimerGroupName,
                      TimerGroupDesc, opts::TimeRewrite);
 
   auto ELF64LEFile = cast<ELF64LEObjectFile>(InputFile);
   const ELFFile<ELF64LE> &Obj = ELF64LEFile->getELFFile();
 
   BC->StartFunctionAddress = Obj.getHeader().e_entry;
LLVM_DEBUG(
                << Twine::utohexstr(Section.getAddress()) << ":0x"
                << Twine::utohexstr(Section.getAddress() + Section.getSize())
                << "\n");
     if (isDebugSection(SectionName))
       HasDebugInfo = true;
     if (isKSymtabSection(SectionName))
       opts::LinuxKernelMode = true;
   }
 
+  // Set IsRelro section attribute based on PT_GNU_RELRO segment.
+  markGnuRelroSections();
+
   if (HasDebugInfo && !opts::UpdateDebugSections && !opts::AggregateOnly) {
     errs() << "BOLT-WARNING: debug info will be stripped from the binary. "
               "Use -update-debug-sections to keep it.\n";
   }
 
   HasTextRelocations = (bool)BC->getUniqueSectionByName(".rela.text");
   HasSymbolTable = (bool)BC->getUniqueSectionByName(".symtab");
   LSDASection = BC->getUniqueSectionByName(".gcc_except_table");

bolt/test/X86/pt_gnu_relro.s

+# REQUIRES: system-linux
+
+# Check that BOLT recognizes PT_GNU_RELRO segment and marks respective sections
+# accordingly.
+
+# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-linux %s -o %t.o -relax-relocations
+# RUN: ld.lld %t.o -o %t.exe -q --no-relax
+# RUN: llvm-readelf -We %t.exe | FileCheck --check-prefix=READELF %s
+# Unfortunately there's no direct way to extract a segment to section mapping
+# for a given section from readelf. Use the fool-proof way of matching
+# readelf output line-by-line.
+# READELF:      Program Headers:
+# READELF-NEXT: Type Offset {{.*}}
+# READELF-NEXT: PHDR
+# READELF-NEXT: LOAD
+# READELF-NEXT: LOAD
+# READELF-NEXT: LOAD
+# READELF-NEXT: GNU_RELRO
+# (GNU_RELRO is segment 4)
+
+# READELF: Section to Segment mapping:
+# READELF: 04 .got
+
+# RUN: llvm-bolt %t.exe --relocs -o /dev/null -v=1 \
+# RUN:   |& FileCheck --check-prefix=BOLT %s
+# BOLT: BOLT-INFO: marking .got as GNU_RELRO
+
+  .globl _start
+  .type _start, %function
+_start:
+  .cfi_startproc
+  jmp *foo@GOTPCREL(%rip)
+  ret
+  .cfi_endproc
+  .size _start, .-_start
+
+  .globl foo
+  .type foo, %function
+foo:
+  .cfi_startproc
+  ret
+  .cfi_endproc
+  .size foo, .-foo