From cd09c2b2abf2c03b80d0d48c9828004ade64a8f8 Mon Sep 17 00:00:00 2001 From: nabijaczleweli Date: Tue, 27 Mar 2018 23:09:35 +0200 Subject: [PATCH 1/3] Flush executables to disk after linkage A problem caused by not doing so in Chrome has been reported here: https://randomascii.wordpress.com/2018/02/25/compiler-bug-linker-bug-windows-kernel-bug/amp/ File::sync_all() calls FlushFileBuffers() down the line, causing potentially unflushed buffers on high I/O-load systems to flush and prevent nasty non-reproducible bugs. The force-flush is only done on Windows and if the linker exited successfully Closes #48545 --- src/librustc_trans/back/link.rs | 36 ++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/src/librustc_trans/back/link.rs b/src/librustc_trans/back/link.rs index 75ba83a7c620a..33f6ce9975e53 100644 --- a/src/librustc_trans/back/link.rs +++ b/src/librustc_trans/back/link.rs @@ -692,7 +692,7 @@ fn link_natively(sess: &Session, loop { i += 1; prog = time(sess, "running linker", || { - exec_linker(sess, &mut cmd, tmpdir) + exec_linker(sess, &mut cmd, out_filename, tmpdir) }); let output = match prog { Ok(ref output) => output, @@ -819,7 +819,7 @@ fn link_natively(sess: &Session, } } -fn exec_linker(sess: &Session, cmd: &mut Command, tmpdir: &Path) +fn exec_linker(sess: &Session, cmd: &mut Command, out_filename: &Path, tmpdir: &Path) -> io::Result { // When attempting to spawn the linker we run a risk of blowing out the @@ -867,7 +867,37 @@ fn exec_linker(sess: &Session, cmd: &mut Command, tmpdir: &Path) fs::write(&file, &bytes)?; cmd2.arg(format!("@{}", file.display())); info!("invoking linker {:?}", cmd2); - return cmd2.output(); + let output = cmd2.output(); + flush_linked_file(&output, out_filename)?; + return output; + + #[cfg(unix)] + fn flush_linked_file(_: &io::Result, _: &Path) -> io::Result<()> { + Ok(()) + } + + #[cfg(windows)] + fn flush_linked_file(command_output: &io::Result, out_filename: &Path) + -> io::Result<()> + { + // On Windows, under high I/O load, output buffers are sometimes not flushed, + // even long after process exit, causing nasty, non-reproducible output bugs. + // + // File::sync_all() calls FlushFileBuffers() down the line, which solves the problem. + // + // А full writeup of the original Chrome bug can be found at + // randomascii.wordpress.com/2018/02/25/compiler-bug-linker-bug-windows-kernel-bug/amp + + if let &Ok(ref out) = command_output { + if out.status.success() { + if let Ok(of) = fs::File::open(out_filename) { + of.sync_all()?; + } + } + } + + Ok(()) + } #[cfg(unix)] fn command_line_too_big(err: &io::Error) -> bool { From 3787106be6e3a9be377aecc8600a2e57aa1afb58 Mon Sep 17 00:00:00 2001 From: nabijaczleweli Date: Thu, 29 Mar 2018 17:50:57 +0200 Subject: [PATCH 2/3] Also protect first attempt --- src/librustc_trans/back/link.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/librustc_trans/back/link.rs b/src/librustc_trans/back/link.rs index 33f6ce9975e53..46defb9e73343 100644 --- a/src/librustc_trans/back/link.rs +++ b/src/librustc_trans/back/link.rs @@ -833,7 +833,11 @@ fn exec_linker(sess: &Session, cmd: &mut Command, out_filename: &Path, tmpdir: & // there instead of looking at the command line. if !cmd.very_likely_to_exceed_some_spawn_limit() { match cmd.command().stdout(Stdio::piped()).stderr(Stdio::piped()).spawn() { - Ok(child) => return child.wait_with_output(), + Ok(child) => { + let output = child.wait_with_output(); + flush_linked_file(&output, out_filename)?; + return output; + } Err(ref e) if command_line_too_big(e) => { info!("command line to linker was too big: {}", e); } From e1d3c471d75bbc4360eee17178ccb32dce348542 Mon Sep 17 00:00:00 2001 From: nabijaczleweli Date: Sat, 31 Mar 2018 19:12:29 +0200 Subject: [PATCH 3/3] Open the file as write before trying to flush it This should be enough and shouldn't require append(true) since we're not explicitly writing anything so we're not flushing it so we've no risk of overwriting it --- src/librustc_trans/back/link.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_trans/back/link.rs b/src/librustc_trans/back/link.rs index 46defb9e73343..5da4667af910c 100644 --- a/src/librustc_trans/back/link.rs +++ b/src/librustc_trans/back/link.rs @@ -894,7 +894,7 @@ fn exec_linker(sess: &Session, cmd: &mut Command, out_filename: &Path, tmpdir: & if let &Ok(ref out) = command_output { if out.status.success() { - if let Ok(of) = fs::File::open(out_filename) { + if let Ok(of) = fs::OpenOptions::new().write(true).open(out_filename) { of.sync_all()?; } }