Skip to content

Commit 4155643

Browse files
pczarnpczarn
committed
Fix double evaluation of read+write operands
Stop read+write expressions from expanding into two occurences in the AST. Add a bool to indicate whether an operand in output position if read+write or not. Fixes rust-lang#14936
1 parent 3570095 commit 4155643

File tree

12 files changed

+105
-37
lines changed

12 files changed

+105
-37
lines changed

‎src/librustc/middle/cfg/construct.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -473,14 +473,15 @@ impl<'a> CFGBuilder<'a> {
473473
ast::ExprInlineAsm(ref inline_asm) => {
474474
let inputs = inline_asm.inputs.iter();
475475
let outputs = inline_asm.outputs.iter();
476-
fn extract_expr<A>(&(_, expr): &(A, Gc<ast::Expr>)) -> Gc<ast::Expr> { expr }
477476
let post_inputs = self.exprs(inputs.map(|a| {
478477
debug!("cfg::construct InlineAsm id:{} input:{:?}", expr.id, a);
479-
extract_expr(a)
478+
let &(_, expr) = a;
479+
expr
480480
}), pred);
481481
let post_outputs = self.exprs(outputs.map(|a| {
482482
debug!("cfg::construct InlineAsm id:{} output:{:?}", expr.id, a);
483-
extract_expr(a)
483+
let &(_, expr, _) = a;
484+
expr
484485
}), post_inputs);
485486
self.add_node(expr.id, [post_outputs])
486487
}

‎src/librustc/middle/expr_use_visitor.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -380,8 +380,9 @@ impl<'d,'t,TYPER:mc::Typer> ExprUseVisitor<'d,'t,TYPER> {
380380
self.consume_expr(&**input);
381381
}
382382

383-
for &(_, ref output) in ia.outputs.iter() {
384-
self.mutate_expr(expr, &**output, JustWrite);
383+
for &(_, ref output, is_rw) in ia.outputs.iter() {
384+
self.mutate_expr(expr, &**output,
385+
if is_rw { WriteAndRead } else { JustWrite });
385386
}
386387
}
387388

‎src/librustc/middle/liveness.rs

+5-4
Original file line numberDiff line numberDiff line change
@@ -1192,9 +1192,10 @@ impl<'a> Liveness<'a> {
11921192
}
11931193

11941194
ExprInlineAsm(ref ia) => {
1195-
let succ = ia.outputs.iter().rev().fold(succ, |succ, &(_, ref expr)| {
1196-
// see comment on lvalues in
1197-
// propagate_through_lvalue_components()
1195+
1196+
let succ = ia.outputs.iter().rev().fold(succ, |succ, &(_, ref expr, _)| {
1197+
// see comment on lvalues
1198+
// in propagate_through_lvalue_components()
11981199
let succ = self.write_lvalue(&**expr, succ, ACC_WRITE);
11991200
self.propagate_through_lvalue_components(&**expr, succ)
12001201
});
@@ -1437,7 +1438,7 @@ fn check_expr(this: &mut Liveness, expr: &Expr) {
14371438
}
14381439

14391440
// Output operands must be lvalues
1440-
for &(_, ref out) in ia.outputs.iter() {
1441+
for &(_, ref out, _) in ia.outputs.iter() {
14411442
this.check_lvalue(&**out);
14421443
this.visit_expr(&**out, ());
14431444
}

‎src/librustc/middle/trans/asm.rs

+18-3
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,27 @@ pub fn trans_inline_asm<'a>(bcx: &'a Block<'a>, ia: &ast::InlineAsm)
3636

3737
let temp_scope = fcx.push_custom_cleanup_scope();
3838

39+
let mut ext_inputs = Vec::new();
40+
let mut ext_constraints = Vec::new();
41+
3942
// Prepare the output operands
40-
let outputs = ia.outputs.iter().map(|&(ref c, ref out)| {
43+
let outputs = ia.outputs.iter().enumerate().map(|(i, &(ref c, ref out, is_rw))| {
4144
constraints.push((*c).clone());
4245

4346
let out_datum = unpack_datum!(bcx, expr::trans(bcx, &**out));
4447
output_types.push(type_of::type_of(bcx.ccx(), out_datum.ty));
45-
out_datum.val
48+
let val = out_datum.val;
49+
if is_rw {
50+
ext_inputs.push(unpack_result!(bcx, {
51+
callee::trans_arg_datum(bcx,
52+
expr_ty(bcx, &**out),
53+
out_datum,
54+
cleanup::CustomScope(temp_scope),
55+
callee::DontAutorefArg)
56+
}));
57+
ext_constraints.push(i.to_string());
58+
}
59+
val
4660

4761
}).collect::<Vec<_>>();
4862

@@ -58,14 +72,15 @@ pub fn trans_inline_asm<'a>(bcx: &'a Block<'a>, ia: &ast::InlineAsm)
5872
cleanup::CustomScope(temp_scope),
5973
callee::DontAutorefArg)
6074
})
61-
}).collect::<Vec<_>>();
75+
}).collect::<Vec<_>>().append(ext_inputs.as_slice());
6276

6377
// no failure occurred preparing operands, no need to cleanup
6478
fcx.pop_custom_cleanup_scope(temp_scope);
6579

6680
let mut constraints =
6781
String::from_str(constraints.iter()
6882
.map(|s| s.get().to_string())
83+
.chain(ext_constraints.move_iter())
6984
.collect::<Vec<String>>()
7085
.connect(",")
7186
.as_slice());

‎src/librustc/middle/trans/debuginfo.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -3729,7 +3729,7 @@ fn populate_scope_map(cx: &CrateContext,
37293729
walk_expr(cx, &**exp, scope_stack, scope_map);
37303730
}
37313731

3732-
for &(_, ref exp) in outputs.iter() {
3732+
for &(_, ref exp, _) in outputs.iter() {
37333733
walk_expr(cx, &**exp, scope_stack, scope_map);
37343734
}
37353735
}

‎src/librustc/middle/typeck/check/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -3332,7 +3332,7 @@ fn check_expr_with_unifier(fcx: &FnCtxt,
33323332
for &(_, ref input) in ia.inputs.iter() {
33333333
check_expr(fcx, &**input);
33343334
}
3335-
for &(_, ref out) in ia.outputs.iter() {
3335+
for &(_, ref out, _) in ia.outputs.iter() {
33363336
check_expr(fcx, &**out);
33373337
}
33383338
fcx.write_nil(id);

‎src/libsyntax/ast.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -958,9 +958,9 @@ pub enum AsmDialect {
958958
pub struct InlineAsm {
959959
pub asm: InternedString,
960960
pub asm_str_style: StrStyle,
961-
pub clobbers: InternedString,
961+
pub outputs: Vec<(InternedString, Gc<Expr>, bool)>,
962962
pub inputs: Vec<(InternedString, Gc<Expr>)>,
963-
pub outputs: Vec<(InternedString, Gc<Expr>)>,
963+
pub clobbers: InternedString,
964964
pub volatile: bool,
965965
pub alignstack: bool,
966966
pub dialect: AsmDialect

‎src/libsyntax/ext/asm.rs

+4-14
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,6 @@ pub fn expand_asm(cx: &mut ExtCtxt, sp: Span, tts: &[ast::TokenTree])
5959

6060
let mut state = Asm;
6161

62-
let mut read_write_operands = Vec::new();
63-
6462
'statement: loop {
6563
match state {
6664
Asm => {
@@ -100,8 +98,6 @@ pub fn expand_asm(cx: &mut ExtCtxt, sp: Span, tts: &[ast::TokenTree])
10098
let output = match constraint.get().slice_shift_char() {
10199
(Some('='), _) => None,
102100
(Some('+'), operand) => {
103-
// Save a reference to the output
104-
read_write_operands.push((outputs.len(), out));
105101
Some(token::intern_and_get_ident(format!(
106102
"={}",
107103
operand).as_slice()))
@@ -112,7 +108,8 @@ pub fn expand_asm(cx: &mut ExtCtxt, sp: Span, tts: &[ast::TokenTree])
112108
}
113109
};
114110

115-
outputs.push((output.unwrap_or(constraint), out));
111+
let is_rw = output.is_some();
112+
outputs.push((output.unwrap_or(constraint), out, is_rw));
116113
}
117114
}
118115
Inputs => {
@@ -202,21 +199,14 @@ pub fn expand_asm(cx: &mut ExtCtxt, sp: Span, tts: &[ast::TokenTree])
202199
}
203200
}
204201

205-
// Append an input operand, with the form of ("0", expr)
206-
// that links to an output operand.
207-
for &(i, out) in read_write_operands.iter() {
208-
inputs.push((token::intern_and_get_ident(i.to_string().as_slice()),
209-
out));
210-
}
211-
212202
MacExpr::new(box(GC) ast::Expr {
213203
id: ast::DUMMY_NODE_ID,
214204
node: ast::ExprInlineAsm(ast::InlineAsm {
215205
asm: token::intern_and_get_ident(asm.get()),
216206
asm_str_style: asm_str_style.unwrap(),
217-
clobbers: token::intern_and_get_ident(cons.as_slice()),
218-
inputs: inputs,
219207
outputs: outputs,
208+
inputs: inputs,
209+
clobbers: token::intern_and_get_ident(cons.as_slice()),
220210
volatile: volatile,
221211
alignstack: alignstack,
222212
dialect: dialect

‎src/libsyntax/fold.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1189,8 +1189,8 @@ pub fn noop_fold_expr<T: Folder>(e: Gc<Expr>, folder: &mut T) -> Gc<Expr> {
11891189
inputs: a.inputs.iter().map(|&(ref c, input)| {
11901190
((*c).clone(), folder.fold_expr(input))
11911191
}).collect(),
1192-
outputs: a.outputs.iter().map(|&(ref c, out)| {
1193-
((*c).clone(), folder.fold_expr(out))
1192+
outputs: a.outputs.iter().map(|&(ref c, out, is_rw)| {
1193+
((*c).clone(), folder.fold_expr(out), is_rw)
11941194
}).collect(),
11951195
.. (*a).clone()
11961196
})

‎src/libsyntax/print/pprust.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -1671,8 +1671,14 @@ impl<'a> State<'a> {
16711671
try!(self.word_space(":"));
16721672

16731673
try!(self.commasep(Inconsistent, a.outputs.as_slice(),
1674-
|s, &(ref co, ref o)| {
1675-
try!(s.print_string(co.get(), ast::CookedStr));
1674+
|s, &(ref co, ref o, is_rw)| {
1675+
match co.get().slice_shift_char() {
1676+
(Some('='), operand) if is_rw => {
1677+
try!(s.print_string(format!("+{}", operand).as_slice(),
1678+
ast::CookedStr))
1679+
}
1680+
_ => try!(s.print_string(co.get(), ast::CookedStr))
1681+
}
16761682
try!(s.popen());
16771683
try!(s.print_expr(&**o));
16781684
try!(s.pclose());

‎src/libsyntax/visit.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -854,11 +854,11 @@ pub fn walk_expr<E: Clone, V: Visitor<E>>(visitor: &mut V, expression: &Expr, en
854854
ExprParen(ref subexpression) => {
855855
visitor.visit_expr(&**subexpression, env.clone())
856856
}
857-
ExprInlineAsm(ref assembler) => {
858-
for &(_, ref input) in assembler.inputs.iter() {
857+
ExprInlineAsm(ref ia) => {
858+
for &(_, ref input) in ia.inputs.iter() {
859859
visitor.visit_expr(&**input, env.clone())
860860
}
861-
for &(_, ref output) in assembler.outputs.iter() {
861+
for &(_, ref output, _) in ia.outputs.iter() {
862862
visitor.visit_expr(&**output, env.clone())
863863
}
864864
}

‎src/test/run-pass/issue-14936.rs

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
#![feature(asm, macro_rules)]
12+
13+
type History = Vec<&'static str>;
14+
15+
fn wrap<A>(x:A, which: &'static str, history: &mut History) -> A {
16+
history.push(which);
17+
x
18+
}
19+
20+
macro_rules! demo {
21+
( $output_constraint:tt ) => {
22+
{
23+
let mut x: int = 0;
24+
let y: int = 1;
25+
26+
let mut history: History = vec!();
27+
unsafe {
28+
asm!("mov ($1), $0"
29+
: $output_constraint (*wrap(&mut x, "out", &mut history))
30+
: "r"(&wrap(y, "in", &mut history)));
31+
}
32+
assert_eq!((x,y), (1,1));
33+
assert_eq!(history.as_slice(), &["out", "in"]);
34+
}
35+
}
36+
}
37+
38+
#[cfg(target_arch = "x86")]
39+
#[cfg(target_arch = "x86_64")]
40+
fn main() {
41+
fn out_write_only_expr_then_in_expr() {
42+
demo!("=r")
43+
}
44+
45+
fn out_read_write_expr_then_in_expr() {
46+
demo!("+r")
47+
}
48+
49+
out_write_only_expr_then_in_expr();
50+
out_read_write_expr_then_in_expr();
51+
}
52+
53+
#[cfg(not(target_arch = "x86"), not(target_arch = "x86_64"))]
54+
pub fn main() {}

0 commit comments

Comments
 (0)