Details

Reviewers

kib
emaste

Commits

rGa7d137fcbcac: rtld: Support DT_RELR relative relocation format

Summary

PIE and shared objects usually have many relative relocations. In
2017/2018, a compact relative relocation format RELR was proposed on
https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/GxjM0L-PBAAJ
("Proposal for a new section type SHT_RELR") and is a pre-standard.
RELR usually takes 3% or smaller space than R_*_RELATIVE relocations.
The virtual memory size of a mostly statically linked PIE is typically 5~10% smaller.

ld.lld --pack-dyn-relocs=relr emits RELR relocations. DT_RELR has been
adopted by Android bionic, Linux kernel's arm64 port, Chrome OS (patched
glibc).

This patch adds DT_RELR support to rtld-elf.

Author: Fangrui Song <i@maskray.me>

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

fbsd-phab_maskray.me created this revision.Oct 16 2021, 6:43 PM

Herald added subscribers: Contributor Reviews (src), bdragon, imp. · View Herald TranscriptOct 16 2021, 6:43 PM

fbsd-phab_maskray.me requested review of this revision.Oct 16 2021, 6:43 PM

Harbormaster completed remote builds in B42169: Diff 96958.Oct 16 2021, 6:43 PM

RELR is like a modern version of Firefox’s ELF hack https://glandium.org/blog/?p=1177#relocations

About "pre-standard": Cary Coutant took over generic-abi maintenanceship (https://groups.google.com/g/generic-abi/c/9OO5vhxb00Y "Ongoing Maintenance of the gABI") and plans to apply changes including RELR: https://sourceware.org/pipermail/libc-alpha/2021-October/131781.html

My testing on amd64:

# To build just libexec/rtld-elf
MAKEOBJDIRPREFIX=$PWD/obj/default make -j 12 buildenv
export CPUTYPE=  # zsh hack
make -C libexec/rtld-elf

cd /tmp/c
objdir=$HOME/freebsd/obj/default/usr/home/ray/freebsd/amd64.amd64
clang -Wl,--dynamic-linker=$objdir/libexec/rtld-elf/ld-elf.so.1.full -L/tmp/opt/lib -L$objdir/lib -Wl,-rpath=$objdir/lib/libc -fpic -shared b.c -o b.so -Wl,-Bsymbolic -Wl,--pack-dyn-relocs=relr
clang -Wl,--dynamic-linker=$objdir/libexec/rtld-elf/ld-elf.so.1.full -L/tmp/opt/lib -L$objdir/lib -Wl,-rpath=$objdir/lib/libc -fpie -pie a.c ./b.so -o a -Wl,--pack-dyn-relocs=relr
./a

// b.c
#include <stdio.h>

int vb;
int fb() {
  return vb++;
}
// a.c
#include <stdio.h>

int fb();
int main() {
  printf("%d\n", fb());
  printf("%d\n", fb());
  printf("%d\n", fb());
  printf("%d\n", fb());
}

const char* -> const char *

Harbormaster completed remote builds in B42171: Diff 96962.Oct 16 2021, 7:02 PM

fbsd-phab_maskray.me edited the summary of this revision. (Show Details)Oct 16 2021, 7:14 PM

kib added inline comments.Oct 16 2021, 8:17 PM

libexec/rtld-elf/rtld.c
3167	Please use normal style(9) conventions for the new code. Most important, use tab for indent, not 4 spaces.
3173	You are checking the lowest bit there, right? I believe `& 1` is much more common way to do this. Initially I was quite confused by the code, but apparently they changed encoding against the initial proposal. Also they made all entries 64bit, right? IMO the documentation in the tangled thread on some list/google groups is not enough argument to start supporting this stuff. The last activity occured in 2018.
3181	What does this 8 mean?
sys/sys/elf32.h
155 ↗	(On Diff #96962)	If they changed relocation entry to be 64bit always, why this?
sys/sys/elf_common.h
622 ↗	(On Diff #96962)	Is this value used somehow?

kib added a subscriber: emaste.Oct 16 2021, 8:34 PM

kib removed a subscriber: emaste.

address comments

Harbormaster completed remote builds in B42172: Diff 96963.Oct 16 2021, 8:49 PM

fbsd-phab_maskray.me added inline comments.Oct 16 2021, 8:49 PM

libexec/rtld-elf/rtld.c
3167	OK. I'll use this neovim script for my FreeBSD code: :so ~/Dev/freebsd/tools/tools/editing/freebsd.vim :call FreeBSD_Style() :set ts=8 sw=4 sts=0 noet
3173	Yes, the format changed. Ali Bahrami's comment (scroll to the bottom) is the format used by ld.lld, llvm-readelf -r, Android bionic, Linux kernel, and Chrome OS. Someone may need to implement `readelf -r` for FreeBSD's readelf.
3181	CHAR_BIT = 8. I just switched to CHAR_BIT for clarity. A bitmap entry describes 63 locations on a 64-bit machine. where skips 63 locations.
sys/sys/elf32.h
155 ↗	(On Diff #96962)	They use `typedef Elf32_Word Elf32_Relr;` for ELFCLASS32. ld.lld --pack-dyn-relocs=relr and llvm-readelf -r use 32-bit on ELFCLASS32 as well (llvm-project `llvm/include/llvm/BinaryFormat/ELF.h`)
sys/sys/elf_common.h
622 ↗	(On Diff #96962)	I just added an assert for DT_RELRENT.

ld.lld's RELR is robust since 2019-04 (https://reviews.llvm.org/D67164) when I fixed an oscillation bug.

Better test:

% cat relr.c
static int o, x;

#define ELEMS O O O O O O O O X X X X X X X O O X O O X X X E X E E O X O E
#define E 0,

#define O &o,
#define X &x,
void *arr[] = { ELEMS };
#undef O
#undef X

#define O 1,
#define X 2,
static char val[] = { ELEMS };

int main()
{
  for (int i = 0; i < sizeof (arr) / sizeof (arr[0]); i++)
    if (!((arr[i] == 0 && val[i] == 0) ||
          (arr[i] == &o && val[i] == 1) ||
          (arr[i] == &x && val[i] == 2)))
      return 1;
  return 0;
}

% cat /tmp/opt/lib/libc.so 
/* $FreeBSD: releng/12.2/lib/libc/libc.ldscript 258283 2013-11-17 22:52:17Z peter $ */
GROUP ( libc/libc.so.7 libc_nonshared/libc_nonshared.a libssp_nonshared/libssp_nonshared.a )
% clang -Wl,--dynamic-linker=$objdir/libexec/rtld-elf/ld-elf.so.1.full -L/tmp/opt/lib -L$objdir/lib -Wl,-rpath=$objdir/lib/libc -fpie -pie relr.c -o relr -Wl,--pack-dyn-relocs=relr
% ./relr  # good

# Without DT_RELR support, segfault:
% clang -fpie -pie relr.c -o relr -Wl,--pack-dyn-relocs=relr
% ./relr
[1]    45961 segmentation fault (core dumped)  ./relr

Please split the patch into two: one for sys/ adding the needed definitions, and one for rtld. Then send me complete git patches with all metadata and tags filled.

libexec/rtld-elf/rtld.c
3172	There should be a blank line after the local declaration.
3178	If you use local inside block, why not move its declaration into for loop init clause?
3179	!= 0

address style issues

rebase on D32526

Harbormaster completed remote builds in B42175: Diff 96966.Oct 16 2021, 9:35 PM

fbsd-phab_maskray.me added a parent revision: D32526: sys: Add definitions for RELR relative relocation format.Oct 16 2021, 9:35 PM

git format-patch -2 --stdout of D32526 + D32524: https://gist.github.com/MaskRay/4df58dc970a377d04ec93a983685f1d4

This revision was not accepted when it landed; it landed in state Needs Review.Oct 16 2021, 11:39 PM

Closed by commit rGa7d137fcbcac: rtld: Support DT_RELR relative relocation format (authored by fbsd-phab_maskray.me, committed by kib). · Explain Why

This revision was automatically updated to reflect the committed changes.

kib added a commit: rGa7d137fcbcac: rtld: Support DT_RELR relative relocation format.

arichardson added a subscriber: arichardson.Oct 17 2021, 6:48 AM

arichardson added inline comments.

libexec/rtld-elf/rtld.c
3167	For most code git-clang-format should be almost correct. However rtld still uses different indentation for some functions...
3173	Or we just switch to MK_LLVM_BINUTILS by default :)

emaste added inline comments.Oct 18 2021, 1:55 PM

libexec/rtld-elf/rtld.c
3173	Or both - even when `MK_LLVM_BINUTILS` lands we presumably will not MFC it, but people may well run readelf from older FreeBSD branches to inspect objects built with RELR.

Folks may be interested in

https://maskray.me/blog/2021-10-31-relative-relocations-and-relr#time-travel-compatibility
https://groups.google.com/g/generic-abi/c/yOy5jfTYek4 ("Proposal: New DT_REQUIRED tag to identify required features")

@fbsd-phab_maskray.me tell the Linux folks that they don't want to be too far behind FreeBSD :)

rtld: Support DT_RELR relative relocation format
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 96971

libexec/rtld-elf/rtld.h

libexec/rtld-elf/rtld.c

rtld: Support DT_RELR relative relocation formatClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 96971

libexec/rtld-elf/rtld.h

libexec/rtld-elf/rtld.c

rtld: Support DT_RELR relative relocation format
ClosedPublic
Actions

Revision Contents
Changeset List