From de7845ac72e01b491b4ed352f23c2c9a73efc45b Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 15 Apr 2014 07:25:22 -0700 Subject: [PATCH 1/3] rustc: Fix passing errors from LLVM to rustc Many of the instances of setting a global error variable ended up leaving a dangling pointer into free'd memory. This changes the method of error transmission to strdup any error and "relinquish ownership" to rustc when it gets an error. The corresponding Rust code will then free the error as necessary. Closes #12865 --- src/librustc/back/link.rs | 2 +- src/rustllvm/PassWrapper.cpp | 4 ++-- src/rustllvm/RustWrapper.cpp | 36 +++++++++++++++++++++++------------- src/rustllvm/rustllvm.h | 2 +- 4 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/librustc/back/link.rs b/src/librustc/back/link.rs index 9fd3894d7948e..f6846acaa9918 100644 --- a/src/librustc/back/link.rs +++ b/src/librustc/back/link.rs @@ -60,7 +60,7 @@ pub fn llvm_err(sess: &Session, msg: ~str) -> ! { if cstr == ptr::null() { sess.fatal(msg); } else { - let err = CString::new(cstr, false); + let err = CString::new(cstr, true); let err = str::from_utf8_lossy(err.as_bytes()); sess.fatal(msg + ": " + err.as_slice()); } diff --git a/src/rustllvm/PassWrapper.cpp b/src/rustllvm/PassWrapper.cpp index 32bac73debfb9..021dda4976550 100644 --- a/src/rustllvm/PassWrapper.cpp +++ b/src/rustllvm/PassWrapper.cpp @@ -75,7 +75,7 @@ LLVMRustCreateTargetMachine(const char *triple, const llvm::Target *TheTarget = TargetRegistry::lookupTarget(Trip.getTriple(), Error); if (TheTarget == NULL) { - LLVMRustError = Error.c_str(); + LLVMRustSetLastError(Error.c_str()); return NULL; } @@ -178,7 +178,7 @@ LLVMRustWriteOutputFile(LLVMTargetMachineRef Target, raw_fd_ostream OS(path, ErrorInfo, raw_fd_ostream::F_Binary); #endif if (ErrorInfo != "") { - LLVMRustError = ErrorInfo.c_str(); + LLVMRustSetLastError(ErrorInfo.c_str()); return false; } formatted_raw_ostream FOS(OS); diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index 3fe1b1380da01..ec33b750358bb 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -23,18 +23,28 @@ using namespace llvm; using namespace llvm::sys; using namespace llvm::object; -const char *LLVMRustError; +static char *LastError; extern "C" LLVMMemoryBufferRef LLVMRustCreateMemoryBufferWithContentsOfFile(const char *Path) { LLVMMemoryBufferRef MemBuf = NULL; - LLVMCreateMemoryBufferWithContentsOfFile(Path, &MemBuf, - const_cast(&LLVMRustError)); + char *err = NULL; + LLVMCreateMemoryBufferWithContentsOfFile(Path, &MemBuf, &err); + if (err != NULL) { + LLVMRustSetLastError(err); + } return MemBuf; } -extern "C" const char *LLVMRustGetLastError(void) { - return LLVMRustError; +extern "C" char *LLVMRustGetLastError(void) { + char *ret = LastError; + LastError = NULL; + return ret; +} + +void LLVMRustSetLastError(const char *err) { + free((void*) LastError); + LastError = strdup(err); } extern "C" void @@ -609,14 +619,14 @@ LLVMRustLinkInExternalBitcode(LLVMModuleRef dst, char *bc, size_t len) { MemoryBuffer* buf = MemoryBuffer::getMemBufferCopy(StringRef(bc, len)); ErrorOr Src = llvm::getLazyBitcodeModule(buf, Dst->getContext()); if (!Src) { - LLVMRustError = Src.getError().message().c_str(); + LLVMRustSetLastError(Src.getError().message().c_str()); delete buf; return false; } std::string Err; if (Linker::LinkModules(Dst, *Src, Linker::DestroySource, &Err)) { - LLVMRustError = Err.c_str(); + LLVMRustSetLastError(Err.c_str()); return false; } return true; @@ -629,13 +639,13 @@ LLVMRustLinkInExternalBitcode(LLVMModuleRef dst, char *bc, size_t len) { std::string Err; Module *Src = llvm::getLazyBitcodeModule(buf, Dst->getContext(), &Err); if (!Src) { - LLVMRustError = Err.c_str(); + LLVMRustSetLastError(Err.c_str()); delete buf; return false; } if (Linker::LinkModules(Dst, Src, Linker::DestroySource, &Err)) { - LLVMRustError = Err.c_str(); + LLVMRustSetLastError(Err.c_str()); return false; } return true; @@ -648,12 +658,12 @@ LLVMRustOpenArchive(char *path) { std::unique_ptr buf; error_code err = MemoryBuffer::getFile(path, buf); if (err) { - LLVMRustError = err.message().c_str(); + LLVMRustSetLastError(err.message().c_str()); return NULL; } Archive *ret = new Archive(buf.release(), err); if (err) { - LLVMRustError = err.message().c_str(); + LLVMRustSetLastError(err.message().c_str()); return NULL; } return ret; @@ -664,12 +674,12 @@ LLVMRustOpenArchive(char *path) { OwningPtr buf; error_code err = MemoryBuffer::getFile(path, buf); if (err) { - LLVMRustError = err.message().c_str(); + LLVMRustSetLastError(err.message().c_str()); return NULL; } Archive *ret = new Archive(buf.take(), err); if (err) { - LLVMRustError = err.message().c_str(); + LLVMRustSetLastError(err.message().c_str()); return NULL; } return ret; diff --git a/src/rustllvm/rustllvm.h b/src/rustllvm/rustllvm.h index 42c60e72baba7..5722eea48d7d8 100644 --- a/src/rustllvm/rustllvm.h +++ b/src/rustllvm/rustllvm.h @@ -68,4 +68,4 @@ #include #endif -extern const char* LLVMRustError; +void LLVMRustSetLastError(const char*); From c62daa6ed3866d8649ec8654a09c57f4078e1fdd Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 15 Apr 2014 07:35:17 -0700 Subject: [PATCH 2/3] rustc: Give a friendlier error when writing deps When an error is encountered when writing dependencies, this presents a nicer error rather than an ICE. Closes #13517 --- src/librustc/driver/driver.rs | 44 +++++++++++-------- .../error-writing-dependencies/Makefile | 8 ++++ .../error-writing-dependencies/foo.rs | 11 +++++ 3 files changed, 44 insertions(+), 19 deletions(-) create mode 100644 src/test/run-make/error-writing-dependencies/Makefile create mode 100644 src/test/run-make/error-writing-dependencies/foo.rs diff --git a/src/librustc/driver/driver.rs b/src/librustc/driver/driver.rs index cf0e7e161c1fc..6674ef8abb4ac 100644 --- a/src/librustc/driver/driver.rs +++ b/src/librustc/driver/driver.rs @@ -495,7 +495,7 @@ pub fn stop_after_phase_5(sess: &Session) -> bool { fn write_out_deps(sess: &Session, input: &Input, outputs: &OutputFilenames, - krate: &ast::Crate) -> io::IoResult<()> { + krate: &ast::Crate) { let id = link::find_crate_id(krate.attrs.as_slice(), outputs.out_filestem); let mut out_filenames = Vec::new(); @@ -524,28 +524,34 @@ fn write_out_deps(sess: &Session, StrInput(..) => { sess.warn("can not write --dep-info without a filename \ when compiling stdin."); - return Ok(()); + return }, }, - _ => return Ok(()), + _ => return, }; - // Build a list of files used to compile the output and - // write Makefile-compatible dependency rules - let files: Vec<~str> = sess.codemap().files.borrow() - .iter().filter_map(|fmap| { - if fmap.is_real_file() { - Some(fmap.name.clone()) - } else { - None - } - }).collect(); - let mut file = try!(io::File::create(&deps_filename)); - for path in out_filenames.iter() { - try!(write!(&mut file as &mut Writer, - "{}: {}\n\n", path.display(), files.connect(" "))); + let result = (|| { + // Build a list of files used to compile the output and + // write Makefile-compatible dependency rules + let files: Vec<~str> = sess.codemap().files.borrow() + .iter().filter(|fmap| fmap.is_real_file()) + .map(|fmap| fmap.name.clone()) + .collect(); + let mut file = try!(io::File::create(&deps_filename)); + for path in out_filenames.iter() { + try!(write!(&mut file as &mut Writer, + "{}: {}\n\n", path.display(), files.connect(" "))); + } + Ok(()) + })(); + + match result { + Ok(()) => {} + Err(e) => { + sess.fatal(format!("error writing dependencies to `{}`: {}", + deps_filename.display(), e)); + } } - Ok(()) } pub fn compile_input(sess: Session, cfg: ast::CrateConfig, input: &Input, @@ -569,7 +575,7 @@ pub fn compile_input(sess: Session, cfg: ast::CrateConfig, input: &Input, krate, &id); (outputs, expanded_crate, ast_map) }; - write_out_deps(&sess, input, &outputs, &expanded_crate).unwrap(); + write_out_deps(&sess, input, &outputs, &expanded_crate); if stop_after_phase_2(&sess) { return; } diff --git a/src/test/run-make/error-writing-dependencies/Makefile b/src/test/run-make/error-writing-dependencies/Makefile new file mode 100644 index 0000000000000..9f91618bda493 --- /dev/null +++ b/src/test/run-make/error-writing-dependencies/Makefile @@ -0,0 +1,8 @@ +-include ../tools.mk + +all: + # Let's get a nice error message + $(RUSTC) foo.rs --dep-info foo/bar/baz 2>&1 | \ + grep "error writing dependencies" + # Make sure the filename shows up + $(RUSTC) foo.rs --dep-info foo/bar/baz 2>&1 | grep "baz" diff --git a/src/test/run-make/error-writing-dependencies/foo.rs b/src/test/run-make/error-writing-dependencies/foo.rs new file mode 100644 index 0000000000000..8ae3d072362ed --- /dev/null +++ b/src/test/run-make/error-writing-dependencies/foo.rs @@ -0,0 +1,11 @@ +// Copyright 2014 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 b0d85e30b75fb7a2568dba9a535e8ea8e0291fef Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 15 Apr 2014 07:47:26 -0700 Subject: [PATCH 3/3] rustc: Don't die when a crate id can't be inferred The filestem of the desired output isn't necessarily a valid crate id, and calling unwrap() will trigger an ICE in rustc. This tries a little harder to infer a "valid crate id" from a crate, with an eventual fallback to a generic crate id if alll else fails. Closes #11107 --- src/librustc/back/link.rs | 5 ++++- src/test/run-make/weird-output-filenames/Makefile | 9 +++++++++ src/test/run-make/weird-output-filenames/foo.rs | 11 +++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 src/test/run-make/weird-output-filenames/Makefile create mode 100644 src/test/run-make/weird-output-filenames/foo.rs diff --git a/src/librustc/back/link.rs b/src/librustc/back/link.rs index f6846acaa9918..17a09aa490901 100644 --- a/src/librustc/back/link.rs +++ b/src/librustc/back/link.rs @@ -516,7 +516,10 @@ pub mod write { pub fn find_crate_id(attrs: &[ast::Attribute], out_filestem: &str) -> CrateId { match attr::find_crateid(attrs) { - None => from_str(out_filestem).unwrap(), + None => from_str(out_filestem).unwrap_or_else(|| { + let mut s = out_filestem.chars().filter(|c| c.is_XID_continue()); + from_str(s.collect::<~str>()).or(from_str("rust-out")).unwrap() + }), Some(s) => s, } } diff --git a/src/test/run-make/weird-output-filenames/Makefile b/src/test/run-make/weird-output-filenames/Makefile new file mode 100644 index 0000000000000..9d852bce6f6a0 --- /dev/null +++ b/src/test/run-make/weird-output-filenames/Makefile @@ -0,0 +1,9 @@ +-include ../tools.mk + +all: + $(RUSTC) foo.rs -o $(TMPDIR)/.foo + rm $(TMPDIR)/.foo + $(RUSTC) foo.rs -o $(TMPDIR)/.foo.bar + rm $(TMPDIR)/.foo.bar + $(RUSTC) foo.rs -o $(TMPDIR)/+foo+bar + rm $(TMPDIR)/+foo+bar diff --git a/src/test/run-make/weird-output-filenames/foo.rs b/src/test/run-make/weird-output-filenames/foo.rs new file mode 100644 index 0000000000000..8ae3d072362ed --- /dev/null +++ b/src/test/run-make/weird-output-filenames/foo.rs @@ -0,0 +1,11 @@ +// Copyright 2014 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() {}