rust-lang / backtrace-rs

Backtraces in Rust
https://docs.rs/backtrace
Other
537 stars 246 forks source link

Backtrace frame ip's not relativized in SGX #471

Open phlip9 opened 2 years ago

phlip9 commented 2 years ago

Hi! I've noticed the current backtrace logic in std isn't working properly for the fortanix sgx target. More specifically, the instruction pointer addresses aren't getting relativized from absolute addresses (e.g. 0x7f74ec1870a5) to relative offsets (e.g., 0x1b09d9). The lack of symbolizing appears to be intentional, however.

Example

# bad - absolute addresses

thread 'main' panicked at 'foo', src/main.rs:6:5
stack backtrace:
0: 0x7f74ec4a1072 - <unknown>
1: 0x7f74ec4bdb1e - <unknown>
2: 0x7f74ec49dfa3 - <unknown>
3: 0x7f74ec4a2927 - <unknown>

# good - addresses relative to image base 0x7f74eb000000

thread 'main' panicked at 'foo', src/main.rs:6:5
stack backtrace:
0: 0x14a1072 - <unknown>
1: 0x14bdb1e - <unknown>
2: 0x149dfa3 - <unknown>
3: 0x14a2927 - <unknown>

In the second case, users can easily consume backtraces with an external symbolizer like addr2line.

Problem

It looks like this code block isn't getting included due to the extra feature = "std" requirement:

// https://github.com/rust-lang/backtrace-rs/blob/master/src/print.rs#L221

// To reduce TCB size in Sgx enclave, we do not want to implement symbol
// resolution functionality.  Rather, we can print the offset of the
// address here, which could be later mapped to correct function.
#[cfg(all(feature = "std", target_env = "sgx", target_vendor = "fortanix"))]
{
    let image_base = std::os::fortanix_sgx::mem::image_base();
    frame_ip = usize::wrapping_sub(frame_ip as usize, image_base as _) as _;
}

I'm guessing the feature = "std" requirement was included because std::os::fortanix_sgx::mem::image_base lives in std.

I'm not too familiar with the whole situation around rust std's dependency on backtrace-rs and how that restricts std imports. From my cursory look, std actually inlines the backtrace-rs crate here std/src/lib.rs:581?

What would be a good resolution here? From my POV, we have three options:

  1. Only allow backtrace-rs from std for this target and just remove the feature requirement.
  2. Some #[cfg(..)] magic to call std::os::fortanix_sgx::mem::image_base() even when backtrace-rs is included by std.
  3. duplicate and inline std::os::fortanix_sgx::mem::image_base() into backtrace-rs.

Thoughts?

workingjubilee commented 1 year ago

I think https://github.com/rust-lang/backtrace-rs/pull/268 is related?