Skip to content

[Backtracing][Linux] Fix crash handler for musl. #82624

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions stdlib/public/RuntimeModule/Elf.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1064,8 +1064,8 @@ struct ElfSymbolTable<SomeElfTraits: ElfTraits>: ElfSymbolTableProtocol {
continue
}

// Ignore anything undefined
if symbol.st_shndx == SHN_UNDEF {
// Ignore anything undefined or absolute
if symbol.st_shndx == SHN_UNDEF || symbol.st_shndx == SHN_ABS {
continue
}

Expand Down Expand Up @@ -1190,6 +1190,8 @@ final class ElfImage<SomeElfTraits: ElfTraits>
var sectionHeaders: [Traits.Shdr]?
var shouldByteSwap: Bool { return header.shouldByteSwap }

var imageBase: ImageSource.Address

@_specialize(kind: full, where SomeElfTraits == Elf32Traits)
@_specialize(kind: full, where SomeElfTraits == Elf64Traits)
required init(source: ImageSource,
Expand Down Expand Up @@ -1222,11 +1224,21 @@ final class ElfImage<SomeElfTraits: ElfTraits>

var phdrs: [Traits.Phdr] = []
var phAddr = ImageSource.Address(header.e_phoff)
var minAddr: Traits.Address? = nil
for _ in 0..<header.e_phnum {
let phdr = maybeSwap(try source.fetch(from: phAddr, as: Traits.Phdr.self))
phdrs.append(phdr)
phAddr += ImageSource.Address(header.e_phentsize)

if phdr.p_type == .PT_LOAD {
if let oldMinAddr = minAddr {
minAddr = min(oldMinAddr, phdr.p_vaddr)
} else {
minAddr = phdr.p_vaddr
}
}
}
imageBase = ImageSource.Address(exactly: minAddr ?? 0)!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any benefit to failing gracefully here for values that aren't representable, like minAddr.map { ImageSource.Address(exactly: $0) } ?? 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure. On the one hand, that would stop it crashing in such a case. On the other, I think that's rather unlikely in practice (ImageSource.Address is usually UInt64).

programHeaders = phdrs

if source.isMappedImage {
Expand Down
9 changes: 6 additions & 3 deletions stdlib/public/RuntimeModule/SymbolicatedBacktrace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -466,9 +466,6 @@ public struct SymbolicatedBacktrace: CustomStringConvertible {
for frame in backtrace.frames {
let address = frame.adjustedProgramCounter
if let imageNdx = theImages.indexOfImage(at: address) {
let relativeAddress = ImageSource.Address(
address - theImages[imageNdx].baseAddress
)
let name = theImages[imageNdx].name ?? "<unknown>"
var symbol: Symbol = Symbol(imageIndex: imageNdx,
imageName: name,
Expand Down Expand Up @@ -527,13 +524,19 @@ public struct SymbolicatedBacktrace: CustomStringConvertible {
if let hit = cache.lookup(path: theImages[imageNdx].path) {
switch hit {
case let .elf32Image(image):
let relativeAddress = ImageSource.Address(
address - theImages[imageNdx].baseAddress
) + image.imageBase
if let theSymbol = lookupSymbol(image: image,
at: imageNdx,
named: name,
address: relativeAddress) {
symbol = theSymbol
}
case let .elf64Image(image):
let relativeAddress = ImageSource.Address(
address - theImages[imageNdx].baseAddress
) + image.imageBase
if let theSymbol = lookupSymbol(image: image,
at: imageNdx,
named: name,
Expand Down
47 changes: 35 additions & 12 deletions stdlib/public/runtime/CrashHandlerLinux.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,15 @@
#include <string.h>
#include <unistd.h>

#define DEBUG_MEMSERVER 0

#if DEBUG_MEMSERVER
#include <stdio.h>
#define memserver_error(x) perror(x)
#else
#define memserver_error(x)
#endif

#include "swift/Runtime/Backtrace.h"

#include <cstring>
Expand Down Expand Up @@ -86,8 +95,8 @@ ssize_t safe_read(int fd, void *buf, size_t len) {
ssize_t ret;
do {
ret = read(fd, buf, len);
} while (ret < 0 && errno == EINTR);
if (ret < 0)
} while (ret <= 0 && errno == EINTR);
if (ret <= 0)
return ret;
total += ret;
ptr += ret;
Expand All @@ -106,8 +115,8 @@ ssize_t safe_write(int fd, const void *buf, size_t len) {
ssize_t ret;
do {
ret = write(fd, buf, len);
} while (ret < 0 && errno == EINTR);
if (ret < 0)
} while (ret <= 0 && errno == EINTR);
if (ret <= 0)
return ret;
total += ret;
ptr += ret;
Expand Down Expand Up @@ -657,20 +666,28 @@ memserver_start()
int fds[2];

ret = socketpair(AF_UNIX, SOCK_STREAM, 0, fds);
if (ret < 0)
if (ret < 0) {
memserver_error("memserver_start: socketpair failed");
return ret;
}

memserver_fd = fds[0];
ret = clone(memserver_entry, memserver_stack + sizeof(memserver_stack),
#if MEMSERVER_USE_PROCESS
0,
#else
CLONE_THREAD | CLONE_VM | CLONE_FILES
| CLONE_FS | CLONE_IO | CLONE_SIGHAND,
#ifndef __musl__
// Can't use CLONE_THREAD on musl because the clone() function
// there returns EINVAL if we do.
CLONE_THREAD | CLONE_SIGHAND |
#endif
CLONE_VM | CLONE_FILES | CLONE_FS | CLONE_IO,
#endif
NULL);
if (ret < 0)
if (ret < 0) {
memserver_error("memserver_start: clone failed");
return ret;
}

#if MEMSERVER_USE_PROCESS
memserver_pid = ret;
Expand Down Expand Up @@ -718,7 +735,7 @@ memserver_entry(void *dummy __attribute__((unused))) {
int fd = memserver_fd;
int result = 1;

#if MEMSERVER_USE_PROCESS
#if MEMSERVER_USE_PROCESS || defined(__musl__)
prctl(PR_SET_NAME, "[backtrace]");
#endif

Expand All @@ -743,8 +760,10 @@ memserver_entry(void *dummy __attribute__((unused))) {
ssize_t ret;

ret = safe_read(fd, &req, sizeof(req));
if (ret != sizeof(req))
if (ret != sizeof(req)) {
memserver_error("memserver: terminating because safe_read() returned wrong size");
break;
}

uint64_t addr = req.addr;
uint64_t bytes = req.len;
Expand All @@ -761,15 +780,19 @@ memserver_entry(void *dummy __attribute__((unused))) {
resp.len = ret;

ret = safe_write(fd, &resp, sizeof(resp));
if (ret != sizeof(resp))
if (ret != sizeof(resp)) {
memserver_error("memserver: terminating because safe_write() failed");
goto fail;
}

if (resp.len < 0)
break;

ret = safe_write(fd, memserver_buffer, resp.len);
if (ret != resp.len)
if (ret != resp.len) {
memserver_error("memserver: terminating because safe_write() failed (2)");
goto fail;
}

addr += resp.len;
bytes -= resp.len;
Expand Down