Skip to content

Commit 5f3429b

Browse files
authoredAug 25, 2022
fix(miri): Resolve Miri's concerns around unsafe code (#197)
1 parent 12279f8 commit 5f3429b

File tree

9 files changed

+388
-153
lines changed

9 files changed

+388
-153
lines changed
 

‎.github/workflows/ci.yml

+18
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,24 @@ jobs:
4545
- name: Run tests
4646
run: cargo test --all --verbose --features fancy
4747

48+
miri:
49+
name: Miri
50+
runs-on: ubuntu-latest
51+
52+
steps:
53+
- uses: actions/checkout@v1
54+
- name: Install Rust
55+
uses: actions-rs/toolchain@v1
56+
with:
57+
profile: minimal
58+
toolchain: nightly
59+
components: miri,rust-src
60+
override: true
61+
- name: Run tests with miri
62+
env:
63+
MIRIFLAGS: -Zmiri-disable-isolation -Zmiri-strict-provenance
64+
run: cargo miri test --all --verbose --features fancy
65+
4866
minimal_versions:
4967
name: Minimal versions check
5068
runs-on: ${{ matrix.os }}

‎src/eyreish/context.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use super::error::ContextError;
1+
use super::error::{ContextError, ErrorImpl};
22
use super::{Report, WrapErr};
33
use core::fmt::{self, Debug, Display, Write};
44

@@ -116,7 +116,7 @@ where
116116
D: Display,
117117
{
118118
fn source(&self) -> Option<&(dyn StdError + 'static)> {
119-
Some(self.error.inner.error())
119+
unsafe { Some(ErrorImpl::error(self.error.inner.by_ref())) }
120120
}
121121
}
122122

@@ -159,23 +159,23 @@ where
159159
D: Display,
160160
{
161161
fn code<'a>(&'a self) -> Option<Box<dyn Display + 'a>> {
162-
self.error.inner.diagnostic().code()
162+
unsafe { ErrorImpl::diagnostic(self.error.inner.by_ref()).code() }
163163
}
164164

165165
fn severity(&self) -> Option<crate::Severity> {
166-
self.error.inner.diagnostic().severity()
166+
unsafe { ErrorImpl::diagnostic(self.error.inner.by_ref()).severity() }
167167
}
168168

169169
fn help<'a>(&'a self) -> Option<Box<dyn Display + 'a>> {
170-
self.error.inner.diagnostic().help()
170+
unsafe { ErrorImpl::diagnostic(self.error.inner.by_ref()).help() }
171171
}
172172

173173
fn url<'a>(&'a self) -> Option<Box<dyn Display + 'a>> {
174-
self.error.inner.diagnostic().url()
174+
unsafe { ErrorImpl::diagnostic(self.error.inner.by_ref()).url() }
175175
}
176176

177177
fn labels<'a>(&'a self) -> Option<Box<dyn Iterator<Item = LabeledSpan> + 'a>> {
178-
self.error.inner.diagnostic().labels()
178+
unsafe { ErrorImpl::diagnostic(self.error.inner.by_ref()).labels() }
179179
}
180180

181181
fn source_code(&self) -> Option<&dyn crate::SourceCode> {

‎src/eyreish/error.rs

+146-134
Large diffs are not rendered by default.

‎src/eyreish/fmt.rs

+11-9
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,20 @@
1-
use super::error::ErrorImpl;
1+
use super::{error::ErrorImpl, ptr::Ref};
22
use core::fmt;
33

44
impl ErrorImpl<()> {
5-
pub(crate) fn display(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
6-
self.handler
5+
pub(crate) unsafe fn display(this: Ref<'_, Self>, f: &mut fmt::Formatter<'_>) -> fmt::Result {
6+
this.deref()
7+
.handler
78
.as_ref()
8-
.map(|handler| handler.display(self.error(), f))
9-
.unwrap_or_else(|| core::fmt::Display::fmt(self.diagnostic(), f))
9+
.map(|handler| handler.display(Self::error(this), f))
10+
.unwrap_or_else(|| core::fmt::Display::fmt(Self::diagnostic(this), f))
1011
}
1112

12-
pub(crate) fn debug(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
13-
self.handler
13+
pub(crate) unsafe fn debug(this: Ref<'_, Self>, f: &mut fmt::Formatter<'_>) -> fmt::Result {
14+
this.deref()
15+
.handler
1416
.as_ref()
15-
.map(|handler| handler.debug(self.diagnostic(), f))
16-
.unwrap_or_else(|| core::fmt::Debug::fmt(self.diagnostic(), f))
17+
.map(|handler| handler.debug(Self::diagnostic(this), f))
18+
.unwrap_or_else(|| core::fmt::Debug::fmt(Self::diagnostic(this), f))
1719
}
1820
}

‎src/eyreish/mod.rs

+7-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
clippy::wrong_self_convention
66
)]
77
use core::fmt::Display;
8-
use core::mem::ManuallyDrop;
98

109
use std::error::Error as StdError;
1110

@@ -34,12 +33,15 @@ use crate::MietteHandler;
3433

3534
use error::ErrorImpl;
3635

36+
use self::ptr::Own;
37+
3738
mod context;
3839
mod error;
3940
mod fmt;
4041
mod into_diagnostic;
4142
mod kind;
4243
mod macros;
44+
mod ptr;
4345
mod wrapper;
4446

4547
/**
@@ -50,9 +52,12 @@ Core Diagnostic wrapper type.
5052
You can just replace `use`s of `eyre::Report` with `miette::Report`.
5153
*/
5254
pub struct Report {
53-
inner: ManuallyDrop<Box<ErrorImpl<()>>>,
55+
inner: Own<ErrorImpl<()>>,
5456
}
5557

58+
unsafe impl Sync for Report {}
59+
unsafe impl Send for Report {}
60+
5661
///
5762
pub type ErrorHook =
5863
Box<dyn Fn(&(dyn Diagnostic + 'static)) -> Box<dyn ReportHandler> + Sync + Send + 'static>;

‎src/eyreish/ptr.rs

+188
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
use std::{marker::PhantomData, ptr::NonNull};
2+
3+
#[repr(transparent)]
4+
/// A raw pointer that owns its pointee
5+
pub(crate) struct Own<T>
6+
where
7+
T: ?Sized,
8+
{
9+
pub(crate) ptr: NonNull<T>,
10+
}
11+
12+
unsafe impl<T> Send for Own<T> where T: ?Sized {}
13+
unsafe impl<T> Sync for Own<T> where T: ?Sized {}
14+
15+
impl<T> Copy for Own<T> where T: ?Sized {}
16+
17+
impl<T> Clone for Own<T>
18+
where
19+
T: ?Sized,
20+
{
21+
fn clone(&self) -> Self {
22+
*self
23+
}
24+
}
25+
26+
impl<T> Own<T>
27+
where
28+
T: ?Sized,
29+
{
30+
pub(crate) fn new(ptr: Box<T>) -> Self {
31+
Own {
32+
ptr: unsafe { NonNull::new_unchecked(Box::into_raw(ptr)) },
33+
}
34+
}
35+
36+
pub(crate) fn cast<U: CastTo>(self) -> Own<U::Target> {
37+
Own {
38+
ptr: self.ptr.cast(),
39+
}
40+
}
41+
42+
pub(crate) unsafe fn boxed(self) -> Box<T> {
43+
Box::from_raw(self.ptr.as_ptr())
44+
}
45+
46+
pub(crate) fn by_ref<'a>(&self) -> Ref<'a, T> {
47+
Ref {
48+
ptr: self.ptr,
49+
lifetime: PhantomData,
50+
}
51+
}
52+
53+
pub(crate) fn by_mut<'a>(self) -> Mut<'a, T> {
54+
Mut {
55+
ptr: self.ptr,
56+
lifetime: PhantomData,
57+
}
58+
}
59+
}
60+
61+
#[allow(explicit_outlives_requirements)]
62+
#[repr(transparent)]
63+
/// A raw pointer that represents a shared borrow of its pointee
64+
pub(crate) struct Ref<'a, T>
65+
where
66+
T: ?Sized,
67+
{
68+
pub(crate) ptr: NonNull<T>,
69+
lifetime: PhantomData<&'a T>,
70+
}
71+
72+
impl<'a, T> Copy for Ref<'a, T> where T: ?Sized {}
73+
74+
impl<'a, T> Clone for Ref<'a, T>
75+
where
76+
T: ?Sized,
77+
{
78+
fn clone(&self) -> Self {
79+
*self
80+
}
81+
}
82+
83+
impl<'a, T> Ref<'a, T>
84+
where
85+
T: ?Sized,
86+
{
87+
pub(crate) fn new(ptr: &'a T) -> Self {
88+
Ref {
89+
ptr: NonNull::from(ptr),
90+
lifetime: PhantomData,
91+
}
92+
}
93+
94+
pub(crate) fn from_raw(ptr: NonNull<T>) -> Self {
95+
Ref {
96+
ptr,
97+
lifetime: PhantomData,
98+
}
99+
}
100+
101+
pub(crate) fn cast<U: CastTo>(self) -> Ref<'a, U::Target> {
102+
Ref {
103+
ptr: self.ptr.cast(),
104+
lifetime: PhantomData,
105+
}
106+
}
107+
108+
pub(crate) fn by_mut(self) -> Mut<'a, T> {
109+
Mut {
110+
ptr: self.ptr,
111+
lifetime: PhantomData,
112+
}
113+
}
114+
115+
pub(crate) fn as_ptr(self) -> *const T {
116+
self.ptr.as_ptr() as *const T
117+
}
118+
119+
pub(crate) unsafe fn deref(self) -> &'a T {
120+
&*self.ptr.as_ptr()
121+
}
122+
}
123+
124+
#[allow(explicit_outlives_requirements)]
125+
#[repr(transparent)]
126+
/// A raw pointer that represents a unique borrow of its pointee
127+
pub(crate) struct Mut<'a, T>
128+
where
129+
T: ?Sized,
130+
{
131+
pub(crate) ptr: NonNull<T>,
132+
lifetime: PhantomData<&'a mut T>,
133+
}
134+
135+
impl<'a, T> Copy for Mut<'a, T> where T: ?Sized {}
136+
137+
impl<'a, T> Clone for Mut<'a, T>
138+
where
139+
T: ?Sized,
140+
{
141+
fn clone(&self) -> Self {
142+
*self
143+
}
144+
}
145+
146+
impl<'a, T> Mut<'a, T>
147+
where
148+
T: ?Sized,
149+
{
150+
pub(crate) fn cast<U: CastTo>(self) -> Mut<'a, U::Target> {
151+
Mut {
152+
ptr: self.ptr.cast(),
153+
lifetime: PhantomData,
154+
}
155+
}
156+
157+
pub(crate) fn by_ref(self) -> Ref<'a, T> {
158+
Ref {
159+
ptr: self.ptr,
160+
lifetime: PhantomData,
161+
}
162+
}
163+
164+
pub(crate) fn extend<'b>(self) -> Mut<'b, T> {
165+
Mut {
166+
ptr: self.ptr,
167+
lifetime: PhantomData,
168+
}
169+
}
170+
171+
pub(crate) unsafe fn deref_mut(self) -> &'a mut T {
172+
&mut *self.ptr.as_ptr()
173+
}
174+
}
175+
176+
impl<'a, T> Mut<'a, T> {
177+
pub(crate) unsafe fn read(self) -> T {
178+
self.ptr.as_ptr().read()
179+
}
180+
}
181+
182+
pub(crate) trait CastTo {
183+
type Target;
184+
}
185+
186+
impl<T> CastTo for T {
187+
type Target = T;
188+
}

‎src/handler.rs

+9
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ impl MietteHandlerOpts {
261261
}
262262
}
263263

264+
#[cfg(not(miri))]
264265
pub(crate) fn get_width(&self) -> usize {
265266
self.width.unwrap_or_else(|| {
266267
terminal_size::terminal_size()
@@ -269,6 +270,14 @@ impl MietteHandlerOpts {
269270
.0 as usize
270271
})
271272
}
273+
274+
#[cfg(miri)]
275+
// miri doesn't support a syscall (specifically ioctl)
276+
// performed by terminal_size, which causes test execution to fail
277+
// so when miri is running we'll just fallback to a constant
278+
pub(crate) fn get_width(&self) -> usize {
279+
self.width.unwrap_or(80)
280+
}
272281
}
273282

274283
/**

‎src/source_impls.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ fn context_info<'a>(
7373
}
7474

7575
if offset >= (span.offset() + span.len()).saturating_sub(1) {
76-
let starting_offset = before_lines_starts.get(0).copied().unwrap_or_else(|| {
76+
let starting_offset = before_lines_starts.first().copied().unwrap_or_else(|| {
7777
if context_lines_before == 0 {
7878
span.offset()
7979
} else {

‎tests/compiletest.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#[rustversion::attr(not(nightly), ignore)]
2+
#[cfg_attr(miri, ignore)]
23
#[test]
34
fn ui() {
45
let t = trybuild::TestCases::new();

0 commit comments

Comments
 (0)
Please sign in to comment.