Adjust frame IP in backtraces relative to image base for SGX target

[mzohreva]

This is followup to rust-lang/backtrace-rs#566.

The backtraces printed by panic! or generated by std::backtrace::Backtrace in SGX target are not usable. The frame addresses need to be relative to image base address so they can be used for symbol resolution. Here's an example panic backtrace generated before this change:

$ cargo r --target x86_64-fortanix-unknown-sgx
...
stack backtrace:
   0:     0x7f8fe401d3a5 - <unknown>
   1:     0x7f8fe4034780 - <unknown>
   2:     0x7f8fe401c5a3 - <unknown>
   3:     0x7f8fe401d1f5 - <unknown>
   4:     0x7f8fe401e6f6 - <unknown>

Here's the same panic after this change:

$ cargo +stage1 r --target x86_64-fortanix-unknown-sgx
stack backtrace:
   0:            0x198bf - <unknown>
   1:            0x3d181 - <unknown>
   2:            0x26164 - <unknown>
   3:            0x19705 - <unknown>
   4:            0x1ef36 - <unknown>

cc @jethrogb and @workingjubilee

[rustbot]

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

[mzohreva]

I don't quite understand why this is a problem:

Error: tidy error: /checkout/library/std/src/backtrace.rs:330: platform-specific cfg: cfg(all(target_vendor = "fortanix", target_env = "sgx"))

[workingjubilee]

@mzohreva We try to keep platform-specific configuration code confined to the sys module, because otherwise we have 5 different abstraction layers bleeding into each other all the time.

[mzohreva]

@workingjubilee so does that mean that we'd need to introduce a new API in sys that is only doing something in SGX and a no-op for all others? If so, can you point me to some existing example please?

[workingjubilee]

so does that mean that we'd need to introduce a new API in sys that is only doing something in SGX and a no-op for all others?

Yes, basically. This is part of why I asked questions like "is there ANY other way to do this?" during review.

[workingjubilee]

There is a file in tidy which contains a hardcoded list of exceptions, but it should generally never be added to, as doing so is... an unhappy last-resort, as any such exception applies to all ~450 lines of code in that file, and it means someone has to clean it up anyways in good time.

Other PRs which did this "refactor code into the sys module" (as part of a longer-term strategy of potentially extracting sys into its own crate) motion are

• move pal cfgs in f32 and f64 to sys #108485
• move personality to sys #108796
• Move RawOsError defination to sys #115493
• Move __thread_local_inner to sys #108927

[Mark-Simulacrum]

Is this API implying that the backtrace generation is going to do more than math with the pointer value here? i.e., read/write from it? It seems odd to me to call from_exposed_addr here, vs. invalid_mut, which feels like it more appropriately describes the opaque nature of this pointer and theoretically hints that no one should read from it.

[mzohreva]

No, backtrace is only using this for adjusting frame IPs. Having read the docs in https://doc.rust-lang.org/nightly/std/ptr/index.html I was under the impression that doing pointer arithmetic also requires strict provenance.

[mzohreva]

See https://github.com/rust-lang/backtrace-rs/blob/master/src/backtrace/libunwind.rs#L38-L55

[Mark-Simulacrum]

I would not expect arithmetic with non-unsafe functions to ever require provenance. It looks like that code is operating on usize even, so it's not even doing pointer arithmetic I think.

[Mark-Simulacrum]

@bors r+

[bors]

📌 Commit b576dd2 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

[bors]

⌛ Testing commit b576dd2 with merge d052f6f...

[bors]

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing d052f6f to master...

[rust-timer]

Finished benchmarking commit (d052f6f): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	4.8%	[4.8%, 4.8%]	1
Improvements ✅ (primary)	-2.7%	[-2.7%, -2.7%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.7%	[-2.7%, -2.7%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 676.828s -> 675.196s (-0.24%)
Artifact size: 313.81 MiB -> 313.81 MiB (0.00%)