For FRU, evaluate field expressions (into scratch temps) before base expression.

Fix #23112.

Author: pnkfelix

src/librustc_trans/trans/expr.rs

@@ -1494,8 +1494,45 @@ pub fn trans_adt<'a, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
     // panic occur before the ADT as a whole is ready.
     let custom_cleanup_scope = fcx.push_custom_cleanup_scope();
 
-    // First we trans the base, if we have one, to the dest
-    if let Some(base) = optbase {
+    if ty::type_is_simd(bcx.tcx(), ty) {
+        // Issue 23112: The original logic appeared vulnerable to same
+        // order-of-eval bug. But, SIMD values are tuple-structs;
+        // i.e. functional record update (FRU) syntax is unavailable.
+        //
+        // To be safe, double-check that we did not get here via FRU.
+        assert!(optbase.is_none());
+
+        // This is the constructor of a SIMD type, such types are
+        // always primitive machine types and so do not have a
+        // destructor or require any clean-up.
+        let llty = type_of::type_of(bcx.ccx(), ty);
+
+        // keep a vector as a register, and running through the field
+        // `insertelement`ing them directly into that register
+        // (i.e. avoid GEPi and `store`s to an alloca) .
+        let mut vec_val = C_undef(llty);
+
+        for &(i, ref e) in fields {
+            let block_datum = trans(bcx, &**e);
+            bcx = block_datum.bcx;
+            let position = C_uint(bcx.ccx(), i);
+            let value = block_datum.datum.to_llscalarish(bcx);
+            vec_val = InsertElement(bcx, vec_val, value, position);
+        }
+        Store(bcx, vec_val, addr);
+    } else if let Some(base) = optbase {
+        // Issue 23112: If there is a base, then order-of-eval
+        // requires field expressions eval'ed before base expression.
+
+        // First, trans field expressions to temporary scratch values.
+        let scratch_vals: Vec<_> = fields.iter().map(|&(i, ref e)| {
+            let datum = unpack_datum!(bcx, trans(bcx, &**e));
+            (i, datum)
+        }).collect();
+
+        debug_location.apply(bcx.fcx);
+
+        // Second, trans the base to the dest.
         assert_eq!(discr, 0);
 
         match ty::expr_kind(bcx.tcx(), &*base.expr) {
@@ -1514,31 +1551,14 @@ pub fn trans_adt<'a, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
                 }
             }
         }
-    }
-
-    debug_location.apply(bcx.fcx);
-
-    if ty::type_is_simd(bcx.tcx(), ty) {
-        // This is the constructor of a SIMD type, such types are
-        // always primitive machine types and so do not have a
-        // destructor or require any clean-up.
-        let llty = type_of::type_of(bcx.ccx(), ty);
-
-        // keep a vector as a register, and running through the field
-        // `insertelement`ing them directly into that register
-        // (i.e. avoid GEPi and `store`s to an alloca) .
-        let mut vec_val = C_undef(llty);
 
-        for &(i, ref e) in fields {
-            let block_datum = trans(bcx, &**e);
-            bcx = block_datum.bcx;
-            let position = C_uint(bcx.ccx(), i);
-            let value = block_datum.datum.to_llscalarish(bcx);
-            vec_val = InsertElement(bcx, vec_val, value, position);
+        // Finally, move scratch field values into actual field locations
+        for (i, datum) in scratch_vals.into_iter() {
+            let dest = adt::trans_field_ptr(bcx, &*repr, addr, discr, i);
+            bcx = datum.store_to(bcx, dest);
         }
-        Store(bcx, vec_val, addr);
     } else {
-        // Now, we just overwrite the fields we've explicitly specified
+        // No base means we can write all fields directly in place.
         for &(i, ref e) in fields {
             let dest = adt::trans_field_ptr(bcx, &*repr, addr, discr, i);
             let e_ty = expr_ty_adjusted(bcx, &**e);

src/test/run-pass/struct-order-of-eval-1.rs

@@ -12,11 +12,12 @@ struct S { f0: String, f1: int }
 
 pub fn main() {
     let s = "Hello, world!".to_string();
-    let _s = S {
+    let s = S {
         f0: s.to_string(),
         ..S {
             f0: s,
             f1: 23
         }
     };
+    assert_eq!(s.f0, "Hello, world!");
 }

src/test/run-pass/struct-order-of-eval-2.rs

@@ -15,8 +15,9 @@ struct S {
 
 pub fn main() {
     let s = "Hello, world!".to_string();
-    let _s = S {
+    let s = S {
         f1: s.to_string(),
         f0: s
     };
+    assert_eq!(s.f0, "Hello, world!");
 }

src/test/run-pass/struct-order-of-eval-3.rs

@@ -0,0 +1,46 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Checks that functional-record-update order-of-eval is as expected
+// even when no Drop-implementations are involved.
+
+use std::sync::atomic::{Ordering, AtomicUsize, ATOMIC_USIZE_INIT};
+
+struct W { wrapped: u32 }
+struct S { f0: W, _f1: i32 }
+
+pub fn main() {
+    const VAL: u32 = 0x89AB_CDEF;
+    let w = W { wrapped: VAL };
+    let s = S {
+        f0: { event(0x01); W { wrapped: w.wrapped + 1 } },
+        ..S {
+            f0: { event(0x02); w},
+            _f1: 23
+        }
+    };
+    assert_eq!(s.f0.wrapped, VAL + 1);
+    let actual = event_log();
+    let expect = 0x01_02;
+    assert!(expect == actual,
+            "expect: 0x{:x} actual: 0x{:x}", expect, actual);
+}
+
+static LOG: AtomicUsize = ATOMIC_USIZE_INIT;
+
+fn event_log() -> usize {
+    LOG.load(Ordering::SeqCst)
+}
+
+fn event(tag: u8) {
+    let old_log = LOG.load(Ordering::SeqCst);
+    let new_log = (old_log << 8) + tag as usize;
+    LOG.store(new_log, Ordering::SeqCst);
+}

src/test/run-pass/struct-order-of-eval-4.rs

@@ -0,0 +1,43 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Checks that struct-literal expression order-of-eval is as expected
+// even when no Drop-implementations are involved.
+
+use std::sync::atomic::{Ordering, AtomicUsize, ATOMIC_USIZE_INIT};
+
+struct W { wrapped: u32 }
+struct S { f0: W, _f1: i32 }
+
+pub fn main() {
+    const VAL: u32 = 0x89AB_CDEF;
+    let w = W { wrapped: VAL };
+    let s = S {
+        _f1: { event(0x01); 23 },
+        f0: { event(0x02); w },
+    };
+    assert_eq!(s.f0.wrapped, VAL);
+    let actual = event_log();
+    let expect = 0x01_02;
+    assert!(expect == actual,
+            "expect: 0x{:x} actual: 0x{:x}", expect, actual);
+}
+
+static LOG: AtomicUsize = ATOMIC_USIZE_INIT;
+
+fn event_log() -> usize {
+    LOG.load(Ordering::SeqCst)
+}
+
+fn event(tag: u8) {
+    let old_log = LOG.load(Ordering::SeqCst);
+    let new_log = (old_log << 8) + tag as usize;
+    LOG.store(new_log, Ordering::SeqCst);
+}