Skip to content

Commit

Permalink
Fix SelectionDAG Graph Printing on Windows
Browse files Browse the repository at this point in the history
Currently, when compiling to IR (presumably at the clang level) LLVM
mangles symbols and sometimes they have illegal file characters
including `?`'s in them. This causes a problem when building a graph via
llc on Windows because the code currently passes the machine function
name all the way down to the Windows API which frequently returns error
123  **ERROR_INVALID_NAME**
https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-

Thus, we need to remove those illegal characters from the machine
function name before generating a graph, which is the purpose of this
patch.
https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file

I've created a static helper function replace_illegal_filename_chars
which within GraphWriter.cpp to help with replacing illegal file
character names before generating a dot graph filename.

Reviewed By: rnk

Differential Revision: https://reviews.llvm.org/D76863
  • Loading branch information
Justice Adams authored and rnk committed May 6, 2020
1 parent 0274c79 commit 89c7451
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 5 deletions.
5 changes: 1 addition & 4 deletions llvm/include/llvm/Support/GraphWriter.h
Expand Up @@ -330,11 +330,8 @@ std::string WriteGraph(const GraphType &G, const Twine &Name,
const Twine &Title = "",
std::string Filename = "") {
int FD;
// Windows can't always handle long paths, so limit the length of the name.
std::string N = Name.str();
N = N.substr(0, std::min<std::size_t>(N.size(), 140));
if (Filename.empty()) {
Filename = createGraphFilename(N, FD);
Filename = createGraphFilename(Name.str(), FD);
} else {
std::error_code EC = sys::fs::openFileForWrite(Filename, FD);

Expand Down
27 changes: 26 additions & 1 deletion llvm/lib/Support/GraphWriter.cpp
Expand Up @@ -76,10 +76,35 @@ StringRef llvm::DOT::getColorString(unsigned ColorNumber) {
return Colors[ColorNumber % NumColors];
}

static std::string replaceIllegalFilenameChars(std::string Filename,
const char ReplacementChar) {
#ifdef _WIN32
std::string IllegalChars = "\\/:?\"<>|";
#else
std::string IllegalChars = "/";
#endif

for (char IllegalChar : IllegalChars) {
std::replace(Filename.begin(), Filename.end(), IllegalChar,
ReplacementChar);
}

return Filename;
}

std::string llvm::createGraphFilename(const Twine &Name, int &FD) {
FD = -1;
SmallString<128> Filename;
std::error_code EC = sys::fs::createTemporaryFile(Name, "dot", FD, Filename);

// Windows can't always handle long paths, so limit the length of the name.
std::string N = Name.str();
N = N.substr(0, std::min<std::size_t>(N.size(), 140));

// Replace illegal characters in graph Filename with '_' if needed
std::string CleansedName = replaceIllegalFilenameChars(N, '_');

std::error_code EC =
sys::fs::createTemporaryFile(CleansedName, "dot", FD, Filename);
if (EC) {
errs() << "Error: " << EC.message() << "\n";
return "";
Expand Down

0 comments on commit 89c7451

Please sign in to comment.