Skip to content

Commit

Permalink
Add platform::POpen.
Browse files Browse the repository at this point in the history 
Fixes ClangInternalState::differentContent showing no differences when there
actually are, just less than 128 characters in length.

Invoking diff on Windows with --ignore-matching-lines arguments will fail because the command string is too long. Using Windows APIs to launch the process succeeds.

Signed-off-by: Vassil Vassilev <vvasilev@cern.ch>
  • Loading branch information
@marsupial
marsupial authored and sftnight committed Oct 18, 2016
1 parent 182954b commit 3e765d8
Show file tree
Hide file tree
Showing 5 changed files with 171 additions and 58 deletions.
4 changes: 2 additions & 2 deletions include/cling/Interpreter/ClangInternalState.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<const char*>* ignores = 0) const;

static void printLookupTables(llvm::raw_ostream& Out, clang::ASTContext& C);
Expand Down
11 changes: 11 additions & 0 deletions include/cling/Utils/Platform.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<char>& Buf,
bool StdErrToStdOut = false);

#if defined(LLVM_ON_UNIX)

#if defined(__APPLE__)
Expand Down
86 changes: 37 additions & 49 deletions lib/Interpreter/ClangInternalState.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
//------------------------------------------------------------------------------

#include "cling/Interpreter/ClangInternalState.h"
#include "cling/Utils/Platform.h"

#include "clang/AST/ASTContext.h"
#include "clang/Lex/Preprocessor.h"
Expand All @@ -29,11 +30,6 @@
#include <string>
#include <time.h>

#ifdef WIN32
#define popen _popen
#define pclose _pclose
#endif

using namespace clang;

namespace cling {
Expand All @@ -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();
}

Expand Down Expand Up @@ -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
Expand All @@ -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<const char*>* ignores/*=0*/) const {
const char* type,
const llvm::SmallVectorImpl<const char*>* ignores/*=0*/) const {

std::string diffCall = m_DiffCommand;
if (ignores) {
for (size_t i = 0, e = ignores->size(); i < e; ++i) {
Expand All @@ -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<DumpLookupTables> {
Expand Down
30 changes: 23 additions & 7 deletions lib/Utils/PlatformPosix.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#if defined(LLVM_ON_UNIX)

#include "cling/Utils/Paths.h"
#include "llvm/ADT/SmallString.h"

#include <string>
#include <dlfcn.h>
Expand Down Expand Up @@ -84,6 +85,25 @@ std::string NormalizePath(const std::string& Path) {
return std::string();
}

bool Popen(const std::string& Cmd, llvm::SmallVectorImpl<char>& 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<std::string>& Paths) {
#if defined(__APPLE__) || defined(__CYGWIN__)
Paths.push_back("/usr/local/lib/");
Expand All @@ -98,13 +118,9 @@ bool GetSystemLibraryPaths(llvm::SmallVectorImpl<std::string>& 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)");
Expand Down
98 changes: 98 additions & 0 deletions lib/Utils/PlatformWin.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,104 @@ bool GetSystemLibraryPaths(llvm::SmallVectorImpl<std::string>& Paths) {
return true;
}

static void CloseHandle(HANDLE H) {
if (::CloseHandle(H) == 0)
ReportLastError("CloseHandle");
}

bool Popen(const std::string& Cmd, llvm::SmallVectorImpl<char>& 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
Expand Down

0 comments on commit 3e765d8

Please sign in to comment.