Do not overwrite input file if it has no extension #34005

estebank · 2016-06-01T08:40:53Z

Check wether the source file has the same path as the executable to be
generated and fail early if it would overwrite the source file with the
executable.

% echo 'fn main(){}' > file && ./rustc file
error: the input file would be overwritten by the generated executable
error: aborting due to previous error

Fixes #13019

rust-highfive · 2016-06-01T08:41:06Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Aatch (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

LeoTestard · 2016-06-01T09:40:24Z

src/librustc_driver/lib.rs

+    // the source file.
+    let output_filenames = driver::build_output_filenames(&input, &odir, &ofile, &[], &sess);
+    match input_file_path {
+        Some(ifile) => if output_filenames.path(OutputType::Exe) == ifile {


Hmm... Why checking only the executable output type? rustc can generate bytecode or assembly too and, as far as I know, all could overwrite the input file.

LeoTestard · 2016-06-02T16:14:08Z

src/librustc/session/config.rs

+
+    pub fn is_path_used(&self, path: PathBuf) -> bool {
+        for flavor in self.outputs.keys() {
+            if self.path(*flavor) == path {


There is a .iter() method that directly returns an iterator over (key, value) pairs. In fact, in this context, I think you do not even care about keys and could even use .values() to iterate over values only instead.

I felt that doing it this way I was hedging against path ever changing behaviour. Having said that, I just pushed a change that doesn't use path for this check.

estebank · 2016-06-03T17:23:26Z

r? @LeoTestard

LeoTestard · 2016-06-03T18:43:16Z

@estebank I don't have the r+ rights yet, I usually ask someone (typically @pnkfelix) to r+ once I reviewed the changes. :P But looks good to me! Maybe you might want to trigger a new Travis build? (This one failed for some reason...)

estebank · 2016-06-03T19:00:36Z

@estebank I don't have the r+ rights yet, I usually ask someone (typically @pnkfelix) to r+ once I reviewed the changes. :P

r? @pnkfelix

But looks good to me!

Thanks, @LeoTestard!

Maybe you might want to trigger a new Travis build? (This one failed for some reason...)

I've been seeing travis failures when setting up the state, I have no idea why that is, but all PRs seemed to be affected for the last couple of days. Just rebased -i and pushed to trigger the build.

jonas-schievink · 2016-07-18T12:48:19Z

@estebank Travis fail is legit

pnkfelix · 2016-07-18T15:05:51Z

In particular here is the error:

failures:

---- [run-pass] run-pass/compiler-calls.rs stdout ----

error: test run failed!
status: exit code: 101
command: x86_64-unknown-linux-gnu/test/run-pass-fulldeps/compiler-calls.stage2-x86_64-unknown-linux-gnu 
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
error: the input file would be overwritten by the generated executable
thread 'main' panicked at 'assertion failed: `(left == right)` (left: `10`, right: `30`)', /build/src/test/run-pass-fulldeps/compiler-calls.rs:86
note: Run with `RUST_BACKTRACE=1` for a backtrace.

------------------------------------------

thread '[run-pass] run-pass/compiler-calls.rs' panicked at 'explicit panic', /build/src/tools/compiletest/src/runtest.rs:2243
note: Run with `RUST_BACKTRACE=1` for a backtrace.


failures:
    [run-pass] run-pass/compiler-calls.rs

test result: FAILED. 58 passed; 1 failed; 0 ignored; 0 measured

I have not attempted to dissect the problem completely; I thought the problem might be that its interpreted the test as overwriting compiler-calls, but the first argument is thrown away by the option parser, so now I don't know what the actual problem is.

@estebank in any case, you may want to revise your error message to actual include the input file name your code is predicting will be overwritten. That will make it easier for users trying to understand your message to connect it up to the potentially erroneous input they provided.

bors · 2016-08-01T11:53:35Z

☔ The latest upstream changes (presumably #34830) made this pull request unmergeable. Please resolve the merge conflicts.

nagisa · 2016-08-23T12:18:15Z

Breaking change for anybody scripting such behaviour.

Check wether the source file has the same path as the executable to be generated and fail early if it would overwrite the source file with the executable. ```bash % echo 'fn main(){}' > file && ./rustc file error: the input file "file" would be overwritten by the generated executable error: aborting due to previous error % echo 'fn main(){}' > file.rs && ./rustc file.rs -o file.rs error: the input file "file.rs" would be overwritten by the generated executable error: aborting due to previous error ```

pnkfelix · 2016-09-22T11:45:52Z

I'm closing due to inactivity, but feel free to open a new pull request if you rebase and fix the aforementioned problems.

rust-highfive assigned Aatch Jun 1, 2016

LeoTestard reviewed Jun 1, 2016
View reviewed changes

estebank force-pushed the sourcefilecheck branch from c29b8de to a0a5be1 Compare June 1, 2016 16:41

LeoTestard reviewed Jun 2, 2016
View reviewed changes

estebank force-pushed the sourcefilecheck branch from 98e5565 to ac1b32a Compare June 3, 2016 18:57

rust-highfive assigned pnkfelix and unassigned Aatch Jun 3, 2016

estebank force-pushed the sourcefilecheck branch 3 times, most recently from e2ef035 to cc6cd1f Compare June 7, 2016 01:20

estebank force-pushed the sourcefilecheck branch from cc6cd1f to f3c2f86 Compare July 7, 2016 00:22

estebank mentioned this pull request Aug 22, 2016

It is not possible to compile a file named "rust.rs" on Linux #35887

Closed

estebank force-pushed the sourcefilecheck branch from f3c2f86 to b5f8cdf Compare September 1, 2016 01:22

estebank force-pushed the sourcefilecheck branch from b5f8cdf to 9745d4e Compare September 16, 2016 23:06

pnkfelix closed this Sep 22, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not overwrite input file if it has no extension #34005

Do not overwrite input file if it has no extension #34005

estebank commented Jun 1, 2016

rust-highfive commented Jun 1, 2016

LeoTestard Jun 1, 2016 •

edited

Loading

estebank Jun 1, 2016

LeoTestard Jun 2, 2016

estebank Jun 2, 2016

estebank commented Jun 3, 2016

LeoTestard commented Jun 3, 2016

estebank commented Jun 3, 2016

jonas-schievink commented Jul 18, 2016

pnkfelix commented Jul 18, 2016

bors commented Aug 1, 2016

nagisa commented Aug 23, 2016

pnkfelix commented Sep 22, 2016

Do not overwrite input file if it has no extension #34005

Do not overwrite input file if it has no extension #34005

Conversation

estebank commented Jun 1, 2016

rust-highfive commented Jun 1, 2016

LeoTestard Jun 1, 2016 • edited Loading

Choose a reason for hiding this comment

estebank Jun 1, 2016

Choose a reason for hiding this comment

LeoTestard Jun 2, 2016

Choose a reason for hiding this comment

estebank Jun 2, 2016

Choose a reason for hiding this comment

estebank commented Jun 3, 2016

LeoTestard commented Jun 3, 2016

estebank commented Jun 3, 2016

jonas-schievink commented Jul 18, 2016

pnkfelix commented Jul 18, 2016

bors commented Aug 1, 2016

nagisa commented Aug 23, 2016

pnkfelix commented Sep 22, 2016

LeoTestard Jun 1, 2016 •

edited

Loading