fix clippy (and MIR printing) handling of ConstValue::Indirect slices

Author: RalfJung

compiler/rustc_middle/src/mir/interpret/value.rs

@@ -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,
@@ -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

compiler/rustc_middle/src/mir/mod.rs

@@ -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 => {

src/tools/clippy/clippy_utils/src/consts.rs

@@ -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() {

tests/mir-opt/const_prop/mutable_variable_aggregate.main.ConstProp.diff

@@ -27,7 +27,7 @@
       }
 + }
 + 
-+ alloc6 (size: 8, align: 4) {
++ alloc7 (size: 8, align: 4) {
 +     2a 00 00 00 63 00 00 00                         │ *...c...
 + }
 + 

tests/mir-opt/const_prop/tuple_literal_propagation.main.ConstProp.panic-abort.diff

@@ -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                         │ ........
 + }
 + 

tests/mir-opt/const_prop/tuple_literal_propagation.main.ConstProp.panic-unwind.diff

@@ -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                         │ ........
 + }
 +