Skip to content

[lldb][Windows] Fixed Host::Kill() #99721

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 21, 2024

Conversation

slydiman
Copy link
Contributor

@slydiman slydiman commented Jul 19, 2024

HostProcessWindows::Terminate() correctly uses m_process which type is process_t (HANDLE) to call ::TerminateProcess(). But Host::Kill() uses a cast from pid, which is wrong.

This patch fixes #51793

@slydiman slydiman requested a review from JDevlieghere as a code owner July 19, 2024 23:51
@llvmbot llvmbot added the lldb label Jul 19, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 19, 2024

@llvm/pr-subscribers-lldb

Author: Dmitry Vasilyev (slydiman)

Changes

HostProcessWindows::Terminate() correctly uses m_process which type is process_t (HANDLE) to call ::TerminateProcess(). But Host::Kill() uses a cast from pid, which is wrong.


Full diff: https://github.com/llvm/llvm-project/pull/99721.diff

1 Files Affected:

  • (modified) lldb/source/Host/windows/Host.cpp (+3-1)
diff --git a/lldb/source/Host/windows/Host.cpp b/lldb/source/Host/windows/Host.cpp
index 6908f0003eaf7..642092f61d924 100644
--- a/lldb/source/Host/windows/Host.cpp
+++ b/lldb/source/Host/windows/Host.cpp
@@ -103,7 +103,9 @@ lldb::thread_t Host::GetCurrentThread() {
 }
 
 void Host::Kill(lldb::pid_t pid, int signo) {
-  TerminateProcess((HANDLE)pid, 1);
+  AutoHandle handle(::OpenProcess(PROCESS_TERMINATE, FALSE, pid), nullptr);
+  if (handle.IsValid())
+    ::TerminateProcess(handle.get(), 1);
 }
 
 const char *Host::GetSignalAsCString(int signo) { return NULL; }

@slydiman slydiman requested a review from labath July 19, 2024 23:54
HostProcessWindows::Terminate() correctly uses m_process which type is process_t (HANDLE) to call ::TerminateProcess().
But Host::Kill() uses a cast from pid, which is wrong.

This patch fixes llvm#51793
@slydiman slydiman force-pushed the fix-lldb-windows-host-kill branch from 0712380 to c033ba3 Compare July 20, 2024 09:18
Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat

@slydiman slydiman merged commit f6eb89c into llvm:main Jul 21, 2024
6 checks passed
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary:
HostProcessWindows::Terminate() correctly uses m_process which type is
process_t (HANDLE) to call ::TerminateProcess(). But Host::Kill() uses a
cast from pid, which is wrong.

This patch fixes #51793

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251618
@slydiman slydiman deleted the fix-lldb-windows-host-kill branch July 25, 2024 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SBPlatform::Kill does not seem to work on windows
3 participants