Skip to content

Commit

Permalink
Revert "Remove backtrace"
Browse files Browse the repository at this point in the history
This reverts commit 6ce8e01.
  • Loading branch information
leighmcculloch committed Sep 17, 2024
1 parent 6ce8e01 commit 4558728
Show file tree
Hide file tree
Showing 6 changed files with 139 additions and 8 deletions.
61 changes: 61 additions & 0 deletions Cargo.lock

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

8 changes: 8 additions & 0 deletions cackle.toml
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,14 @@ allow_apis = [
[pkg.rand]
allow_unsafe = true

[pkg.backtrace]
allow_apis = [
"env",
"fs",
"thread",
]
allow_unsafe = true

[pkg.rand_chacha]
allow_apis = [
"rand",
Expand Down
6 changes: 3 additions & 3 deletions soroban-env-common/src/vmcaller_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,9 @@ macro_rules! vmcaller_none_function_helper {
=>
{
// We call `augment_err_result` here to give the Env a chance to attach
// context to any error that was generated by code that didn't have an
// Env on hand when creating the error. This will at least localize the
// error to a given Env call.
// context (eg. a backtrace) to any error that was generated by code
// that didn't have an Env on hand when creating the error. This will at
// least localize the error to a given Env call.
fn $fn_id(&self, $($arg:$type),*) -> Result<$ret, Self::Error> {
// Check the ledger protocol version against the function-specified
// boundaries, this prevents calling a host function using the host
Expand Down
4 changes: 3 additions & 1 deletion soroban-env-host/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ rand_chacha = "0.3.1"
num-traits = "0.2.17"
num-integer = "0.1.45"
num-derive = "0.4.1"
backtrace = { version = "0.3.69", optional = true }
k256 = {version = "0.13.1", default-features = false, features = ["ecdsa", "arithmetic"]}
p256 = {version = "0.13.2", default-features = false, features = ["ecdsa", "arithmetic"]}
ecdsa = {version = "0.16.7", default-features = false}
Expand Down Expand Up @@ -85,6 +86,7 @@ wasmprinter = "0.2.72"
expect-test = "1.4.1"
more-asserts = "0.3.1"
pretty_assertions = "1.4.0"
backtrace = "0.3.69"
serde_json = "1.0.108"
arbitrary = "1.3.2"
lstsq = "0.5.0"
Expand All @@ -103,7 +105,7 @@ default-features = false
features = ["arbitrary"]

[features]
testutils = ["soroban-env-common/testutils", "recording_mode"]
testutils = ["soroban-env-common/testutils", "recording_mode", "dep:backtrace"]
next = ["soroban-env-common/next", "stellar-xdr/next"]
tracy = ["dep:tracy-client", "soroban-env-common/tracy"]
recording_mode = []
Expand Down
2 changes: 1 addition & 1 deletion soroban-env-host/src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -820,7 +820,7 @@ impl EnvBase for Host {
// will display a relatively ugly message like "thread panicked at Box<dyn
// Any>" to stderr, when it is much more useful to the user if we have it
// print the result of HostError::Debug, with its glorious Error,
// site-of-origin debug log.
// site-of-origin backtrace and debug log.
//
// To get it to do that, we have to call `panic!()`, not `panic_any`.
// Personally I think this is a glaring weakness of `panic_any` but we are
Expand Down
66 changes: 63 additions & 3 deletions soroban-env-host/src/host/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use crate::{
ConversionError, EnvBase, Error, Host, TryFromVal, U32Val, Val,
};

#[cfg(any(test, feature = "testutils"))]
use backtrace::{Backtrace, BacktraceFrame};
use core::fmt::Debug;
use std::{
cell::{Ref, RefCell, RefMut},
Expand All @@ -16,6 +18,8 @@ use super::metered_clone::MeteredClone;
#[derive(Clone)]
pub(crate) struct DebugInfo {
events: Events,
#[cfg(any(test, feature = "testutils"))]
backtrace: Backtrace,
}

#[derive(Clone)]
Expand Down Expand Up @@ -62,13 +66,68 @@ impl DebugInfo {
}
Ok(())
}

#[cfg(not(any(test, feature = "testutils")))]
fn write_backtrace(&self, _f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
Ok(())
}

#[cfg(any(test, feature = "testutils"))]
fn write_backtrace(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
// We do a little trimming here, skipping the first two frames (which
// are always into, from, and one or more Host::err_foo calls) and all
// the frames _after_ the short-backtrace marker that rust compiles-in.

fn frame_name_matches(frame: &BacktraceFrame, pat: &str) -> bool {
for sym in frame.symbols() {
match sym.name() {
Some(sn) if format!("{:}", sn).contains(pat) => {
return true;
}
_ => (),
}
}
false
}

fn frame_is_short_backtrace_start(frame: &BacktraceFrame) -> bool {
frame_name_matches(frame, "__rust_begin_short_backtrace")
}

fn frame_is_initial_error_plumbing(frame: &BacktraceFrame) -> bool {
frame_name_matches(frame, "::from")
|| frame_name_matches(frame, "::into")
|| frame_name_matches(frame, "host::err")
|| frame_name_matches(frame, "Host::err")
|| frame_name_matches(frame, "Host>::err")
|| frame_name_matches(frame, "::augment_err_result")
|| frame_name_matches(frame, "::with_shadow_mode")
|| frame_name_matches(frame, "::with_debug_mode")
|| frame_name_matches(frame, "::maybe_get_debug_info")
|| frame_name_matches(frame, "::map_err")
}
let mut bt = self.backtrace.clone();
bt.resolve();
let frames: Vec<BacktraceFrame> = bt
.frames()
.iter()
.skip_while(|f| frame_is_initial_error_plumbing(f))
.take_while(|f| !frame_is_short_backtrace_start(f))
.cloned()
.collect();
let bt: Backtrace = frames.into();
writeln!(f)?;
writeln!(f, "Backtrace (newest first):")?;
writeln!(f, "{:?}", bt)
}
}

impl Debug for HostError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
writeln!(f, "HostError: {:?}", self.error)?;
if let Some(info) = &self.info {
info.write_events(f)
info.write_events(f)?;
info.write_backtrace(f)
} else {
writeln!(f, "DebugInfo not available")
}
Expand Down Expand Up @@ -202,7 +261,7 @@ impl Host {
/// [Error], and when running in [DiagnosticMode::Debug] additionally
/// records a diagnostic event with the provided `msg` and `args` and then
/// enriches the returned [Error] with [DebugInfo] in the form of a
/// snapshot of the [Events] buffer.
/// [Backtrace] and snapshot of the [Events] buffer.
pub(crate) fn error(&self, error: Error, msg: &str, args: &[Val]) -> HostError {
let mut he = HostError::from(error);
self.with_debug_mode(|| {
Expand Down Expand Up @@ -237,7 +296,8 @@ impl Host {
self.with_debug_mode(|| {
if let Ok(events_ref) = self.0.events.try_borrow() {
let events = events_ref.externalize(self)?;
res = Some(Box::new(DebugInfo { events }));
let backtrace = Backtrace::new_unresolved();
res = Some(Box::new(DebugInfo { backtrace, events }));
}
Ok(())
});
Expand Down

0 comments on commit 4558728

Please sign in to comment.