From 1d808d106bc88be89eba1354e5dd724bb5816726 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Sat, 17 Nov 2018 13:50:04 +0100
Subject: [PATCH 1/2] When popping in CTFE, perform validation before jumping
 to next statement to have a better span for the error

---
 src/librustc_mir/interpret/eval_context.rs | 33 +++++++++++-----------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs
index e6267012dc2..f9c28753c2d 100644
--- a/src/librustc_mir/interpret/eval_context.rs
+++ b/src/librustc_mir/interpret/eval_context.rs
@@ -492,23 +492,6 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
         let frame = self.stack.pop().expect(
             "tried to pop a stack frame, but there were none",
         );
-        match frame.return_to_block {
-            StackPopCleanup::Goto(block) => {
-                self.goto_block(block)?;
-            }
-            StackPopCleanup::None { cleanup } => {
-                if !cleanup {
-                    // Leak the locals. Also skip validation, this is only used by
-                    // static/const computation which does its own (stronger) final
-                    // validation.
-                    return Ok(());
-                }
-            }
-        }
-        // Deallocate all locals that are backed by an allocation.
-        for local in frame.locals {
-            self.deallocate_local(local)?;
-        }
         // Validate the return value.
         if let Some(return_place) = frame.return_place {
             if M::enforce_validity(self) {
@@ -530,6 +513,22 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
             // Uh, that shouldn't happen... the function did not intend to return
             return err!(Unreachable);
         }
+        // Jump to new block -- *after* validation so that the spans make more sense.
+        match frame.return_to_block {
+            StackPopCleanup::Goto(block) => {
+                self.goto_block(block)?;
+            }
+            StackPopCleanup::None { cleanup } => {
+                if !cleanup {
+                    // Leak the locals.
+                    return Ok(());
+                }
+            }
+        }
+        // Deallocate all locals that are backed by an allocation.
+        for local in frame.locals {
+            self.deallocate_local(local)?;
+        }
 
         if self.stack.len() > 1 { // FIXME should be "> 0", printing topmost frame crashes rustc...
             debug!("CONTINUING({}) {}", self.cur_frame(), self.frame().instance);

From bcf82efe081dd12e4b6708241f5a1d165d103cae Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Sat, 17 Nov 2018 14:51:45 +0100
Subject: [PATCH 2/2] deallocate locals before validation, to catch dangling
 references

---
 src/librustc_mir/interpret/eval_context.rs | 30 ++++++++++++++--------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs
index f9c28753c2d..148442deaad 100644
--- a/src/librustc_mir/interpret/eval_context.rs
+++ b/src/librustc_mir/interpret/eval_context.rs
@@ -492,7 +492,24 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
         let frame = self.stack.pop().expect(
             "tried to pop a stack frame, but there were none",
         );
-        // Validate the return value.
+        // Abort early if we do not want to clean up: We also avoid validation in that case,
+        // because this is CTFE and the final value will be thoroughly validated anyway.
+        match frame.return_to_block {
+            StackPopCleanup::Goto(_) => {},
+            StackPopCleanup::None { cleanup } => {
+                if !cleanup {
+                    assert!(self.stack.is_empty(), "only the topmost frame should ever be leaked");
+                    // Leak the locals, skip validation.
+                    return Ok(());
+                }
+            }
+        }
+        // Deallocate all locals that are backed by an allocation.
+        for local in frame.locals {
+            self.deallocate_local(local)?;
+        }
+        // Validate the return value. Do this after deallocating so that we catch dangling
+        // references.
         if let Some(return_place) = frame.return_place {
             if M::enforce_validity(self) {
                 // Data got changed, better make sure it matches the type!
@@ -518,16 +535,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
             StackPopCleanup::Goto(block) => {
                 self.goto_block(block)?;
             }
-            StackPopCleanup::None { cleanup } => {
-                if !cleanup {
-                    // Leak the locals.
-                    return Ok(());
-                }
-            }
-        }
-        // Deallocate all locals that are backed by an allocation.
-        for local in frame.locals {
-            self.deallocate_local(local)?;
+            StackPopCleanup::None { .. } => {}
         }
 
         if self.stack.len() > 1 { // FIXME should be "> 0", printing topmost frame crashes rustc...