diff --git a/include/cling/Interpreter/ClangInternalState.h b/include/cling/Interpreter/ClangInternalState.h index 327eccec4b..8a2a5b9ac3 100644 --- a/include/cling/Interpreter/ClangInternalState.h +++ b/include/cling/Interpreter/ClangInternalState.h @@ -72,12 +72,12 @@ namespace cling { ///\brief Runs diff on two files. ///\param[in] file1 - A file to diff ///\param[in] file2 - A file to diff - ///\param[in] differences - The differences if any between file1 and file2 + ///\param[in] type - The type/name of the differences to print. ///\param[in] ignores - A list of differences to ignore. ///\returns true if there is difference in the contents. /// bool differentContent(const std::string& file1, const std::string& file2, - std::string& differences, + const char* type = nullptr, const llvm::SmallVectorImpl* ignores = 0) const; static void printLookupTables(llvm::raw_ostream& Out, clang::ASTContext& C); diff --git a/include/cling/Utils/Platform.h b/include/cling/Utils/Platform.h index 294e5acc9d..df46b84d16 100644 --- a/include/cling/Utils/Platform.h +++ b/include/cling/Utils/Platform.h @@ -61,6 +61,17 @@ namespace platform { /// bool IsMemoryValid(const void *P); + ///\brief Invoke a command and read it's output. + /// + /// \param [in] Cmd - Command and arguments to invoke. + /// \param [out] Buf - Buffer to write output to. + /// \param [in] StdErrToStdOut - Redirect stderr to stdout. + /// + /// \returns whether any output was written to Buf + /// + bool Popen(const std::string& Cmd, llvm::SmallVectorImpl& Buf, + bool StdErrToStdOut = false); + #if defined(LLVM_ON_UNIX) #if defined(__APPLE__) diff --git a/lib/Interpreter/ClangInternalState.cpp b/lib/Interpreter/ClangInternalState.cpp index f5280c4fee..5e06bb2f21 100644 --- a/lib/Interpreter/ClangInternalState.cpp +++ b/lib/Interpreter/ClangInternalState.cpp @@ -8,6 +8,7 @@ //------------------------------------------------------------------------------ #include "cling/Interpreter/ClangInternalState.h" +#include "cling/Utils/Platform.h" #include "clang/AST/ASTContext.h" #include "clang/Lex/Preprocessor.h" @@ -29,11 +30,6 @@ #include #include -#ifdef WIN32 -#define popen _popen -#define pclose _pclose -#endif - using namespace clang; namespace cling { @@ -42,7 +38,12 @@ namespace cling { llvm::Module* M, CodeGenerator* CG, const std::string& name) : m_ASTContext(AC), m_Preprocessor(PP), m_CodeGen(CG), m_Module(M), - m_DiffCommand("diff -u --text "), m_Name(name), m_DiffPair(nullptr) { +#if defined(LLVM_ON_WIN32) + m_DiffCommand("diff.exe -u --text "), +#else + m_DiffCommand("diff -u --text "), +#endif + m_Name(name), m_DiffPair(nullptr) { store(); } @@ -153,23 +154,14 @@ namespace cling { builtinNames.push_back(".*__builtin.*"); - if (differentContent(m_LookupTablesFile, m_DiffPair->m_LookupTablesFile, - differences, &builtinNames)) { - llvm::errs() << "Differences in the lookup tables\n"; - llvm::errs() << differences << "\n"; - differences = ""; - } - if (differentContent(m_IncludedFilesFile, m_DiffPair->m_IncludedFilesFile, - differences)) { - llvm::errs() << "Differences in the included files\n"; - llvm::errs() << differences << "\n"; - differences = ""; - } - if (differentContent(m_ASTFile, m_DiffPair->m_ASTFile, differences)) { - llvm::errs() << "Differences in the AST \n"; - llvm::errs() << differences << "\n"; - differences = ""; - } + differentContent(m_LookupTablesFile, m_DiffPair->m_LookupTablesFile, + "lookup tables", &builtinNames); + + differentContent(m_IncludedFilesFile, m_DiffPair->m_IncludedFilesFile, + "included files"); + + differentContent(m_ASTFile, m_DiffPair->m_ASTFile, "AST"); + if (m_Module) { assert(m_CodeGen && "Must have CodeGen set"); // We want to skip the intrinsics @@ -179,24 +171,19 @@ namespace cling { if (I->isIntrinsic()) builtinNames.push_back(I->getName().data()); - if (differentContent(m_LLVMModuleFile, m_DiffPair->m_LLVMModuleFile, - differences, &builtinNames)) { - llvm::errs() << "Differences in the llvm Module \n"; - llvm::errs() << differences << "\n"; - differences = ""; - } - } - if (differentContent(m_MacrosFile, m_DiffPair->m_MacrosFile, differences)){ - llvm::errs() << "Differences in the Macro Definitions \n"; - llvm::errs() << differences << "\n"; - differences = ""; + differentContent(m_LLVMModuleFile, m_DiffPair->m_LLVMModuleFile, + "llvm Module", &builtinNames); } + + differentContent(m_MacrosFile, m_DiffPair->m_MacrosFile, + "Macro Definitions"); } bool ClangInternalState::differentContent(const std::string& file1, const std::string& file2, - std::string& differences, - const llvm::SmallVectorImpl* ignores/*=0*/) const { + const char* type, + const llvm::SmallVectorImpl* ignores/*=0*/) const { + std::string diffCall = m_DiffCommand; if (ignores) { for (size_t i = 0, e = ignores->size(); i < e; ++i) { @@ -205,22 +192,23 @@ namespace cling { diffCall += ".*\""; } } + diffCall += " "; + diffCall += file1; + diffCall += " "; + diffCall += file2; - FILE* pipe = popen((diffCall + " " + file1 + " " + file2).c_str() , "r"); - assert(pipe && "Error creating the pipe"); - assert(differences.empty() && "Must be empty"); - - char buffer[128]; - while(!feof(pipe)) { - if(fgets(buffer, 128, pipe) != NULL) - differences += buffer; - } - pclose(pipe); + llvm::SmallString<1024> Difs; + platform::Popen(diffCall, Difs); - if (!differences.empty()) - llvm::errs() << diffCall << " " << file1 << " " << file2 << "\n"; + if (Difs.empty()) + return false; - return !differences.empty(); + if (type) { + llvm::errs() << diffCall << "\n"; + llvm::errs() << "Differences in the " << type << ":\n"; + llvm::errs() << Difs << "\n"; + } + return true; } class DumpLookupTables : public RecursiveASTVisitor { diff --git a/lib/Utils/PlatformPosix.cpp b/lib/Utils/PlatformPosix.cpp index 88ab2e6462..8b429ba198 100644 --- a/lib/Utils/PlatformPosix.cpp +++ b/lib/Utils/PlatformPosix.cpp @@ -12,6 +12,7 @@ #if defined(LLVM_ON_UNIX) #include "cling/Utils/Paths.h" +#include "llvm/ADT/SmallString.h" #include #include @@ -84,6 +85,25 @@ std::string NormalizePath(const std::string& Path) { return std::string(); } +bool Popen(const std::string& Cmd, llvm::SmallVectorImpl& Buf, bool RdE) { + if (FILE *PF = ::popen(RdE ? (Cmd + " 2>&1").c_str() : Cmd.c_str(), "r")) { + Buf.resize(0); + const size_t Chunk = Buf.capacity_in_bytes(); + while (true) { + const size_t Len = Buf.size(); + Buf.resize(Len + Chunk); + const size_t R = ::fread(&Buf[Len], sizeof(char), Chunk, PF); + if (R < Chunk) { + Buf.resize(Len + R); + break; + } + } + ::pclose(PF); + return !Buf.empty(); + } + return false; +} + bool GetSystemLibraryPaths(llvm::SmallVectorImpl& Paths) { #if defined(__APPLE__) || defined(__CYGWIN__) Paths.push_back("/usr/local/lib/"); @@ -98,13 +118,9 @@ bool GetSystemLibraryPaths(llvm::SmallVectorImpl& Paths) { Paths.push_back("/lib64/"); #endif #else - std::string Result; - if (FILE* F = ::popen("LD_DEBUG=libs LD_PRELOAD=DOESNOTEXIST ls 2>&1", "r")) { - char Buf[1024]; - while (::fgets(&Buf[0], sizeof(Buf), F)) - Result += Buf; - ::pclose(F); - } + llvm::SmallString<1024> Buf; + platform::Popen("LD_DEBUG=libs LD_PRELOAD=DOESNOTEXIST ls", Buf, true); + const llvm::StringRef Result = Buf.str(); const std::size_t NPos = std::string::npos; const std::size_t LD = Result.find("(LD_LIBRARY_PATH)"); diff --git a/lib/Utils/PlatformWin.cpp b/lib/Utils/PlatformWin.cpp index 20109e12ad..ca847f8131 100644 --- a/lib/Utils/PlatformWin.cpp +++ b/lib/Utils/PlatformWin.cpp @@ -587,6 +587,104 @@ bool GetSystemLibraryPaths(llvm::SmallVectorImpl& Paths) { return true; } +static void CloseHandle(HANDLE H) { + if (::CloseHandle(H) == 0) + ReportLastError("CloseHandle"); +} + +bool Popen(const std::string& Cmd, llvm::SmallVectorImpl& Buf, bool RdE) { + Buf.resize(0); + + SECURITY_ATTRIBUTES saAttr; + saAttr.nLength = sizeof(SECURITY_ATTRIBUTES); + saAttr.bInheritHandle = TRUE; + saAttr.lpSecurityDescriptor = NULL; + + HANDLE Process = ::GetCurrentProcess(); + HANDLE hOutputReadTmp, hOutputRead, hOutputWrite, hErrorWrite; + + if (::CreatePipe(&hOutputReadTmp, &hOutputWrite, &saAttr, 0) == 0) + return false; + if (RdE) { + if (::DuplicateHandle(Process, hOutputWrite, Process, &hErrorWrite, 0, TRUE, + DUPLICATE_SAME_ACCESS) == 0) { + ReportLastError("DuplicateHandle"); + ::CloseHandle(hOutputReadTmp); + ::CloseHandle(hOutputWrite); + return false; + } + } + + // Create new output read handle. Set the Properties to FALSE, otherwise the + // child inherits the properties and, as a result, non-closeable handles to + // the pipes are created. + if (::DuplicateHandle(Process, hOutputReadTmp, Process, &hOutputRead, 0, + FALSE, DUPLICATE_SAME_ACCESS) == 0) { + ReportLastError("DuplicateHandle"); + ::CloseHandle(hOutputReadTmp); + ::CloseHandle(hOutputWrite); + if (RdE) + ::CloseHandle(hErrorWrite); + return false; + } + + // Close inheritable copies of the handles you do not want to be inherited. + CloseHandle(hOutputReadTmp); + + STARTUPINFOA si; + ZeroMemory(&si, sizeof(si)); + si.cb = sizeof(si); + si.dwFlags = STARTF_USESTDHANDLES; + si.hStdOutput = hOutputWrite; + if (RdE) + si.hStdError = hErrorWrite; + + PROCESS_INFORMATION pi; + ZeroMemory(&pi, sizeof(pi)); + + // https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425%28v=vs.85%29.aspx + // CreateProcessW can write back to second arguement, CreateProcessA not + BOOL Result = ::CreateProcessA(NULL, (LPSTR)Cmd.c_str(), NULL, NULL, TRUE, 0, + NULL, NULL, &si, &pi); + DWORD Err = ::GetLastError(); + + // Close pipe handles (do not continue to modify the parent) to make sure + // that no handles to the write end of the output pipe are maintained in this + // process or else the pipe will not close when the child process exits and + // the ReadFile will hang. + CloseHandle(hOutputWrite); + if (RdE) + CloseHandle(hErrorWrite); + + if (Result != 0) { + DWORD dwRead; + const size_t Chunk = Buf.capacity_in_bytes(); + while (true) { + const size_t Len = Buf.size(); + Buf.resize(Len + Chunk); + Result = ::ReadFile(hOutputRead, &Buf[Len], Chunk, &dwRead, NULL); + if (!Result || !dwRead) { + Err = ::GetLastError(); + if (Err != ERROR_BROKEN_PIPE) + ReportError(Err, "ReadFile"); + Buf.resize(Len); + break; + } + if (dwRead < Chunk) + Buf.resize(Len + dwRead); + } + + // Close process and thread handles. + CloseHandle(pi.hProcess); + CloseHandle(pi.hThread); + } else + ReportError(Err, "CreateProcess"); + + CloseHandle(hOutputRead); + + return !Buf.empty(); +} + } // namespace platform } // namespace utils } // namespace cling