martine/ninja — Issue #206: Subprocess::OnPipeReady may use undefined value on Windows
Author: qhuo
Created: 2012-01-24 16:02:15+00:00
State: open

We encountered a ninja crash with the following stack dump when a compilation task fails.

0012f528 00416601 e06d7363 00000001 00000003 kernel32!RaiseException+0x53
0012f560 00413988 0012f570 00443f54 0043ec30 ninja!_CxxThrowException+0x46 [f:\sp\vctools\crt_bld\self_x86\crt\prebuild\eh\throw.cpp @ 161]
0012f57c 004039f3 23f700fe 7c91005d 00402b9a ninja!operator new+0x69 [f:\sp\vctools\crt_bld\self_x86\crt\src\new.cpp @ 63]
0012f590 00402bb3 23f700fe 00421667 0012fc04 ninja!std::allocator::allocate+0x13
0012fc10 0040439e 23f700fd 23f6f0fd 01749a70 ninja!std::basic_string,std::allocator >::_Copy+0x93
0012fc2c 00429a68 01749aa8 00001000 01236f70 ninja!std::basic_string,std::allocator >::append+0x8e
0012fc40 00429bf3 00000000 0012fc8c 01236f68 ninja!Subprocess::OnPipeReady+0x58
0012fc68 00431285 0000000f 0012fe7c 00000001 ninja!SubprocessSet::DoWork+0x43
0012fc8c 004324c1 0012fcbc 0012fce4 6e59e457 ninja!RealCommandRunner::WaitForCommand+0x25
0012fd10 0043505f 0012feb4 6e59e5d7 00000a28 ninja!Builder::Build+0x161
0012ff3c 004351f3 00000001 6e59e787 00000a28 ninja!main_unsafe+0xfbf
0012ff70 004164ca 00000005 01234900 012349f0 ninja!main+0x53
0012ffc0 7c817077 00000000 0140f574 7ffd4000 ninja!__tmainCRTStartup+0x15f [f:\sp\vctools\crt_bld\self_x86\crt\src\crt0.c @ 327]
0012fff0 00000000 00416521 00000000 00000000 kernel32!BaseProcessStart+0x23

I've not been able to reproduce the crash. But I had a look of the involved functions anyway, and I think there is something odd in the OnPipeReady() function.

It calls GetOverlappedResult(), if it is successful, and the bytes returned is non-zero, it appends the “bytes” bytes data from overlapped_buf_ to buf_ (a std::string). However, the only time overlapped_buf_ is used is through the ReadFile function, also in OnPipeReady() after the append() call. So it seems to me that GetOverlappedResult() and append() should operate on the result of the previous ReadFile() call. However, the very first GetOverlappedResult() call is the result of ConnectNamedPipe(), which according to MSDN, its output “bytes” argument is undefined.

I am only guessing that this might be the reason that causes the crash. Anyway, it would be better to fix the code. Such as adding a flag to indicate whether the current pending operation if ReadFile.