From 735a610f47b029d6620abfda04615f70f49c5ec6 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 29 Nov 2019 11:57:17 +0100 Subject: [PATCH 1/4] Only memoize const fn calls during const eval Miri and other engines may want to execute the function in order to detect UB inside of them. --- src/librustc_mir/const_eval.rs | 14 ++++++++++++++ src/librustc_mir/interpret/terminator.rs | 14 +------------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 0967b25788557..005d3b217eb3d 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -336,6 +336,20 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, _unwind: Option // unwinding is not supported in consts ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> { debug!("eval_fn_call: {:?}", instance); + + // If this function is a `const fn` then as an optimization we can query this + // evaluation immediately. + // + // For the moment we only do this for functions which take no arguments + // (or all arguments are ZSTs) so that we don't memoize too much. + if ecx.tcx.is_const_fn_raw(instance.def.def_id()) && + args.iter().all(|a| a.layout.is_zst()) + { + let gid = GlobalId { instance, promoted: None }; + ecx.eval_const_fn_call(gid, ret)?; + return Ok(None); + } + // Only check non-glue functions if let ty::InstanceDef::Item(def_id) = instance.def { // Execution might have wandered off into other crates, so we cannot do a stability- diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 7285836d1c318..ed037570d6db8 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -284,18 +284,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ty::InstanceDef::DropGlue(..) | ty::InstanceDef::CloneShim(..) | ty::InstanceDef::Item(_) => { - // If this function is a `const fn` then as an optimization we can query this - // evaluation immediately. - // - // For the moment we only do this for functions which take no arguments - // (or all arguments are ZSTs) so that we don't memoize too much. - if self.tcx.is_const_fn_raw(instance.def.def_id()) && - args.iter().all(|a| a.layout.is_zst()) - { - let gid = GlobalId { instance, promoted: None }; - return self.eval_const_fn_call(gid, ret); - } - // We need MIR for this fn let body = match M::find_fn(self, instance, args, ret, unwind)? { Some(body) => body, @@ -463,7 +451,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// Evaluate a const function where all arguments (if any) are zero-sized types. /// The evaluation is memoized thanks to the query system. - fn eval_const_fn_call( + pub (crate) fn eval_const_fn_call( &mut self, gid: GlobalId<'tcx>, ret: Option<(PlaceTy<'tcx, M::PointerTag>, mir::BasicBlock)>, From f15197e83c4c0e2963ef11e2e8bdb14491247582 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 30 Nov 2019 17:53:02 +0100 Subject: [PATCH 2/4] `find_fn` -> `find_mir_or_eval_fn` rename --- src/librustc_mir/const_eval.rs | 4 ++-- src/librustc_mir/interpret/machine.rs | 2 +- src/librustc_mir/interpret/terminator.rs | 2 +- src/librustc_mir/transform/const_prop.rs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 005d3b217eb3d..133ab31524723 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -328,14 +328,14 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, false // for now, we don't enforce validity } - fn find_fn( + fn find_mir_or_eval_fn( ecx: &mut InterpCx<'mir, 'tcx, Self>, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx>], ret: Option<(PlaceTy<'tcx>, mir::BasicBlock)>, _unwind: Option // unwinding is not supported in consts ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> { - debug!("eval_fn_call: {:?}", instance); + debug!("find_mir_or_eval_fn: {:?}", instance); // If this function is a `const fn` then as an optimization we can query this // evaluation immediately. diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index b7cde626415a7..7f5873cebf621 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -146,7 +146,7 @@ pub trait Machine<'mir, 'tcx>: Sized { /// nor just jump to `ret`, but instead push their own stack frame.) /// Passing `dest`and `ret` in the same `Option` proved very annoying when only one of them /// was used. - fn find_fn( + fn find_mir_or_eval_fn( ecx: &mut InterpCx<'mir, 'tcx, Self>, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx, Self::PointerTag>], diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index ed037570d6db8..674051ecd8f16 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -285,7 +285,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ty::InstanceDef::CloneShim(..) | ty::InstanceDef::Item(_) => { // We need MIR for this fn - let body = match M::find_fn(self, instance, args, ret, unwind)? { + let body = match M::find_mir_or_eval_fn(self, instance, args, ret, unwind)? { Some(body) => body, None => return Ok(()), }; diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 8de16308e8375..22f25aa93a392 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -139,7 +139,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { false } - fn find_fn( + fn find_mir_or_eval_fn( _ecx: &mut InterpCx<'mir, 'tcx, Self>, _instance: ty::Instance<'tcx>, _args: &[OpTy<'tcx>], From 55840b0209a77bcf7884c8818585f792637927c2 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 30 Nov 2019 18:22:11 +0100 Subject: [PATCH 3/4] Don't repeat the `is_const_fn_raw` check --- src/librustc_mir/const_eval.rs | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 133ab31524723..e299d4b164f1b 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -337,25 +337,23 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>> { debug!("find_mir_or_eval_fn: {:?}", instance); - // If this function is a `const fn` then as an optimization we can query this - // evaluation immediately. - // - // For the moment we only do this for functions which take no arguments - // (or all arguments are ZSTs) so that we don't memoize too much. - if ecx.tcx.is_const_fn_raw(instance.def.def_id()) && - args.iter().all(|a| a.layout.is_zst()) - { - let gid = GlobalId { instance, promoted: None }; - ecx.eval_const_fn_call(gid, ret)?; - return Ok(None); - } - // Only check non-glue functions if let ty::InstanceDef::Item(def_id) = instance.def { // Execution might have wandered off into other crates, so we cannot do a stability- // sensitive check here. But we can at least rule out functions that are not const // at all. - if !ecx.tcx.is_const_fn_raw(def_id) { + if ecx.tcx.is_const_fn_raw(def_id) { + // If this function is a `const fn` then as an optimization we can query this + // evaluation immediately. + // + // For the moment we only do this for functions which take no arguments + // (or all arguments are ZSTs) so that we don't memoize too much. + if args.iter().all(|a| a.layout.is_zst()) { + let gid = GlobalId { instance, promoted: None }; + ecx.eval_const_fn_call(gid, ret)?; + return Ok(None); + } + } else { // Some functions we support even if they are non-const -- but avoid testing // that for const fn! We certainly do *not* want to actually call the fn // though, so be sure we return here. From af8f1416e1bc004c15f2cb305bbc26e0af5478e1 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 4 Dec 2019 16:38:58 +0100 Subject: [PATCH 4/4] Update src/librustc_mir/interpret/terminator.rs Co-Authored-By: Ralf Jung --- src/librustc_mir/interpret/terminator.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 674051ecd8f16..91d304cf78e52 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -451,6 +451,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// Evaluate a const function where all arguments (if any) are zero-sized types. /// The evaluation is memoized thanks to the query system. + // FIXME: Consider moving this to `const_eval.rs`. pub (crate) fn eval_const_fn_call( &mut self, gid: GlobalId<'tcx>,