From 1b894b3cec60ec03a01bb5b7d87594d19295e6b3 Mon Sep 17 00:00:00 2001 From: Frederich Munch Date: Tue, 11 Jul 2017 15:10:42 -0400 Subject: [PATCH 1/3] Fix destruction ordering on shutdown. cling is currently tearing down the text input before the Interpreter is destroyed. Leading to problems with pipes and file handles being close in the text input layer. --- include/cling/UserInterface/UserInterface.h | 9 ++- lib/UserInterface/UserInterface.cpp | 62 ++++++++++++--------- tools/driver/cling.cpp | 5 +- 3 files changed, 48 insertions(+), 28 deletions(-) diff --git a/include/cling/UserInterface/UserInterface.h b/include/cling/UserInterface/UserInterface.h index b8ebad3a50..893e9396be 100644 --- a/include/cling/UserInterface/UserInterface.h +++ b/include/cling/UserInterface/UserInterface.h @@ -20,17 +20,24 @@ namespace cling { /// class UserInterface { private: + class TextInputHolder; + std::unique_ptr m_TextInput; std::unique_ptr m_MetaProcessor; ///\brief Prints cling's startup logo /// void PrintLogo(); public: - UserInterface(Interpreter& interp); + UserInterface(); ~UserInterface(); MetaProcessor* getMetaProcessor() { return m_MetaProcessor.get(); } + ///\brief Attach this instance to the given Interpreter. + /// @param[in] Interp - The interpreter to attach to. + /// + void attach(Interpreter& Interp); + ///\brief Drives the interactive prompt talking to the user. /// @param[in] nologo - whether to show cling's welcome logo or not /// diff --git a/lib/UserInterface/UserInterface.cpp b/lib/UserInterface/UserInterface.cpp index febd5d97ca..c7b60115e3 100644 --- a/lib/UserInterface/UserInterface.cpp +++ b/lib/UserInterface/UserInterface.cpp @@ -46,57 +46,67 @@ namespace { return true; } }; +} + +namespace cling { ///\brief Delays ~TextInput until after ~StreamReader and ~TerminalDisplay /// - class TextInputHolder { + class UserInterface::TextInputHolder { textinput::StreamReader* m_Reader; textinput::TerminalDisplay* m_Display; textinput::TextInput m_Input; + class HistoryFile { + llvm::SmallString<512> Path; + + public: + HistoryFile(const char* Env = "CLING_NOHISTORY", + const char* Name = ".cling_history") { + if (getenv(Env)) return; + // History file is $HOME/.cling_history + if (llvm::sys::path::home_directory(Path)) + llvm::sys::path::append(Path, Name); + } + const char* c_str() { return Path.empty() ? nullptr : Path.c_str(); } + }; + public: - TextInputHolder(llvm::SmallString<512>& Hist) + TextInputHolder(HistoryFile HistFile = HistoryFile()) : m_Reader(textinput::StreamReader::Create()), m_Display(textinput::TerminalDisplay::Create()), - m_Input(*m_Reader, *m_Display, Hist.empty() ? 0 : Hist.c_str()) {} + m_Input(*m_Reader, *m_Display, HistFile.c_str()) { + } ~TextInputHolder() { delete m_Reader; delete m_Display; } - textinput::TextInput* operator -> () { return &m_Input; } + operator textinput::TextInput& () { return m_Input; } }; -} -namespace cling { - - UserInterface::UserInterface(Interpreter& interp) { - m_MetaProcessor.reset(new MetaProcessor(interp, cling::outs())); + UserInterface::UserInterface() { llvm::install_fatal_error_handler(&CompilationException::throwingHandler); } - UserInterface::~UserInterface() {} + UserInterface::~UserInterface() { llvm::remove_fatal_error_handler(); } + + void UserInterface::attach(Interpreter& Interp) { + m_MetaProcessor.reset(new MetaProcessor(Interp, cling::outs())); + } void UserInterface::runInteractively(bool nologo /* = false */) { - if (!nologo) { + assert(m_MetaProcessor.get() && "Not attached to an Interpreter."); + if (!nologo) PrintLogo(); - } - - llvm::SmallString<512> histfilePath; - if (!getenv("CLING_NOHISTORY")) { - // History file is $HOME/.cling_history - if (llvm::sys::path::home_directory(histfilePath)) - llvm::sys::path::append(histfilePath, ".cling_history"); - } - TextInputHolder TI(histfilePath); + m_TextInput.reset(new TextInputHolder); + textinput::TextInput& TI = *m_TextInput; // Inform text input about the code complete consumer // TextInput owns the TabCompletion. - UITabCompletion* Completion = - new UITabCompletion(m_MetaProcessor->getInterpreter()); - TI->SetCompletion(Completion); + TI.SetCompletion(new UITabCompletion(m_MetaProcessor->getInterpreter())); bool Done = false; std::string Line; @@ -107,9 +117,9 @@ namespace cling { m_MetaProcessor->getOuts().flush(); { MetaProcessor::MaybeRedirectOutputRAII RAII(*m_MetaProcessor); - TI->SetPrompt(Prompt.c_str()); - Done = TI->ReadInput() == textinput::TextInput::kRREOF; - TI->TakeInput(Line); + TI.SetPrompt(Prompt.c_str()); + Done = TI.ReadInput() == textinput::TextInput::kRREOF; + TI.TakeInput(Line); if (Done && Line.empty()) break; } diff --git a/tools/driver/cling.cpp b/tools/driver/cling.cpp index 15089dfea0..943e284d6e 100644 --- a/tools/driver/cling.cpp +++ b/tools/driver/cling.cpp @@ -53,6 +53,9 @@ static int checkDiagErrors(clang::CompilerInstance* CI, unsigned* OutErrs = 0) { int main( int argc, char **argv ) { + // Force the UserInterface to be destroyed last, so any file manipulation, + // pipes, or dups are active for the entire process. + cling::UserInterface Ui; llvm::llvm_shutdown_obj shutdownTrigger; @@ -105,7 +108,7 @@ int main( int argc, char **argv ) { for (const std::string& Lib : Opts.LibsToLoad) Interp.loadFile(Lib); - cling::UserInterface Ui(Interp); + Ui.attach(Interp); // If we are not interactive we're supposed to parse files if (!Opts.IsInteractive()) { for (const std::string &Input : Opts.Inputs) { From 3a00e2c34656eef12871159b8a53a2ef38dee1f6 Mon Sep 17 00:00:00 2001 From: Frederich Munch Date: Tue, 11 Jul 2017 16:49:37 -0400 Subject: [PATCH 2/3] Windows: Fix DeRegisterEHFrames from erasing a possibly invalid iterator. DeRegisterEHFrames can be called without a a prior call to RegisterEHFrames. This can easily be detected so just do nothing when that occurs. --- lib/Utils/PlatformWin.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/Utils/PlatformWin.cpp b/lib/Utils/PlatformWin.cpp index 25fbf6f06d..cccf360c92 100644 --- a/lib/Utils/PlatformWin.cpp +++ b/lib/Utils/PlatformWin.cpp @@ -788,11 +788,17 @@ void RegisterEHFrames(uintptr_t ImgBs, const EHFrameInfos& Frames, bool Block) { void DeRegisterEHFrames(uintptr_t ImgBase, const EHFrameInfos& Frames) { if (Frames.empty()) return; - assert(getImageBaseMap().find(ImgBase) != getImageBaseMap().end()); - // Remove the ImageBase from lookup + // There is a chance that DeRegisterEHFrames will have been called without a + // preceeding call to RegisterEHFrames. Rather than tracking such cases, + // just ignore the requests when ImgBase was never registered. ImageBaseMap& Unwind = getImageBaseMap(); - Unwind.erase(Unwind.find(ImgBase)); + auto Itr = Unwind.find(ImgBase); + if (Itr == Unwind.end()) + return; + + // Remove the ImageBase from lookup + Unwind.erase(Itr); // Unregister all the PRUNTIME_FUNCTIONs for (auto&& Frame : Frames) From 942350658797037effb344f9d6be6f5a5fccf97d Mon Sep 17 00:00:00 2001 From: Frederich Munch Date: Wed, 26 Jul 2017 03:20:40 -0400 Subject: [PATCH 3/3] Windows: Fix exception handling frames not being registered. --- lib/Interpreter/IncrementalJIT.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/Interpreter/IncrementalJIT.cpp b/lib/Interpreter/IncrementalJIT.cpp index 23c3f8ec22..4f5a40402b 100644 --- a/lib/Interpreter/IncrementalJIT.cpp +++ b/lib/Interpreter/IncrementalJIT.cpp @@ -251,12 +251,11 @@ class Azog: public RTDyldMemoryManager { // the fact that we're lazily emitting object files: The only way you can // get more than one set of objects loaded but not yet finalized is if // they were loaded during relocation of another set. - if (m_jit.m_UnfinalizedSections.size() == 1) { #ifdef CLING_WIN_SEH_EXCEPTIONS - platform::RegisterEHFrames(getBaseAddr(), m_EHFrames, true); + platform::RegisterEHFrames(getBaseAddr(), m_EHFrames, true); #endif + if (m_jit.m_UnfinalizedSections.size() == 1) return getExeMM()->finalizeMemory(ErrMsg); - } return false; };