diff --git a/CHANGELOG.md b/CHANGELOG.md index 50d12a4fb..a3f379e72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ **Fixes**: +- Native/macOS: fix module `image_size` computation, which could have caused the symbolicator to misattribute every frame to the lowest-addressed image (typically `dyld` or `libsystem`). ([#1740](https://github.com/getsentry/sentry-native/pull/1740)) - Native: raise `SENTRY_CRASH_MAX_MODULES` from `512` to `2048` so processes that load many shared libraries no longer have their minidump module list truncated, which left frames in unrecorded modules without a `debug_id` and unsymbolicatable. ([#1738](https://github.com/getsentry/sentry-native/pull/1738)) - Reject overly deep msgpack payloads during deserialization. ([#1727](https://github.com/getsentry/sentry-native/pull/1727)) diff --git a/src/backends/native/sentry_crash_handler.c b/src/backends/native/sentry_crash_handler.c index 6b6671d45..fe588d938 100644 --- a/src/backends/native/sentry_crash_handler.c +++ b/src/backends/native/sentry_crash_handler.c @@ -562,31 +562,30 @@ crash_signal_handler(int signum, siginfo_t *info, void *context) const struct mach_header_64 *header64 = (const struct mach_header_64 *)header; const uint8_t *cmds = (const uint8_t *)(header64 + 1); + bool has_size = false; + bool has_uuid = false; - for (uint32_t j = 0; j < header64->ncmds && j < 256; j++) { + for (uint32_t j = 0; + j < header64->ncmds && j < 256 && (!has_size || !has_uuid); + j++) { const struct load_command *cmd = (const struct load_command *)cmds; if (cmd->cmd == LC_SEGMENT_64) { const struct segment_command_64 *seg = (const struct segment_command_64 *)cmd; - // Skip __PAGEZERO which has vmsize=4GB on 64-bit and - // would vastly inflate the module size - if (seg->initprot == 0 && seg->maxprot == 0) { - cmds += cmd->cmdsize; - if (cmd->cmdsize == 0) - break; - continue; - } - uint64_t seg_end = seg->vmaddr + seg->vmsize; - if (seg_end > size) { - size = seg_end; + // image_size on macOS is the __TEXT segment's vmsize: + // https://github.com/getsentry/sentry-native/blob/82dec4b8518de32586c4a761a3a9df4aed9d7b8d/src/modulefinder/sentry_modulefinder_apple.c#L65-L69 + if (memcmp(seg->segname, "__TEXT", 7) == 0) { + size = seg->vmsize; + has_size = true; } } else if (cmd->cmd == LC_UUID) { // Extract UUID for symbolication const struct uuid_command *uuid_cmd = (const struct uuid_command *)cmd; signal_safe_memcpy(module->uuid, uuid_cmd->uuid, 16); + has_uuid = true; } cmds += cmd->cmdsize; diff --git a/tests/assertions.py b/tests/assertions.py index 5592f3989..7ec07e876 100644 --- a/tests/assertions.py +++ b/tests/assertions.py @@ -182,6 +182,29 @@ def assert_event_meta( ) +def assert_debug_meta_images_do_not_overlap(event): + """Validate ``event["debug_meta"]["images"]`` shape. + + The half-open ranges ``[image_addr, image_addr + image_size)`` must not + overlap -- the symbolicator relies on this to attribute frames to images. + """ + images = event["debug_meta"]["images"] + assert len(images) > 0, "debug_meta should contain at least one image" + + ranges = [] + for image in images: + name = image.get("code_file") or image.get("debug_file") or "" + addr = int(image["image_addr"], 16) + ranges.append((addr, addr + image["image_size"], name)) + + ranges.sort() + for (a_start, a_end, a_name), (b_start, b_end, b_name) in zip(ranges, ranges[1:]): + assert a_end <= b_start, ( + f"image ranges overlap: {a_name} [{a_start:#x}, {a_end:#x}) " + f"vs {b_name} [{b_start:#x}, {b_end:#x})" + ) + + def is_valid_hex(s): if not s.lower().startswith("0x"): return False diff --git a/tests/test_integration_native.py b/tests/test_integration_native.py index 00efc711f..81d9e6a9a 100644 --- a/tests/test_integration_native.py +++ b/tests/test_integration_native.py @@ -20,6 +20,7 @@ ) from .assertions import ( assert_breadcrumb, + assert_debug_meta_images_do_not_overlap, assert_meta, assert_native_crash, assert_session, @@ -903,6 +904,8 @@ def test_crash_mode_native_only(cmake, httpserver): # Should have debug_meta assert "debug_meta" in event assert len(event["debug_meta"]["images"]) > 0 + if sys.platform == "darwin": + assert_debug_meta_images_do_not_overlap(event) def test_crash_mode_native_with_minidump(cmake, httpserver): @@ -947,6 +950,8 @@ def test_crash_mode_native_with_minidump(cmake, httpserver): # Should have debug_meta assert "debug_meta" in event + if sys.platform == "darwin": + assert_debug_meta_images_do_not_overlap(event) def test_native_cache_consent(cmake, httpserver):