Improve crash backtraces for non-reproducible faults#2880
Open
DL6ER wants to merge 1 commit into
Open
Conversation
…acktrace fallback. Add mapping safety checks and log the unwind source so field crash reports show whether frames came from signal context or fallback. Signed-off-by: Dominik <dl6er@dl6er.de>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves crash diagnostics in src/signals.c by preferring backtrace collection from the interrupted signal context (frame-pointer walk) and falling back to _Unwind_Backtrace when that fails. It also adds mapping-safety checks for the signal-context unwinder and logs which unwinding source was used so crash reports are easier to interpret.
Changes:
- Add signal-context-based frame collection (x86_64/aarch64) with
_Unwind_Backtracefallback and record the unwind source in logs. - Add
/proc/self/maps-based readability/executability checks to reduce unsafe memory dereferences during frame-pointer walking. - Refine frame resolution result categories (
FRAME_UNRESOLVABLEvsFRAME_UNRESOLVED) and adjust backtrace logging accordingly.
Comments suppressed due to low confidence (1)
src/signals.c:317
- The
log_frame()doc comment describing return values is now out of sync with theframe_resultenum: it mentions onlyFRAME_UNRESOLVED/FRAME_SKIPPED, but the function can also returnFRAME_UNRESOLVABLE. Please update the comment to reflect the new enum semantics so future changes don’t mis-handle the result codes.
// Log one backtrace frame as a single line.
// Resolved: " #N func_name src/file.c:line"
// Unresolved: " #N 0xADDRESS (reason)"
// Returns FRAME_RESOLVED when addr2line produced a result, FRAME_UNRESOLVED
// when addr2line was tried but failed, FRAME_SKIPPED when it was not attempted.
static enum frame_result log_frame(const int idx, const void *addr, const void *rel_addr)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+132
to
+161
| // True when [addr, addr+bytes) is inside a readable mapping in /proc/self/maps. | ||
| static bool is_readable_range(const uintptr_t addr, const size_t bytes) | ||
| { | ||
| if(bytes == 0u) | ||
| return false; | ||
|
|
||
| FILE *maps = fopen("/proc/self/maps", "r"); | ||
| if(maps == NULL) | ||
| return false; | ||
|
|
||
| bool readable = false; | ||
| char line[512]; | ||
| while(fgets(line, sizeof(line), maps) != NULL) | ||
| { | ||
| uintptr_t start = 0, end = 0; | ||
| char perms[8] = { 0 }; | ||
| const int n = sscanf(line, "%" SCNxPTR "-%" SCNxPTR " %7s %*s %*s %*s", | ||
| &start, &end, perms); | ||
| if(n != 3 || perms[0] != 'r') | ||
| continue; | ||
|
|
||
| if(addr >= start && addr < end && (size_t)(end - addr) >= bytes) | ||
| { | ||
| readable = true; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| fclose(maps); | ||
| return readable; |
Comment on lines
+164
to
+191
| // True when addr points into an executable mapping. | ||
| static bool is_executable_address(const uintptr_t addr) | ||
| { | ||
| FILE *maps = fopen("/proc/self/maps", "r"); | ||
| if(maps == NULL) | ||
| return false; | ||
|
|
||
| bool executable = false; | ||
| char line[512]; | ||
| while(fgets(line, sizeof(line), maps) != NULL) | ||
| { | ||
| uintptr_t start = 0, end = 0; | ||
| char perms[8] = { 0 }; | ||
| const int n = sscanf(line, "%" SCNxPTR "-%" SCNxPTR " %7s %*s %*s %*s", | ||
| &start, &end, perms); | ||
| if(n != 3 || perms[2] != 'x') | ||
| continue; | ||
|
|
||
| if(addr >= start && addr < end) | ||
| { | ||
| executable = true; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| fclose(maps); | ||
| return executable; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this implement/fix?
Prefer signal-context frame walking in crash handlers, with
_Unwind_Backtracefallback. Add mapping safety checks and log the unwind source so field crash reports show whether frames came from signal context or fallback.Related issue or feature (if applicable): N/A
Pull request in docs with documentation (if applicable): N/A
By submitting this pull request, I confirm the following:
git rebase)Checklist:
developmentbranch.