Coredump: implementation on MMU targets

[etiaro]

Description

Implement configurable dumping of serialized memory, registers and metadata on a fault that allows for post-mortem debugging of crashed process using GDB after parsing on host machine.

Motivation and Context

RTOS-1043 Research core dump and backtrace techniques
RTOS-1049 Implement core dump on MMU targets

Example coredump log

This is the most minimalistic coredump example (only stack memory and registers, one thread), generated on ia32.
Allows for backtrace and shows all stack variables in GDB.

_____________COREDUMP_START_____________
/usr/bin/coredump_test: 14 #PF
f0VMRgEBAf4JAAQAAwAB/gcANP4LADQAIAAC/gcABAAAAHT+CwAkAQAAJAH+CgABAAAAmAEAAIxf/gYAdAAAAHQAAAAG/gcABQAAAJAAAAABAAAAQ09SRf4QAA7+CwAn/kcAeFY0EiMAAAAjAAAAIwAAADP+BwBchAQIGwAAAEYyAACMXwAAI/4HAAUAAABsAAAAAgAAAENPUkX+BAB/AwAAIAEAAP//AAAAhAQIGwAAAIxfAAAj/jwAgABA/gUAZmbIBED+BQBmZsgEQDL+CwB/D38D/ggAMv4HAHhWNBL+CABmZshCmv4Fmck//hAA+CAAALiIBAj+IAD4IAAAY9k7lA==
______________COREDUMP_END______________

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Chore (refactoring, style fixes, git/CI config, submodule management, no code logic changes)

How Has This Been Tested?

• Already covered by automatic testing.
• New test added: (add PR link here).
• Tested by hand on: ia32-qemu, riscv64-qemu, armv7a9-zynq7000-qemu

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing linter checks and tests passed.
• My changes generate no new compilation warnings for any of the targets.

Special treatment

• This PR needs additional PRs to work (list the PRs, preferably in merge-order).
• I will merge this PR by myself when appropriate.

[gemini-code-assist]

Hello @etiaro, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Hello team,

Gemini here to provide a summary of this pull request. This PR introduces the core dump functionality into the system. The primary goal is to implement a configurable mechanism to dump serialized memory, registers, and metadata of a process upon encountering a fault. This output is intended to be in a format suitable for post-mortem debugging using tools like GDB, specifically the ELF core dump format. The changes involve adding new core dump specific files, integrating the core dump generation into the exception handling paths for various architectures (ARMv7a, IA32, RISC-V64), and implementing a thread freezing mechanism to ensure a consistent state during the dump.

Highlights

• Core Dump Implementation: Adds the core logic for generating ELF-formatted core dumps (proc/coredump.c, proc/coredump.h). This includes handling ELF headers, program headers (for notes and memory segments), and note sections (thread status, floating-point registers, auxiliary vector).
• Architecture Support: Integrates core dump generation into the exception handling routines for ARMv7a, IA32, and RISC-V64. This involves adding architecture-specific definitions for core dump sizes, ELF machine types, and functions to extract general-purpose and auxiliary register sets (hal/*/exceptions.c, hal/*/arch/exceptions.h).
• Thread Freezing: Introduces a mechanism to freeze threads within a process (proc/threads.c, proc/threads.h). This is essential to prevent threads from changing state while the core dump is being generated, ensuring a consistent snapshot of the process state.
• Context Saving: Adds logic to save the user-space CPU context (userContext field in thread_t) upon entering the kernel via exceptions or syscalls (proc/threads.c, syscalls.c, vm/map.c). This saved context is then used when generating the core dump.
• Output Encoding: The core dump output is encoded using Base64 with simple Run-Length Encoding (RLE) compression and includes a CRC32 checksum (proc/coredump.c). This allows the dump to be output via serial console or log.
• Configurable Memory Dumping: The core dump can be configured to dump only the crashing thread's stack (default), all threads' stacks (COREDUMP_MEM_ALL_STACKS), or all readable/writable memory segments (COREDUMP_MEM_ALL).

Changelog

Click here to see the changelog

• hal/armv7a/arch/exceptions.h
  • +Added defines for core dump note sizes (SIZE_COREDUMP_GREGSET, SIZE_COREDUMP_THREADAUX, SIZE_COREDUMP_GENAUX).
  • +Added define for ARM ELF machine type (HAL_ELF_MACHINE).
• hal/armv7a/exceptions.c
  • +Included proc/elf.h.
  • +Extracted exception mnemonic lookup into a separate function hal_exceptionMnemonic (lines 53-60).
  • +Added hal_excToCpuCtx to get the CPU context from the exception context (lines 244-247).
  • +Added hal_coredumpGRegset to dump general-purpose registers for ARMv7a (lines 250-270).
  • +Added hal_coredumpThreadAux to dump thread auxiliary notes (VFP context) for ARMv7a (lines 271-286).
  • +Added hal_coredumpGeneralAux to dump general auxiliary notes (HWCAP) for ARMv7a (lines 295-314).
• hal/exceptions.h
  • +Added declaration for hal_exceptionMnemonic (line 31).
  • +Added declarations for core dump related HAL functions (hal_excToCpuCtx, hal_coredumpGRegset, hal_coredumpThreadAux, hal_coredumpGeneralAux) (lines 43-52).
• hal/ia32/arch/exceptions.h
  • +Added defines for core dump note sizes (SIZE_COREDUMP_GREGSET, SIZE_COREDUMP_THREADAUX, SIZE_COREDUMP_GENAUX).
  • +Added define for IA32 ELF machine type (HAL_ELF_MACHINE).
• hal/ia32/exceptions.c
  • +Included proc/elf.h.
  • +Extracted exception mnemonic lookup into a separate function hal_exceptionMnemonic (lines 116-126).
  • +Updated hal_exceptionsDumpContext to use hal_exceptionMnemonic (line 146).
  • +Added hal_excToCpuCtx to get the CPU context from the exception context (lines 314-317).
  • +Added hal_coredumpGRegset to dump general-purpose registers for IA32 (lines 320-342).
  • +Added hal_coredumpThreadAux to dump thread auxiliary notes (FPU context) for IA32 (lines 345-358).
  • +Added hal_coredumpGeneralAux (empty implementation) for IA32 (lines 361-363).
• hal/riscv64/arch/cpu.h
  • +Renamed pc field to ra in cpu_context_t struct (line 125).
• hal/riscv64/arch/exceptions.h
  • +Added defines for core dump note sizes (SIZE_COREDUMP_GREGSET, SIZE_COREDUMP_THREADAUX, SIZE_COREDUMP_GENAUX).
  • +Added define for RISC-V 64-bit ELF machine type (HAL_ELF_MACHINE).
• hal/riscv64/cpu.c
  • +Updated hal_cpuCreateContext to initialize ctx->ra instead of ctx->pc (line 141).
• hal/riscv64/exceptions.c
  • +Included proc/elf.h.
  • +Extracted exception mnemonic lookup into a separate function hal_exceptionMnemonic (lines 35-44).
  • +Updated hal_exceptionsDumpContext to use hal_exceptionMnemonic (line 54) and dump ctx->ra instead of ctx->pc (line 59).
  • +Updated hal_exceptionsPC to return ctx->sepc instead of ctx->pc (line 179).
  • +Added hal_excToCpuCtx to get the CPU context from the exception context (lines 257-260).
  • +Added hal_coredumpGRegset to dump general-purpose registers for RISC-V64 (lines 263-300).
  • +Added hal_coredumpThreadAux to dump thread auxiliary notes (FPU context) for RISC-V64 (lines 303-317).
  • +Added hal_coredumpGeneralAux (empty implementation) for RISC-V64 (lines 320-322).
• proc/Makefile
  • +Added coredump.o to the list of objects to be built (line 8).
• proc/coredump.c
  • +New file implementing the core dump generation logic.
  • +Includes ELF structure definitions, note types, and functions for writing the ELF header, program headers, and notes.
  • +Implements Base64 encoding with RLE compression and CRC32 checksumming for the output.
  • +Includes logic to dump memory segments based on configuration (COREDUMP_MEM_ALL, COREDUMP_MEM_ALL_STACKS).
  • +Uses thread freezing (proc_freeze, proc_unfreeze) to ensure a consistent state during the dump.
• proc/coredump.h
  • +New file declaring the coredump_dump function and the coredump_threadinfo_t structure used to pass thread information.
• proc/elf.h
  • +Added various ELF definitions relevant to core dumps, including ELFMAG, EI_* constants, ET_CORE, PT_NOTE, NT_PRSTATUS, NT_FPREGSET, NT_AUXV, NT_ARM_VFP (lines 35-108).
  • +Added Elf32_Nhdr and Elf64_Nhdr structures for note headers (lines 156-160, 216-220).
  • +Corrected e_hsize to e_ehsize in Elf32_Ehdr (line 121).
• proc/process.c
  • +Added calls to threads_saveUserContext and coredump_dump in the process_exception handler (lines 277, 279).
  • +Added a call to threads_saveUserContext in the process_illegal handler (line 301).
• proc/threads.c
  • +Included coredump.h.
  • +Added logic in _threads_schedule to handle threads in FREEZING or FROZEN states (lines 580-582, 599-602).
  • +Initialized the new freeze field to RUNNING in proc_threadCreate (line 779).
  • +Initialized the new userContext field to NULL in proc_threadCreate (line 792).
  • +Added logic in _proc_threadDequeue to set freeze state to FROZEN if the thread was FREEZING (lines 1070-1073).
  • +Implemented threads_saveUserContext to store the user CPU context (lines 2092-2105).
  • +Implemented _threads_userContext to retrieve the appropriate user context (lines 2108-2117).
  • +Implemented proc_freeze to freeze all threads within a process (lines 2120-2163).
  • +Implemented proc_unfreeze to unfreeze all threads within a process (lines 2166-2184).
  • +Implemented coredump_threadsInfo to collect information about process threads for the core dump (lines 2187-2239).
• proc/threads.h
  • +Included coredump.h.
  • +Added freeze state enum (RUNNING, FREEZER, FREEZING, FROZEN) (line 41).
  • +Added freeze field to thread_t struct (line 69).
  • +Added userContext field to thread_t struct (line 95).
  • +Added declarations for new thread/process functions (threads_saveUserContext, _threads_userContext, proc_freeze, proc_unfreeze, coredump_threadsInfo) (lines 226-238).
• syscalls.c
  • +Added call to threads_saveUserContext at the beginning of syscalls_dispatch to save the context upon entering the kernel (line 1894).
  • +Added call to threads_saveUserContext(NULL) before returning from syscalls_dispatch to clear the saved context (line 1906).
• vm/map.c
  • +Added call to threads_saveUserContext at the beginning of map_pageFault to save the context (line 740).
  • +Added call to coredump_dump in the PAGEFAULTSTOP block (line 747).
  • +Added call to coredump_dump if vm_mapForce fails (line 767).

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

A crash, a sudden halt,
The system finds a fault.
Registers are saved,
Memory is braved,
A core dump tells the tale.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request implements a configurable coredump feature, which is a significant addition for post-mortem debugging. The implementation includes architecture-specific register and auxiliary data dumping for ARM, IA32, and RISC-V, along with ELF-compatible output formatting, RLE and Base64 encoding for the dump data. The changes also introduce a process freezing mechanism to ensure coredump consistency.

The overall structure and approach seem sound. The code is well-organized with clear separation between generic coredump logic and architecture-specific helpers. The use of conditional compilation for output methods (serial/log) and memory dump scope (all memory, all stacks, or just current stack) provides good flexibility.

I've identified a couple of areas that need attention, detailed in the comments below. Additionally, the PR author has noted that documentation changes are required but not yet updated; this should be addressed before merging.

Summary of Findings

• Hardcoded AT_HWCAP Value with TODO: In hal/armv7a/exceptions.c, the AT_HWCAP value for the coredump's general auxiliary vector is hardcoded as 0x8b8d7 with a TODO. This should be clarified, defined as a constant, or dynamically determined to improve clarity and maintainability.
• Potential Typo in Coredump Function Call: In vm/map.c, within the PAGEFAULTSTOP conditional block, proc_coredump(n, ctx) is called. The coredump function introduced in this PR is named coredump_dump. This might be a typo that could lead to errors if PAGEFAULTSTOP is enabled.
• Documentation Update Required: The pull request description indicates that documentation changes are required for this new feature, but the corresponding checkbox for updating documentation is unchecked. This should be addressed.

Merge Readiness

This pull request introduces a valuable coredump feature. The implementation is largely solid. However, there are a couple of issues identified (a TODO for a hardcoded value and a potential function call typo) that should be addressed before merging. Additionally, the pending documentation update should be completed.

As an AI, I am not authorized to approve pull requests. Please ensure these changes are reviewed and approved by other maintainers before merging.

[github-actions]

Unit Test Results

8 039 tests  +9   7 515 ✅ +9   40m 39s ⏱️ +3s
  479 suites ±0     524 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit afe2296. ± Comparison against base commit 7db978d.

♻️ This comment has been updated with latest results.

Fixed reported issues

[agkaminski]

Partial review, will tackle the rest later

[etiaro]

Long tests with multiple processes and threads unveiled a few problems:

• spawning new threads at the same time when the process is being killed sometimes led to remaining dangling, alive threads of presumably killed process (even on master),
• spawning new threads at the same time when the process is being freezed sometimes led to issues with not being frozen and added to ready queue twice after unfreeze,
• occasionally deadlocks happened if frozen threads were enqueued for obtaining kernel locks.

These problems are solved in last two commits.

[nalajcie]

• spawning new threads at the same time when the process is being killed sometimes led to remaining dangling, alive threads of presumably killed process (even on master),

How easy is to trigger the error on master branch? If it's a common occurrence - maybe we want to fix it ASAP on master (and maybe backport to release branch?)

[etiaro]

How easy is to trigger the error on master branch?

It's hard to trigger, I was able to do that only by spawning massive amounts of threads of which some were instantly failing, and even in this setup it doesn't always happen.

[agkaminski]

Name collision with libc name, might cause problems in the future

[agkaminski]

How does this work? Won't this cause pr_fpvalid offset to be always invalid?

[etiaro]

It's equivalent to declaring two structures:
elf_prstatus_header,
elf_prstatus_footer,
and dumping in order header, hal_coredumpGRegset(), footer.

I just combined it in single struct and used pr_reg as a marker to put real arch-dependent pr_reg here in actual dump. The intention was to achieve: &prstatus->pr_reg == &prstatus->pr_fpvalid.
After quick read, zero-length arrays is legacy GCC extension and it's rather discouraged, not really intended to use this way, just legacy way for C99's [] array (variable length at the end of struct).

Alternatively, whole struct could be moved to HAL (which will affect buffer size and require filling its entries inside HAL function). This adds some redundancy inside HAL.
Current solution isn't perfect either - it turned out that GDB is not working with SPARC-linux coredumps and require a hack inside its HAL to modify this struct into (luckily larger) Solaris's version of elf_prstatus.

[etiaro]

tl;dr
It works only on GCC and it's hacky. I wonder if I should:

1. move whole struct into HAL adding redundant code in all architectures except SPARC
2. split it into header/footer and keep hacky solution in SPARC's HAL (next PR)
3. forget about this struct and only remember its size and pr_pid offset, as that's the only used (most probably probably the only useful) field for now

[agkaminski]

This is somewhat iffy in scope of C standard and MISRA - yes, we provide aligned buffer, but in pedantic view this is still unwanted method.

It is best to pass the desired type all the way down, in this case it depends on the architecture - perhaps per arch typedef?

On the other hand memcpy can be used instead, most likely less efficient approach.

Not really a merge blocker, I'll leave this for @Darchiv to decide

[etiaro]

Regarding the previous comment, would it be any better to define struct elf_prstatus including pr_regs in HAL which would just be filled here, and make a cast to this new type in caller?

[agkaminski]

Why do this stuff with locks type etc? We can allow thread freeze only and only if either:

• threads is not in supervisor mode,
• thread is sleeping.

This assumption would make matter way simpler and we wouldn't need to classify locks

[etiaro]

If we freeze the thread that is sleeping, and

1. it can be waiting for a lock - then will sooner or later get that lock, and if it's still freezed, then that lock can cause deadlock, and will block other waiting threads
2. it can't be waiting for any lock - then we have to wait until that thread gets the locks its waiting for - but it can get deadlocked if that lock is "user.mutex" lock.

Another solution would be to only wait to free acquired locks, dequeue freezed threads and put them in queue on unfreeze, but the queue mechanism is used not only for locks and I didn't have an idea how to do that reasonably well that wouldn't be a harsh to maintain. Also, this solution still requires distinguishing "user.mutex", as it can never get freed.

[agkaminski]

You're freezing all threads, so there's no risk of deadlock regarding user mutexes.

But it's a valid point, something has to be done regarding queues. Perhaps we could use interruption mechanism (from signals) to force threads to vacate interruptible queues and freeze only in user space or on return to user space?

[agkaminski]

Another solution is to skip frozen threads on dequeue, but yeah, it might get ugly quick in edge cases (e.g. how to proceed when we only have frozen thread on queue and it becomes unfrozen?).

Side note - IMHO freezing is not really needed for coredump mechanism - we can simply kill process on fault and collect thread stack as they die

[etiaro]

You're freezing all threads, so there's no risk of deadlock regarding user mutexes.

The exception thread can be holding this lock, if another thread is waiting to acquire it and freezer is waiting for this acquisition then that's a deadlock

Side note - IMHO freezing is not really needed for coredump mechanism - we can simply kill process on fault and collect thread stack as they die

I'm working on dumping running processes on request. That's where freezing will really be required.
Also, the thread that gets a SIGSEGV can handle that signal and keep working (probably not recommended, but possible)

Perhaps we could use interruption mechanism (from signals) to force threads to vacate interruptible queues

I missed that proc_mutexLock is interruptible. That indeed solves the problem. I'll remove the attribute and just interrupt them out of the queues.

[etiaro]

Rewriting to userspace coredump-server and ELF core file creation.
Bugfixes moved to separate PRs #671 #672