Skip to content

Commit b8f972f

Browse files
borsbors
committed
Auto merge of rust-lang#2727 - RalfJung:flags-and-libc, r=RalfJung
make flag checks reobust against multi-bit flags and make eval_libc functions ICE on any problem
2 parents 0876519 + f0bb7c7 commit b8f972f

File tree

14 files changed

+213
-189
lines changed

14 files changed

+213
-189
lines changed

‎src/tools/miri/src/helpers.rs‎

Lines changed: 40 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -138,55 +138,77 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
138138
.unwrap_or_else(|| panic!("failed to find required Rust item: {path:?}"))
139139
}
140140

141-
/// Evaluates the scalar at the specified path. Returns Some(val)
142-
/// if the path could be resolved, and None otherwise
143-
fn eval_path_scalar(&self, path: &[&str]) -> InterpResult<'tcx, Scalar<Provenance>> {
141+
/// Evaluates the scalar at the specified path.
142+
fn eval_path_scalar(&self, path: &[&str]) -> Scalar<Provenance> {
144143
let this = self.eval_context_ref();
145144
let instance = this.resolve_path(path, Namespace::ValueNS);
146145
let cid = GlobalId { instance, promoted: None };
147146
// We don't give a span -- this isn't actually used directly by the program anyway.
148-
let const_val = this.eval_global(cid, None)?;
147+
let const_val = this
148+
.eval_global(cid, None)
149+
.unwrap_or_else(|err| panic!("failed to evaluate required Rust item: {path:?}\n{err}"));
149150
this.read_scalar(&const_val.into())
151+
.unwrap_or_else(|err| panic!("failed to read required Rust item: {path:?}\n{err}"))
150152
}
151153

152154
/// Helper function to get a `libc` constant as a `Scalar`.
153-
fn eval_libc(&self, name: &str) -> InterpResult<'tcx, Scalar<Provenance>> {
155+
fn eval_libc(&self, name: &str) -> Scalar<Provenance> {
154156
self.eval_path_scalar(&["libc", name])
155157
}
156158

157159
/// Helper function to get a `libc` constant as an `i32`.
158-
fn eval_libc_i32(&self, name: &str) -> InterpResult<'tcx, i32> {
160+
fn eval_libc_i32(&self, name: &str) -> i32 {
159161
// TODO: Cache the result.
160-
self.eval_libc(name)?.to_i32()
162+
self.eval_libc(name).to_i32().unwrap_or_else(|_err| {
163+
panic!("required libc item has unexpected type (not `i32`): {name}")
164+
})
165+
}
166+
167+
/// Helper function to get a `libc` constant as an `u32`.
168+
fn eval_libc_u32(&self, name: &str) -> u32 {
169+
// TODO: Cache the result.
170+
self.eval_libc(name).to_u32().unwrap_or_else(|_err| {
171+
panic!("required libc item has unexpected type (not `u32`): {name}")
172+
})
161173
}
162174

163175
/// Helper function to get a `windows` constant as a `Scalar`.
164-
fn eval_windows(&self, module: &str, name: &str) -> InterpResult<'tcx, Scalar<Provenance>> {
176+
fn eval_windows(&self, module: &str, name: &str) -> Scalar<Provenance> {
165177
self.eval_context_ref().eval_path_scalar(&["std", "sys", "windows", module, name])
166178
}
167179

180+
/// Helper function to get a `windows` constant as a `u32`.
181+
fn eval_windows_u32(&self, module: &str, name: &str) -> u32 {
182+
// TODO: Cache the result.
183+
self.eval_windows(module, name).to_u32().unwrap_or_else(|_err| {
184+
panic!("required Windows item has unexpected type (not `u32`): {module}::{name}")
185+
})
186+
}
187+
168188
/// Helper function to get a `windows` constant as a `u64`.
169-
fn eval_windows_u64(&self, module: &str, name: &str) -> InterpResult<'tcx, u64> {
189+
fn eval_windows_u64(&self, module: &str, name: &str) -> u64 {
170190
// TODO: Cache the result.
171-
self.eval_windows(module, name)?.to_u64()
191+
self.eval_windows(module, name).to_u64().unwrap_or_else(|_err| {
192+
panic!("required Windows item has unexpected type (not `u64`): {module}::{name}")
193+
})
172194
}
173195

174196
/// Helper function to get the `TyAndLayout` of a `libc` type
175-
fn libc_ty_layout(&self, name: &str) -> InterpResult<'tcx, TyAndLayout<'tcx>> {
197+
fn libc_ty_layout(&self, name: &str) -> TyAndLayout<'tcx> {
176198
let this = self.eval_context_ref();
177199
let ty = this
178200
.resolve_path(&["libc", name], Namespace::TypeNS)
179201
.ty(*this.tcx, ty::ParamEnv::reveal_all());
180-
this.layout_of(ty)
202+
this.layout_of(ty).unwrap()
181203
}
182204

183205
/// Helper function to get the `TyAndLayout` of a `windows` type
184-
fn windows_ty_layout(&self, name: &str) -> InterpResult<'tcx, TyAndLayout<'tcx>> {
206+
fn windows_ty_layout(&self, name: &str) -> TyAndLayout<'tcx> {
185207
let this = self.eval_context_ref();
186208
let ty = this
187209
.resolve_path(&["std", "sys", "windows", "c", name], Namespace::TypeNS)
188210
.ty(*this.tcx, ty::ParamEnv::reveal_all());
189-
this.layout_of(ty)
211+
this.layout_of(ty).unwrap()
190212
}
191213

192214
/// Project to the given *named* field of the mplace (which must be a struct or union type).
@@ -609,14 +631,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
609631
if target.families.iter().any(|f| f == "unix") {
610632
for &(name, kind) in UNIX_IO_ERROR_TABLE {
611633
if err_kind == kind {
612-
return this.eval_libc(name);
634+
return Ok(this.eval_libc(name));
613635
}
614636
}
615637
throw_unsup_format!("io error {:?} cannot be translated into a raw os error", err_kind)
616638
} else if target.families.iter().any(|f| f == "windows") {
617639
// FIXME: we have to finish implementing the Windows equivalent of this.
618640
use std::io::ErrorKind::*;
619-
this.eval_windows(
641+
Ok(this.eval_windows(
620642
"c",
621643
match err_kind {
622644
NotFound => "ERROR_FILE_NOT_FOUND",
@@ -627,7 +649,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
627649
err_kind
628650
),
629651
},
630-
)
652+
))
631653
} else {
632654
throw_unsup_format!(
633655
"converting io::Error into errnum is unsupported for OS {}",
@@ -647,7 +669,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
647669
if target.families.iter().any(|f| f == "unix") {
648670
let errnum = errnum.to_i32()?;
649671
for &(name, kind) in UNIX_IO_ERROR_TABLE {
650-
if errnum == this.eval_libc_i32(name)? {
672+
if errnum == this.eval_libc_i32(name) {
651673
return Ok(Some(kind));
652674
}
653675
}

‎src/tools/miri/src/shims/env.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
170170
))
171171
}
172172
None => {
173-
let envvar_not_found = this.eval_windows("c", "ERROR_ENVVAR_NOT_FOUND")?;
173+
let envvar_not_found = this.eval_windows("c", "ERROR_ENVVAR_NOT_FOUND");
174174
this.set_last_error(envvar_not_found)?;
175175
Scalar::from_u32(0) // return zero upon failure
176176
}
@@ -240,7 +240,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
240240
Ok(0) // return zero on success
241241
} else {
242242
// name argument is a null pointer, points to an empty string, or points to a string containing an '=' character.
243-
let einval = this.eval_libc("EINVAL")?;
243+
let einval = this.eval_libc("EINVAL");
244244
this.set_last_error(einval)?;
245245
Ok(-1)
246246
}
@@ -274,15 +274,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
274274
this.deallocate_ptr(var, None, MiriMemoryKind::Runtime.into())?;
275275
this.update_environ()?;
276276
}
277-
Ok(this.eval_windows("c", "TRUE")?)
277+
Ok(this.eval_windows("c", "TRUE"))
278278
} else {
279279
let value = this.read_os_str_from_wide_str(value_ptr)?;
280280
let var_ptr = alloc_env_var_as_wide_str(&name, &value, this)?;
281281
if let Some(var) = this.machine.env_vars.map.insert(name, var_ptr) {
282282
this.deallocate_ptr(var, None, MiriMemoryKind::Runtime.into())?;
283283
}
284284
this.update_environ()?;
285-
Ok(this.eval_windows("c", "TRUE")?)
285+
Ok(this.eval_windows("c", "TRUE"))
286286
}
287287
}
288288

@@ -306,7 +306,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
306306
Ok(0)
307307
} else {
308308
// name argument is a null pointer, points to an empty string, or points to a string containing an '=' character.
309-
let einval = this.eval_libc("EINVAL")?;
309+
let einval = this.eval_libc("EINVAL");
310310
this.set_last_error(einval)?;
311311
Ok(-1)
312312
}
@@ -335,7 +335,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
335335
if this.write_path_to_c_str(&cwd, buf, size)?.0 {
336336
return Ok(buf);
337337
}
338-
let erange = this.eval_libc("ERANGE")?;
338+
let erange = this.eval_libc("ERANGE");
339339
this.set_last_error(erange)?;
340340
}
341341
Err(e) => this.set_last_error_from_io_error(e.kind())?,
@@ -411,14 +411,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
411411
this.reject_in_isolation("`SetCurrentDirectoryW`", reject_with)?;
412412
this.set_last_error_from_io_error(ErrorKind::PermissionDenied)?;
413413

414-
return this.eval_windows("c", "FALSE");
414+
return Ok(this.eval_windows("c", "FALSE"));
415415
}
416416

417417
match env::set_current_dir(path) {
418-
Ok(()) => this.eval_windows("c", "TRUE"),
418+
Ok(()) => Ok(this.eval_windows("c", "TRUE")),
419419
Err(e) => {
420420
this.set_last_error_from_io_error(e.kind())?;
421-
this.eval_windows("c", "FALSE")
421+
Ok(this.eval_windows("c", "FALSE"))
422422
}
423423
}
424424
}

‎src/tools/miri/src/shims/time.rs‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,26 +36,26 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
3636
// Linux further distinguishes regular and "coarse" clocks, but the "coarse" version
3737
// is just specified to be "faster and less precise", so we implement both the same way.
3838
absolute_clocks = vec![
39-
this.eval_libc_i32("CLOCK_REALTIME")?,
40-
this.eval_libc_i32("CLOCK_REALTIME_COARSE")?,
39+
this.eval_libc_i32("CLOCK_REALTIME"),
40+
this.eval_libc_i32("CLOCK_REALTIME_COARSE"),
4141
];
4242
// The second kind is MONOTONIC clocks for which 0 is an arbitrary time point, but they are
4343
// never allowed to go backwards. We don't need to do any additonal monotonicity
4444
// enforcement because std::time::Instant already guarantees that it is monotonic.
4545
relative_clocks = vec![
46-
this.eval_libc_i32("CLOCK_MONOTONIC")?,
47-
this.eval_libc_i32("CLOCK_MONOTONIC_COARSE")?,
46+
this.eval_libc_i32("CLOCK_MONOTONIC"),
47+
this.eval_libc_i32("CLOCK_MONOTONIC_COARSE"),
4848
];
4949
}
5050
"macos" => {
51-
absolute_clocks = vec![this.eval_libc_i32("CLOCK_REALTIME")?];
52-
relative_clocks = vec![this.eval_libc_i32("CLOCK_MONOTONIC")?];
51+
absolute_clocks = vec![this.eval_libc_i32("CLOCK_REALTIME")];
52+
relative_clocks = vec![this.eval_libc_i32("CLOCK_MONOTONIC")];
5353
// Some clocks only seem to exist in the aarch64 version of the target.
5454
if this.tcx.sess.target.arch == "aarch64" {
5555
// `CLOCK_UPTIME_RAW` supposed to not increment while the system is asleep... but
5656
// that's not really something a program running inside Miri can tell, anyway.
5757
// We need to support it because std uses it.
58-
relative_clocks.push(this.eval_libc_i32("CLOCK_UPTIME_RAW")?);
58+
relative_clocks.push(this.eval_libc_i32("CLOCK_UPTIME_RAW"));
5959
}
6060
}
6161
target => throw_unsup_format!("`clock_gettime` is not supported on target OS {target}"),
@@ -68,7 +68,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
6868
this.machine.clock.now().duration_since(this.machine.clock.anchor())
6969
} else {
7070
// Unsupported clock.
71-
let einval = this.eval_libc("EINVAL")?;
71+
let einval = this.eval_libc("EINVAL");
7272
this.set_last_error(einval)?;
7373
return Ok(Scalar::from_i32(-1));
7474
};
@@ -94,7 +94,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
9494
// Using tz is obsolete and should always be null
9595
let tz = this.read_pointer(tz_op)?;
9696
if !this.ptr_is_null(tz)? {
97-
let einval = this.eval_libc("EINVAL")?;
97+
let einval = this.eval_libc("EINVAL");
9898
this.set_last_error(einval)?;
9999
return Ok(-1);
100100
}
@@ -118,9 +118,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
118118
this.assert_target_os("windows", "GetSystemTimeAsFileTime");
119119
this.check_no_isolation("`GetSystemTimeAsFileTime`")?;
120120

121-
let NANOS_PER_SEC = this.eval_windows_u64("time", "NANOS_PER_SEC")?;
122-
let INTERVALS_PER_SEC = this.eval_windows_u64("time", "INTERVALS_PER_SEC")?;
123-
let INTERVALS_TO_UNIX_EPOCH = this.eval_windows_u64("time", "INTERVALS_TO_UNIX_EPOCH")?;
121+
let NANOS_PER_SEC = this.eval_windows_u64("time", "NANOS_PER_SEC");
122+
let INTERVALS_PER_SEC = this.eval_windows_u64("time", "INTERVALS_PER_SEC");
123+
let INTERVALS_TO_UNIX_EPOCH = this.eval_windows_u64("time", "INTERVALS_TO_UNIX_EPOCH");
124124
let NANOS_PER_INTERVAL = NANOS_PER_SEC / INTERVALS_PER_SEC;
125125
let SECONDS_TO_UNIX_EPOCH = INTERVALS_TO_UNIX_EPOCH / INTERVALS_PER_SEC;
126126

@@ -226,7 +226,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
226226
let duration = match this.read_timespec(&this.deref_operand(req_op)?)? {
227227
Some(duration) => duration,
228228
None => {
229-
let einval = this.eval_libc("EINVAL")?;
229+
let einval = this.eval_libc("EINVAL");
230230
this.set_last_error(einval)?;
231231
return Ok(-1);
232232
}

‎src/tools/miri/src/shims/tls.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -303,12 +303,12 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
303303
return Ok(());
304304
}
305305
let thread_callback =
306-
this.eval_windows("thread_local_key", "p_thread_callback")?.to_pointer(this)?;
306+
this.eval_windows("thread_local_key", "p_thread_callback").to_pointer(this)?;
307307
let thread_callback = this.get_ptr_fn(thread_callback)?.as_instance()?;
308308

309309
// FIXME: Technically, the reason should be `DLL_PROCESS_DETACH` when the main thread exits
310310
// but std treats both the same.
311-
let reason = this.eval_windows("c", "DLL_THREAD_DETACH")?;
311+
let reason = this.eval_windows("c", "DLL_THREAD_DETACH");
312312

313313
// The signature of this function is `unsafe extern "system" fn(h: c::LPVOID, dwReason: c::DWORD, pv: c::LPVOID)`.
314314
// FIXME: `h` should be a handle to the current module and what `pv` should be is unknown

‎src/tools/miri/src/shims/unix/foreign_items.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
196196
// Align must be power of 2, and also at least ptr-sized (POSIX rules).
197197
// But failure to adhere to this is not UB, it's an error condition.
198198
if !align.is_power_of_two() || align < this.pointer_size().bytes() {
199-
let einval = this.eval_libc_i32("EINVAL")?;
199+
let einval = this.eval_libc_i32("EINVAL");
200200
this.write_int(einval, dest)?;
201201
} else {
202202
if size == 0 {
@@ -243,7 +243,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
243243
];
244244
let mut result = None;
245245
for &(sysconf_name, value) in sysconfs {
246-
let sysconf_name = this.eval_libc_i32(sysconf_name)?;
246+
let sysconf_name = this.eval_libc_i32(sysconf_name);
247247
if sysconf_name == name {
248248
result = Some(value(this));
249249
break;
@@ -480,7 +480,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
480480
None => format!("<unknown errnum in strerror_r: {errnum}>"),
481481
};
482482
let (complete, _) = this.write_os_str_to_c_str(OsStr::new(&formatted), buf, buflen)?;
483-
let ret = if complete { 0 } else { this.eval_libc_i32("ERANGE")? };
483+
let ret = if complete { 0 } else { this.eval_libc_i32("ERANGE") };
484484
this.write_int(ret, dest)?;
485485
}
486486
"getpid" => {
@@ -495,7 +495,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
495495
if this.frame_in_std() => {
496496
let [_attr, guard_size] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
497497
let guard_size = this.deref_operand(guard_size)?;
498-
let guard_size_layout = this.libc_ty_layout("size_t")?;
498+
let guard_size_layout = this.libc_ty_layout("size_t");
499499
this.write_scalar(Scalar::from_uint(this.machine.page_size, guard_size_layout.size), &guard_size.into())?;
500500

501501
// Return success (`0`).
@@ -589,7 +589,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
589589
this.write_null(dest)?;
590590
} else {
591591
this.write_null(&result.into())?;
592-
this.write_scalar(this.eval_libc("ERANGE")?, dest)?;
592+
this.write_scalar(this.eval_libc("ERANGE"), dest)?;
593593
}
594594
}
595595

0 commit comments

Comments
 (0)