From df1c61d303c6557434e3dc0dd8d32eb3599d8b40 Mon Sep 17 00:00:00 2001 From: varkor Date: Fri, 5 Jan 2018 00:31:40 +0000 Subject: [PATCH 1/8] Warn when rustc output conflicts with existing directories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the compiled executable would conflict with a directory, display a rustc error instead of a verbose and potentially-confusing linker error. This is a usability improvement, and doesn’t actually change behaviour with regards to compilation success. This addresses the concern in #35887. --- src/librustc/session/config.rs | 44 ++++++++++++++----- src/librustc_driver/driver.rs | 19 +++++--- .../Makefile | 7 +++ .../foo.rs | 11 +++++ 4 files changed, 64 insertions(+), 17 deletions(-) create mode 100644 src/test/run-make/output-filename-conflicts-with-directory/Makefile create mode 100644 src/test/run-make/output-filename-conflicts-with-directory/foo.rs diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index b9546143a05..b04376000c0 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -549,23 +549,43 @@ impl OutputFilenames { format!("{}{}", self.out_filestem, self.extra) } + fn check_output(&self, f: F) -> Option where F: Fn(PathBuf) -> Option { + match self.single_output_file { + Some(ref output_path) => { + f(output_path.clone()) + }, + None => { + for k in self.outputs.keys() { + let output_path = self.path(k.to_owned()); + if let Some(result) = f(output_path) { + return Some(result); + } + } + None + } + } + } + pub fn contains_path(&self, input_path: &PathBuf) -> bool { let input_path = input_path.canonicalize().ok(); if input_path.is_none() { return false } - match self.single_output_file { - Some(ref output_path) => output_path.canonicalize().ok() == input_path, - None => { - for k in self.outputs.keys() { - let output_path = self.path(k.to_owned()); - if output_path.canonicalize().ok() == input_path { - return true; - } - } - false - } - } + let check = |output_path: PathBuf| { + if output_path.canonicalize().ok() == input_path { + Some(()) + } else { None } + }; + self.check_output(check).is_some() + } + + pub fn conflicts_with_dir(&self) -> Option { + let check = |output_path: PathBuf| { + if output_path.is_dir() { + Some(output_path) + } else { None } + }; + self.check_output(check) } } diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index e97d83ed1ee..5875ccbfd58 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -125,11 +125,20 @@ pub fn compile_input(trans: Box, // Ensure the source file isn't accidentally overwritten during compilation. match *input_path { Some(ref input_path) => { - if outputs.contains_path(input_path) && sess.opts.will_create_output_file() { - sess.err(&format!( - "the input file \"{}\" would be overwritten by the generated executable", - input_path.display())); - return Err(CompileIncomplete::Stopped); + if sess.opts.will_create_output_file() { + if outputs.contains_path(input_path) { + sess.err(&format!( + "the input file \"{}\" would be overwritten by the generated executable", + input_path.display())); + return Err(CompileIncomplete::Stopped); + } + if let Some(dir_path) = outputs.conflicts_with_dir() { + sess.err(&format!( + "the generated executable for the input file \"{}\" conflicts with the \ + existing directory \"{}\'", + input_path.display(), dir_path.display())); + return Err(CompileIncomplete::Stopped); + } } }, None => {} diff --git a/src/test/run-make/output-filename-conflicts-with-directory/Makefile b/src/test/run-make/output-filename-conflicts-with-directory/Makefile new file mode 100644 index 00000000000..68f72094ce6 --- /dev/null +++ b/src/test/run-make/output-filename-conflicts-with-directory/Makefile @@ -0,0 +1,7 @@ +-include ../tools.mk + +all: + cp foo.rs $(TMPDIR)/foo.rs + mkdir $(TMPDIR)/foo + $(RUSTC) $(TMPDIR)/foo.rs 2>&1 \ + | $(CGREP) -e "the generated executable for the input file \".*foo\.rs\" conflicts with the existing directory \".*foo\'" diff --git a/src/test/run-make/output-filename-conflicts-with-directory/foo.rs b/src/test/run-make/output-filename-conflicts-with-directory/foo.rs new file mode 100644 index 00000000000..046d27a9f0f --- /dev/null +++ b/src/test/run-make/output-filename-conflicts-with-directory/foo.rs @@ -0,0 +1,11 @@ +// Copyright 2017 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. + +fn main() {} From dc274e68a7085491519cb4f3c5613c0321751c6f Mon Sep 17 00:00:00 2001 From: varkor Date: Fri, 5 Jan 2018 00:41:37 +0000 Subject: [PATCH 2/8] Fix tidy error --- src/librustc_driver/driver.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 5875ccbfd58..c6b51f69406 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -128,7 +128,8 @@ pub fn compile_input(trans: Box, if sess.opts.will_create_output_file() { if outputs.contains_path(input_path) { sess.err(&format!( - "the input file \"{}\" would be overwritten by the generated executable", + "the input file \"{}\" would be overwritten by the generated \ + executable", input_path.display())); return Err(CompileIncomplete::Stopped); } From 3d55974be4142244d98007c9557ac93e9b9b7d0d Mon Sep 17 00:00:00 2001 From: varkor Date: Fri, 5 Jan 2018 10:15:52 +0000 Subject: [PATCH 3/8] Fix quotation mark --- src/librustc_driver/driver.rs | 2 +- .../run-make/output-filename-conflicts-with-directory/Makefile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index c6b51f69406..05bf5e5a7d9 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -136,7 +136,7 @@ pub fn compile_input(trans: Box, if let Some(dir_path) = outputs.conflicts_with_dir() { sess.err(&format!( "the generated executable for the input file \"{}\" conflicts with the \ - existing directory \"{}\'", + existing directory \"{}\"", input_path.display(), dir_path.display())); return Err(CompileIncomplete::Stopped); } diff --git a/src/test/run-make/output-filename-conflicts-with-directory/Makefile b/src/test/run-make/output-filename-conflicts-with-directory/Makefile index 68f72094ce6..74dea9ce72b 100644 --- a/src/test/run-make/output-filename-conflicts-with-directory/Makefile +++ b/src/test/run-make/output-filename-conflicts-with-directory/Makefile @@ -4,4 +4,4 @@ all: cp foo.rs $(TMPDIR)/foo.rs mkdir $(TMPDIR)/foo $(RUSTC) $(TMPDIR)/foo.rs 2>&1 \ - | $(CGREP) -e "the generated executable for the input file \".*foo\.rs\" conflicts with the existing directory \".*foo\'" + | $(CGREP) -e "the generated executable for the input file \".*foo\.rs\" conflicts with the existing directory \".*foo\"" From 45e962ecc023749fed2bb926c3386059c8e1407a Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 7 Jan 2018 01:11:23 +0000 Subject: [PATCH 4/8] Use correct output file paths for error checking --- src/librustc/session/config.rs | 39 -------------- src/librustc_driver/driver.rs | 98 ++++++++++++++++++++++++---------- 2 files changed, 70 insertions(+), 67 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index b04376000c0..af91e294db6 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -548,45 +548,6 @@ impl OutputFilenames { pub fn filestem(&self) -> String { format!("{}{}", self.out_filestem, self.extra) } - - fn check_output(&self, f: F) -> Option where F: Fn(PathBuf) -> Option { - match self.single_output_file { - Some(ref output_path) => { - f(output_path.clone()) - }, - None => { - for k in self.outputs.keys() { - let output_path = self.path(k.to_owned()); - if let Some(result) = f(output_path) { - return Some(result); - } - } - None - } - } - } - - pub fn contains_path(&self, input_path: &PathBuf) -> bool { - let input_path = input_path.canonicalize().ok(); - if input_path.is_none() { - return false - } - let check = |output_path: PathBuf| { - if output_path.canonicalize().ok() == input_path { - Some(()) - } else { None } - }; - self.check_output(check).is_some() - } - - pub fn conflicts_with_dir(&self) -> Option { - let check = |output_path: PathBuf| { - if output_path.is_dir() { - Some(output_path) - } else { None } - }; - self.check_output(check) - } } pub fn host_triple() -> &'static str { diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 05bf5e5a7d9..a8ba795845f 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -121,33 +121,8 @@ pub fn compile_input(trans: Box, }; let outputs = build_output_filenames(input, outdir, output, &krate.attrs, sess); - - // Ensure the source file isn't accidentally overwritten during compilation. - match *input_path { - Some(ref input_path) => { - if sess.opts.will_create_output_file() { - if outputs.contains_path(input_path) { - sess.err(&format!( - "the input file \"{}\" would be overwritten by the generated \ - executable", - input_path.display())); - return Err(CompileIncomplete::Stopped); - } - if let Some(dir_path) = outputs.conflicts_with_dir() { - sess.err(&format!( - "the generated executable for the input file \"{}\" conflicts with the \ - existing directory \"{}\"", - input_path.display(), dir_path.display())); - return Err(CompileIncomplete::Stopped); - } - } - }, - None => {} - } - let crate_name = ::rustc_trans_utils::link::find_crate_name(Some(sess), &krate.attrs, input); - let ExpansionResult { expanded_crate, defs, analysis, resolutions, mut hir_forest } = { phase_2_configure_and_expand( sess, @@ -166,8 +141,33 @@ pub fn compile_input(trans: Box, } )? }; + + let output_paths = generated_output_paths(sess, &outputs, &crate_name); - write_out_deps(sess, &outputs, &crate_name); + // Ensure the source file isn't accidentally overwritten during compilation. + match *input_path { + Some(ref input_path) => { + if sess.opts.will_create_output_file() { + if output_contains_path(&output_paths, input_path) { + sess.err(&format!( + "the input file \"{}\" would be overwritten by the generated \ + executable", + input_path.display())); + return Err(CompileIncomplete::Stopped); + } + if let Some(dir_path) = output_conflicts_with_dir(&output_paths) { + sess.err(&format!( + "the generated executable for the input file \"{}\" conflicts with the \ + existing directory \"{}\"", + input_path.display(), dir_path.display())); + return Err(CompileIncomplete::Stopped); + } + } + }, + None => {} + } + + write_out_deps(sess, &outputs, &output_paths); if sess.opts.output_types.contains_key(&OutputType::DepInfo) && sess.opts.output_types.keys().count() == 1 { return Ok(()) @@ -1111,7 +1111,10 @@ fn escape_dep_filename(filename: &FileName) -> String { filename.to_string().replace(" ", "\\ ") } -fn write_out_deps(sess: &Session, outputs: &OutputFilenames, crate_name: &str) { +// Returns all the paths that correspond to generated files. +fn generated_output_paths(sess: &Session, + outputs: &OutputFilenames, + crate_name: &str) -> Vec { let mut out_filenames = Vec::new(); for output_type in sess.opts.output_types.keys() { let file = outputs.path(*output_type); @@ -1135,7 +1138,46 @@ fn write_out_deps(sess: &Session, outputs: &OutputFilenames, crate_name: &str) { } } } + out_filenames +} +// Runs `f` on every output file path and returns the first non-None result, or None if `f` +// returns None for every file path. +fn check_output(output_paths: &Vec, f: F) -> Option + where F: Fn(&PathBuf) -> Option { + for output_path in output_paths { + if let Some(result) = f(output_path) { + return Some(result); + } + } + None +} + +pub fn output_contains_path(output_paths: &Vec, input_path: &PathBuf) -> bool { + let input_path = input_path.canonicalize().ok(); + if input_path.is_none() { + return false + } + let check = |output_path: &PathBuf| { + if output_path.canonicalize().ok() == input_path { + Some(()) + } else { None } + }; + check_output(output_paths, check).is_some() +} + +pub fn output_conflicts_with_dir(output_paths: &Vec) -> Option { + let check = |output_path: &PathBuf| { + if output_path.is_dir() { + Some(output_path.clone()) + } else { None } + }; + check_output(output_paths, check) +} + +fn write_out_deps(sess: &Session, + outputs: &OutputFilenames, + out_filenames: &Vec) { // Write out dependency rules to the dep-info file if requested if !sess.opts.output_types.contains_key(&OutputType::DepInfo) { return; @@ -1154,7 +1196,7 @@ fn write_out_deps(sess: &Session, outputs: &OutputFilenames, crate_name: &str) { .map(|fmap| escape_dep_filename(&fmap.name)) .collect(); let mut file = fs::File::create(&deps_filename)?; - for path in &out_filenames { + for path in out_filenames { write!(file, "{}: {}\n\n", path.display(), files.join(" "))?; } From ba76092c2ef771f4b4463ec5e879f12194e59ec5 Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 7 Jan 2018 12:03:24 +0000 Subject: [PATCH 5/8] Fix tidy error --- src/librustc_driver/driver.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index a8ba795845f..4fe51138f9a 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -141,7 +141,7 @@ pub fn compile_input(trans: Box, } )? }; - + let output_paths = generated_output_paths(sess, &outputs, &crate_name); // Ensure the source file isn't accidentally overwritten during compilation. From d97da7d536b85bda9ff9255b1cd27f1285143930 Mon Sep 17 00:00:00 2001 From: varkor Date: Tue, 9 Jan 2018 19:55:36 +0000 Subject: [PATCH 6/8] Minor refactoring --- src/librustc_driver/driver.rs | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 4fe51138f9a..1ab5f19c2e2 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -145,26 +145,23 @@ pub fn compile_input(trans: Box, let output_paths = generated_output_paths(sess, &outputs, &crate_name); // Ensure the source file isn't accidentally overwritten during compilation. - match *input_path { - Some(ref input_path) => { - if sess.opts.will_create_output_file() { - if output_contains_path(&output_paths, input_path) { - sess.err(&format!( - "the input file \"{}\" would be overwritten by the generated \ - executable", - input_path.display())); - return Err(CompileIncomplete::Stopped); - } - if let Some(dir_path) = output_conflicts_with_dir(&output_paths) { - sess.err(&format!( - "the generated executable for the input file \"{}\" conflicts with the \ - existing directory \"{}\"", - input_path.display(), dir_path.display())); - return Err(CompileIncomplete::Stopped); - } + if let Some(ref input_path) = *input_path { + if sess.opts.will_create_output_file() { + if output_contains_path(&output_paths, input_path) { + sess.err(&format!( + "the input file \"{}\" would be overwritten by the generated \ + executable", + input_path.display())); + return Err(CompileIncomplete::Stopped); } - }, - None => {} + if let Some(dir_path) = output_conflicts_with_dir(&output_paths) { + sess.err(&format!( + "the generated executable for the input file \"{}\" conflicts with the \ + existing directory \"{}\"", + input_path.display(), dir_path.display())); + return Err(CompileIncomplete::Stopped); + } + } } write_out_deps(sess, &outputs, &output_paths); From 9a2f02df6655f65e5a7892e853cc2112e28b89e8 Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 29 Jan 2018 12:50:36 +0000 Subject: [PATCH 7/8] Warn when `-C extra-filename` flag is used with `-o` --- src/librustc_driver/driver.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 1ab5f19c2e2..df8860823ad 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -1378,6 +1378,9 @@ pub fn build_output_filenames(input: &Input, if *odir != None { sess.warn("ignoring --out-dir flag due to -o flag."); } + if !sess.opts.cg.extra_filename.is_empty() { + sess.warn("ignoring -C extra-filename flag due to -o flag."); + } let cur_dir = Path::new(""); From e92bdb9828ff19afc92c292b289be1790f2ee116 Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 29 Jan 2018 14:38:50 +0000 Subject: [PATCH 8/8] Specify output filenames for compatibility with Windows --- src/librustc_driver/driver.rs | 15 +++++++++------ .../Makefile | 2 +- .../foo.rs | 2 +- .../output-filename-overwrites-input/Makefile | 5 ++++- .../output-filename-overwrites-input/bar.rs | 11 +++++++++++ .../output-filename-overwrites-input/foo.rs | 2 +- 6 files changed, 27 insertions(+), 10 deletions(-) create mode 100644 src/test/run-make/output-filename-overwrites-input/bar.rs diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index df8860823ad..50c19b5a99a 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -142,7 +142,7 @@ pub fn compile_input(trans: Box, )? }; - let output_paths = generated_output_paths(sess, &outputs, &crate_name); + let output_paths = generated_output_paths(sess, &outputs, output.is_some(), &crate_name); // Ensure the source file isn't accidentally overwritten during compilation. if let Some(ref input_path) = *input_path { @@ -1111,16 +1111,19 @@ fn escape_dep_filename(filename: &FileName) -> String { // Returns all the paths that correspond to generated files. fn generated_output_paths(sess: &Session, outputs: &OutputFilenames, + exact_name: bool, crate_name: &str) -> Vec { let mut out_filenames = Vec::new(); for output_type in sess.opts.output_types.keys() { let file = outputs.path(*output_type); match *output_type { - OutputType::Exe => { - for output in sess.crate_types.borrow().iter() { + // If the filename has been overridden using `-o`, it will not be modified + // by appending `.rlib`, `.exe`, etc., so we can skip this transformation. + OutputType::Exe if !exact_name => { + for crate_type in sess.crate_types.borrow().iter() { let p = ::rustc_trans_utils::link::filename_for_input( sess, - *output, + *crate_type, crate_name, outputs ); @@ -1376,10 +1379,10 @@ pub fn build_output_filenames(input: &Input, Some(out_file.clone()) }; if *odir != None { - sess.warn("ignoring --out-dir flag due to -o flag."); + sess.warn("ignoring --out-dir flag due to -o flag"); } if !sess.opts.cg.extra_filename.is_empty() { - sess.warn("ignoring -C extra-filename flag due to -o flag."); + sess.warn("ignoring -C extra-filename flag due to -o flag"); } let cur_dir = Path::new(""); diff --git a/src/test/run-make/output-filename-conflicts-with-directory/Makefile b/src/test/run-make/output-filename-conflicts-with-directory/Makefile index 74dea9ce72b..74e5dcfcf36 100644 --- a/src/test/run-make/output-filename-conflicts-with-directory/Makefile +++ b/src/test/run-make/output-filename-conflicts-with-directory/Makefile @@ -3,5 +3,5 @@ all: cp foo.rs $(TMPDIR)/foo.rs mkdir $(TMPDIR)/foo - $(RUSTC) $(TMPDIR)/foo.rs 2>&1 \ + $(RUSTC) $(TMPDIR)/foo.rs -o $(TMPDIR)/foo 2>&1 \ | $(CGREP) -e "the generated executable for the input file \".*foo\.rs\" conflicts with the existing directory \".*foo\"" diff --git a/src/test/run-make/output-filename-conflicts-with-directory/foo.rs b/src/test/run-make/output-filename-conflicts-with-directory/foo.rs index 046d27a9f0f..3f07b46791d 100644 --- a/src/test/run-make/output-filename-conflicts-with-directory/foo.rs +++ b/src/test/run-make/output-filename-conflicts-with-directory/foo.rs @@ -1,4 +1,4 @@ -// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. // diff --git a/src/test/run-make/output-filename-overwrites-input/Makefile b/src/test/run-make/output-filename-overwrites-input/Makefile index 0554627d677..6377038b7be 100644 --- a/src/test/run-make/output-filename-overwrites-input/Makefile +++ b/src/test/run-make/output-filename-overwrites-input/Makefile @@ -2,8 +2,11 @@ all: cp foo.rs $(TMPDIR)/foo - $(RUSTC) $(TMPDIR)/foo 2>&1 \ + $(RUSTC) $(TMPDIR)/foo -o $(TMPDIR)/foo 2>&1 \ | $(CGREP) -e "the input file \".*foo\" would be overwritten by the generated executable" + cp bar.rs $(TMPDIR)/bar.rlib + $(RUSTC) $(TMPDIR)/bar.rlib -o $(TMPDIR)/bar.rlib 2>&1 \ + | $(CGREP) -e "the input file \".*bar.rlib\" would be overwritten by the generated executable" $(RUSTC) foo.rs 2>&1 && $(RUSTC) -Z ls $(TMPDIR)/foo 2>&1 cp foo.rs $(TMPDIR)/foo.rs $(RUSTC) $(TMPDIR)/foo.rs -o $(TMPDIR)/foo.rs 2>&1 \ diff --git a/src/test/run-make/output-filename-overwrites-input/bar.rs b/src/test/run-make/output-filename-overwrites-input/bar.rs new file mode 100644 index 00000000000..8e4e35fdee6 --- /dev/null +++ b/src/test/run-make/output-filename-overwrites-input/bar.rs @@ -0,0 +1,11 @@ +// Copyright 2018 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. + +#![crate_type = "lib"] diff --git a/src/test/run-make/output-filename-overwrites-input/foo.rs b/src/test/run-make/output-filename-overwrites-input/foo.rs index 046d27a9f0f..3f07b46791d 100644 --- a/src/test/run-make/output-filename-overwrites-input/foo.rs +++ b/src/test/run-make/output-filename-overwrites-input/foo.rs @@ -1,4 +1,4 @@ -// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. //