From d3fec7fb456138c83b84e38ce785ea6bfa59c30b Mon Sep 17 00:00:00 2001 From: James Y Knight Date: Wed, 20 Nov 2019 10:08:18 -0500 Subject: [PATCH] LLD: Don't use the stderrOS stream in link before it's reassigned. Remove the lld::enableColors function, as it just obscures which stream it's affecting, and replace with explicit calls to the stream's enable_colors. Also, assign the stderrOS and stdoutOS globals first in link function, just to ensure nothing might use them. (Either change individually fixes the issue of using the old stream, but both together seems best.) Follow-up to b11386f9be9b2dc7276a758d64f66833da10bdea. Differential Revision: https://reviews.llvm.org/D70492 --- lld/COFF/Driver.cpp | 8 ++++---- lld/COFF/DriverUtils.cpp | 8 ++++---- lld/Common/ErrorHandler.cpp | 2 -- lld/ELF/Driver.cpp | 8 ++++---- lld/ELF/DriverUtils.cpp | 8 ++++---- lld/MinGW/Driver.cpp | 3 ++- lld/include/lld/Common/ErrorHandler.h | 2 -- lld/lib/Driver/DarwinLdDriver.cpp | 8 ++++---- lld/wasm/Driver.cpp | 16 ++++++++-------- 9 files changed, 30 insertions(+), 33 deletions(-) diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp index 8a9ba92899c..f770fff80bc 100644 --- a/lld/COFF/Driver.cpp +++ b/lld/COFF/Driver.cpp @@ -64,15 +64,15 @@ LinkerDriver *driver; bool link(ArrayRef args, bool canExitEarly, raw_ostream &stdoutOS, raw_ostream &stderrOS) { + lld::stdoutOS = &stdoutOS; + lld::stderrOS = &stderrOS; + errorHandler().logName = args::getFilenameWithoutExe(args[0]); errorHandler().errorLimitExceededMsg = "too many errors emitted, stopping now" " (use /errorlimit:0 to see all errors)"; errorHandler().exitEarly = canExitEarly; - enableColors(stderrOS.has_colors()); - - lld::stdoutOS = &stdoutOS; - lld::stderrOS = &stderrOS; + stderrOS.enable_colors(stderrOS.has_colors()); config = make(); symtab = make(); diff --git a/lld/COFF/DriverUtils.cpp b/lld/COFF/DriverUtils.cpp index cb23c9bf734..301a5c5efa8 100644 --- a/lld/COFF/DriverUtils.cpp +++ b/lld/COFF/DriverUtils.cpp @@ -773,15 +773,15 @@ static void handleColorDiagnostics(opt::InputArgList &args) { if (!arg) return; if (arg->getOption().getID() == OPT_color_diagnostics) { - enableColors(true); + lld::errs().enable_colors(true); } else if (arg->getOption().getID() == OPT_no_color_diagnostics) { - enableColors(false); + lld::errs().enable_colors(false); } else { StringRef s = arg->getValue(); if (s == "always") - enableColors(true); + lld::errs().enable_colors(true); else if (s == "never") - enableColors(false); + lld::errs().enable_colors(false); else if (s != "auto") error("unknown option: --color-diagnostics=" + s); } diff --git a/lld/Common/ErrorHandler.cpp b/lld/Common/ErrorHandler.cpp index e26d78c2871..91ea6799907 100644 --- a/lld/Common/ErrorHandler.cpp +++ b/lld/Common/ErrorHandler.cpp @@ -51,8 +51,6 @@ ErrorHandler &lld::errorHandler() { return handler; } -void lld::enableColors(bool enable) { lld::errs().enable_colors(enable); } - void lld::exitLld(int val) { // Delete any temporary file, while keeping the memory mapping open. if (errorHandler().outputBuffer) diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp index 25a9b292054..f7ce7358bb7 100644 --- a/lld/ELF/Driver.cpp +++ b/lld/ELF/Driver.cpp @@ -77,15 +77,15 @@ static void readConfigs(opt::InputArgList &args); bool link(ArrayRef args, bool canExitEarly, raw_ostream &stdoutOS, raw_ostream &stderrOS) { + lld::stdoutOS = &stdoutOS; + lld::stderrOS = &stderrOS; + errorHandler().logName = args::getFilenameWithoutExe(args[0]); errorHandler().errorLimitExceededMsg = "too many errors emitted, stopping now (use " "-error-limit=0 to see all errors)"; errorHandler().exitEarly = canExitEarly; - enableColors(stderrOS.has_colors()); - - lld::stdoutOS = &stdoutOS; - lld::stderrOS = &stderrOS; + stderrOS.enable_colors(stderrOS.has_colors()); inputSections.clear(); outputSections.clear(); diff --git a/lld/ELF/DriverUtils.cpp b/lld/ELF/DriverUtils.cpp index 53134d0cbfe..9fcb36e8167 100644 --- a/lld/ELF/DriverUtils.cpp +++ b/lld/ELF/DriverUtils.cpp @@ -59,15 +59,15 @@ static void handleColorDiagnostics(opt::InputArgList &args) { if (!arg) return; if (arg->getOption().getID() == OPT_color_diagnostics) { - enableColors(true); + lld::errs().enable_colors(true); } else if (arg->getOption().getID() == OPT_no_color_diagnostics) { - enableColors(false); + lld::errs().enable_colors(false); } else { StringRef s = arg->getValue(); if (s == "always") - enableColors(true); + lld::errs().enable_colors(true); else if (s == "never") - enableColors(false); + lld::errs().enable_colors(false); else if (s != "auto") error("unknown option: --color-diagnostics=" + s); } diff --git a/lld/MinGW/Driver.cpp b/lld/MinGW/Driver.cpp index 950156a8a8f..1a256b44efb 100644 --- a/lld/MinGW/Driver.cpp +++ b/lld/MinGW/Driver.cpp @@ -161,10 +161,11 @@ searchLibrary(StringRef name, ArrayRef searchPaths, bool bStatic) { // then call coff::link. bool mingw::link(ArrayRef argsArr, bool canExitEarly, raw_ostream &stdoutOS, raw_ostream &stderrOS) { - enableColors(stderrOS.has_colors()); lld::stdoutOS = &stdoutOS; lld::stderrOS = &stderrOS; + stderrOS.enable_colors(stderrOS.has_colors()); + MinGWOptTable parser; opt::InputArgList args = parser.parse(argsArr.slice(1)); diff --git a/lld/include/lld/Common/ErrorHandler.h b/lld/include/lld/Common/ErrorHandler.h index ac6f93fc088..8eed38af8c8 100644 --- a/lld/include/lld/Common/ErrorHandler.h +++ b/lld/include/lld/Common/ErrorHandler.h @@ -117,8 +117,6 @@ private: /// Returns the default error handler. ErrorHandler &errorHandler(); -void enableColors(bool enable); - inline void error(const Twine &msg) { errorHandler().error(msg); } inline LLVM_ATTRIBUTE_NORETURN void fatal(const Twine &msg) { errorHandler().fatal(msg); diff --git a/lld/lib/Driver/DarwinLdDriver.cpp b/lld/lib/Driver/DarwinLdDriver.cpp index d372e6c94c5..10e0aa3d9fe 100644 --- a/lld/lib/Driver/DarwinLdDriver.cpp +++ b/lld/lib/Driver/DarwinLdDriver.cpp @@ -1145,15 +1145,15 @@ static void createFiles(MachOLinkingContext &ctx, bool Implicit) { /// This is where the link is actually performed. bool link(llvm::ArrayRef args, bool CanExitEarly, raw_ostream &StdoutOS, raw_ostream &StderrOS) { + lld::stdoutOS = &StdoutOS; + lld::stderrOS = &StderrOS; + errorHandler().logName = args::getFilenameWithoutExe(args[0]); errorHandler().errorLimitExceededMsg = "too many errors emitted, stopping now (use " "'-error-limit 0' to see all errors)"; errorHandler().exitEarly = CanExitEarly; - enableColors(StderrOS.has_colors()); - - lld::stdoutOS = &StdoutOS; - lld::stderrOS = &StderrOS; + StderrOS.enable_colors(StderrOS.has_colors()); MachOLinkingContext ctx; if (!parse(args, ctx)) diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp index d1e81caf2c5..80a717a533c 100644 --- a/lld/wasm/Driver.cpp +++ b/lld/wasm/Driver.cpp @@ -80,14 +80,14 @@ private: bool link(ArrayRef args, bool canExitEarly, raw_ostream &stdoutOS, raw_ostream &stderrOS) { + lld::stdoutOS = &stdoutOS; + lld::stderrOS = &stderrOS; + errorHandler().logName = args::getFilenameWithoutExe(args[0]); errorHandler().errorLimitExceededMsg = "too many errors emitted, stopping now (use " "-error-limit=0 to see all errors)"; - enableColors(stderrOS.has_colors()); - - lld::stdoutOS = &stdoutOS; - lld::stderrOS = &stderrOS; + stderrOS.enable_colors(stderrOS.has_colors()); config = make(); symtab = make(); @@ -135,15 +135,15 @@ static void handleColorDiagnostics(opt::InputArgList &args) { if (!arg) return; if (arg->getOption().getID() == OPT_color_diagnostics) { - enableColors(true); + lld::errs().enable_colors(true); } else if (arg->getOption().getID() == OPT_no_color_diagnostics) { - enableColors(false); + lld::errs().enable_colors(false); } else { StringRef s = arg->getValue(); if (s == "always") - enableColors(true); + lld::errs().enable_colors(true); else if (s == "never") - enableColors(false); + lld::errs().enable_colors(false); else if (s != "auto") error("unknown option: --color-diagnostics=" + s); }