I noticed that we were populating this StringBuilder and then throwing
away the result while profiling `true` with UserSpace emulator.
Before:
courage:~ $ time -n 1000 true
Timing report: 3454 ms
==============
Command: true
Average time: 3.45 ms (median: 3, stddev: 3.42, min: 0, max:11)
Excluding first: 3.45 ms (median: 3, stddev: 3.42, min: 0, max:11)
After:
courage:~ $ time -n 1000 true
Timing report: 3308 ms
==============
Command: true
Average time: 3.30 ms (median: 3, stddev: 3.28, min: 0, max:12)
Excluding first: 3.30 ms (median: 3, stddev: 3.29, min: 0, max:12)
The DT_RELR relocation is a relatively new relocation encoding designed
to achieve space-efficient relative relocations in PIE programs.
The description of the format is available here:
https://groups.google.com/g/generic-abi/c/bX460iggiKg/m/Pi9aSwwABgAJ
It works by using a bitmap to store the offsets which need to be
relocated. Even entries are *address* entries: they contain an address
(relative to the base of the executable) which needs to be relocated.
Subsequent even entries are *bitmap* entries: "1" bits encode offsets
(in word size increments) relative to the last address entry which need
to be relocated.
This is in contrast to the REL/RELA format, where each entry takes up
2/3 machine words. Certain kinds of relocations store useful data in
that space (like the name of the referenced symbol), so not everything
can be encoded in this format. But as position-independent executables
and shared libraries tend to have a lot of relative relocations, a
specialized encoding for them absolutely makes sense.
The authors of the format suggest an overall 5-20% reduction in the file
size of various programs. Due to our extensive use of dynamic linking
and us not stripping debug info, relative relocations don't make up such
a large portion of the binary's size, so the measurements will tend to
skew to the lower side of the spectrum.
The following measurements were made with the x86-64 Clang toolchain:
- The kernel contains 290989 relocations. Enabling RELR decreased its
size from 30 MiB to 23 MiB.
- LibUnicodeData contains 190262 relocations, almost all of them
relative. Its file size changed from 17 MiB to 13 MiB.
- /bin/WebContent contains 1300 relocations, 66% of which are relative
relocations. With RELR, its size changed from 832 KiB to 812 KiB.
This change was inspired by the following blog post:
https://maskray.me/blog/2021-10-31-relative-relocations-and-relr
Apologies for the enormous commit, but I don't see a way to split this
up nicely. In the vast majority of cases it's a simple change. A few
extra places can use TRY instead of manual error checking though. :^)
This change lays the foundation for making the require_promise return
an error hand handling the process abort outside of the syscall
implementations, to avoid cases where we would leak resources.
It also has the advantage that it makes removes a gs pointer read
to look up the current thread, then process for every syscall. We
can instead go through the Process this pointer in most cases.
When the member is initialized in the constructor body, but also has a
default constructor, you pay for default construction to just throw it
away. In this case a StringView is light weight to initialize, but we
might as well fix all cases we find.
Found by PVS-Studio: https://pvs-studio.com/en/docs/warnings/v818/
Consider the situation where two shared libraries libA and libB, both
depending (as in having a NEEDED dtag) on libC. libA is first
dlopen()-ed, which produces libC to be mapped and linked. When libB is
dlopen()-ed the DynamicLinker would re-map and re-link libC though,
causing any previous references to its old location to be invalid. And
if libA's PLT has been patched to point to libC's symbols, then any
further invocations to libA will cause the code to jump to a virtual
address that isn't mapped anymore, therefore causing a crash. This
situation was reported in #10014, although the setup was more convolved
in the ticket.
This commit fixes the issue by distinguishing between a main program
loading being performed by Loader.so, and a dlopen() call. The main
difference between these two cases is that in the former the
s_globals_objects maps is always empty, while in the latter it might
already contain dependencies for the library being dlopen()-ed. Hence,
when collecting dependencies to map and link, dlopen() should skip those
that are present in the global map to avoid the issue described above.
With this patch the original issue seen in #10014 is gone, with all
python3 modules (so far) loading correctly.
A unit test reproducing a simplified issue is also included in this
commit. The unit test includes the building of two dynamic libraries A
and B with both depending on libline.so (and B also depending on A); the
test then dlopen()s libA, invokes one its function, then does the same
with libB.
These are found in some libraries, and LibELF doesn't know how to handle
them, not even their name. Adding these definitions should at least help
readelf display information correctly, but more work is needed to
actually implement them.
A copy of the same mapping was found both in LibELF and in the readelf
utility, which uses LibELF; keeping them both is redundant and removing
the duplicate saves (a bit of) space.
The dynamic loader was mistakenly assuming that there are only two types
of program load headers: text (RX) and data (RW).
Now that we're linking with `-z separate-code`, we will also get some
read-onlydata (R) segments. These can be memory-mapped directly without
making a private per-process copy.
To solve this, the code now instead separates the headers into map/copy
instead of text/data. Writable segments get copied, while non-writable
segments get memory-mapped. :^)
GNU ld sometimes generates zero-sized PT_LOAD headers when running with
the "-z separate-code" option. Let's not choke on such headers, we can
just ignore them and move along.
Optimizations:
- Make sure `DT_SYMTAB` is a string view literal, instead of string.
- DynamicObject::HashSection::lookup_sysv_symbol should be using
raw_name() from symbol comparison to avoid needlessly calling
`strlen`, when the StrinView::operator= walks the cstring without
calling `strlen` first.
- DynamicObject::HashSection::lookup_gnu_symbol shouldn't create a
symbol unless we know the hashes match first.
In order to test these changes I enabled Undefined behavior sanitizer
which creates a huge amount of relocations, and then ran the browser
with the help argument 100 times. The browser is a fairly big app with
a few different libraries being loaded, so it seemed liked a good
target.
Command: `time -n 100 br --help`
Before:
```
Timing report:
==============
Command: br --help
Average time: 3897.679931 ms
Excluding first: 3901.242431 ms
```
After:
```
Timing report:
==============
Command: br --help
Average time: 3612.860107 ms
Excluding first: 3613.54541 ms
```
These integer => pointer => integer conversions were technically prone
to UB, since they were used as offsets (which are perfectly fine to be
zero), but we calculated them with pointer arithmetic. This made Clang
insert pointer overflow UBSAN checks, which trigger in case of a zero
result.
When loading libraries, it is required that each library uses the same
instance of each symbol, and that they use the one from the executable
if any. This is barely noticeable if done incorrectly; except that it
completely breaks RTTI on Clang. This switches the hash map to be
ordered; tested to work for Clang by @Bertaland
