Auto merge of rust-lang#98936 - matthiaskrgr:rollup-dvr0ucm, r=matthiaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#98860 (adjust dangling-int-ptr error message)
 - rust-lang#98888 (interpret: fix CheckedBinOp behavior when overflow checking is disabled)
 - rust-lang#98889 (Add regression test for rust-lang#79467)
 - rust-lang#98895 (bootstrap.py: Always use `.exe` for Windows)
 - rust-lang#98920 (adapt issue-37945 codegen test to accept any order of ops)
 - rust-lang#98921 (Refactor: remove a redundant mutable variable)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup

Author: bors

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -217,7 +217,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     sym::mul_with_overflow => BinOp::Mul,
                     _ => bug!(),
                 };
-                self.binop_with_overflow(bin_op, &lhs, &rhs, dest)?;
+                self.binop_with_overflow(
+                    bin_op, /*force_overflow_checks*/ true, &lhs, &rhs, dest,
+                )?;
             }
             sym::saturating_add | sym::saturating_sub => {
                 let l = self.read_immediate(&args[0])?;

compiler/rustc_const_eval/src/interpret/machine.rs

@@ -144,6 +144,9 @@ pub trait Machine<'mir, 'tcx>: Sized {
         true
     }
 
+    /// Whether CheckedBinOp MIR statements should actually check for overflow.
+    fn check_binop_checks_overflow(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
+
     /// Entry point for obtaining the MIR of anything that should get evaluated.
     /// So not just functions and shims, but also const/static initializers, anonymous
     /// constants, ...
@@ -468,6 +471,11 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
         true
     }
 
+    #[inline(always)]
+    fn check_binop_checks_overflow(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool {
+        true
+    }
+
     #[inline(always)]
     fn call_extra_fn(
         _ecx: &mut InterpCx<$mir, $tcx, Self>,
@@ -513,7 +521,7 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
         _ecx: &InterpCx<$mir, $tcx, Self>,
         addr: u64,
     ) -> Pointer<Option<AllocId>> {
-        Pointer::new(None, Size::from_bytes(addr))
+        Pointer::from_addr(addr)
     }
 
     #[inline(always)]
@@ -523,7 +531,7 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
     ) -> InterpResult<$tcx, Pointer<Option<AllocId>>> {
         // Allow these casts, but make the pointer not dereferenceable.
         // (I.e., they behave like transmutation.)
-        Ok(Pointer::new(None, Size::from_bytes(addr)))
+        Ok(Pointer::from_addr(addr))
     }
 
     #[inline(always)]

compiler/rustc_const_eval/src/interpret/operator.rs

@@ -12,9 +12,13 @@ use super::{ImmTy, Immediate, InterpCx, Machine, PlaceTy};
 impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     /// Applies the binary operation `op` to the two operands and writes a tuple of the result
     /// and a boolean signifying the potential overflow to the destination.
+    ///
+    /// `force_overflow_checks` indicates whether overflow checks should be done even when
+    /// `tcx.sess.overflow_checks()` is `false`.
     pub fn binop_with_overflow(
         &mut self,
         op: mir::BinOp,
+        force_overflow_checks: bool,
         left: &ImmTy<'tcx, M::PointerTag>,
         right: &ImmTy<'tcx, M::PointerTag>,
         dest: &PlaceTy<'tcx, M::PointerTag>,
@@ -26,6 +30,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             "type mismatch for result of {:?}",
             op,
         );
+        // As per https://github.com/rust-lang/rust/pull/98738, we always return `false` in the 2nd
+        // component when overflow checking is disabled.
+        let overflowed =
+            overflowed && (force_overflow_checks || M::check_binop_checks_overflow(self));
+        // Write the result to `dest`.
         if let Abi::ScalarPair(..) = dest.layout.abi {
             // We can use the optimized path and avoid `place_field` (which might do
             // `force_allocation`).

compiler/rustc_const_eval/src/interpret/place.rs

@@ -188,7 +188,7 @@ impl<'tcx, Tag: Provenance> MPlaceTy<'tcx, Tag> {
     #[inline]
     pub fn dangling(layout: TyAndLayout<'tcx>) -> Self {
         let align = layout.align.abi;
-        let ptr = Pointer::new(None, Size::from_bytes(align.bytes())); // no provenance, absolute address
+        let ptr = Pointer::from_addr(align.bytes()); // no provenance, absolute address
         // `Poison` this to make sure that the pointer value `ptr` is never observable by the program.
         MPlaceTy { mplace: MemPlace { ptr, meta: MemPlaceMeta::Poison }, layout, align }
     }

compiler/rustc_const_eval/src/interpret/step.rs

@@ -185,7 +185,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 let left = self.read_immediate(&self.eval_operand(left, None)?)?;
                 let layout = binop_right_homogeneous(bin_op).then_some(left.layout);
                 let right = self.read_immediate(&self.eval_operand(right, layout)?)?;
-                self.binop_with_overflow(bin_op, &left, &right, &dest)?;
+                self.binop_with_overflow(
+                    bin_op, /*force_overflow_checks*/ false, &left, &right, &dest,
+                )?;
             }
 
             UnaryOp(un_op, ref operand) => {

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

@@ -186,17 +186,17 @@ pub enum CheckInAllocMsg {
 
 impl fmt::Display for CheckInAllocMsg {
     /// When this is printed as an error the context looks like this:
-    /// "{msg}0x01 is not a valid pointer".
+    /// "{msg}{pointer} is a dangling pointer".
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(
             f,
             "{}",
             match *self {
                 CheckInAllocMsg::DerefTest => "dereferencing pointer failed: ",
                 CheckInAllocMsg::MemoryAccessTest => "memory access failed: ",
-                CheckInAllocMsg::PointerArithmeticTest => "pointer arithmetic failed: ",
+                CheckInAllocMsg::PointerArithmeticTest => "out-of-bounds pointer arithmetic: ",
                 CheckInAllocMsg::OffsetFromTest => "out-of-bounds offset_from: ",
-                CheckInAllocMsg::InboundsTest => "",
+                CheckInAllocMsg::InboundsTest => "out-of-bounds pointer use: ",
             }
         )
     }
@@ -350,14 +350,12 @@ impl fmt::Display for UndefinedBehaviorInfo<'_> {
                 ptr_size = ptr_size.bytes(),
                 ptr_size_p = pluralize!(ptr_size.bytes()),
             ),
-            DanglingIntPointer(0, CheckInAllocMsg::InboundsTest) => {
-                write!(f, "null pointer is not a valid pointer for this operation")
-            }
-            DanglingIntPointer(0, msg) => {
-                write!(f, "{msg}null pointer is not a valid pointer")
-            }
             DanglingIntPointer(i, msg) => {
-                write!(f, "{msg}{i:#x} is not a valid pointer")
+                write!(
+                    f,
+                    "{msg}{pointer} is a dangling pointer (it has no provenance)",
+                    pointer = Pointer::<Option<AllocId>>::from_addr(*i),
+                )
             }
             AlignmentCheckFailed { required, has } => write!(
                 f,

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

@@ -181,7 +181,17 @@ impl<Tag: Provenance> fmt::Debug for Pointer<Option<Tag>> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self.provenance {
             Some(tag) => Provenance::fmt(&Pointer::new(tag, self.offset), f),
-            None => write!(f, "{:#x}", self.offset.bytes()),
+            None => write!(f, "{:#x}[noalloc]", self.offset.bytes()),
+        }
+    }
+}
+
+impl<Tag: Provenance> fmt::Display for Pointer<Option<Tag>> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        if self.provenance.is_none() && self.offset.bytes() == 0 {
+            write!(f, "null pointer")
+        } else {
+            fmt::Debug::fmt(self, f)
         }
     }
 }
@@ -226,9 +236,14 @@ impl<Tag> Pointer<Option<Tag>> {
 }
 
 impl<Tag> Pointer<Option<Tag>> {
+    #[inline(always)]
+    pub fn from_addr(addr: u64) -> Self {
+        Pointer { provenance: None, offset: Size::from_bytes(addr) }
+    }
+
     #[inline(always)]
     pub fn null() -> Self {
-        Pointer { provenance: None, offset: Size::ZERO }
+        Pointer::from_addr(0)
     }
 }
 

compiler/rustc_middle/src/mir/syntax.rs

@@ -993,8 +993,9 @@ pub enum Rvalue<'tcx> {
 
     /// Same as `BinaryOp`, but yields `(T, bool)` with a `bool` indicating an error condition.
     ///
-    /// When overflow checking is disabled, the error condition is false. Otherwise, the error
-    /// condition is determined as described below.
+    /// When overflow checking is disabled and we are generating run-time code, the error condition
+    /// is false. Otherwise, and always during CTFE, the error condition is determined as described
+    /// below.
     ///
     /// For addition, subtraction, and multiplication on integers the error condition is set when
     /// the infinite precision result would be unequal to the actual result.

compiler/rustc_trait_selection/src/traits/fulfill.rs

@@ -131,8 +131,6 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> {
         let span = debug_span!("select", obligation_forest_size = ?self.predicates.len());
         let _enter = span.enter();
 
-        let mut errors = Vec::new();
-
         // Process pending obligations.
         let outcome: Outcome<_, _> = self.predicates.process_obligations(&mut FulfillProcessor {
             selcx,
@@ -142,7 +140,8 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> {
         // FIXME: if we kept the original cache key, we could mark projection
         // obligations as complete for the projection cache here.
 
-        errors.extend(outcome.errors.into_iter().map(to_fulfillment_error));
+        let errors: Vec<FulfillmentError<'tcx>> =
+            outcome.errors.into_iter().map(to_fulfillment_error).collect();
 
         debug!(
             "select({} predicates remaining, {} errors) done",
@@ -728,7 +727,7 @@ impl<'a, 'b, 'tcx> FulfillProcessor<'a, 'b, 'tcx> {
                 }
                 return ProcessResult::Changed(vec![]);
             } else {
-                tracing::debug!("Does NOT hold: {:?}", obligation);
+                debug!("Does NOT hold: {:?}", obligation);
             }
         }
 

src/bootstrap/bootstrap.py

@@ -152,6 +152,10 @@ def run(args, verbose=False, exception=False, is_bootstrap=False, **kwargs):
     if verbose:
         print("running: " + ' '.join(args))
     sys.stdout.flush()
+    # Ensure that the .exe is used on Windows just in case a Linux ELF has been
+    # compiled in the same directory.
+    if os.name == 'nt' and not args[0].endswith('.exe'):
+        args[0] += '.exe'
     # Use Popen here instead of call() as it apparently allows powershell on
     # Windows to not lock up waiting for input presumably.
     ret = subprocess.Popen(args, **kwargs)