Core dump deduplication

[nchaimov]

This PR adds a feature to Spindle to deduplicate core dumps by unique crash site, so that only one representative core dump is produced for a given crash site.

The operation is as follows:

• In the client, a signal handler is registered for signals that indicate a crash.
• When the signal handler is invoked, if an application signal handler has been registered, it is given an opportunity to fix the problem. After running the application handler, for reads, we retry the read and check if it succeeds; for writes, we check the maps and check whether the faulting address is now in a page that permits writes.
• If there was no application handler, or the application handler did not fix the problem, then for the first thread that crashes in a process, we determine the library and offset of the crash and send this to the server. For SIGABRT, the abort_msg is used instead, if set.
• The server checks whether it has already seen this crash site before; if so, it responds that the process was not selected. If not, it forwards the request up the tree, where the process is repeated. The first of a given crash site to reach the root is selected as the representative.
• Upon receiving a response, the client, if not selected, sets its own core size limit to zero to suppress the core dump.

In order to work with application-registered signal handlers, Spindle intercepts sigaction, signal, bsd_signal and sysv_signal and stores handlers registered through these. To remain invisible to the application, queries for registered signal handlers returns the application's handler rather than Spindle's, or SIG_DFL if no application handler has been registered.

There is a new set of tests which test various crash patterns (for example, all same crash sites; all different crash sites; two sets; mixed abort and segfault). There are also tests which verify that we get the correct faulting page when an access spans two pages with different permissions. There are a set of tests of application handlers which test the safepoint pattern as used by Java and Julia. A test driver script runs the tests and verifies that they crash or exit cleanly as required; that the correct number of core dumps was produced; and that the top frames of the core dumps match the expected crash site.

Known Limitations

• The current implementation registers an altstack only on the main thread. Stack overflows on other threads may leave the signal handler without any stack space of its own. Fixing this would require wrapping all thread creations and exits to allocate and register the altstack upon thread creation, and free the altstack upon thread exit.
• There is not currently a central log of which processes crashed at which sites (although this information is recorded across the various log files if Spindle is run at debug log level 2 or higher). Fixing this will require that all requests reach the root rather than short-circuiting once a server recognizes a previously-seen crash site.

I have observed intermittent failures in GitHub Actions where the Flux tests or the Slurm plugin tests hang. I believe these are existing issues and not due to the code in this PR, as we have seen the same failures in the existing devel branch, and, in the case of the Slurm plugin hang, the logs indicate a problem in cachepath consensus.

[mplegendre]

One big change needed around PHDR lookups from link maps. Otherwise it's small changes.

I didn't in-depth examine the server and network code. I expect we'll be adjusting that to fit in the crashing logging, and I'll take more of a look then.

[mplegendre]

That's a decent size static buffer, and I'm expecting we'll default run with the ALTSTACK off (once we get the configuration options to make that disable-able). How about making this a pointer instead of a buffer, and doing an anonymous mmap on it if we're using ALTSTACK.

[nchaimov]

I've replaced the static buffer as requested

[mplegendre]

Can we remove the CRASH_HANDLER_ENABLED define in general? I'd prefer to keep ifdefs to the minimum. Otherwise it's easy to get compilation errors that can slip in with rarely tested define combinations.

There are, of course, necessary defines such as the architecture specific code below. I've no objections to those. And there are occasional places where you absolutely need the define to compile, such as library/header existance.

As an alternative you can use the define in the FE config parsing to decide of crash handling is on/off by default (or make the define hardcode to off there). Then use a dynamic test throughout the rest of the code base.

[nchaimov]

I've removed CRASH_HANDLER_ENABLED; the crash handler is now always compiled.

[mplegendre]

This test won't work for libraries with compile-time-determined load addresses. You don't see as much of them anymore, but for a while RHEL was shipping system libraries that were pre-compiled to always load at the same address in every process, which produced faster load times but were more vulnerable to injection attacks. It's still legal to make libraries that do that, even if not common.

For these libraries the load_base will be 0. You calculated symbol/table/etc address in memory by adding the binary's 'offset' (which was the compile-time memory address) to the 0 load base.
This code would fault reading from 0 if it found one of those libraries.

It's absolutely obnoxious to get phdrs from link_maps (no idea why they didn't just give us a pointer). We'd previously talked about using dl_iterate_phdr at library load time to maintain a mapping of link maps to phdrs (which might be useful to other parts of spindle). I think that's still the best bet.

[nchaimov]

dl_iterate_phdr turned out not to work because it only iterates over the objects in the current namespace, so instead we read the program headers from the file. New tests have been added with an executable and a library with a fixed load address.

[mplegendre]

And this needs the load_base passed to it in order to handle PIE executables.

If you get the above find_pc_et_dyn function fixed up correctly, you should be able to use the same code for executables and libraries. Each has corner cases where they behave like the other, so you essentially need the same code to handle both.

[nchaimov]

We now handle PIE executables, and I've added a test with a PIE executable.

[mplegendre]

This is exciting. The r_debug extended is new enough I've never used it or run with it. But I also can't easily glance to know if this is right. And I don't see a test that would trigger this in the testsuite.

Have you run any spindle client crashes through this system? That would exercise this.

[nchaimov]

There was a test that crashes in a dlmopen'ed library, but it actually happened to pass even without this because the fallback was still able to deduplicate. I've changed the test so that it now checks for both correct deduplication, and that the crash site key contained the correct library.

[mplegendre]

I didn't know about PROCMAP_QUERY. Generally looks fine for this.

As a tangent... this is a new kernel API that could reveal spindle's changes to an application. We may need to start intercepting IOCTL and looking for/fixing PROCMAP_QUERY. But that doesn't need doing here.

[mplegendre]

I'm unsure, but are you confident SIGBUS should be here? Are there situations/applications that hit a SIGBUS and take corrective action?

[nchaimov]

There is a case where the application can fix SIGBUS: reading past the end of a memory-mapped file produces SIGBUS, and the application's handler can fix it by extending the file. I've added two tests for this: one where the application handler extends the file (and Spindle shouldn't and doesn't handle it as a crash), and one where the application handler doesn't (and Spindle should and does handle it).