diff --git a/src/librustc_trans/trans/expr.rs b/src/librustc_trans/trans/expr.rs index 60455119d5872..e1c264feb00bd 100644 --- a/src/librustc_trans/trans/expr.rs +++ b/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); 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 6cebd17496a52..a64477242c08f 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 786f080bb9ee3..359ecdab630ec 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 0000000000000..856ed7c105e8a --- /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 0000000000000..25923beffdde4 --- /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); +}