[1/2] Don't close thread handles provided by WaitForDebugEvent

Message ID 20200524222448.11955-1-ssbssa@yahoo.de
State New
Headers show
Series
  • [1/2] Don't close thread handles provided by WaitForDebugEvent
Related show

Commit Message

Kevin Buettner via Gdb-patches May 24, 2020, 10:24 p.m.
I sometimes encountered a weird breakpoint failure when using start:

(gdb) start
Temporary breakpoint 2 at 0x40162d: file gdb-25911.c, line 4.
Starting program: C:\src\tests\gdb-25911.exe
Warning:
Cannot insert breakpoint 2.
Cannot access memory at address 0x401628

After trying a lot of combinations, I found a way to reproduce it:

(gdb) file gdb-25987.exe
Reading symbols from gdb-25987.exe...
(gdb) start
Temporary breakpoint 1 at 0x401638: file gdb-25987.cpp, line 13.
Starting program: C:\src\tests\gdb-25987.exe

Temporary breakpoint 1, main () at gdb-25987.cpp:13
13      int main() {
(gdb) c
Continuing.

Program received signal SIGSEGV, Segmentation fault.
MyClass::call (this=0x3d20d0) at gdb-25987.cpp:8
8           *(char*)(nullptr) = 1;
(gdb) kill
Kill the program being debugged? (y or n) y
[Inferior 1 (process 1140) killed]
(gdb) file gdb-25911.exe
Load new symbol table from "gdb-25911.exe"? (y or n) y
Reading symbols from gdb-25911.exe...
(gdb) start
Temporary breakpoint 2 at 0x40162d: file gdb-25911.c, line 4.
Starting program: C:\src\tests\gdb-25911.exe
Warning:
Cannot insert breakpoint 2.
Cannot access memory at address 0x401628

Command aborted.

The actual failure was that ReadProcessMemory used a process handle that
was no longer valid.
And the underlying reason was that the windows_thread_info destructor
closes a thread handle that was provided earlier by WaitForDebugEvent.

But since this is not allowed (and it was actually already closed at this
point, and the handle value reused), this closed another still-needed handle.

gdb/ChangeLog:

2020-05-24  Hannes Domani  <ssbssa@yahoo.de>

	* nat/windows-nat.c (windows_thread_info::~windows_thread_info):
	Don't close thread handle.
---
 gdb/nat/windows-nat.c | 1 -
 1 file changed, 1 deletion(-)

-- 
2.26.2

Comments

Tom Tromey May 26, 2020, 8:29 p.m. | #1
>>>>> "Hannes" == Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> writes:


Hannes> 2020-05-24  Hannes Domani  <ssbssa@yahoo.de>

Hannes> 	* nat/windows-nat.c (windows_thread_info::~windows_thread_info):
Hannes> 	Don't close thread handle.

Thanks, I found this, which seems to agree with your analysis:

https://docs.microsoft.com/en-us/windows/win32/api/debugapi/nf-debugapi-waitfordebugevent

So, I think this patch is ok.

Tom

Patch

diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 8c2092a51d..709a9d3a31 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -51,7 +51,6 @@  bool ignore_first_breakpoint = false;
 
 windows_thread_info::~windows_thread_info ()
 {
-  CloseHandle (h);
 }
 
 void