There are three reasons for this change:
* It's consistent with MSYS2, which already renamed console.exe to
winpty.exe.
* winpty.exe is less likely to clash with another program.
* winpty.exe is easier to search for online. If someone unfamiliar with
winpty is asked to run console.exe, they might have a hard time figuring
out where console.exe comes from.
The old behavior can be restored by passing UNIX_ADAPTER_EXE=console.exe
to `make`, or by simply renaming the binary after-the-fact, or with
`alias`, etc.
* Harmonize the subsystem of every binary to CONSOLE. It apparently
doesn't matter for DLLs, and it's easier if they're all the same. I
examined various DLLs on my system, and they're very inconsistent.
(e.g. kernel32.dll is CONSOLE, but shell32.dll is GUI.)
* When using a recent XP-targeting toolset (v120_xp or v140_xp), it's
necessary to explicitly specify the subsystem, so list it in
configurations.gypi.
* node.js changed its common.gypi to disable exception handling[1], so
explicitly turn it back on in winpty.gyp. Placing the setting in
winpty.gyp's 'target_defaults' block doesn't seem to override a setting
in an included file's 'target_defaults' block, so instead enable EH on
each target.
* For consistency with other gyp files I see, use quoted integers instead
of bare integers for settings. Affects the 'RuntimeLibrary' setting.
* Provide a documented way in configurations.gypi for setting the
XP-specific toolsets.
[1] da9eff80a3
MSVC complained that GetVersionEx was deprecated when the /SDL checks were
enabled.
Comment added to WindowsVersion.cc:
Allow use of deprecated functions (i.e. GetVersionEx). We need to use
GetVersionEx for the old MinGW toolchain and with MSVC when it targets XP.
Having two code paths makes code harder to test, and it's not obvious how
to detect the presence of a new enough SDK. (Including ntverp.h and
examining VER_PRODUCTBUILD apparently works, but even then, MinGW-w64 and
MSVC seem to use different version numbers.)
This change avoids the unnecessary round-trip to the ANSI (or OEM?) code
page, and it also allows removing the mbstowcs calls, which simplifies the
code and fixes MSVC /SDL compliance.
In certain situations (e.g. when it has the SeImpersonatePrivilege
privilege), a named pipe server can impersonate a named pipe client and
execute in its security context (e.g. open the client user's files).
There's no reason for a winpty debugserver to need this ability, so
restrict it in the client.
It seems unlikely to me that this commit fixes a genuine security issue,
but it also seems like a decent precaution to take. Clients using winpty
don't connect to the DebugServer named pipe unless the WINPTY_DEBUG
environment variable is set, and Windows restricts the ability to
impersonate named clients (at least as of XP SP2).
The str array need an extra element for the NUL terminator. The previous
code didn't suffer from memory-unsafety, but winpty_snprintf truncated the
last character of the marker, and trying to write NUL to the WinXP console
didn't work. (I suspect WinXP turned the NUL into a space, so
findSyncMarker couldn't find it later.)
* In general, harmonize the debugserver named pipe with the other named
pipe instances: don't let remote users log messages and fail if the
pipe already exists.
Also:
* Removed some obsoleted comments.
* Factor out the Handle class. Rename to OwnedHandle. Merge some
improvements from libwinpty-rewrite, including fixing move-assignment
so that it closes the existing handle, if one exists.
* Set the PIPE_REJECT_REMOTE_CLIENTS flag on Vista and up.
* Set a DACL on named pipes that gives full control to the built-in
administrators, the local system account, and the current security
token's owner SID. This is the same DACL that is used by default,
except that the default also grants read access to the Everyone group
and the anonymous account.
* The createSecurityDescriptorOwnerFullControlEveryoneWrite function is
not currently used (or tested), but I think I'll use it in the debug
server to allow collecting trace output from other accounts on the
machine. (I think I'll make that behavior optional.)
* PIDs are recycled, so include the system time.
* A program running on the same machine could predict the names of pipes
winpty uses and block winpty, causing a denial-of-service. I'm not sure
this is really a concern, but I suppose in principle it is? Try to
guard against it by appending random bytes to the pipe name. The
CreatePrivateNamespace API looks relevant, but I'm not sure it applies
to named pipes, and it's Vista only.
* Rename modifiableWString to vectorWithNulFromString.
* Create a vectorFromString that omits the NUL terminator.
* Stop triply-terminated non-empty environment blocks.
* Stop adding an extra NUL terminator to environment blocks in
handleStartProcessPacket. (env.data() didn't necessarily add a
terminator in C++03, but it does in C++11.)
* Previously, most places in the codebase used "%ls", which relies on the
CRT to do a wide-to-mbcs conversion, and I think(?) that uses locale
settings?
* Do the conversion explicitly using narrowString, which is renamed to
utf8FromWide and moved to a new file, src/shared/StringUtil.cc.
Remove all references to the iostream and sstream headers from code in the
src directory.
This change reduces the sizes of winpty.dll and winpty-agent.exe. For
example:
Cygwin's x86_64-w64-mingw32-g++ 4.9.2 compiler:
- winpty.dll: 622 KiB to 513 KiB
- winpty-agent.exe: 742 KiB to 572 KiB
MSVC 2015, using gyp + configurations.gypi targeting x64:
- winpty.dll: 985 KiB to 545 KiB
- winpty-agent.exe: 1108 KiB to 642 KiB
* Avoid std::stringstream because it's inefficient/bloated.
* Explicitly handle decode errors, by throwing ReadBuffer::DecodeError.
For the time being, this exception can only be thrown in the agent, but
I anticipate it being thrown in the libwinpty code eventually.
* Catch encoding errors a bit better.
* Be more precise about the integer types used for length. e.g.
Use 64 bits on the wire, but otherwise use size_t/uint64_t where
appropriate.
* Replace memcpy with std::copy to make MSVC's /SDL security checks happy.
It's a somewhat silly/arbitrary change. I don't think it will affect
performance, but I haven't benchmarked it.
* This commit changes the RPC protocol that libwinpty uses to communicate
with winpty-agent.
The existing snprintf functions were not C99-conformant:
- The _vsnprintf* functions do not accept all of the C99 specifiers,
like %zu for size_t or (on WinXP) %lld for long long.
- MinGW always provides vsnprintf, but the implementation sometimes just
calls MSVCRT's _vsnprintf*. In that case, it won't handle all C99
specifiers and the return value won't be conformant.
Changes:
- winpty_snprintf accepts C90(?) specifiers, returns -1 on error
(including truncation) and should work with the inttypes.h printf
macros.
- Enable __attribute__((printf)) on the winpty_[v]snprintf functions and
also on trace(). Fix a HANDLE(void*) <-> unsigned int mismatch in
NamedPipe.cc.
- Switch most sprintf() call sites to winpty_snprintf for improved safety
guarantees. (This change gets the code closer to compiling with MSVC's
/sdl flag, which forbids "unsafe" memory/string functions.)
* In agent: if the console is frozen, then calling abort hangs forever,
because abort tries to write to stderr, which blocks. Instead,
use WM_CLOSE to close the console window. If that doesn't work after
waiting a while, then exit.
We need to do the same thing doing normal agent shutdown, so factor out
an agentShutdown function. It's a bit unhappy living in
WinptyAssert.cc, but it works.
* Outside agent: trace an assertion failure message, then delegate to the
ordinary assert() function. If that's somehow turned off, then call
abort().
The gyp file already worked this way, but now the Makefile does too.
The motivation is to let the shared source files have different error
handling behavior (e.g. assert/ASSERT/stderr-print/abort/exception) in
winpty.dll and the other targets.
Currently, when a header file is removed (or renamed), there is a stale
dependency file still referring to the header, and GNU make aborts, because
the header is missing. Attempt to work around this by adding a pattern
rule to generate any header in `src/`. The rule prints a warning message.
The C source file whose header was missing will be recompiled because a
dependency was rebuilt. Once it is, the removed header should be removed
from the dependency file.
(Aside: FWIW, the "ninja" build tool knows directly about depfiles, and it
does not have this problem.)
The variables can be specified on the make command-line instead, and the
user could have unrelated variables that happen to have the name winpty
is using.
I verified that MSYS2 is passing UNIX_ADAPTER_EXE and PREFIX on the
command-line. ship/ship.py already passes PREFIX on the command-line.
Previously, for each object file, the Makefile spawned a subprocess for
each of: echo, mkdir, dirname, and git. Most of these can be eliminated:
- echo: The gmake function $(info ...) is mostly good enough -- it
affects the --dry-run/-n output, which might be undesired.
- mkdir: We only need to invoke mkdir once per directory. It's tricky
to get the optimal behavior, but it's possible to do better than before.
- dirname: The gmake function $(dir ...) is good enough.
- git: only needed once per build, to get the commit hash.
MinGW-w64 is still supported, as is the MinGW compiler distributed by
mingw.org.
Cygwin's MinGW toolchain uses the 4.0-1 mingw-runtime package by
default, which appears to correspond to the "WSL-4" 4.x version of the
MinGW project's mingwrt packages. (The version numbers in the
/usr/i686-pc-mingw32/sys-root/mingw/include/_mingw.h file are also
consistent with this theory.)
The WSL-4 branch was repudiated by the MinGW project as a mistake [1].
The MinGW project claims no responsibility for Cygwin's MinGW packages
[2]. There were several releases in short succession [3]:
4.0.3 2013-09-22
4.0.2 2013-09-15
4.0.1 2013-09-14
4.0.0 2013-08-23
There have been no 4.x releases since then, though there have been more
3.x releases (3.21 and 3.21.1). The FILE_FLAG_FIRST_PIPE_INSTANCE bug
that this project has worked around was fixed in 4.0.3 [4], which was
never propagated to the Cygwin package.
The MinGW packages were apparently orphaned on 2014-09-18 [5] and are
still orphaned [6].
The g++ compiler in this package is still 4.7.3 (released on 2013-04-11),
which is almost three three years old. While writing the "GenRandom"
module, I hit a bug where a non-static data member initializer of
pointer-to-function-type segfaulted the compiler. (gcc seemed to have
confused the field with a pure virtual method.)
[1] http://sourceforge.net/p/mingw/bugs/1673/#5260/41ec
[2] http://sourceforge.net/p/mingw/bugs/1673/#2556/35d4
[3] http://sourceforge.net/projects/mingw/files/MinGW/Base/mingwrt/
[4] http://sourceforge.net/p/mingw/bugs/2050/
[5] https://cygwin.com/ml/cygwin/2014-09/msg00289.html
[6] https://cygwin.com/cygwin-pkg-maint
