[llvm] r192096 - Windows: Be more explicit with Win32 APIs
Aaron Ballman
aaron at aaronballman.com
Mon Oct 7 05:49:06 PDT 2013
On Mon, Oct 7, 2013 at 5:52 AM, David Majnemer <david.majnemer at gmail.com> wrote:
> Author: majnemer
> Date: Mon Oct 7 04:52:36 2013
> New Revision: 192096
>
> URL: http://llvm.org/viewvc/llvm-project?rev=192096&view=rev
> Log:
> Windows: Be more explicit with Win32 APIs
>
> This addresses several issues in a similar vein:
> - Use the Unicode APIs when possible, running nm on clang shows that we
> only use Unicode APIs except for FormatMessage, CreateSemaphore, and
> GetModuleHandle. AFAICT, the latter two are coming from MinGW and
> not LLVM itself.
At this point, would it make more sense for us to turn on the UNICODE
and _UNICODE defines to ensure that we are using the Unicode APIs even
when forgetting to add W?
> - Make getMainExecutable more resilient. It previously considered
> return values of zero from ::GetModuleFileNameA to be acceptable.
>
> Modified:
> llvm/trunk/lib/Support/Windows/Path.inc
> llvm/trunk/lib/Support/Windows/Program.inc
> llvm/trunk/lib/Support/Windows/RWMutex.inc
> llvm/trunk/lib/Support/Windows/Signals.inc
>
> Modified: llvm/trunk/lib/Support/Windows/Path.inc
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Path.inc?rev=192096&r1=192095&r2=192096&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Support/Windows/Path.inc (original)
> +++ llvm/trunk/lib/Support/Windows/Path.inc Mon Oct 7 04:52:36 2013
> @@ -46,9 +46,9 @@ namespace {
> /*__in*/ LPCWSTR lpTargetFileName,
> /*__in*/ DWORD dwFlags);
>
> - PtrCreateSymbolicLinkW create_symbolic_link_api = PtrCreateSymbolicLinkW(
> - ::GetProcAddress(::GetModuleHandleA("kernel32.dll"),
> - "CreateSymbolicLinkW"));
> + PtrCreateSymbolicLinkW create_symbolic_link_api =
> + PtrCreateSymbolicLinkW(::GetProcAddress(
> + ::GetModuleHandleW(L"Kernel32.dll"), "CreateSymbolicLinkW"));
>
> error_code TempDir(SmallVectorImpl<wchar_t> &result) {
> retry_temp_dir:
> @@ -216,9 +216,28 @@ namespace sys {
> namespace fs {
>
> std::string getMainExecutable(const char *argv0, void *MainExecAddr) {
> - char pathname[MAX_PATH];
> - DWORD ret = ::GetModuleFileNameA(NULL, pathname, MAX_PATH);
> - return ret != MAX_PATH ? pathname : "";
> + SmallVector<wchar_t, MAX_PATH> PathName;
> + DWORD Size = ::GetModuleFileNameW(NULL, PathName.data(), PathName.capacity());
> +
> + // A zero return value indicates a failure other than insufficient space.
> + if (Size == 0)
> + return "";
> +
> + // Insufficient space is determined by a return value equal to the size of
> + // the buffer passed in.
> + if (Size == PathName.capacity())
> + return "";
> +
> + // On success, GetModuleFileNameW returns the number of characters written to
> + // the buffer not including the NULL terminator.
> + PathName.set_size(Size);
> +
> + // Convert the result from UTF-16 to UTF-8.
> + SmallVector<char, MAX_PATH> PathNameUTF8;
> + if (UTF16ToUTF8(PathName.data(), PathName.size(), PathNameUTF8))
> + return "";
It's early and I have a cold, so forgive me if this is a bit off...
When you set the PathName to have the size without the null
terminator, then convert it to UTF-8, does that not come back without
the null terminator as well? Then we pass that into std::string's
constructor without a null terminator, but without specifying the size
in bytes it should contain. That seems like a buffer overflow waiting
to happen, unless my fuzzy brain is not yet working this morning
(which is entirely possible).
> +
> + return std::string(PathNameUTF8.data());
> }
>
> UniqueID file_status::getUniqueID() const {
> @@ -672,12 +691,11 @@ error_code mapped_file_region::init(int
> case priv: flprotect = PAGE_WRITECOPY; break;
> }
>
> - FileMappingHandle = ::CreateFileMapping(FileHandle,
> - 0,
> - flprotect,
> - (Offset + Size) >> 32,
> - (Offset + Size) & 0xffffffff,
> - 0);
> + FileMappingHandle =
> + ::CreateFileMappingW(FileHandle, 0, flprotect,
> + (Offset + Size) >> 32,
> + (Offset + Size) & 0xffffffff,
> + 0);
> if (FileMappingHandle == NULL) {
> error_code ec = windows_error(GetLastError());
> if (FileDescriptor) {
>
> Modified: llvm/trunk/lib/Support/Windows/Program.inc
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Program.inc?rev=192096&r1=192095&r2=192096&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Support/Windows/Program.inc (original)
> +++ llvm/trunk/lib/Support/Windows/Program.inc Mon Oct 7 04:52:36 2013
> @@ -335,7 +335,7 @@ static bool Execute(ProcessInfo &PI, Str
> // Assign the process to a job if a memory limit is defined.
> ScopedJobHandle hJob;
> if (memoryLimit != 0) {
> - hJob = CreateJobObject(0, 0);
> + hJob = CreateJobObjectW(0, 0);
> bool success = false;
> if (hJob) {
> JOBOBJECT_EXTENDED_LIMIT_INFORMATION jeli;
>
> Modified: llvm/trunk/lib/Support/Windows/RWMutex.inc
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/RWMutex.inc?rev=192096&r1=192095&r2=192096&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Support/Windows/RWMutex.inc (original)
> +++ llvm/trunk/lib/Support/Windows/RWMutex.inc Mon Oct 7 04:52:36 2013
> @@ -48,7 +48,7 @@ static bool loadSRW() {
> if (!sChecked) {
> sChecked = true;
>
> - HMODULE hLib = ::LoadLibrary(TEXT("Kernel32"));
> + HMODULE hLib = ::LoadLibraryW(L"Kernel32.dll");
> if (hLib) {
> fpInitializeSRWLock =
> (VOID (WINAPI *)(PSRWLOCK))::GetProcAddress(hLib,
>
> Modified: llvm/trunk/lib/Support/Windows/Signals.inc
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Signals.inc?rev=192096&r1=192095&r2=192096&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Support/Windows/Signals.inc (original)
> +++ llvm/trunk/lib/Support/Windows/Signals.inc Mon Oct 7 04:52:36 2013
> @@ -135,7 +135,7 @@ typedef PVOID (WINAPI *fpSymFunctionTabl
> static fpSymFunctionTableAccess64 SymFunctionTableAccess64;
>
> static bool load64BitDebugHelp(void) {
> - HMODULE hLib = ::LoadLibrary(TEXT("Dbghelp.dll"));
> + HMODULE hLib = ::LoadLibraryW(L"Dbghelp.dll");
> if (hLib) {
> StackWalk64 = (fpStackWalk64)
> ::GetProcAddress(hLib, "StackWalk64");
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
~Aaron
More information about the llvm-commits
mailing list