Check if out-dir exists before compiling / closes #12865

[derwolfe]

Compilation can fail if the supplied out-dir or -o contain paths that do not exist.
The error provided from rust is currently unclear:

$ echo'fn main() {}'| rustc - -o foo/bar/baz error: error: internal compiler error: unexpected failure
note: the compiler hit an unexpected failure path. this is a bug.
note: we would appreciate a bug report: http://static.rust-lang.org/doc/master/complement-bugreport.html
note: run with `RUST_BACKTRACE=1`for a backtrace
task 'rustc' failed at 'called `Option::unwrap()` on a `None` value', /Users/acrichton/code/rust2/src/libstd/option.rs:264

The solution is to check if the paths exist before compilation occurs. If a supplied path contains non-existent directories, then compilation will fail with one of two errors:

if out-dir contains a non-existent path:

$ echo'fn main() {}'| rustc - --out-dir foo/
error: output directory does not exist

if -o contains a non-existent path

$ echo'fn main() {}'| rustc - -o foo/bar/baz error: output directory does not exist, file cannot be created

Fixes#12865

[pnkfelix]

I idly wonder if this could be the same place to add a fix for #13098

More importantly, a more robust fix in the face of potential concurrent file system operations is to handle the error when you get the error variant of IoResult.

But I do not know how much more hacking that requires.

(conditions were intended for cases like this, so one need not pepper the code with result checks, but those were dropped from our libs. ..)

[flaper87]

I think these checks should go into driver::driver::build_output_filenames were a filename and a dirname is picked if none is passed through the CLI.

[flaper87]

Besides checking whether the path exists or not, it should also check if ofile exists and that it's not a directory, This should also fix #13098

[alexcrichton]

This change needs to be accompanied with tests, I would recommend writing a run-make test which all live inside of src/test/run-make. The tests are basically just a Makefile that gets run. There should be a bunch of examples to poke around at.

[derwolfe]

Thank you all for your helpful feedback. I'll be able to work on the changes later today/this evening.

[alexcrichton]

This second clause doesn't need to check for odir.exists() because the previous clause ensured that. This also may be better expressed as if !odir.is_dir() to catch more things other than files (pipes, sockets, etc).

[alexcrichton]

Thanks again for this, nice work! Could you squash the commits into one when you're done as well?

Feel free to comment on the PR whenever you force-push it, sadly github doesn't send out notification for force pushes!

[flaper87]

Great job, thanks.

It would be nice to have the out-dir / out-file paths as part of the error messages.

[derwolfe]

I force-pushed my latest set of changes.

FlaPer87: I've integrated the paths into the error messages. Right now, I am using absolute paths, though i'm not sure that is the best choice (that is, using abs paths vs just pulling the param through).

Thanks!

[alexcrichton]

It may be a bit misleading to include the full directory when it wasn't passed on the command line, perhaps this should just be odir.display() rather than the absolute version?

[derwolfe]

Hi - I have force pushed my changes. I've replaced the absolute paths with path.display(). Thanks!

[flaper87]

this looks lot better, thanks a lot @derwolfe

I'm still wondering whether these changes should go into driver::driver::build_output_filenames instead but I probably wouldn't block this patch on that.

[alexcrichton]

On second though (sorry I didn't think of this sooner), I agree with @flaper87. I think this belongs in build_output_filenames rather than in rustc/lib.rs. The doesn't currently handle the case where -o or --out-dir wasn't specified, but the output is a directory. Additionally, it also doesn't handle the case where you have --emit=foo,bar,baz and one of the output formats is a directory.

I think that all temporary/destination files are threaded through the OutputFilenames structure, so I believe that instrumenting that structure would work. You will have to instrument both the path and temp_path methods to check if the destination is a directory or whether some parent directory doesn't exist.

I'm personally inclined to just fix the ICE rather than add extra instrumentation all over the place for one small class of errors with this information in mind. As long as something is printed at some point about a missing directory or file mismatch, I don't think it's too bad.

[derwolfe]

Just an update - Over the next few days I'm going to try to fix the ICE instead of pushing in the changes to OutputFilenames.

[derwolfe]

I've also noticed a bit of a change/behavior that I didn't notice before.

At certain times rustc provides the following error originating in LLVM:

error could not write output 'bar/foo.o': No such file or directory.

Other times, the ICE shows up.

Here is the backtrace for the ICE:

 1: 0x10816aa64 - rt::backtrace::imp::write::h6ff97601cd85c669bic::v0.10.pre
2: 0x1080ce986 - rt::unwind::begin_unwind_inner::had4ea80c59b4e4ccGSb::v0.10.pre
3: 0x1080cddf8 - rt::unwind::begin_unwind::h1967b54e14ed098agSb::v0.10.pre
4: 0x108156091 - fmt::format_unsafe::hb9439769a55e7b959GS::v0.10.pre
5: 0x1081023a1 - fmt::format::h640be17b8c27dff6RGS::v0.10.pre
6: 0x10788f052 - diagnostic::print_diagnostic::hbe82976f475fca9af2b::v0.10.pre
7: 0x10788ce5a - diagnostic::EmitterWriter.Emitter::emit::h5cbba14e4efdf45dg8b::v0.10.pre
8: 0x10788ab74 - diagnostic::Handler::fatal::h663e396a5887c37fhSb::v0.10.pre
9: 0x1056fd4e8 - driver::session::Session::fatal::ha6dc5c91e6a3445ffzg::v0.10.pre
10: 0x105cd01b9 - back::link::llvm_err::h0a7a8ea2c2cda6725H0::v0.10.pre
11: 0x105cd038f - back::link::WriteOutputFile::hc4b77c1b45dd3e201I0::v0.10.pre
12: 0x105cd546f - back::link::write::run_passes::closure.82771
13: 0x105a603cf - util::common::time::h4fdfbe47d3a023bdj3g::v0.10.pre
14: 0x105cd1436 - back::link::write::run_passes::h92628adb80c67dc3gL0::v0.10.pre
15: 0x105daf081 - driver::driver::phase_5_run_llvm_passes::closure.89109
16: 0x105a603cf - util::common::time::h4fdfbe47d3a023bdj3g::v0.10.pre
17: 0x105daef1d - driver::driver::phase_5_run_llvm_passes::h85995d8a659584b4R1e::v0.10.pre
18: 0x105db1efa - driver::driver::compile_input::hec1bd171ed00cd26Qgf::v0.10.pre
19: 0x105dd64b1 - run_compiler::ha71d7d9551dbb5667Tm::v0.10.pre
20: 0x105dea3cd - main_args::closure.91027
21: 0x105de8be2 - monitor::closure.90912
22: 0x105de478b - task::TaskBuilder::try::closure.90687
23: 0x10567367c - task::spawn_opts::closure.7878
24: 0x108165df8 - rt::task::Task::run::closure.41317
25: 0x1081708dc - rust_try
26: 0x108165c77 - rt::task::Task::run::h2b84c2d9ef782ee4hI9::v0.10.pre
27: 0x1056734ff - task::spawn_opts::closure.7850
28: 0x1081693b6 - rt::thread::thread_start::h960160d5a0cf68b5Joa::v0.10.pre
29: 0x7fff86e0b899 - _pthread_body
30: 0x7fff86e0b72a - _pthread_struct_init

[alexcrichton]

@derwolfe, this looks like there may be corruption of LLVM's last error when handing it over to rust. It may not be getting allocated properly or it may be getting free'd/stomped over.

[alexcrichton]

Closing due to inactivity, but I'd love to see this ICE fixed, so feel free to reopen!

[derwolfe]

Sorry for the inactivity on this. I'll keep working on the issue.

If anyone has any tips on how to get lldb to work with rustc when compiled with debug information, I'd appreciate the help. I've had no luck setting line level break points with either lldb or gdb on OS X where the bug is present. But, I'll keep trying to get something working. Thanks!