Skip to content

Commit

Permalink
#1315 Improve error reporting stack traces, tested on macOS
Browse files Browse the repository at this point in the history
increases binary size considerably, but at the moment, this has priority
  • Loading branch information
helgoboss committed Nov 6, 2024
1 parent 2c09a88 commit 132d85a
Show file tree
Hide file tree
Showing 8 changed files with 151 additions and 40 deletions.
14 changes: 0 additions & 14 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ jobs:
extension-file-name: "not-yet-supported"
target: x86_64-unknown-linux-gnu
use-cross: false
strip-cmd: strip
profile: release
features: ""
# - artifact: linux-aarch64
Expand All @@ -66,7 +65,6 @@ jobs:
# extension-file-name: ""
# target: aarch64-unknown-linux-gnu
# use-cross: true
# strip-cmd: aarch64-linux-gnu-strip
# profile: release
# features: ""
# - artifact: linux-armv7
Expand All @@ -76,7 +74,6 @@ jobs:
# extension-file-name: ""
# target: armv7-unknown-linux-gnueabihf
# use-cross: true
# strip-cmd: arm-linux-gnueabihf-strip
# profile: release
# features: ""
env:
Expand Down Expand Up @@ -138,17 +135,6 @@ jobs:
command: build
args: --features "${{ matrix.features }}" --profile ${{ matrix.profile }} --target ${{ matrix.target }}
use-cross: ${{ matrix.use-cross }}
# Strip debug symbols (Linux and macOS)
# TODO-medium We could replace this with Cargo's recent built-in strip function
- name: Strip debug symbols from Linux binary
if: startsWith(matrix.os, 'ubuntu-')
run: |
cp target/${{ matrix.target }}/${{ matrix.profile }}/${{ matrix.lib-file-name }} target/${{ matrix.target }}/${{ matrix.profile }}/libhelgobox-debug.so
${{ matrix.strip-cmd }} target/${{ matrix.target }}/${{ matrix.profile }}/${{ matrix.lib-file-name }}
- name: Strip debug symbols from macOS binary
if: matrix.os == 'macos-latest'
run: |
strip -u -r target/${{ matrix.target }}/${{ matrix.profile }}/${{ matrix.lib-file-name }}
# Upload
- name: Upload plug-in and extension to artifact
uses: actions/upload-artifact@v4
Expand Down
109 changes: 109 additions & 0 deletions CONTRIBUTING.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,115 @@ To be used like this:
. Copy the error report to the clipboard.
. Execute the action.

=== Differences between debug levels

==== macOS

Insights:

* The size difference between `debug = 1` and `debug = 2` is almost nothing (both 58 MB), and there's nothing to gain from `debug = 2` in terms of stack traces.
* The size difference between `debug = 0` and `debug = 1` is around 5 MB (53 MB vs. 58 MB), and `debug = 1` only makes a difference if the source files exist (showing line numbers), and only for panics.
* Hard crash stack traces are completely independent of the `debug` value.
They are always helpful except when stripping the symbols.
* `strip = symbols` leads to the smallest binaries (38 MB) but also to completely useless stack traces, both in soft and hard crashes.
However, `split-debuginfo = "packed" seems to fix this at least for hard crashes, at a similar-sized binary (not for panics though) ... even if the DSYM directories are not on disk. What also fixes this for hard crashes is stripping via `strip -u -r`.
We shouldn't do that anymore!
* `strip = debuginfo` leads to an okay size reduction (53 MB) but removes line numbers even if source files exist.
However, `split-debuginfo = "packed"` solves this by creating dSYM directories, as long as they are there.

Takeaway:

* We should build with `debug = 2`
** While `debug = 1` is actually enough for most purposes, it can't hurt building with `debug = 2` since we strip debuginfo anyway.
So there's no size difference for the final binary.
That way we have more debuginfo on the server whenever we need it.
* `strip = debuginfo` is the max we can strip away if we want panics to contain something useful
* `strip = symbols` is only okay if we are fine with bogus stack traces in panics.
In that case, we must use `split-debuginfo = "packed"` to get at least detailed stack traces in case of hard crashes.
* We should use `split-debuginfo = "packed"` in all cases.
* Would be good to find a way to leverage symbols for panic stack traces, but I think there is none.

===== "debug = 0"

.Panic (sources don't matter)
----
7: 0x120531450 - helgobox::infrastructure::plugin::sandbox::execute::h9608b1370cb08106
----

.Hard crash (sources don't matter)
----
4 helgobox-arm64.vst.dylib 0x120527df4 _$LT$helgobox..domain..targets..track_volume_target..TrackVolumeTarget$u20$as$u20$helgoboss_learn..mode..target..Target$GT$::current_value::hff7df2fe68cec5d5 + 20
----

===== "debug = 1"

.Panic if sources exist
----
7: 0x13071ffdc - helgobox::infrastructure::plugin::sandbox::execute::hb564445c2d9211ee
at /Users/helgoboss/Documents/projects/dev/realearn/main/src/infrastructure/plugin/sandbox.rs:3:5
----

.Panic if sources are gone
----
7: 0x140f1ffdc - helgobox::infrastructure::plugin::sandbox::execute::hb564445c2d9211ee
----

.Hard crash (sources don't matter)
----
6 helgobox-arm64.vst.dylib 0x1302d4a64 _$LT$helgobox..domain..mapping..CompoundMappingTarget$u20$as$u20$helgoboss_learn..mode..target..Target$GT$::current_value::hadfcb1be993900b3 + 20 (mapping.rs:2498) [inlined]
----

===== "debug = 2"

.Panic if sources exist
----
7: 0x12160dafc - helgobox::infrastructure::plugin::sandbox::execute::h18fb689d4112e2d3
at /Users/helgoboss/Documents/projects/dev/realearn/main/src/infrastructure/plugin/sandbox.rs:3:5
----

.Panic if sources are gone
----
7: 0x153f81afc - helgobox::infrastructure::plugin::sandbox::execute::h18fb689d4112e2d3
----

.Hard crash (sources don't matter)
----
6 helgobox-arm64.vst.dylib 0x1212d8148 _$LT$helgobox..domain..mapping..CompoundMappingTarget$u20$as$u20$helgoboss_learn..mode..target..Target$GT$::current_value::h15041e3226fa455d + 20 (mapping.rs:2498) [inlined]
----

===== "debug = 2; strip = debuginfo"

.Panic (sources don't matter)
----
7: 0x138616570 - helgobox::infrastructure::plugin::sandbox::execute::hd0d406afe4d62df9
----

.Hard crash (sources don't matter)
----
4 helgobox-arm64.vst.dylib 0x13885aab4 _$LT$helgobox..domain..targets..track_volume_target..TrackVolumeTarget$u20$as$u20$helgoboss_learn..mode..target..Target$GT$::current_value::h35c07d80eb0a312b + 20
----

===== "debug = 2; strip = symbols"

.Soft crash (sources don't matter)
----
0: 0x14bdef0ec - _NSEEL_HOSTSTUB_EnterMutex
1: 0x14bd56f08 - _NSEEL_HOSTSTUB_EnterMutex
2: 0x14bfbe4e4 - _cpp_to_rust_ProjectStateContext_SetTempFlag
3: 0x14bfbddcc - _cpp_to_rust_ProjectStateContext_SetTempFlag
4: 0x14bfbca08 - _cpp_to_rust_ProjectStateContext_SetTempFlag
5: 0x14bfbdabc - _cpp_to_rust_ProjectStateContext_SetTempFlag
6: 0x14c075710 - _cpp_to_rust_ProjectStateContext_SetTempFlag
7: 0x14af50aa0 - _ReaperPluginEntry
8: 0x14bd58fcc - _NSEEL_HOSTSTUB_EnterMutex
9: 0x14bd82844 - _NSEEL_HOSTSTUB_EnterMutex
----

.Hard crash (sources don't matter)
----
7 helgobox-arm64.vst.dylib 0x14b34b5e0 0x14a65c000 + 13563360
----

== Documentation

All documentation is written in AsciiDoc.
Expand Down
49 changes: 32 additions & 17 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,11 @@ cached = "0.53.1"
imageproc = "0.25.0"

[profile.release]
# This is important for having line numbers in bug reports.
# There's a long reasoning about those values in CONTRIBUTING.adoc
debug = 2
# Switching to "symbols" will reduce size even more but leads to useless stack traces when panicking
strip = "debuginfo"
split-debuginfo = "packed"

[profile.dev-llvm-out-of-memory-fix]
inherits = "dev"
Expand Down
2 changes: 1 addition & 1 deletion main/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ once_cell.workspace = true
derivative.workspace = true
chrono.workspace = true
smallvec = "1.7.0"
backtrace = "0.3"
backtrace = "0.3.74"
regex.workspace = true
enum-map.workspace = true
# For generating controller file names from controller names
Expand Down
7 changes: 3 additions & 4 deletions main/src/infrastructure/plugin/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,12 @@ pub const ACTION_DEFS: &[ActionDef] = &[
requires_instance: true,
..DEFAULT_DEF
},
#[cfg(debug_assertions)]
ActionDef {
section: ActionSection::General,
command_name: "HB_SANDBOX",
action_name: "Execute sandbox",
command_name: "HB_PANIC",
action_name: "Simulate error",
developer: true,
op: crate::infrastructure::plugin::sandbox::execute,
op: crate::infrastructure::plugin::sandbox::simulate_error,
..DEFAULT_DEF
},
];
Expand Down
1 change: 0 additions & 1 deletion main/src/infrastructure/plugin/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ pub use actions::*;
mod dynamic_toolbar;
mod hidden_helper_panel;
pub mod persistent_toolbar;
#[cfg(debug_assertions)]
mod sandbox;
mod unit_shell;
pub use unit_shell::*;
Expand Down
4 changes: 2 additions & 2 deletions main/src/infrastructure/plugin/sandbox.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! Some experimental code for trying things out.
pub fn execute() {
panic!("Panic test");
pub fn simulate_error() {
panic!("Simulated error");
}

0 comments on commit 132d85a

Please sign in to comment.