Conversation
|
Add an arm branch to the vendored libunwind CMake so Linux arm (32-bit) targets compile the appropriate source set from src/arm/ and include tdep-arm/. Source list mirrors the upstream 1.8.3 Makefile.am ARCH_ARM block; the arm sources in the tree are byte-identical to the release tarball (verified with diff -r against v1.8.3). Extend the existing CI test matrix with a CROSS_ARM32 entry on ubuntu-24.04 using gcc-arm-linux-gnueabihf to cross-compile the inproc backend. The arm32 binaries are not executable on the amd64 runner, so this row skips pytest and runs a direct cmake --build against SENTRY_BACKEND=inproc instead. Update VENDORING.md to move arm from "kept but NOT built" to Supported Architectures, and adjust the maintenance steps to reference all four supported architectures. Refs GH-1587 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
29d51ae to
db28bf4
Compare
There was a problem hiding this comment.
LGTM, at least in the assumed limit.
QEMU-user was considered but ruled out: its synthetic signal frames don't match the kernel layout that libunwind's signal-frame detection expects, so the crash/signal test paths would fail for qemu reasons rather than real arm32 issues.
Well, the thing is, if we add an ARM build, users will run it. So, even if a test configuration doesn't cover signal handling on ARM, it could at least cover everything else.
libunwind will also be used for stacktraces in normal operation, meaning we have feedback on that, too. So we could disable the crash tests, with !is_arm() (or whatever) in the integration tests.
This is not a blocker for this issue, just something to think about.
Enable the pytest suite on the arm32 cross-compile matrix row by installing qemu-user-static + binfmt-support so the kernel transparently dispatches armhf binaries to qemu, and wiring the cross-compile cmake options through get_platform_cmake_args so pytest's build fixtures produce armhf binaries. Drop the standalone cmake build step now that pytest drives the build. Rename the matrix flag from CROSS_ARM32 to TEST_ARM32 for consistency with TEST_X86 / TEST_MINGW. Add is_arm32 and is_qemu conditions to tests/conditions.py (orthogonal: arm32 is the target arch, qemu is the execution environment). No test skips are wired yet — the intent is to observe which tests actually fail under qemu-user before adding narrowly-scoped skipif markers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Observed failures from local docker runs of pytest in an ubuntu arm64 container with armhf multiarch + qemu-user. Apply narrow skips for the confirmed-failing paths: - has_crashpad: crashpad's arm32 cross-build fails (gcc psABI-driven errors in the nested C++ sort/vector paths). - has_native: the native backend's arm32 cross-build fails too. - Breakpad builds fine for arm32, but its crash+restore flow doesn't survive qemu-user signal handling (cross-thread crash coordination). Inline `or is_qemu` on each breakpad crash-test skipif so the build- only test_static_breakpad keeps running. - test_dotnet_signals_* / test_aot_signals_*: .NET on armhf inside qemu isn't a real thing. - test_capture_http / test_exception_and_session_http / test_capture_stdout: dladdr-based symbolication returns frames without function/package under qemu-user. - test_inproc_abort_stdout / test_inproc_crash_stdout_before_send / test_inproc_crash_stdout_discarding_on_crash: inproc crash flows that don't reproduce under qemu-user signal frames. - test_static_lib: ldd on foreign-arch binaries doesn't work under qemu, and the 'file' output check had no TEST_ARM32 branch. - unwinder unit test: unreliable under qemu-user. Result from the local docker run: 1114 passed, 0 failed, 139 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
os.environ.get returns a string (or None), and pytest's @skipif treats
a str argument as a Python expression to eval. On matrix rows where
TEST_ARM32/TEST_QEMU aren't set, GitHub substitutes empty string, and
eval("") raises SyntaxError. Forcing bool sidesteps the string path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
These tests only inspect strings embedded in libsentry.so (version, platform, variant, build id) and don't exercise any backend, but they were building with the platform default. Aligns them with the sibling tests in the same file that already pass SENTRY_BACKEND=none, and avoids dragging crashpad into the build on platforms where it's not needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Crashpad now builds for arm32 (submodule bump carries the .S stack-note %progbits fix). Drop the `and not is_arm32` gate from has_crashpad so the integration suite runs, and add `@pytest.mark.skipif(is_qemu, ...)` to each of the 16 test functions whose crash-and-restore flow doesn't survive qemu-user (external crashpad_handler + cross-process IPC, same class as breakpad/native). The non-crash tests (proxy config, retry-on-failure, crash_after_shutdown, etc.) stay enabled and pass. Result on the arm32 qemu-user CI row: 5 passed, 35 skipped, 0 failed (was: 5 passed, 23 failed, 12 skipped). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 16 skipifs added in c5830d1 were based on an earlier qemu run where all crashpad tests failed with "error while loading shared libraries: libstdc++.so.6" because libstdc++6:armhf wasn't installed. Once that package is added to the CI install step, a local docker run shows that several crashpad crash tests actually pass under qemu-user (e.g. test_logger_enabled_when_crashed[crashpad]). Remove the preemptive skipifs so CI observes which crashpad tests genuinely fail, then narrower skipifs can be added based on evidence. Also add libstdc++6:armhf to the arm32 dependency install step so any C++-linked example/test (including crashpad's sentry_example) can actually load at runtime. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
I guess arm32 was never supported, and #1587 was just mislabeled as a bug by the reporter themselves. 😅 I thought it was a regression introduced by the vendored libunwind. |
write_thread_context emitted "Unsupported architecture for Linux" on __arm__, blocking the native backend from building for armhf. Add the arm32 branch that maps the Linux ucontext (arm_r0..arm_r10, arm_fp, arm_ip, arm_sp, arm_lr, arm_pc, arm_cpsr) into the existing minidump_context_arm_t. VFP/NEON state is left zeroed (matches the i386 branch's punt on FPU capture). The register mapping follows breakpad's reference implementation (external/breakpad/src/client/linux/dump_writer_common/ucontext_reader.cc, UContextReader::FillCPUContext), translated into the named-field layout used by sentry's minidump_context_arm_t. The context_flags constant uses the Microsoft-aligned CONTEXT_ARM=0x00200000 (same numbering convention the other arches in this file already follow), not breakpad's custom 0x40000000. Verified by cross-compiling SENTRY_BACKEND=native for armhf in an ubuntu arm64 docker container. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Native backend builds for armhf now that write_thread_context has the __arm__ branch. Drop the `and not is_arm32` gate from has_native so the integration suite runs. Without preemptive is_qemu skipifs — let CI surface the actual failing tests (applying the "don't skip until confirmed" lesson from the crashpad pass). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Native backend's cross-process sentry-crash daemon needs prctl(PR_SET_PTRACER) and ptrace for the handler to read the crashed process's memory. qemu-user emulates neither reliably — the handler logs "prctl(PR_SET_PTRACER) failed: Invalid argument" and the subsequent read fails. Under a local docker run with the current setup, 24 of ~27 native-parametrized tests fail; the 3 "passes" are all spurious (tests asserting absence of crash state, accidentally satisfied when the daemon can't produce state). Unlike crashpad where a meaningful subset of tests exercises only non-crash paths (proxy config, retry, etc.), native integration tests are almost uniformly crash-driven. Per-test skipifs would preserve near-zero coverage, so gate has_native itself. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the has_native qemu gate with per-usage inline `or is_qemu`, mirroring the breakpad pattern. The native backend itself builds and links fine under qemu — it's the sentry-crash daemon's ptrace-based crash handling that breaks. So has_native stays clean (so test_static_native and any other non-crash native tests still run), and only the crash-path skipifs pick up the extra qemu clause: - test_integration_native.py file-level pytestmark (every test in this file drives a crash) - test_integration_logger.py native parametrize marks - test_integration_logs.py, test_integration_metrics.py, test_integration_http.py function-level skipifs for their native-crash tests Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Yeah, it has nothing to do with |
|
The PR got a bit noisy, but it runs now most tests on arm32:
Not sure what the reporter uses but older Raspberry Pi's are also potential targets. I don't remember when they switched to 64-bit by default, but I think all versions up to RPI4 still have 32-bit support. |
No, absolutely, there are many embedded targets that run on 32-bit ARMs, but the question is not what exists, but what is supported. And besides Android, nothing but 64-bit was officially supported, although certain paths will work just fine. |
ptrace_capture_thread() and write_thread_list_stream() each declare a local SP variable and assign it inside a per-arch #if/#elif ladder that only covered x86_64, aarch64, and i386. On __arm__, the variable fell through both ladders and was used uninitialized — either to gate a ptrace-based stack capture or as the base address for read_process_memory(), producing a garbage pointer and likely a crash or corrupted minidump. Add the missing __arm__ branches that read uc_mcontext.arm_sp. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…bit ARM ptrace_get_thread_registers had x86_64 / aarch64 / i386 branches but no __arm__ branch. On arm32 it returned false without populating the thread's ucontext, so every non-crashed thread landed in the minidump without registers or stack — multi-threaded crash reports on arm32 were effectively single-threaded. Add the arm32 branch using PTRACE_GETREGS (same mechanism the x86_64 / i386 branches use; works on arm32 too) and map the returned `struct user_regs` uregs[0..16] into the ucontext_t's arm_* fields. No VFP/NEON capture yet — matches the i386 branch's punt on FPU state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
minidump_context_arm_t was 72 bytes (integers + cpsr), but the on-disk minidump spec for ARM contexts — as implemented by breakpad (MDRawContextARM) and rust-minidump (CONTEXT_ARM, the reader Sentry's ingest uses to deserialize these streams) — is 368 bytes, with a trailing float-save area (fpscr u64, 32x u64 VFP regs, 8x u32 extra). rust-minidump's struct is explicitly called out as a "Breakpad extension": https://github.com/rust-minidump/rust-minidump/blob/d4fefc765aad35b3bef569d53c1680eadab5a268/minidump-common/src/format.rs#L1034-L1055 Since get_context_size() returns sizeof(minidump_context_arm_t), the stream size recorded in the minidump was too small and downstream parsers couldn't deserialize the thread context. Add the missing float-save trailing fields. They're left zero- initialized (we don't capture VFP state yet — same punt the i386 branch makes for FPU) but the layout now matches what readers expect. The other arches in this file (i386, aarch64) already carry their full structs this way. Note: crashpad's in-memory MinidumpContextARM is 364 bytes (u32 fpscr), a 4-byte divergence from breakpad despite crashpad's comment saying it's "included for compatibility with breakpad." That's a crashpad-internal quirk; on-disk minidumps follow the breakpad layout that rust-minidump reads. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a3ecf71. Configure here.
The ARM32 write_thread_context branch was emitting context_flags = 0x00200003 (Microsoft Windows CONTEXT_ARM | CONTROL | INTEGER). That matched the pattern the other arches use in this file, but ARM32 is the one arch where breakpad explicitly diverges from Microsoft's numbering, with a comment in minidump_cpu_arm.h: // This value was chosen to avoid likely conflicts with MD_CONTEXT_* // for other CPUs. #define MD_CONTEXT_ARM 0x40000000 rust-minidump (Sentry's ingest reader) keys off the breakpad value: https://github.com/rust-minidump/rust-minidump/blob/d4fefc765aad35b3bef569d53c1680eadab5a268/minidump-common/src/format.rs#L801-L804 With 0x00200000, the parser wouldn't identify the stream as an ARM context and registers would be dropped from the crash report. Switch to 0x40000003 to match breakpad / rust-minidump / crashpad (all of which use 0x40000000 for ARM specifically). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
See also: getsentry/crashpad#150 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Adds 32-bit ARM support to Linux builds across all three affected components.
vendor/libunwind/CMakeLists.txtgets an ARM branch that compiles the appropriate source set fromsrc/arm/andinclude/tdep-arm/. Source list mirrors the upstream 1.8.3ARCH_ARMblock: https://github.com/libunwind/libunwind/blob/v1.8.3/src/Makefile.am#L481-L501src/backends/native/minidump/sentry_minidump_linux.cgrows a__arm__branch inwrite_thread_contextthat maps the Linux ucontext intominidump_context_arm_t, following breakpad'sucontext_reader.cc.CONTEXT_ARMuses Microsoft's0x00200000numbering to match the other arches in the file.external/crashpadis bumped to pick up a one-character%progbitsfix incrashpad_info_note.S—@progbitsis silently dropped on ARM since@is the comment char there, which was causing the assembler to reject the rest of the.sectiondirective.Close: #1587