msys2-runtime: Various fixes for argv[] determination

When Windows programs called MSYS2/Cygwin programs argv[] handling
was something of a mess. It's still a bit of a mess, but at least
it works for the more common use-cases [*].

Mainly, if globify () detected an argument that looked like a dos
path ('C:', '"C:' or \\) then it wouldn't handle proceeding literal,
escaped quotes correctly (the " got dumped, while the \ remained).

Also, strace was setting env. var MSYS="noglob" and I can't figure
out why. This made investigating the globify () problem a lot more
difficult than it should've been.

Finally, build_argv conflated winshell with (winshell && allow_glob)
when they should be orthogonal.

Also added a bunch of debug_printf () calls.

[*] It's possible that I broke the corner-case of:
    '"C:\some\quoted\\"folder with spaces\"\may\not\work"'
    .. in order to support:
    "C:/some/quoted/\"folder with spaces\"/now/does/work"'
    .. so I might revisit this soon.
This commit is contained in:
Ray Donnelly
2015-08-24 01:31:36 +01:00
parent 900744becd
commit 7388276a41
7 changed files with 266 additions and 62 deletions

View File

@@ -1,34 +0,0 @@
From 45c46bb855f10831f3080b30f1a9998e51814216 Mon Sep 17 00:00:00 2001
From: Alexpux <alexey.pawlow@gmail.com>
Date: Mon, 6 Jul 2015 06:44:59 +0300
Subject: [PATCH] Do not convert environment for strace
strace is a Windows program so MSYS2 will convert
all arguments and environment vars and that makes
debugging msys2 software with strace very tricky.
---
winsup/cygwin/spawn.cc | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index df5777f..230ddd5 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -546,11 +546,13 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
bool switch_user = ::cygheap->user.issetuid ()
&& (::cygheap->user.saved_uid
!= ::cygheap->user.real_uid);
+ bool keep_posix = (iscmd (argv[0], "strace.exe")
+ || iscmd (argv[0], "strace")) ? true : real_path.iscygexec ();
moreinfo->envp = build_env (envp, envblock, moreinfo->envc,
real_path.iscygexec (),
switch_user ? ::cygheap->user.primary_token ()
: NULL,
- real_path.iscygexec ());
+ keep_posix);
if (!moreinfo->envp || !envblock)
{
set_errno (E2BIG);
--
2.4.5

View File

@@ -0,0 +1,45 @@
From 0681f64468e3966e179abf5fbe9e6d702a7ae777 Mon Sep 17 00:00:00 2001
From: Ray Donnelly <mingw.android@gmail.com>
Date: Fri, 21 Aug 2015 12:52:09 +0100
Subject: [PATCH 1/5] dcrt0.cc: Untangle allow_glob from winshell
Otherwise if globbing is allowed and we get called from a
Windows program, build_argv thinks we've been called from
a Cygwin program.
---
winsup/cygwin/dcrt0.cc | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc
index 5bd1f1d..b7600b6 100644
--- a/winsup/cygwin/dcrt0.cc
+++ b/winsup/cygwin/dcrt0.cc
@@ -294,7 +294,7 @@ globify (char *word, char **&argv, int &argc, int &argvlen)
/* Build argv, argc from string passed from Windows. */
static void __stdcall
-build_argv (char *cmd, char **&argv, int &argc, int winshell)
+build_argv (char *cmd, char **&argv, int &argc, int winshell, int glob)
{
int argvlen = 0;
int nesting = 0; // monitor "nesting" from insert_file
@@ -354,7 +354,7 @@ build_argv (char *cmd, char **&argv, int &argc, int winshell)
}
/* Add word to argv file after (optional) wildcard expansion. */
- if (!winshell || !argc || !globify (word, argv, argc, argvlen))
+ if (!glob || !argc || !globify (word, argv, argc, argvlen))
{
debug_printf ("argv[%d] = '%s'", argc, word);
argv[argc++] = word;
@@ -959,6 +959,7 @@ dll_crt0_1 (void *)
/* Scan the command line and build argv. Expand wildcards if not
called from another cygwin process. */
build_argv (line, __argv, __argc,
+ NOTSTATE (myself, PID_CYGPARENT),
NOTSTATE (myself, PID_CYGPARENT) && allow_glob);
/* Convert argv[0] to posix rules if it's currently blatantly
--
2.5.0

View File

@@ -0,0 +1,60 @@
From b918037c52b21913054f1e7b62ba8ca2d79b6ee9 Mon Sep 17 00:00:00 2001
From: Ray Donnelly <mingw.android@gmail.com>
Date: Mon, 24 Aug 2015 00:48:06 +0100
Subject: [PATCH 2/5] dcrt0.cc (globify): Don't quote literal strings
differently when dos_spec
Reverts 25ba8f306f3099caf8397859019e936b90510e8d. I can't figure out what
the intention was. I'm sure I'll find out soon enough when everything breaks.
This change means that input of:
'"C:/test.exe SOME_VAR=\"literal quotes\""'
becomes:
'C:/test.exe SOME_VAR="literal quotes"'
instead of:
'C:/test.exe SOME_VAR=\literal quotes\'
.. which is at least consistent with the result for:
'"no_drive_or_colon SOME_VAR=\"literal quotes\""'
The old result of course resulted in the quoted string being split into
two arguments at the space which is clearly not intended.
I *guess* backslashes in dos paths may have been the issue here?
If so I don't care since we should not use them, ever, esp. not at
the expense of sensible forward-slash-containing input.
---
winsup/cygwin/dcrt0.cc | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc
index b7600b6..89e095e 100644
--- a/winsup/cygwin/dcrt0.cc
+++ b/winsup/cygwin/dcrt0.cc
@@ -239,10 +239,20 @@ globify (char *word, char **&argv, int &argc, int &argvlen)
char quote = *s;
while (*++s && *s != quote)
{
+ /* This used to be:
if (dos_spec || *s != '\\')
- /* nothing */;
+ // nothing
else if (s[1] == quote || s[1] == '\\')
s++;
+ With commit message:
+ dcrt0.cc (globify): Don't use \ quoting when apparently quoting a DOS path
+ spec, even within a quoted string.
+ But that breaks the "literal quotes" part of '"C:/test.exe SOME_VAR=\"literal quotes\""'
+ giving: 'C:/test.exe SOME_VAR=\literal quotes\' (with \'s between each character)
+ instead of 'C:/test.exe SOME_VAR="literal quotes"' (with \'s between each character)
+ */
+ if (*s == '\\' && (s[1] == quote || s[1] == '\\'))
+ s++;
*p++ = '\\';
size_t cnt = isascii (*s) ? 1 : mbtowc (NULL, s, MB_CUR_MAX);
if (cnt <= 1 || cnt == (size_t)-1)
--
2.5.0

View File

@@ -0,0 +1,67 @@
From c928f9023fd5576cfc627f2b0ad31324c8241b0f Mon Sep 17 00:00:00 2001
From: Ray Donnelly <mingw.android@gmail.com>
Date: Sun, 23 Aug 2015 20:47:30 +0100
Subject: [PATCH 3/5] strace.cc: Don't set MSYS=noglob
Commit message for this code was:
* strace.cc (create_child): Set CYGWIN=noglob when starting new process so that
Cygwin will leave already-parsed the command line alonw."
I can see no reason for it and it badly breaks the ability to use
strace.exe to investigate calling a Cygwin program from a Windows
program, for example:
strace mingw32-make.exe
.. where mingw32-make.exe finds sh.exe and uses it as the shell.
The reason it badly breaks this use-case is because dcrt0.cc depends
on globbing to happen to parse commandlines from Windows programs;
irrespective of whether they contain any glob patterns or not.
See quoted () comment:
"This must have been run from a Windows shell, so preserve
quotes for globify to play with later."
---
winsup/utils/strace.cc | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/winsup/utils/strace.cc b/winsup/utils/strace.cc
index 6d9d727..dc3ef55 100644
--- a/winsup/utils/strace.cc
+++ b/winsup/utils/strace.cc
@@ -355,6 +355,24 @@ create_child (char **argv)
make_command_line (one_line, argv);
SetConsoleCtrlHandler (NULL, 0);
+/* Commit message for this code was:
+"* strace.cc (create_child): Set CYGWIN=noglob when starting new process so that
+
+ Cygwin will leave already-parsed the command line alonw."
+
+ I can see no reason for it and it badly breaks the ability to use
+ strace.exe to investigate calling a Cygwin program from a Windows
+ program, for example:
+ strace mingw32-make.exe
+ .. where mingw32-make.exe finds sh.exe and uses it as the shell.
+ The reason it badly breaks this use-case is because dcrt0.cc depends
+ on globbing to happen to parse commandlines from Windows programs;
+ irrespective of whether they contain any glob patterns or not.
+
+ See quoted () comment:
+ "This must have been run from a Windows shell, so preserve
+ quotes for globify to play with later."
+
const char *cygwin_env = getenv ("MSYS");
const char *space;
if (cygwin_env)
@@ -364,6 +382,7 @@ create_child (char **argv)
char *newenv = (char *) malloc (sizeof ("MSYS=noglob") + strlen (space) + strlen (cygwin_env));
sprintf (newenv, "MSYS=noglob%s%s", space, cygwin_env);
_putenv (newenv);
+*/
ret = CreateProcess (0, one_line.buf, /* command line */
NULL, /* Security */
NULL, /* thread */
--
2.5.0

View File

@@ -0,0 +1,25 @@
From 86e1a43670375071a6285cf89c647063418456ab Mon Sep 17 00:00:00 2001
From: Ray Donnelly <mingw.android@gmail.com>
Date: Fri, 21 Aug 2015 12:18:52 +0100
Subject: [PATCH 4/5] Add debugging for build_argv
---
winsup/cygwin/dcrt0.cc | 2 ++
1 file changed, 2 insertions(+)
diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc
index 89e095e..621d469 100644
--- a/winsup/cygwin/dcrt0.cc
+++ b/winsup/cygwin/dcrt0.cc
@@ -313,6 +313,8 @@ build_argv (char *cmd, char **&argv, int &argc, int winshell, int glob)
argvlen = 0;
argv = NULL;
+ debug_printf ("cmd = '%s', winshell = %d, glob = %d", cmd, winshell, glob);
+
/* Scan command line until there is nothing left. */
while (*cmd)
{
--
2.5.0

View File

@@ -0,0 +1,24 @@
From 8d4c1ebb7911c49c927cf1d2ad88397cb6cba2e1 Mon Sep 17 00:00:00 2001
From: Ray Donnelly <mingw.android@gmail.com>
Date: Fri, 21 Aug 2015 09:52:47 +0100
Subject: [PATCH 5/5] Add debugging for strace make_command_line
---
winsup/utils/strace.cc | 1 +
1 file changed, 1 insertion(+)
diff --git a/winsup/utils/strace.cc b/winsup/utils/strace.cc
index dc3ef55..ead4c04 100644
--- a/winsup/utils/strace.cc
+++ b/winsup/utils/strace.cc
@@ -353,6 +353,7 @@ create_child (char **argv)
flags |= CREATE_NEW_CONSOLE | CREATE_NEW_PROCESS_GROUP;
make_command_line (one_line, argv);
+ printf ("create_child: %s\n", one_line.buf);
SetConsoleCtrlHandler (NULL, 0);
/* Commit message for this code was:
--
2.5.0

View File

@@ -22,30 +22,46 @@ makedepends=('cocom'
'gettext-devel'
'libiconv-devel'
'diffutils')
# options=('debug' '!strip')
source=('msys2-runtime'::'git+https://github.com/Alexpux/Cygwin.git#branch=msys2-master')
md5sums=('SKIP')
options=('debug' '!strip')
source=('msys2-runtime'::'git+https://github.com/Alexpux/Cygwin.git#branch=msys2-master'
'0001-dcrt0.cc-Untangle-allow_glob-from-winshell.patch'
'0002-dcrt0.cc-globify-Don-t-quote-literal-strings-differe.patch'
'0003-strace.cc-Don-t-set-MSYS-noglob.patch'
'0004-Add-debugging-for-build_argv.patch'
'0005-Add-debugging-for-strace-make_command_line.patch')
md5sums=('SKIP'
'c3c22297f0402edf02843e09d3ee9733'
'1d6f11e1106395671e861f372fe1e5de'
'4653f7d0dc64e872400346b02164d1e6'
'cc78a30ea8b6ba377bc1cd6ee23ea146'
'a3b123ebbf961a79815ecad574591a19')
pkgver() {
cd "${srcdir}/msys2-runtime"
cd "${srcdir}"/msys2-runtime
local gitver=$(git describe --tags | sed 's|cygwin-||g' | sed 's|_|.|g')
gitver=${gitver//-*}
#printf "%s.%s" "$(git rev-list --count HEAD)" "$(git rev-parse --short HEAD)"
printf "%s.%s.%s" "${gitver}" "$(git rev-list --count HEAD)" "$(git rev-parse --short HEAD)"
}
prepare() {
cd "$srcdir/msys2-runtime"
cd "${srcdir}"/msys2-runtime
git am "${srcdir}"/0001-dcrt0.cc-Untangle-allow_glob-from-winshell.patch
git am "${srcdir}"/0002-dcrt0.cc-globify-Don-t-quote-literal-strings-differe.patch
git am "${srcdir}"/0003-strace.cc-Don-t-set-MSYS-noglob.patch
git am "${srcdir}"/0004-Add-debugging-for-build_argv.patch
# git am "${srcdir}"/0005-Add-debugging-for-strace-make_command_line.patch
}
build() {
[[ -d $srcdir/build-${CHOST} ]] && rm -rf $srcdir/build-${CHOST}
mkdir -p $srcdir/build-${CHOST} && cd $srcdir/build-${CHOST}
[[ -d "${srcdir}"/build-${CHOST} ]] && rm -rf "${srcdir}"/build-${CHOST}
mkdir -p "${srcdir}"/build-${CHOST} && cd "${srcdir}"/build-${CHOST}
# Gives more verbose compile output when debugging.
local -a extra_config
if check_option "debug" "y"; then
export CCWRAP_VERBOSE=1
OPTIM="-O0"
extra_config+=(--enable-debugging)
else
OPTIM="-O2"
fi
@@ -53,19 +69,20 @@ build() {
CFLAGS="$OPTIM -pipe -ggdb"
CXXFLAGS="$OPTIM -pipe -ggdb"
${srcdir}/msys2-runtime/configure \
"${srcdir}"/msys2-runtime/configure \
--prefix=/usr \
--build=${CHOST} \
--sysconfdir=/etc
--sysconfdir=/etc \
"${extra_config[@]}"
LC_ALL=C make
LC_ALL=C make -j1 DESTDIR=${srcdir}/dest install
LC_ALL=C make -j1 DESTDIR="${srcdir}"/dest install
#pushd ${CHOST}/winsup/cygwin > /dev/null
#LANG=C make libmsys2_s.a
#cp libmsys2_s.a ${srcdir}/dest/usr/${CHOST}/lib/
#cp libmsys2_s.a "${srcdir}"/dest/usr/${CHOST}/lib/
#popd > /dev/null
rm -rf ${srcdir}/dest/etc
rm -rf "${srcdir}"/dest/etc
}
package_msys2-runtime() {
@@ -74,14 +91,14 @@ package_msys2-runtime() {
options=('!strip')
#install=msys2-runtime.install
mkdir -p $pkgdir/usr
cp -rf ${srcdir}/dest/usr/bin $pkgdir/usr/
rm -f $pkgdir/usr/bin/msys-2.0.dbg
rm -f $pkgdir/usr/bin/cyglsa-config
rm -f $pkgdir/usr/bin/cyglsa.dll
rm -f $pkgdir/usr/bin/cyglsa64.dll
rm -f $pkgdir/usr/bin/cygserver-config
cp -rf ${srcdir}/dest/usr/share $pkgdir/usr/
mkdir -p "${pkgdir}"/usr
cp -rf "${srcdir}"/dest/usr/bin "${pkgdir}"/usr/
rm -f "${pkgdir}"/usr/bin/msys-2.0.dbg
rm -f "${pkgdir}"/usr/bin/cyglsa-config
rm -f "${pkgdir}"/usr/bin/cyglsa.dll
rm -f "${pkgdir}"/usr/bin/cyglsa64.dll
rm -f "${pkgdir}"/usr/bin/cygserver-config
cp -rf "${srcdir}"/dest/usr/share "${pkgdir}"/usr/
}
package_msys2-runtime-devel() {
@@ -90,15 +107,15 @@ package_msys2-runtime-devel() {
depends=("msys2-runtime=${pkgver}")
options=('staticlibs' '!strip')
mkdir -p $pkgdir/usr/bin
cp -f ${srcdir}/dest/usr/bin/msys-2.0.dbg $pkgdir/usr/bin/
cp -rLf ${srcdir}/dest/usr/${CHOST}/include $pkgdir/usr/
rm -f $pkgdir/usr/include/iconv.h
rm -f $pkgdir/usr/include/unctrl.h
mkdir -p "${pkgdir}"/usr/bin
cp -f "${srcdir}"/dest/usr/bin/msys-2.0.dbg "${pkgdir}"/usr/bin/
cp -rLf "${srcdir}"/dest/usr/${CHOST}/include "${pkgdir}"/usr/
rm -f "${pkgdir}"/usr/include/iconv.h
rm -f "${pkgdir}"/usr/include/unctrl.h
# provided by libtirpc
rm -fr $pkgdir/usr/include/rpc/
rm -fr "${pkgdir}"/usr/include/rpc/
cp -rLf ${srcdir}/dest/usr/${CHOST}/lib $pkgdir/usr/
cp -rLf "${srcdir}"/dest/usr/${CHOST}/lib "${pkgdir}"/usr/
}
# return 0