Skip to content

Commit

Permalink
fix clippy (and MIR printing) handling of ConstValue::Indirect slices
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Sep 14, 2023
1 parent 7e0d6f3 commit 6a1c887
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 44 deletions.
53 changes: 52 additions & 1 deletion compiler/rustc_middle/src/mir/interpret/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ use rustc_apfloat::{
use rustc_macros::HashStable;
use rustc_target::abi::{HasDataLayout, Size};

use crate::ty::{ParamEnv, ScalarInt, Ty, TyCtxt};
use crate::{
mir::interpret::alloc_range,
ty::{ParamEnv, ScalarInt, Ty, TyCtxt},
};

use super::{
AllocId, ConstAllocation, InterpResult, Pointer, PointerArithmetic, Provenance,
Expand Down Expand Up @@ -114,6 +117,54 @@ impl<'tcx> ConstValue<'tcx> {
pub fn from_target_usize(i: u64, cx: &impl HasDataLayout) -> Self {
ConstValue::Scalar(Scalar::from_target_usize(i, cx))
}

/// Must only be called on constants of type `&str` or `&[u8]`!
pub fn try_get_slice_bytes_for_diagnostics(&self, tcx: TyCtxt<'tcx>) -> Option<&'tcx [u8]> {
let (data, start, end) = match self {
ConstValue::Scalar(_) | ConstValue::ZeroSized => {
bug!("`try_get_slice_bytes` on non-slice constant")
}
&ConstValue::Slice { data, start, end } => (data, start, end),
&ConstValue::Indirect { alloc_id, offset } => {
// The reference itself is stored behind an indirection.
// Load the reference, and then load the actual slice contents.
let a = tcx.global_alloc(alloc_id).unwrap_memory().inner();
let ptr_size = tcx.data_layout.pointer_size;
if a.size() < offset + 2 * ptr_size {
// (partially) dangling reference
return None;
}
// Read the wide pointer components.
let ptr = a
.read_scalar(
&tcx,
alloc_range(offset, ptr_size),
/* read_provenance */ true,
)
.ok()?;
let ptr = ptr.to_pointer(&tcx).ok()?;
let len = a
.read_scalar(
&tcx,
alloc_range(offset + ptr_size, ptr_size),
/* read_provenance */ false,
)
.ok()?;
let len = len.to_target_usize(&tcx).ok()?;
let len: usize = len.try_into().ok()?;
if len == 0 {
return Some(&[]);
}
// Non-empty slice, must have memory. We know this is a relative pointer.
let (inner_alloc_id, offset) = ptr.into_parts();
let data = tcx.global_alloc(inner_alloc_id?).unwrap_memory();
(data, offset.bytes_usize(), offset.bytes_usize() + len)
}
};

// This is for diagnostics only, so we are okay to use `inspect_with_uninit_and_ptr_outside_interpreter`.
Some(data.inner().inspect_with_uninit_and_ptr_outside_interpreter(start..end))
}
}

/// A `Scalar` represents an immediate, primitive value existing outside of a
Expand Down
35 changes: 10 additions & 25 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2887,31 +2887,16 @@ fn pretty_print_const_value<'tcx>(
let u8_type = tcx.types.u8;
match (ct, ty.kind()) {
// Byte/string slices, printed as (byte) string literals.
(ConstValue::Slice { data, start, end }, ty::Ref(_, inner, _)) => {
match inner.kind() {
ty::Slice(t) => {
if *t == u8_type {
// The `inspect` here is okay since we checked the bounds, and `u8` carries
// no provenance (we have an active slice reference here). We don't use
// this result to affect interpreter execution.
let byte_str = data
.inner()
.inspect_with_uninit_and_ptr_outside_interpreter(start..end);
pretty_print_byte_str(fmt, byte_str)?;
return Ok(());
}
}
ty::Str => {
// The `inspect` here is okay since we checked the bounds, and `str` carries
// no provenance (we have an active `str` reference here). We don't use this
// result to affect interpreter execution.
let slice = data
.inner()
.inspect_with_uninit_and_ptr_outside_interpreter(start..end);
fmt.write_str(&format!("{:?}", String::from_utf8_lossy(slice)))?;
return Ok(());
}
_ => {}
(_, ty::Ref(_, inner_ty, _)) if matches!(inner_ty.kind(), ty::Str) => {
if let Some(data) = ct.try_get_slice_bytes_for_diagnostics(tcx) {
fmt.write_str(&format!("{:?}", String::from_utf8_lossy(data)))?;
return Ok(());
}
}
(_, ty::Ref(_, inner_ty, _)) if matches!(inner_ty.kind(), ty::Slice(t) if *t == u8_type) => {
if let Some(data) = ct.try_get_slice_bytes_for_diagnostics(tcx) {
pretty_print_byte_str(fmt, data)?;
return Ok(());
}
}
(ConstValue::Indirect { alloc_id, offset }, ty::Array(t, n)) if *t == u8_type => {
Expand Down
17 changes: 4 additions & 13 deletions src/tools/clippy/clippy_utils/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -671,19 +671,10 @@ pub fn miri_to_const<'tcx>(lcx: &LateContext<'tcx>, result: mir::ConstantKind<'t
ty::RawPtr(_) => Some(Constant::RawPtr(int.assert_bits(int.size()))),
_ => None,
},
mir::ConstantKind::Val(ConstValue::Slice { data, start, end }, _) => match result.ty().kind() {
ty::Ref(_, tam, _) => match tam.kind() {
ty::Str => String::from_utf8(
data.inner()
.inspect_with_uninit_and_ptr_outside_interpreter(start..end)
.to_owned(),
)
.ok()
.map(Constant::Str),
_ => None,
},
_ => None,
},
mir::ConstantKind::Val(cv, _) if matches!(result.ty().kind(), ty::Ref(_, inner_ty, _) if matches!(inner_ty.kind(), ty::Str)) => {
let data = cv.try_get_slice_bytes_for_diagnostics(lcx.tcx)?;
String::from_utf8(data.to_owned()).ok().map(Constant::Str)
}
mir::ConstantKind::Val(ConstValue::Indirect { alloc_id, offset: _ }, _) => {
let alloc = lcx.tcx.global_alloc(alloc_id).unwrap_memory();
match result.ty().kind() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
}
+ }
+
+ alloc6 (size: 8, align: 4) {
+ alloc7 (size: 8, align: 4) {
+ 2a 00 00 00 63 00 00 00 │ *...c...
+ }
+
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@
}
+ }
+
+ alloc8 (size: 8, align: 4) {
+ alloc9 (size: 8, align: 4) {
+ 01 00 00 00 02 00 00 00 │ ........
+ }
+
+ alloc7 (size: 8, align: 4) {
+ alloc8 (size: 8, align: 4) {
+ 01 00 00 00 02 00 00 00 │ ........
+ }
+
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@
}
+ }
+
+ alloc8 (size: 8, align: 4) {
+ alloc9 (size: 8, align: 4) {
+ 01 00 00 00 02 00 00 00 │ ........
+ }
+
+ alloc7 (size: 8, align: 4) {
+ alloc8 (size: 8, align: 4) {
+ 01 00 00 00 02 00 00 00 │ ........
+ }
+
Expand Down

0 comments on commit 6a1c887

Please sign in to comment.