[llvm] [llvm][Support][Windows] Avoid crash calling remove_directories() (PR #118677)
Dmitry Vasilyev via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 9 11:34:02 PST 2024
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/118677
>From bc5973d214d1761c4a433f3f794bb7854d7d342b Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev <dvassiliev at accesssoftek.com>
Date: Wed, 4 Dec 2024 13:30:21 +0400
Subject: [PATCH 1/3] [Windows] Avoid crash calling remove_directories()
We faced an unexpected crash in SHELL32_CallFileCopyHooks() on the buildbot [lldb-remote-linux-win](https://lab.llvm.org/staging/#/builders/197/builds/1066).
The host is Windows Server 2022 w/o any 3rd party shell extensions.
See #118032 for more details.
Based on [this article](https://devblogs.microsoft.com/oldnewthing/20120330-00/?p=7963).
---
llvm/lib/Support/Windows/Path.inc | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/Support/Windows/Path.inc b/llvm/lib/Support/Windows/Path.inc
index c4bd5e24723517..ecea1efefe0098 100644
--- a/llvm/lib/Support/Windows/Path.inc
+++ b/llvm/lib/Support/Windows/Path.inc
@@ -1387,12 +1387,31 @@ std::error_code remove_directories(const Twine &path, bool IgnoreErrors) {
Path16.push_back(0);
Path16.push_back(0);
- SHFILEOPSTRUCTW shfos = {};
- shfos.wFunc = FO_DELETE;
- shfos.pFrom = Path16.data();
- shfos.fFlags = FOF_NO_UI;
+ HRESULT HR = CoInitializeEx(NULL, COINIT_MULTITHREADED);
+ if (SUCCEEDED(HR)) {
+ IFileOperation *FileOp;
+ HR = CoCreateInstance(CLSID_FileOperation, NULL, CLSCTX_ALL,
+ IID_PPV_ARGS(&FileOp));
+ if (SUCCEEDED(HR)) {
+ HR = FileOp->SetOperationFlags(FOF_NO_UI | FOFX_NOCOPYHOOKS);
+ if (SUCCEEDED(HR)) {
+ IShellItem *ShItem = NULL;
+ HR = SHCreateItemFromParsingName(Path16.data(), NULL,
+ IID_PPV_ARGS(&ShItem));
+ if (SUCCEEDED(HR)) {
+ HR = FileOp->DeleteItem(ShItem, NULL);
+ ShItem->Release();
+ }
+ if (SUCCEEDED(HR)) {
+ HR = FileOp->PerformOperations();
+ }
+ }
+ FileOp->Release();
+ }
+ }
+ CoUninitialize();
- int result = ::SHFileOperationW(&shfos);
+ int result = FAILED(HR) ? HRESULT_CODE(HR) : 0;
if (result != 0 && !IgnoreErrors)
return mapWindowsError(result);
return std::error_code();
>From 3fa927241bd1d5bc5ca97cd8578172426db040bb Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev <dvassiliev at accesssoftek.com>
Date: Thu, 5 Dec 2024 00:48:09 +0400
Subject: [PATCH 2/3] Updated with the behavior of SHFileOperationW().
---
llvm/lib/Support/Windows/Path.inc | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/Support/Windows/Path.inc b/llvm/lib/Support/Windows/Path.inc
index ecea1efefe0098..2daec202b227f8 100644
--- a/llvm/lib/Support/Windows/Path.inc
+++ b/llvm/lib/Support/Windows/Path.inc
@@ -1387,29 +1387,31 @@ std::error_code remove_directories(const Twine &path, bool IgnoreErrors) {
Path16.push_back(0);
Path16.push_back(0);
- HRESULT HR = CoInitializeEx(NULL, COINIT_MULTITHREADED);
+ HRESULT HR =
+ CoInitializeEx(NULL, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE);
if (SUCCEEDED(HR)) {
- IFileOperation *FileOp;
+ IFileOperation *FileOp = NULL;
HR = CoCreateInstance(CLSID_FileOperation, NULL, CLSCTX_ALL,
IID_PPV_ARGS(&FileOp));
if (SUCCEEDED(HR)) {
HR = FileOp->SetOperationFlags(FOF_NO_UI | FOFX_NOCOPYHOOKS);
if (SUCCEEDED(HR)) {
+ PIDLIST_ABSOLUTE PIDL = ILCreateFromPathW(Path16.data());
IShellItem *ShItem = NULL;
- HR = SHCreateItemFromParsingName(Path16.data(), NULL,
- IID_PPV_ARGS(&ShItem));
+ HR = SHCreateItemFromIDList(PIDL, IID_PPV_ARGS(&ShItem));
if (SUCCEEDED(HR)) {
HR = FileOp->DeleteItem(ShItem, NULL);
+ if (SUCCEEDED(HR)) {
+ HR = FileOp->PerformOperations();
+ }
ShItem->Release();
}
- if (SUCCEEDED(HR)) {
- HR = FileOp->PerformOperations();
- }
+ ILFree(PIDL);
}
FileOp->Release();
}
+ CoUninitialize();
}
- CoUninitialize();
int result = FAILED(HR) ? HRESULT_CODE(HR) : 0;
if (result != 0 && !IgnoreErrors)
>From 973a23b26552a838b966b5ce796272d81474bba8 Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev <dvassiliev at accesssoftek.com>
Date: Mon, 9 Dec 2024 23:28:01 +0400
Subject: [PATCH 3/3] Refactored.
---
llvm/lib/Support/Windows/Path.inc | 58 +++++++++++++++----------------
1 file changed, 28 insertions(+), 30 deletions(-)
diff --git a/llvm/lib/Support/Windows/Path.inc b/llvm/lib/Support/Windows/Path.inc
index 2daec202b227f8..20aafef9cc8ade 100644
--- a/llvm/lib/Support/Windows/Path.inc
+++ b/llvm/lib/Support/Windows/Path.inc
@@ -25,6 +25,7 @@
// These two headers must be included last, and make sure shlobj is required
// after Windows.h to make sure it picks up our definition of _WIN32_WINNT
#include "llvm/Support/Windows/WindowsSupport.h"
+#include <atlbase.h>
#include <shellapi.h>
#include <shlobj.h>
@@ -1387,36 +1388,33 @@ std::error_code remove_directories(const Twine &path, bool IgnoreErrors) {
Path16.push_back(0);
Path16.push_back(0);
- HRESULT HR =
- CoInitializeEx(NULL, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE);
- if (SUCCEEDED(HR)) {
- IFileOperation *FileOp = NULL;
- HR = CoCreateInstance(CLSID_FileOperation, NULL, CLSCTX_ALL,
- IID_PPV_ARGS(&FileOp));
- if (SUCCEEDED(HR)) {
- HR = FileOp->SetOperationFlags(FOF_NO_UI | FOFX_NOCOPYHOOKS);
- if (SUCCEEDED(HR)) {
- PIDLIST_ABSOLUTE PIDL = ILCreateFromPathW(Path16.data());
- IShellItem *ShItem = NULL;
- HR = SHCreateItemFromIDList(PIDL, IID_PPV_ARGS(&ShItem));
- if (SUCCEEDED(HR)) {
- HR = FileOp->DeleteItem(ShItem, NULL);
- if (SUCCEEDED(HR)) {
- HR = FileOp->PerformOperations();
- }
- ShItem->Release();
- }
- ILFree(PIDL);
- }
- FileOp->Release();
- }
- CoUninitialize();
- }
-
- int result = FAILED(HR) ? HRESULT_CODE(HR) : 0;
- if (result != 0 && !IgnoreErrors)
- return mapWindowsError(result);
- return std::error_code();
+ HRESULT HR;
+ do {
+ HR =
+ CoInitializeEx(NULL, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE);
+ if (FAILED(HR))
+ break;
+ auto Uninitialize = make_scope_exit([] { CoUninitialize(); });
+ CComPtr<IFileOperation> FileOp;
+ HR = FileOp.CoCreateInstance(CLSID_FileOperation);
+ if (FAILED(HR))
+ break;
+ HR = FileOp->SetOperationFlags(FOF_NO_UI | FOFX_NOCOPYHOOKS);
+ if (FAILED(HR))
+ break;
+ PIDLIST_ABSOLUTE PIDL = ILCreateFromPathW(Path16.data());
+ auto FreePIDL = make_scope_exit([&PIDL] { ILFree(PIDL); });
+ CComPtr<IShellItem> ShItem;
+ HR = SHCreateItemFromIDList(PIDL, IID_PPV_ARGS(&ShItem));
+ if (FAILED(HR))
+ break;
+ HR = FileOp->DeleteItem(ShItem, NULL);
+ if (FAILED(HR))
+ break;
+ HR = FileOp->PerformOperations();
+ } while (false);
+ return (IgnoreErrors || SUCCEEDED(HR)) ? std::error_code()
+ : mapWindowsError(HRESULT_CODE(HR));
}
static void expandTildeExpr(SmallVectorImpl<char> &Path) {
More information about the llvm-commits
mailing list