From 568deb7ece889ed2871fa023100cb69c5c086967 Mon Sep 17 00:00:00 2001
From: bjorn3 <17426603+bjorn3@users.noreply.github.com>
Date: Sun, 12 Feb 2023 17:14:28 +0000
Subject: [PATCH] Improve miri comments

---
 src/tools/miri/src/shims/foreign_items.rs | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs
index cc2d4cd640a..167370ef4a8 100644
--- a/src/tools/miri/src/shims/foreign_items.rs
+++ b/src/tools/miri/src/shims/foreign_items.rs
@@ -358,7 +358,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
 
         match allocator_kind {
             AllocatorKind::Global => {
-                // `__rust_*` is defined by `#[global_allocator]` if `#[global_allocator]` is used
+                // When `#[global_allocator]` is used, `__rust_*` is defined by the macro expansion
+                // of this attribute rather than generated by the allocator shim. As such we have
+                // to call the definition produced by `#[global_allocator]` instead of the shim like
+                // in the case of `#[global_allocator]` not existing. Somewhat unintuitively doing
+                // so is done by returning `NotSupported`.
                 return Ok(EmulateByNameResult::NotSupported);
             }
             AllocatorKind::Default => {
@@ -555,6 +559,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             // Rust allocation
             "__rust_alloc" | "miri_alloc" => {
                 let default = |this: &mut MiriInterpCx<'mir, 'tcx>| {
+                    // Only call `check_shim` when `#[global_allocator]` isn't used. The macro
+                    // expansion of `#[global_allocator]` defines this symbol and `check_shim`
+                    // checks that there exists no definition of a shim.
                     let [size, align] = this.check_shim(abi, Abi::Rust, link_name, args)?;
                     let size = this.read_target_usize(size)?;
                     let align = this.read_target_usize(align)?;
@@ -587,6 +594,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             }
             "__rust_alloc_zeroed" => {
                 return this.emulate_allocator(|this| {
+                    // See the comment for `__rust_alloc` why `check_shim` is only called in the
+                    // default case.
                     let [size, align] = this.check_shim(abi, Abi::Rust, link_name, args)?;
                     let size = this.read_target_usize(size)?;
                     let align = this.read_target_usize(align)?;
@@ -610,6 +619,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             }
             "__rust_dealloc" | "miri_dealloc" => {
                 let default = |this: &mut MiriInterpCx<'mir, 'tcx>| {
+                    // See the comment for `__rust_alloc` why `check_shim` is only called in the
+                    // default case.
                     let [ptr, old_size, align] =
                         this.check_shim(abi, Abi::Rust, link_name, args)?;
                     let ptr = this.read_pointer(ptr)?;
@@ -643,6 +654,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             }
             "__rust_realloc" => {
                 return this.emulate_allocator(|this| {
+                    // See the comment for `__rust_alloc` why `check_shim` is only called in the
+                    // default case.
                     let [ptr, old_size, align, new_size] =
                         this.check_shim(abi, Abi::Rust, link_name, args)?;
                     let ptr = this.read_pointer(ptr)?;