Skip to content

Commit

Permalink
Remove more swprintf_s uses (#77228)
Browse files Browse the repository at this point in the history 
* Remove the swprintf_s export

* Convert paths to UTF-8 for flow graph diagnostics in JIT.
  • Loading branch information
@AaronRobinsonMSFT
AaronRobinsonMSFT authored Oct 20, 2022
1 parent 7d06aff commit 35f4967
Show file tree
Hide file tree
Showing 32 changed files with 87 additions and 1,403 deletions.
1 change: 0 additions & 1 deletion src/coreclr/dlls/mscordac/mscordac_unixexports.src
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ nativeStringResourceTable_mscorrc
#_wcsicmp
#_stricmp
#sprintf_s
#swprintf_s
#vsprintf_s
#_snprintf_s
#_snwprintf_s
Expand Down
4 changes: 0 additions & 4 deletions src/coreclr/jit/compiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7839,10 +7839,6 @@ const char* PhaseEnums[] = {
#include "compphases.h"
};

const LPCWSTR PhaseEnumsW[] = {
#define CompPhaseNameMacro(enum_nm, string_nm, hasChildren, parent, measureIR) W(#enum_nm),
#include "compphases.h"
};
#endif // defined(FEATURE_JIT_METHOD_PERF) || DUMP_FLOWGRAPHS

#ifdef FEATURE_JIT_METHOD_PERF
Expand Down
7 changes: 3 additions & 4 deletions src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1408,9 +1408,8 @@ enum Phases
PHASE_NUMBER_OF
};

extern const char* PhaseNames[];
extern const char* PhaseEnums[];
extern const LPCWSTR PhaseEnumsW[];
extern const char* PhaseNames[];
extern const char* PhaseEnums[];

// Specify which checks should be run after each phase
//
Expand Down Expand Up @@ -5328,7 +5327,7 @@ class Compiler
};
const char* fgProcessEscapes(const char* nameIn, escapeMapping_t* map);
static void fgDumpTree(FILE* fgxFile, GenTree* const tree);
FILE* fgOpenFlowGraphFile(bool* wbDontClose, Phases phase, PhasePosition pos, LPCWSTR type);
FILE* fgOpenFlowGraphFile(bool* wbDontClose, Phases phase, PhasePosition pos, const char* type);
bool fgDumpFlowGraph(Phases phase, PhasePosition pos);
#endif // DUMP_FLOWGRAPHS

Expand Down
119 changes: 69 additions & 50 deletions src/coreclr/jit/fgdiagnostic.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,24 @@ void Compiler::fgDumpTree(FILE* fgxFile, GenTree* const tree)
}
}

#ifdef DEBUG
namespace
{
const char* ConvertToUtf8(LPCWSTR wideString, CompAllocator& allocator)
{
int utf8Len = WszWideCharToMultiByte(CP_UTF8, 0, wideString, -1, nullptr, 0, nullptr, nullptr);
if (utf8Len == 0)
return nullptr;

char* alloc = (char*)allocator.allocate<char>(utf8Len);
if (0 == WszWideCharToMultiByte(CP_UTF8, 0, wideString, -1, alloc, utf8Len, nullptr, nullptr))
return nullptr;

return alloc;
}
}
#endif

//------------------------------------------------------------------------
// fgOpenFlowGraphFile: Open a file to dump either the xml or dot format flow graph
//
Expand Down Expand Up @@ -451,14 +469,14 @@ void Compiler::fgDumpTree(FILE* fgxFile, GenTree* const tree)
// Opens a file to which a flowgraph can be dumped, whose name is based on the current
// config vales.
//
FILE* Compiler::fgOpenFlowGraphFile(bool* wbDontClose, Phases phase, PhasePosition pos, LPCWSTR type)
FILE* Compiler::fgOpenFlowGraphFile(bool* wbDontClose, Phases phase, PhasePosition pos, const char* type)
{
FILE* fgxFile;
LPCWSTR prePhasePattern = nullptr; // pre-phase: default (used in Release) is no pre-phase dump
LPCWSTR postPhasePattern = W("*"); // post-phase: default (used in Release) is dump all phases
const char* prePhasePattern = nullptr; // pre-phase: default (used in Release) is no pre-phase dump
const char* postPhasePattern = "*"; // post-phase: default (used in Release) is dump all phases
bool dumpFunction = true; // default (used in Release) is always dump
LPCWSTR filename = nullptr;
LPCWSTR pathname = nullptr;
const char* filename = nullptr;
const char* pathname = nullptr;
const char* escapedString;

if (fgBBcount <= 1)
Expand All @@ -468,19 +486,20 @@ FILE* Compiler::fgOpenFlowGraphFile(bool* wbDontClose, Phases phase, PhasePositi

#ifdef DEBUG
dumpFunction = JitConfig.JitDumpFg().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args);
filename = JitConfig.JitDumpFgFile();
pathname = JitConfig.JitDumpFgDir();

prePhasePattern = JitConfig.JitDumpFgPrePhase();
postPhasePattern = JitConfig.JitDumpFgPhase();
CompAllocator allocator = getAllocatorDebugOnly();
filename = ConvertToUtf8(JitConfig.JitDumpFgFile(), allocator);
pathname = ConvertToUtf8(JitConfig.JitDumpFgDir(), allocator);
prePhasePattern = ConvertToUtf8(JitConfig.JitDumpFgPrePhase(), allocator);
postPhasePattern = ConvertToUtf8(JitConfig.JitDumpFgPhase(), allocator);
#endif // DEBUG

if (!dumpFunction)
{
return nullptr;
}

LPCWSTR phaseName = PhaseEnumsW[phase] + strlen("PHASE_");
const char* phaseName = PhaseEnums[phase] + strlen("PHASE_");

if (pos == PhasePosition::PrePhase)
{
Expand All @@ -489,9 +508,9 @@ FILE* Compiler::fgOpenFlowGraphFile(bool* wbDontClose, Phases phase, PhasePositi
// If pre-phase pattern is not specified, then don't dump for any pre-phase.
return nullptr;
}
else if (*prePhasePattern != W('*'))
else if (*prePhasePattern != '*')
{
if (wcsstr(prePhasePattern, phaseName) == nullptr)
if (strstr(prePhasePattern, phaseName) == nullptr)
{
return nullptr;
}
Expand All @@ -514,9 +533,9 @@ FILE* Compiler::fgOpenFlowGraphFile(bool* wbDontClose, Phases phase, PhasePositi
return nullptr;
}
}
else if (*postPhasePattern != W('*'))
else if (*postPhasePattern != '*')
{
if (wcsstr(postPhasePattern, phaseName) == nullptr)
if (strstr(postPhasePattern, phaseName) == nullptr)
{
return nullptr;
}
Expand All @@ -525,10 +544,10 @@ FILE* Compiler::fgOpenFlowGraphFile(bool* wbDontClose, Phases phase, PhasePositi

if (filename == nullptr)
{
filename = W("default");
filename = "default";
}

if (wcscmp(filename, W("profiled")) == 0)
if (strcmp(filename, "profiled") == 0)
{
if (fgFirstBB->hasProfileWeight())
{
Expand All @@ -539,7 +558,7 @@ FILE* Compiler::fgOpenFlowGraphFile(bool* wbDontClose, Phases phase, PhasePositi
return nullptr;
}
}
if (wcscmp(filename, W("hot")) == 0)
if (strcmp(filename, "hot") == 0)
{
if (info.compMethodInfo->regionKind == CORINFO_REGION_HOT)
{
Expand All @@ -550,7 +569,7 @@ FILE* Compiler::fgOpenFlowGraphFile(bool* wbDontClose, Phases phase, PhasePositi
return nullptr;
}
}
else if (wcscmp(filename, W("cold")) == 0)
else if (strcmp(filename, "cold") == 0)
{
if (info.compMethodInfo->regionKind == CORINFO_REGION_COLD)
{
Expand All @@ -561,7 +580,7 @@ FILE* Compiler::fgOpenFlowGraphFile(bool* wbDontClose, Phases phase, PhasePositi
return nullptr;
}
}
else if (wcscmp(filename, W("jit")) == 0)
else if (strcmp(filename, "jit") == 0)
{
if (info.compMethodInfo->regionKind == CORINFO_REGION_JIT)
{
Expand All @@ -572,13 +591,13 @@ FILE* Compiler::fgOpenFlowGraphFile(bool* wbDontClose, Phases phase, PhasePositi
return nullptr;
}
}
else if (wcscmp(filename, W("all")) == 0)
else if (strcmp(filename, "all") == 0)
{

ONE_FILE_PER_METHOD:;

#define FILENAME_PATTERN W("%S-%S-%s-%S.%s")
#define FILENAME_PATTERN_WITH_NUMBER W("%S-%S-%s-%S~%d.%s")
#define FILENAME_PATTERN "%s-%s-%s-%s.%s"
#define FILENAME_PATTERN_WITH_NUMBER "%s-%s-%s-%s~%d.%s"

const size_t MaxFileNameLength = MAX_PATH_FNAME - 20 /* give us some extra buffer */;

Expand All @@ -590,55 +609,55 @@ FILE* Compiler::fgOpenFlowGraphFile(bool* wbDontClose, Phases phase, PhasePositi
const char* phasePositionString = phasePositionStrings[(unsigned)pos];
const size_t phasePositionStringLen = strlen(phasePositionString);
const char* tierName = compGetTieringName(true);
size_t wCharCount = escapedStringLen + 1 + strlen(phasePositionString) + 1 + wcslen(phaseName) + 1 +
strlen(tierName) + strlen("~999") + 1 + wcslen(type) + 1;
size_t charCount = escapedStringLen + 1 + strlen(phasePositionString) + 1 + strlen(phaseName) + 1 +
strlen(tierName) + strlen("~999") + 1 + strlen(type) + 1;

if (wCharCount > MaxFileNameLength)
if (charCount > MaxFileNameLength)
{
// Crop the escapedString.
wCharCount -= escapedStringLen;
size_t newEscapedStringLen = MaxFileNameLength - wCharCount;
charCount -= escapedStringLen;
size_t newEscapedStringLen = MaxFileNameLength - charCount;
char* newEscapedString = getAllocator(CMK_DebugOnly).allocate<char>(newEscapedStringLen + 1);
strncpy_s(newEscapedString, newEscapedStringLen + 1, escapedString, newEscapedStringLen);
newEscapedString[newEscapedStringLen] = '\0';
escapedString = newEscapedString;
escapedStringLen = newEscapedStringLen;
wCharCount += escapedStringLen;
charCount += escapedStringLen;
}

if (pathname != nullptr)
{
wCharCount += wcslen(pathname) + 1;
charCount += strlen(pathname) + 1;
}
filename = (LPCWSTR)_alloca(wCharCount * sizeof(WCHAR));
filename = (const char*)_alloca(charCount * sizeof(char));

if (pathname != nullptr)
{
swprintf_s((LPWSTR)filename, wCharCount, W("%s\\") FILENAME_PATTERN, pathname, escapedString,
phasePositionString, phaseName, tierName, type);
sprintf_s((char*)filename, charCount, "%s\\" FILENAME_PATTERN, pathname, escapedString, phasePositionString,
phaseName, tierName, type);
}
else
{
swprintf_s((LPWSTR)filename, wCharCount, FILENAME_PATTERN, escapedString, phasePositionString, phaseName,
tierName, type);
sprintf_s((char*)filename, charCount, FILENAME_PATTERN, escapedString, phasePositionString, phaseName,
tierName, type);
}
fgxFile = _wfopen(filename, W("wx")); // Open the file for writing only only if it doesn't already exist
fgxFile = fopen(filename, "wx"); // Open the file for writing only only if it doesn't already exist
if (fgxFile == nullptr)
{
// This filename already exists, so create a different one by appending ~2, ~3, etc...
for (int i = 2; i < 1000; i++)
{
if (pathname != nullptr)
{
swprintf_s((LPWSTR)filename, wCharCount, W("%s\\") FILENAME_PATTERN_WITH_NUMBER, pathname,
escapedString, phasePositionString, phaseName, tierName, i, type);
sprintf_s((char*)filename, charCount, "%s\\" FILENAME_PATTERN_WITH_NUMBER, pathname, escapedString,
phasePositionString, phaseName, tierName, i, type);
}
else
{
swprintf_s((LPWSTR)filename, wCharCount, FILENAME_PATTERN_WITH_NUMBER, escapedString,
phasePositionString, phaseName, tierName, i, type);
sprintf_s((char*)filename, charCount, FILENAME_PATTERN_WITH_NUMBER, escapedString,
phasePositionString, phaseName, tierName, i, type);
}
fgxFile = _wfopen(filename, W("wx")); // Open the file for writing only only if it doesn't already exist
fgxFile = fopen(filename, "wx"); // Open the file for writing only only if it doesn't already exist
if (fgxFile != nullptr)
{
break;
Expand All @@ -652,34 +671,34 @@ FILE* Compiler::fgOpenFlowGraphFile(bool* wbDontClose, Phases phase, PhasePositi
}
*wbDontClose = false;
}
else if (wcscmp(filename, W("stdout")) == 0)
else if (strcmp(filename, "stdout") == 0)
{
fgxFile = jitstdout;
*wbDontClose = true;
}
else if (wcscmp(filename, W("stderr")) == 0)
else if (strcmp(filename, "stderr") == 0)
{
fgxFile = stderr;
*wbDontClose = true;
}
else
{
LPCWSTR origFilename = filename;
size_t wCharCount = wcslen(origFilename) + wcslen(type) + 2;
const char* origFilename = filename;
size_t charCount = strlen(origFilename) + strlen(type) + 2;
if (pathname != nullptr)
{
wCharCount += wcslen(pathname) + 1;
charCount += strlen(pathname) + 1;
}
filename = (LPCWSTR)_alloca(wCharCount * sizeof(WCHAR));
filename = (char*)_alloca(charCount * sizeof(char));
if (pathname != nullptr)
{
swprintf_s((LPWSTR)filename, wCharCount, W("%s\\%s.%s"), pathname, origFilename, type);
sprintf_s((char*)filename, charCount, "%s\\%s.%s", pathname, origFilename, type);
}
else
{
swprintf_s((LPWSTR)filename, wCharCount, W("%s.%s"), origFilename, type);
sprintf_s((char*)filename, charCount, "%s.%s", origFilename, type);
}
fgxFile = _wfopen(filename, W("a+"));
fgxFile = fopen(filename, "a+");
*wbDontClose = false;
}

Expand Down Expand Up @@ -760,7 +779,7 @@ bool Compiler::fgDumpFlowGraph(Phases phase, PhasePosition pos)
const bool displayBlockFlags = false;
#endif // !DEBUG

FILE* fgxFile = fgOpenFlowGraphFile(&dontClose, phase, pos, createDotFile ? W("dot") : W("fgx"));
FILE* fgxFile = fgOpenFlowGraphFile(&dontClose, phase, pos, createDotFile ? "dot" : "fgx");
if (fgxFile == nullptr)
{
return false;
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/pal/inc/mbusafecrt.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,13 @@ extern errno_t _splitpath_s( const char* inPath, char* outDrive, size_t inDriveS
extern errno_t _wsplitpath_s( const WCHAR* inPath, WCHAR* outDrive, size_t inDriveSize, WCHAR* outDirectory, size_t inDirectorySize, WCHAR* outFilename, size_t inFilenameSize, WCHAR* outExtension, size_t inExtensionSize );

extern int sprintf_s( char *string, size_t sizeInBytes, const char *format, ... );
extern int swprintf_s( WCHAR *string, size_t sizeInWords, const WCHAR *format, ... );

extern int _snprintf_s( char *string, size_t sizeInBytes, size_t count, const char *format, ... );
extern int _snwprintf_s( WCHAR *string, size_t sizeInWords, size_t count, const WCHAR *format, ... );

extern int vsprintf_s( char* string, size_t sizeInBytes, const char* format, va_list arglist );
extern int _vsnprintf_s( char* string, size_t sizeInBytes, size_t count, const char* format, va_list arglist );

extern int vswprintf_s( WCHAR* string, size_t sizeInWords, const WCHAR* format, va_list arglist );
extern int _vsnwprintf_s( WCHAR* string, size_t sizeInWords, size_t count, const WCHAR* format, va_list arglist );

extern int sscanf_s( const char *string, const char *format, ... );
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/pal/inc/pal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4154,9 +4154,7 @@ PALIMPORT DLLEXPORT int __cdecl _vsnwprintf_s(WCHAR *, size_t, size_t, const WCH
PALIMPORT DLLEXPORT int __cdecl _snwprintf_s(WCHAR *, size_t, size_t, const WCHAR *, ...);
PALIMPORT DLLEXPORT int __cdecl _snprintf_s(char *, size_t, size_t, const char *, ...);
PALIMPORT DLLEXPORT int __cdecl sprintf_s(char *, size_t, const char *, ... );
PALIMPORT DLLEXPORT int __cdecl swprintf_s(WCHAR *, size_t, const WCHAR *, ... );
PALIMPORT int __cdecl _snwprintf_s(WCHAR *, size_t, size_t, const WCHAR *, ...);
PALIMPORT int __cdecl vswprintf_s( WCHAR *, size_t, const WCHAR *, va_list);
PALIMPORT DLLEXPORT int __cdecl sscanf_s(const char *, const char *, ...);
PALIMPORT DLLEXPORT errno_t __cdecl _itow_s(int, WCHAR *, size_t, int);

Expand Down
32 changes: 2 additions & 30 deletions src/coreclr/pal/inc/rt/safecrt.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1654,7 +1654,7 @@ errno_t __cdecl _wsplitpath_s(

/* vsprintf_s */
/*
* swprintf_s, vsprintf_s, vswprintf_s format a string and copy it into _Dst;
* swprintf_s, vsprintf_s format a string and copy it into _Dst;
* need safecrt.lib and msvcrt.dll;
* will call _SAFECRT_INVALID_PARAMETER if there is not enough space in _Dst;
* will call _SAFECRT_INVALID_PARAMETER if the format string is malformed;
Expand All @@ -1676,36 +1676,8 @@ int __cdecl vsprintf_s(char (&_Dst)[_SizeInBytes], const char *_Format, va_list
}
#endif

/* no inline version of vsprintf_s */

/* swprintf_s, vswprintf_s */
_SAFECRT__EXTERN_C
int __cdecl swprintf_s(WCHAR *_Dst, size_t _SizeInWords, const WCHAR *_Format, ...);
_SAFECRT__EXTERN_C
int __cdecl vswprintf_s(WCHAR *_Dst, size_t _SizeInWords, const WCHAR *_Format, va_list _ArgList);

#if defined(__cplusplus) && _SAFECRT_USE_CPP_OVERLOADS
template <size_t _SizeInWords>
inline
int __cdecl swprintf_s(WCHAR (&_Dst)[_SizeInWords], const WCHAR *_Format, ...)
{
int ret;
va_list _ArgList;
va_start(_ArgList, _Format);
ret = vswprintf_s(_Dst, _SizeInWords, _Format, _ArgList);
va_end(_ArgList);
return ret;
}

template <size_t _SizeInWords>
inline
int __cdecl vswprintf_s(WCHAR (&_Dst)[_SizeInWords], const WCHAR *_Format, va_list _ArgList)
{
return vswprintf_s(_Dst, _SizeInWords, _Format, _ArgList);
}
#endif

/* no inline version of swprintf_s, vswprintf_s */
/* no inline version of swprintf_s */

/* _vsnprintf_s */
/*
Expand Down
Loading

0 comments on commit 35f4967

Please sign in to comment.