diff --git a/src/librustc_trans/trans/expr.rs b/src/librustc_trans/trans/expr.rs index 96d3e16d253b..ecdc7c06bb19 100644 --- a/src/librustc_trans/trans/expr.rs +++ b/src/librustc_trans/trans/expr.rs @@ -1500,31 +1500,14 @@ 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 { - assert_eq!(discr, 0); - - match ty::expr_kind(bcx.tcx(), &*base.expr) { - ty::RvalueDpsExpr | ty::RvalueDatumExpr if !bcx.fcx.type_needs_drop(ty) => { - bcx = trans_into(bcx, &*base.expr, SaveIn(addr)); - }, - ty::RvalueStmtExpr => bcx.tcx().sess.bug("unexpected expr kind for struct base expr"), - _ => { - let base_datum = unpack_datum!(bcx, trans_to_lvalue(bcx, &*base.expr, "base")); - for &(i, t) in &base.fields { - let datum = base_datum.get_element( - bcx, t, |srcval| adt::trans_field_ptr(bcx, &*repr, srcval, discr, i)); - assert!(type_is_sized(bcx.tcx(), datum.ty)); - let dest = adt::trans_field_ptr(bcx, &*repr, addr, discr, i); - bcx = datum.store_to(bcx, dest); - } - } - } - } - - debug_location.apply(bcx.fcx); - 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. @@ -1543,8 +1526,45 @@ pub fn trans_adt<'a, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>, 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) { + ty::RvalueDpsExpr | ty::RvalueDatumExpr if !bcx.fcx.type_needs_drop(ty) => { + bcx = trans_into(bcx, &*base.expr, SaveIn(addr)); + }, + ty::RvalueStmtExpr => bcx.tcx().sess.bug("unexpected expr kind for struct base expr"), + _ => { + let base_datum = unpack_datum!(bcx, trans_to_lvalue(bcx, &*base.expr, "base")); + for &(i, t) in &base.fields { + let datum = base_datum.get_element( + bcx, t, |srcval| adt::trans_field_ptr(bcx, &*repr, srcval, discr, i)); + assert!(type_is_sized(bcx.tcx(), datum.ty)); + let dest = adt::trans_field_ptr(bcx, &*repr, addr, discr, i); + bcx = datum.store_to(bcx, dest); + } + } + } + + // 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); + } } 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); diff --git a/src/test/run-pass/struct-order-of-eval-1.rs b/src/test/run-pass/struct-order-of-eval-1.rs index 6cebd17496a5..a64477242c08 100644 --- a/src/test/run-pass/struct-order-of-eval-1.rs +++ b/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!"); } diff --git a/src/test/run-pass/struct-order-of-eval-2.rs b/src/test/run-pass/struct-order-of-eval-2.rs index 786f080bb9ee..359ecdab630e 100644 --- a/src/test/run-pass/struct-order-of-eval-2.rs +++ b/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!"); } diff --git a/src/test/run-pass/struct-order-of-eval-3.rs b/src/test/run-pass/struct-order-of-eval-3.rs new file mode 100644 index 000000000000..856ed7c105e8 --- /dev/null +++ b/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 or the MIT license +// , 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); +} diff --git a/src/test/run-pass/struct-order-of-eval-4.rs b/src/test/run-pass/struct-order-of-eval-4.rs new file mode 100644 index 000000000000..25923beffdde --- /dev/null +++ b/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 or the MIT license +// , 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); +}