Re: unable to remove oddly-named directory

2024-06-15 Thread Jeremy Drake via Cygwin
On Fri, 14 Jun 2024, Jeremy Drake via Cygwin wrote:

> On Fri, 14 Jun 2024, Jeremy Drake via Cygwin wrote:
>
> > Eliot Moss replied to me privately, and I confirmed as well, that this
> > does not occur with Cygwin 3.5.1-1.  So this appears to be a regression
> > in 3.5.3.
>
> It also does not occur in 3.6.0-0.81 or 3.6.0-0-0.115, so perhaps this was
> already fixed.  The only thing I saw at all relevant had to do with
> globbing, but seemed to be particularly about globbing on dos command
> lines...

I'm going nuts.  Today, none of the versions I tested yesterday are
working.  Maybe I was using the wrong character?  $'\uD800' is broken but
$'\uDC00' is OK.

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: unable to remove oddly-named directory

2024-06-14 Thread Jeremy Drake via Cygwin
On Fri, 14 Jun 2024, Jeremy Drake via Cygwin wrote:

> Eliot Moss replied to me privately, and I confirmed as well, that this
> does not occur with Cygwin 3.5.1-1.  So this appears to be a regression
> in 3.5.3.

It also does not occur in 3.6.0-0.81 or 3.6.0-0-0.115, so perhaps this was
already fixed.  The only thing I saw at all relevant had to do with
globbing, but seemed to be particularly about globbing on dos command
lines...

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: unable to remove oddly-named directory

2024-06-14 Thread Jeremy Drake via Cygwin
On Thu, 13 Jun 2024, Jeremy Drake via Cygwin wrote:

> Thankfully, this can be simply reproduced with the following two bash
> commands (on cygwin 3.5.3):
>
> mkdir -p foo/$'\uD800'
> rm -rf foo

Eliot Moss replied to me privately, and I confirmed as well, that this
does not occur with Cygwin 3.5.1-1.  So this appears to be a regression
in 3.5.3.

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: unable to remove oddly-named directory

2024-06-13 Thread Jeremy Drake via Cygwin
On Thu, 13 Jun 2024, Brian Inglis via Cygwin wrote:

> These reserved surrogate values should probably either be blocked, or encoded
> at
> the file system interface layer so they can be round tripped, like the Windows
> reserved characters, in the BMP or SMP PUAs.
>
> Reserved surrogate ranges are D800-DBFF|DC00-DFFF.

Cygwin already generates these unpaired surrogates in filenames, at least
here:
https://github.com/cygwin/cygwin/blob/84dff09aa0d3ad998551e3f91c61dc0534b0b8bf/winsup/cygwin/syscalls.cc#L342-L350

I was looking into ways to automate cleaning up these files, which would
likely be hampered if things like findutils would suddenly start
disallowing me from finding files with these characters in them.

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


unable to remove oddly-named directory

2024-06-13 Thread Jeremy Drake via Cygwin
Backstory: rust's test suite makes an oddly-named directory as part of a
test:
https://github.com/rust-lang/rust/blob/921645c737f1d6d107a0a10ca5ee129d364dcd7a/tests/run-make/non-unicode-in-incremental-dir/rmake.rs

When trying to clean up after a rust build/test with rm -rf, it results in
a "Directory not empty" error.

Thankfully, this can be simply reproduced with the following two bash
commands (on cygwin 3.5.3):

mkdir -p foo/$'\uD800'
rm -rf foo

This fails with: rm: cannot remove 'foo': Directory not empty
when it should succeed.

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: Please update keychain to 2.8.5 (Updated .cygport file attached)

2024-06-05 Thread Jeremy Drake via Cygwin
On Wed, 5 Jun 2024, Ken Takata via Cygwin wrote:

> > I've updated keychain.cygport for the latest version of keychain.
> > Please find the attached file.
> > Could you include this in the cygwin package repository and release a new 
> > version?
>
> How can we proceed to update keychain to 2.8.5?
>
> There were several requests for updating it.


Maybe try cygwin-a...@cygwin.com list?  I think that's where packaging is
supposed to happen

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


[PATCH v2] Cygwin: /proc//mount*: escape strings.

2024-06-04 Thread Jeremy Drake via Cygwin-patches
In order for these formats to be machine-parseable, characters used as
delimiters must be escaped.  Linux escapes space, tab, newline,
backslash, and hash (because code that parses mounts/mtab and fstab
would handle comments) using octal escapes.  Replicate that behavior
here.

Addresses: https://cygwin.com/pipermail/cygwin/2024-June/256082.html
Signed-off-by: Jeremy Drake 
---

Changes from original:
* forgot to include tab '\t' in characters that need escaping
* I mis-iterpreted how octal escapes work: they don't require a leading 0,
but Linux uses a fixed 3 digit format, which makes calculating the length
cleaner.

 winsup/cygwin/fhandler/process.cc | 76 +++
 1 file changed, 66 insertions(+), 10 deletions(-)

diff --git a/winsup/cygwin/fhandler/process.cc 
b/winsup/cygwin/fhandler/process.cc
index 37bdff84e3..db1763d702 100644
--- a/winsup/cygwin/fhandler/process.cc
+++ b/winsup/cygwin/fhandler/process.cc
@@ -1317,9 +1317,39 @@ extern "C" {
   struct mntent *getmntent (FILE *);
 };

+static size_t
+escape_string_length (const char *str, const char *escapees)
+{
+  size_t i, len = 0;
+
+  for (i = strcspn (str, escapees);
+   str[i];
+   i += strcspn (str + i + 1, escapees) + 1)
+len += 3;
+  return len + i;
+}
+
+static size_t
+escape_string (char *destbuf, const char *str, const char *escapees)
+{
+  size_t s, i;
+  char *p = destbuf;
+
+  for (s = 0, i = strcspn (str, escapees);
+   str[i];
+   s = i + 1, i += strcspn (str + s, escapees) + 1)
+{
+  p = stpncpy (p, str + s, i - s);
+  p += __small_sprintf (p, "\\%03o", (int)(unsigned char) str[i]);
+}
+  p = stpcpy (p, str + s);
+  return (p - destbuf);
+}
+
 static off_t
 format_process_mountstuff (void *data, char *, bool mountinfo)
 {
+  static const char MOUNTSTUFF_ESCAPEES[] = " \t\n\\#";
   _pinfo *p = (_pinfo *) data;
   user_info *u_shared = NULL;
   HANDLE u_hdl = NULL;
@@ -1369,9 +1399,9 @@ format_process_mountstuff (void *data, char *, 
bool mountinfo)
continue;
}
   destbuf = (char *) crealloc_abort (destbuf, len
- + strlen (mnt->mnt_fsname)
- + strlen (mnt->mnt_dir)
- + strlen (mnt->mnt_type)
+ + escape_string_length 
(mnt->mnt_fsname, MOUNTSTUFF_ESCAPEES)
+ + escape_string_length 
(mnt->mnt_dir, MOUNTSTUFF_ESCAPEES)
+ + escape_string_length 
(mnt->mnt_type, MOUNTSTUFF_ESCAPEES)
  + strlen (mnt->mnt_opts)
  + 30);
   if (mountinfo)
@@ -1380,18 +1410,44 @@ format_process_mountstuff (void *data, char *, 
bool mountinfo)
  dev_t dev = pc.exists () ? pc.fs_serial_number () : -1;

  len += __small_sprintf (destbuf + len,
- "%d %d %d:%d / %s %s - %s %s %s\n",
+ "%d %d %d:%d / ",
  iteration, iteration,
- major (dev), minor (dev),
- mnt->mnt_dir, mnt->mnt_opts,
- mnt->mnt_type, mnt->mnt_fsname,
+ major (dev), minor (dev));
+ len += escape_string (destbuf + len,
+   mnt->mnt_dir,
+   MOUNTSTUFF_ESCAPEES);
+ len += __small_sprintf (destbuf + len,
+ " %s - ",
+ mnt->mnt_opts);
+ len += escape_string (destbuf + len,
+   mnt->mnt_type,
+   MOUNTSTUFF_ESCAPEES);
+ destbuf[len++] = ' ';
+ len += escape_string (destbuf + len,
+   mnt->mnt_fsname,
+   MOUNTSTUFF_ESCAPEES);
+ len += __small_sprintf (destbuf + len,
+ " %s\n",
  (pc.fs_flags () & FILE_READ_ONLY_VOLUME)
  ? "ro" : "rw");
}
   else
-   len += __small_sprintf (destbuf + len, "%s %s %s %s %d %d\n",
-   mnt->mnt_fsname, mnt->mnt_dir, mnt->mnt_type,
-   mnt->mnt_opts, mnt->mnt_freq, mnt->mnt_passno);
+{
+ len += escape_string (destbuf + len,
+   mnt->mnt_fsname,
+   MOUNTSTUFF_ESCAPEES);
+ destbuf[len++] = ' ';
+ len += escape_string (destbuf + len,
+   mnt->mnt_dir,
+   MOUNTSTUFF_ESCAPEES);
+ destbuf[len++] = ' ';
+ len += escape_string (destbuf + len,
+   

[PATCH] Cygwin: /proc//mount*: escape strings.

2024-06-04 Thread Jeremy Drake via Cygwin-patches
In order for these formats to be machine-parseable, characters used as
delimiters must be escaped.  Linux escapes space, newline, backslash,
and hash (because code that parses mounts/mtab and fstab would handle
comments) using octal escapes.  Replicate that behavior here.

Addresses: https://cygwin.com/pipermail/cygwin/2024-June/256082.html
Signed-off-by: Jeremy Drake 
---

I took a crack at adding escaping to "mountstuff".  It seems like there
might be other such /proc entries that are expected to be machine-readable
that might need escaping.  As such, perhaps the escape_string* functions
should be in some other file (and not static) so other source files can
call them too.

I made some effort to match formatting, but I may well have missed
something (I just noticed I missed space between function name and open
paren on the new functions, I went ahead and fixed that in the patch
manually in the email client).

 winsup/cygwin/fhandler/process.cc | 81 +++
 1 file changed, 71 insertions(+), 10 deletions(-)

diff --git a/winsup/cygwin/fhandler/process.cc 
b/winsup/cygwin/fhandler/process.cc
index 37bdff84e3..0ab07bd119 100644
--- a/winsup/cygwin/fhandler/process.cc
+++ b/winsup/cygwin/fhandler/process.cc
@@ -1317,9 +1317,44 @@ extern "C" {
   struct mntent *getmntent (FILE *);
 };

+static size_t
+escape_string_length (const char *str, const char *escapees)
+{
+  size_t i, len = 0;
+
+  for (i = strcspn (str, escapees);
+   str[i];
+   i += strcspn (str + i + 1, escapees) + 1)
+if ((unsigned char) str[i] < 8)
+  len += 2;
+else if ((unsigned char) str[i] < 64)
+  len += 3;
+else
+  len += 4;
+  return len + i;
+}
+
+static size_t
+escape_string (char *destbuf, const char *str, const char *escapees)
+{
+  size_t s, i;
+  char *p = destbuf;
+
+  for (s = 0, i = strcspn (str, escapees);
+   str[i];
+   s = i + 1, i += strcspn (str + s, escapees) + 1)
+{
+  p = stpncpy (p, str + s, i - s);
+  p += __small_sprintf (p, "\\0%o", (int)(unsigned char) str[i]);
+}
+  p = stpcpy (p, str + s);
+  return (p - destbuf);
+}
+
 static off_t
 format_process_mountstuff (void *data, char *, bool mountinfo)
 {
+  static const char MOUNTSTUFF_ESCAPEES[] = " \n\\#";
   _pinfo *p = (_pinfo *) data;
   user_info *u_shared = NULL;
   HANDLE u_hdl = NULL;
@@ -1369,9 +1404,9 @@ format_process_mountstuff (void *data, char *, 
bool mountinfo)
continue;
}
   destbuf = (char *) crealloc_abort (destbuf, len
- + strlen (mnt->mnt_fsname)
- + strlen (mnt->mnt_dir)
- + strlen (mnt->mnt_type)
+ + escape_string_length 
(mnt->mnt_fsname, MOUNTSTUFF_ESCAPEES)
+ + escape_string_length 
(mnt->mnt_dir, MOUNTSTUFF_ESCAPEES)
+ + escape_string_length 
(mnt->mnt_type, MOUNTSTUFF_ESCAPEES)
  + strlen (mnt->mnt_opts)
  + 30);
   if (mountinfo)
@@ -1380,18 +1415,44 @@ format_process_mountstuff (void *data, char *, 
bool mountinfo)
  dev_t dev = pc.exists () ? pc.fs_serial_number () : -1;

  len += __small_sprintf (destbuf + len,
- "%d %d %d:%d / %s %s - %s %s %s\n",
+ "%d %d %d:%d / ",
  iteration, iteration,
- major (dev), minor (dev),
- mnt->mnt_dir, mnt->mnt_opts,
- mnt->mnt_type, mnt->mnt_fsname,
+ major (dev), minor (dev));
+ len += escape_string (destbuf + len,
+   mnt->mnt_dir,
+   MOUNTSTUFF_ESCAPEES);
+ len += __small_sprintf (destbuf + len,
+ " %s - ",
+ mnt->mnt_opts);
+ len += escape_string (destbuf + len,
+   mnt->mnt_type,
+   MOUNTSTUFF_ESCAPEES);
+ destbuf[len++] = ' ';
+ len += escape_string (destbuf + len,
+   mnt->mnt_fsname,
+   MOUNTSTUFF_ESCAPEES);
+ len += __small_sprintf (destbuf + len,
+ " %s\n",
  (pc.fs_flags () & FILE_READ_ONLY_VOLUME)
  ? "ro" : "rw");
}
   else
-   len += __small_sprintf (destbuf + len, "%s %s %s %s %d %d\n",
-   mnt->mnt_fsname, mnt->mnt_dir, mnt->mnt_type,
-   mnt->mnt_opts, mnt->mnt_freq, mnt->mnt_passno);
+{
+ len += escape_string 

Re: mount points with whitespace are not escaped

2024-06-03 Thread Jeremy Drake via Cygwin
On Mon, 3 Jun 2024, Jeremy Drake via Cygwin wrote:

> /proc/self/mounts and /proc/self/mountinfo use octal escapes for ' ' and
> \n (I was rather surprised they didn't escape \r also, but I guess they
> don't have to because only ' ' and \n are used as delimiters):

Went looking at Linux source code, I guess they escape ' ' \n and \, and
"recently" added #
(https://github.com/torvalds/linux/commit/ed5fce76b5ea40c87b44cafbe4f3222da8ec981a)

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


mount points with whitespace are not escaped

2024-06-03 Thread Jeremy Drake via Cygwin
Steps to reproduce:

$ mkdir /$'My New\r\nFolder'
$ mount c: /$'My New\r\nFolder'
$ mount
C:/cygwin64/bin on /usr/bin type ntfs (binary,auto)
C:/cygwin64/lib on /usr/lib type ntfs (binary,auto)
C:/cygwin64 on / type ntfs (binary,auto)
C: on /My New
Folder type ntfs (binary,user)

$ cat /proc/self/mounts
C:/cygwin64/bin /usr/bin ntfs binary,auto 1 1
C:/cygwin64/lib /usr/lib ntfs binary,auto 1 1
C:/cygwin64 / ntfs binary,auto 1 1
C: /My New
Folder ntfs binary,user 1 1

$ cat /proc/self/mountinfo
0 0 39488:20815 / /usr/bin binary,auto - ntfs C:/cygwin64/bin rw
1 1 39488:20815 / /usr/lib binary,auto - ntfs C:/cygwin64/lib rw
2 2 39488:20815 / / binary,auto - ntfs C:/cygwin64 rw
3 3 39488:20815 / /My New
Folder binary,user - ntfs C: rw


Expected (what happens on Linux) is that mount outputs the \r\n as ??, but
/proc/self/mounts and /proc/self/mountinfo use octal escapes for ' ' and
\n (I was rather surprised they didn't escape \r also, but I guess they
don't have to because only ' ' and \n are used as delimiters):

$ mount
...
/dev/mapper/XXX on /My New??Folder type ext3 (rw,noatime,data=ordered)

$ cat /proc/self/mounts | xxd
...
0400: 6564 2030 2030 0a2f 6465 762f 646d 2d30  ed 0 0./dev/dm-0
0410: 202f 4d79 5c30 3430 4e65 770d 5c30 3132   /My\040New.\012
0420: 466f 6c64 6572 2065 7874 3320 7277 2c6e  Folder ext3 rw,n
0430: 6f61 7469 6d65 2c64 6174 613d 6f72 6465  oatime,data=orde
0440: 7265 6420 3020 300a  red 0 0.

$ cat /proc/self/mountinfo | xxd
...
04f0: 2f62 696e 202f 4d79 5c30 3430 4e65 770d  /bin /My\040New.
0500: 5c30 3132 466f 6c64 6572 2072 772c 6e6f  \012Folder rw,no
0510: 6174 696d 6520 2d20 6578 7433 202f 6465  atime - ext3 /de
0520: 762f 646d 2d30 2072 772c 6461 7461 3d6f  v/dm-0 rw,data=o
0530: 7264 6572 6564 0ardered.

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


cleanup of in-use files moved to recycle bin

2024-06-03 Thread Jeremy Drake via Cygwin
I'm not necessarily sure that the subject is clear enough, so I want to be
explicit that I'm talking about files (or I guess potentially directories,
though I've never seen that) generated by the `try_to_bin` function in
winsup/cygwin/syscalls.cc.  Specifically, you can generate one with this
simple bash reproducer:

cp /usr/bin/sleep.exe .
./sleep 1000 &
rm -f ./sleep.exe
kill %1
fg

My questions are (starting at the beginning with what I'm trying to
accomplish, and wandering off into the weeds of the various things I've
tried to do that).

1) is there some mechanism in Cygwin that I'm not seeing to clean up these
files?  So far I've confirmed that their creation does not result in the
recycle bin icon turning from 'empty' to 'full' on the desktop, and that
emptying the recycle bin there (when it doesn't think it's already empty)
does not remove them.

2) assuming there is not, I want to make a script using only things
present in a "base system" to clean them up.  This isn't too horribly
difficult, using bash and find, *if I can assume that all Windows volumes
are rooted at drive letters*.  Once I take into account Windows
folder-mounted volumes, I need to be able to find a list of all volume
roots, in a safe way.  I hacked together a sed script that parses the
output of `mountvol`, but that is clearly not safe (not only did it rely
on a message that is likely translated, but mountvol seems to switch to
ANSI output when it is piped or redirected, and any unicode characters
turn into '_') [1].

I made a C program to do this [2], and that's fine, except I really wanted
to avoid having to ship a new binary.  Is there any way in Cygwin to get
this list?  Maybe this is something that should be added to cygpath?

3) in furtherance of figuring that out, I started grepping around for
FindNextVolumeW and GetVolumePathNamesForVolumeNameW in cygwin, and found
the dos_drive_mappings class in winsup/cygwin/mount.cc.  This is
sort-of exposed via the cygwin_internal function CW_ALLOC_DRIVE_MAP,
though the type itself is not.  As a proof-of-concept hack, I wrote
another C program based on this [3], but I don't think that really helps
anything.

4) while pondering this, I keep coming back to the idea that volume folder
mounts could be exposed via the 'normal' mount interface, such as
`getmntent`, `/proc/self/mounts`, `/proc/self/mountinfo`), as children of
the drive-letter cygdrive mounts.  This would make getting the list of
mounts safely a solved problem(*), and would probably also have the bonus
effect of letting `df` list their free space.  Is there a good reason that
this is not already done (other than SHTDI)?

* I found a bug/limitation here, which I will report in a separate email,
because that doesn't depend on any of this.

5) in addition, I ported the C example programs to python ctypes, and
factored out to a small module [4], but that also doesn't really help,
because python is not part of the "base system".  But it might be useful
to somebody else, so I link to it here too.

[1]: https://github.com/msys2/MSYS2-packages/issues/4622#issuecomment-2140990166
[2]: https://gist.github.com/jeremyd2019/8e088a72dfef44ee29ed3442957c1e65
[3]: https://gist.github.com/jeremyd2019/ac2f00ec448e75c4bd3630926201db19
[4]: https://gist.github.com/jeremyd2019/95c2cfd7eef2ed29a339860896deddec

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: [PATCH v4] Cygwin: pthread: Fix a race issue introduced by the commit 2c5433e5da82

2024-06-01 Thread Jeremy Drake via Cygwin-patches
On Sat, 1 Jun 2024, Takashi Yano wrote:

> +  const int destroyed = INT_MIN >> 1;/* 0b1100 */

I thought whether or not right shifting a negative number sign-extends was
undefined in the C/C++ standards?


Re: [PATCH v1] Cygwin: disable high-entropy VA for ldh

2024-05-28 Thread Jeremy Drake via Cygwin-patches
On Tue, 28 May 2024, Jeremy Drake via Cygwin-patches wrote:

> @@ -53,6 +53,7 @@ cygcheck_LDADD = -lz -lwininet -lshlwapi -lpsapi -lntdll

Oops, I accidentally generated this patch against msys2-3.5.3 branch,
rather than cygwin master like the last one.  The only difference is the
line numbers above, and it does apply cleanly to cygwin master, so I won't
send another version unless I'm requested to.


[PATCH v1] Cygwin: disable high-entropy VA for ldh

2024-05-28 Thread Jeremy Drake via Cygwin-patches
If ldd is run against a DLL which links to the Cygwin DLL, ldh will end
up loading the Cygwin DLL dynamically, much like cygcheck or strace.

Addresses: https://cygwin.com/pipermail/cygwin/2024-May/255991.html
Fixes: 60675f1a7eb2 ("Cygwin: decouple shared mem regions from Cygwin DLL")
Signed-off-by: Jeremy Drake 
---
 winsup/utils/mingw/Makefile.am | 1 +
 1 file changed, 1 insertion(+)

diff --git a/winsup/utils/mingw/Makefile.am b/winsup/utils/mingw/Makefile.am
index b89d89490a..07b9f928d4 100644
--- a/winsup/utils/mingw/Makefile.am
+++ b/winsup/utils/mingw/Makefile.am
@@ -53,6 +53,7 @@ cygcheck_LDADD = -lz -lwininet -lshlwapi -lpsapi -lntdll
 cygwin_console_helper_SOURCES = cygwin-console-helper.cc

 ldh_SOURCES = ldh.cc
+ldh_LDFLAGS = ${AM_LDFLAGS} -Wl,--disable-high-entropy-va

 strace_SOURCES = \
path.cc \
-- 
2.45.1.windows.1




Re: [PATCH] Cygwin: disable high-entropy VA for ldh

2024-05-28 Thread Jeremy Drake via Cygwin-patches
On Tue, 28 May 2024, Takashi Yano wrote:

> On Mon, 27 May 2024 22:36:07 -0700 (PDT)
> Jeremy Drake wrote:
> > If ldd is run against a DLL or EXE which links to the Cygwin DLL, ldh
> > will end up loading the Cygwin DLL dynamically, much like cygcheck or
> > strace.
>
> IIUC, ldh is not used for EXE.


You are correct.  I didn't read ldd.cc.  I'll send the patch again with
"or EXE" removed from the commit message.


[PATCH] Cygwin: disable high-entropy VA for ldh

2024-05-27 Thread Jeremy Drake via Cygwin-patches
If ldd is run against a DLL or EXE which links to the Cygwin DLL, ldh
will end up loading the Cygwin DLL dynamically, much like cygcheck or
strace.

Addresses: https://cygwin.com/pipermail/cygwin/2024-May/255991.html
Fixes: 60675f1a7eb2 ("Cygwin: decouple shared mem regions from Cygwin DLL")
Signed-off-by: Jeremy Drake 
---
 winsup/utils/mingw/Makefile.am | 1 +
 1 file changed, 1 insertion(+)

diff --git a/winsup/utils/mingw/Makefile.am b/winsup/utils/mingw/Makefile.am
index d9557d8b50..7f7317ae15 100644
--- a/winsup/utils/mingw/Makefile.am
+++ b/winsup/utils/mingw/Makefile.am
@@ -38,6 +38,7 @@ cygcheck_LDADD = -lz -lwininet -lshlwapi -lpsapi -lntdll
 cygwin_console_helper_SOURCES = cygwin-console-helper.cc

 ldh_SOURCES = ldh.cc
+ldh_LDFLAGS = ${AM_LDFLAGS} -Wl,--disable-high-entropy-va

 strace_SOURCES = \
path.cc \
-- 
2.45.1.windows.1



Re: frequent hangs running ldd

2024-05-24 Thread Jeremy Drake via Cygwin
On Fri, 24 May 2024, Jeremy Drake wrote:

> On Fri, 24 May 2024, Jeremy Drake wrote:
>
> > Looking at !address, it seems Windows put the PEB, TEBs, and stacks in the
> > area where the cygheap should be.  Way to go, ASLR :P
>
> I think the fix for this would be to add -Wl,--disable-high-entropy-va to
> ldh_LDFLAGS, as was done for strace and cygcheck at least.  I used peflags
> -d0 /usr/bin/ldh.exe and I'm not seeing a hang after that.

Sorry, that was peflags -e0 not -d0 (dynamicbase is still on):
$ peflags -v /usr/bin/ldh.exe
/usr/bin/ldh.exe:
coff(0x0226[+executable_image,+line_nums_stripped,+bigaddr,+sepdbg])
pe(0x0140[+dynamicbase,+nxcompat])

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: frequent hangs running ldd

2024-05-24 Thread Jeremy Drake via Cygwin
On Fri, 24 May 2024, Jeremy Drake wrote:

> Looking at !address, it seems Windows put the PEB, TEBs, and stacks in the
> area where the cygheap should be.  Way to go, ASLR :P

I think the fix for this would be to add -Wl,--disable-high-entropy-va to
ldh_LDFLAGS, as was done for strace and cygcheck at least.  I used peflags
-d0 /usr/bin/ldh.exe and I'm not seeing a hang after that.

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: frequent hangs running ldd

2024-05-24 Thread Jeremy Drake via Cygwin
On Fri, 24 May 2024, Jeremy Drake wrote:

> On Fri, 24 May 2024, Jeremy Drake wrote:
>
> > Windbg reports that ldh.exe is already being debugged.  I was able to do a
> > "non-invasive" attach to ldh.exe in windbg, but it doesn't seem to be able
> > to deal with the split debug symbols (gnulink?).  I don't know if gdb can
> > do a non-invasive attach like that (or open a minidump assuming one could
> > be made from a non-invasize attach in windbg).
>
> Seems it can, and at least lldb can load a minidump (unfortunately it's
> not showing source file/line info like gdb does):
> (lldb) bt
> * thread #1, stop reason = Exception 0x8007 encountered at address
> 0x00
>   * frame #0: 0x000180178837 msys-2.0.dll`cygheap_init()

It appears that cygheap is NULL, so I'm guessing that VirtualAlloc failed.
!gle in windbg shows
0:000> !gle
LastErrorValue: (Win32) 0x1e7 (487) - Attempt to access invalid address.
LastStatusValue: (NTSTATUS) 0xc018 - {Conflicting Address Range}  The
specified address range conflicts with the address space.

Looking at !address, it seems Windows put the PEB, TEBs, and stacks in the
area where the cygheap should be.  Way to go, ASLR :P
BaseAddress  EndAddress+1RegionSize Type   State
 Protect Usage
--
+5`e81810008`05a02`1d87f000 MEM_FREE
PAGE_NOACCESS  Free
+8`05a08`05b570000`00157000 MEM_PRIVATE MEM_RESERVE 
   
 8`05b570008`05b580000`1000 MEM_PRIVATE MEM_COMMIT  
PAGE_READWRITE PEB[4628]
 8`05b580008`05b5a0000`2000 MEM_PRIVATE MEM_COMMIT  
PAGE_READWRITE TEB[~0; 4628.31ac]
 8`05b5a0008`05b5c0000`2000 MEM_PRIVATE MEM_COMMIT  
PAGE_READWRITE TEB[~1; 4628.4aac]
 8`05b5c0008`05b5e0000`2000 MEM_PRIVATE MEM_COMMIT  
PAGE_READWRITE TEB[~2; 4628.5840]
 8`05b5e0008`05b60`2000 MEM_PRIVATE MEM_COMMIT  
PAGE_READWRITE TEB[~3; 4628.6b9c]
 8`05b68`05c00`000a MEM_PRIVATE MEM_RESERVE 
   
+8`05c08`05df60000`001f6000 MEM_PRIVATE MEM_RESERVE 
   Stack  [~0; 4628.31ac]
 8`05df60008`05df90000`3000 MEM_PRIVATE MEM_COMMIT  
PAGE_READWRITE | PAGE_GUARDStack  [~0; 4628.31ac]
 8`05df90008`05e00`7000 MEM_PRIVATE MEM_COMMIT  
PAGE_READWRITE Stack  [~0; 4628.31ac]
+8`05e08`05ffb0000`001fb000 MEM_PRIVATE MEM_RESERVE 
   Stack  [~1; 4628.4aac]
 8`05ffb0008`05ffe0000`3000 MEM_PRIVATE MEM_COMMIT  
PAGE_READWRITE | PAGE_GUARDStack  [~1; 4628.4aac]
 8`05ffe0008`06000`2000 MEM_PRIVATE MEM_COMMIT  
PAGE_READWRITE Stack  [~1; 4628.4aac]
+8`06008`061fb0000`001fb000 MEM_PRIVATE MEM_RESERVE 
   Stack  [~2; 4628.5840]
 8`061fb0008`061fe0000`3000 MEM_PRIVATE MEM_COMMIT  
PAGE_READWRITE | PAGE_GUARDStack  [~2; 4628.5840]
 8`061fe0008`06200`2000 MEM_PRIVATE MEM_COMMIT  
PAGE_READWRITE Stack  [~2; 4628.5840]
+8`06208`063fb0000`001fb000 MEM_PRIVATE MEM_RESERVE 
   Stack  [~3; 4628.6b9c]
 8`063fb0008`063fe0000`3000 MEM_PRIVATE MEM_COMMIT  
PAGE_READWRITE | PAGE_GUARDStack  [~3; 4628.6b9c]
 8`063fe0008`06400`2000 MEM_PRIVATE MEM_COMMIT  
PAGE_READWRITE Stack  [~3; 4628.6b9c]
+8`0640  19e`6440  196`5e00 MEM_FREE
PAGE_NOACCESS  Free

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: frequent hangs running ldd

2024-05-24 Thread Jeremy Drake via Cygwin
On Fri, 24 May 2024, Jeremy Drake wrote:

> Windbg reports that ldh.exe is already being debugged.  I was able to do a
> "non-invasive" attach to ldh.exe in windbg, but it doesn't seem to be able
> to deal with the split debug symbols (gnulink?).  I don't know if gdb can
> do a non-invasive attach like that (or open a minidump assuming one could
> be made from a non-invasize attach in windbg).

Seems it can, and at least lldb can load a minidump (unfortunately it's
not showing source file/line info like gdb does):
(lldb) bt
* thread #1, stop reason = Exception 0x8007 encountered at address
0x00
  * frame #0: 0x000180178837 msys-2.0.dll`cygheap_init()
frame #1: 0x000180178b89 msys-2.0.dll`setup_cygheap()
frame #2: 0x0001800488bd msys-2.0.dll`dll_crt0_0()
frame #3: 0x00018007197c msys-2.0.dll`dll_entry(h=0x00018004, 
reason=, static_load=)
frame #4: 0x7ffece99869f 
ntdll.dll`RtlActivateActivationContextUnsafeFast + 303
frame #5: 0x7ffece9dd03d ntdll.dll`RtlEnumerateEntryHashTable + 957
frame #6: 0x7ffece9dcdee ntdll.dll`RtlEnumerateEntryHashTable + 366
frame #7: 0x7ffece9dce60 ntdll.dll`RtlEnumerateEntryHashTable + 480
frame #8: 0x7ffece9dce60 ntdll.dll`RtlEnumerateEntryHashTable + 480
frame #9: 0x7ffece9dce60 ntdll.dll`RtlEnumerateEntryHashTable + 480
frame #10: 0x7ffece9dce60 ntdll.dll`RtlEnumerateEntryHashTable + 480
frame #11: 0x7ffece99d62d ntdll.dll`RtlCopyUnicodeString + 1293
frame #12: 0x7ffece998940 ntdll.dll`RtlImageRvaToSection + 592
frame #13: 0x7ffece988cac ntdll.dll`RtlUnicodeToCustomCPN + 1020
frame #14: 0x7ffece99a25a ntdll.dll`LdrLoadDll + 250
frame #15: 0x7ffecbe961e2 KERNELBASE.dll`LoadLibraryExW + 370
frame #16: 0x7ff6df3414ba ldh.exe
frame #17: 0x7ff6df3412ee ldh.exe
frame #18: 0x7ff6df341406 ldh.exe
frame #19: 0x7ffecdc6257d kernel32.dll`BaseThreadInitThunk + 29
frame #20: 0x7ffece9caa48 ntdll.dll`RtlUserThreadStart + 40
msys-2.0.dll`cygheap_init:
0x180178750 <+0>:   pushq  %rbx
0x180178751 <+1>:   subq   $0x20, %rsp
0x180178755 <+5>:   leaq   0x102de4(%rip), %rcx ; _sys_nerr + 148864
0x18017875c <+12>:  leaq   0x153b2f(%rip), %rdx ; __infinity + 5074
0x180178763 <+19>:  callq  0x1800cce50; muto::init at 32:1
0x180178768 <+24>:  movq   0x102e01(%rip), %rcx ; _sys_nerr + 148912
0x18017876f <+31>:  leaq   0x102e02(%rip), %rax ; _sys_nerr + 148920
0x180178776 <+38>:  cmpq   %rax, %rcx
0x180178779 <+41>:  je 0x1801787d0; <+128> at 294:47
0x18017877b <+43>:  cmpq   $0x0, 0x4870(%rcx)
0x180178783 <+51>:  je 0x1801787a0; <+80> at 311:3
0x180178785 <+53>:  cmpq   $0x0, 0x48a0(%rcx)
0x18017878d <+61>:  je 0x1801787b5; <+101> at 312:14
0x18017878f <+63>:  addq   $0x20, %rsp
0x180178793 <+67>:  popq   %rbx
0x180178794 <+68>:  jmp0x180178680; init_cygheap::init_tls_list at 
638:1
0x180178799 <+73>:  nopl   (%rax)
0x1801787a0 <+80>:  cmpq   $0x0, 0x48a0(%rcx)
0x1801787a8 <+88>:  movq   $0x3, 0x4888(%rcx)
0x1801787b3 <+99>:  jne0x18017878f; <+63> at 314:1
0x1801787b5 <+101>: callq  0x1800c0c10; sigalloc at 125:1
0x1801787ba <+106>: movq   0x102daf(%rip), %rcx ; _sys_nerr + 148912
0x1801787c1 <+113>: addq   $0x20, %rsp
0x1801787c5 <+117>: popq   %rbx
0x1801787c6 <+118>: jmp0x180178680; init_cygheap::init_tls_list at 
638:1
0x1801787cb <+123>: nopl   (%rax,%rax)
0x1801787d0 <+128>: movabsq $0x8, %rbx ; imm = 0x8
0x1801787da <+138>: movl   $0x1, %r9d
0x1801787e0 <+144>: movl   $0x2000, %r8d ; imm = 0x2000
0x1801787e6 <+150>: movabsq $0x2, %rdx ; imm = 0x2
0x1801787f0 <+160>: movq   %rbx, %rcx
0x1801787f3 <+163>: callq  0x180217090; _alloca + 4336
0x1801787f8 <+168>: movq   %rbx, %rcx
0x1801787fb <+171>: movl   $0x4, %r9d
0x180178801 <+177>: movl   $0x1000, %r8d ; imm = 0x1000
0x180178807 <+183>: movl   $0x30, %edx ; imm = 0x30
0x18017880c <+188>: movq   %rax, 0x102d5d(%rip) ; _sys_nerr + 148912
0x180178813 <+195>: callq  0x180217090; _alloca + 4336
0x180178818 <+200>: movq   %rax, %rcx
0x18017881b <+203>: movq   %rax, 0x102d4e(%rip) ; _sys_nerr + 148912
0x180178822 <+210>: leaq   0x48e0(%rax), %rax
0x180178829 <+217>: movq   %rax, 0x102d38(%rip) ; _sys_nerr + 148904
0x180178830 <+224>: leaq   0x3d319(%rip), %rax ; __utf8_mbtowc at 534:1
->  0x180178837 <+231>: movq   %rax, (%rcx)
0x18017883a <+234>: movl   $0x12, 0x4828(%rcx)
0x180178844 <+244>: movq   $0x0, 0x4830(%rcx)
0x18017884f <+255>: jmp0x18017877b; <+43> at 309:3

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: 

Re: frequent hangs running ldd

2024-05-24 Thread Jeremy Drake via Cygwin
On Sat, 25 May 2024, Takashi Yano wrote:

> On Fri, 24 May 2024 14:46:40 -0700 (PDT)
> Jeremy Drake wrote:
> > > Thanks for the report. However, I cannot reproduce the issue.
> > > If it always hangs in GetConsoleProcessList (), I doubt it is not a cygwin
> > > bug but a windows bug.
> > >
> > > By any chance, is the number of processes that attach to the same pty more
> > > than 32768 in your environment?
> > >
> >
> > I doubt it, I was running a shell with this command:
> > find /usr/bin -name \*.dll -printf '%p:\n' -exec ldd '{}' \;
>
> Thanks for the details. I could reproduce the issue.
> It seems that ldh.exe (which is called from ldd?) falls into infinite loop.
> However, gdb cannot attach to ldh.exe...
>

Windbg reports that ldh.exe is already being debugged.  I was able to do a
"non-invasive" attach to ldh.exe in windbg, but it doesn't seem to be able
to deal with the split debug symbols (gnulink?).  I don't know if gdb can
do a non-invasive attach like that (or open a minidump assuming one could
be made from a non-invasize attach in windbg).

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: frequent hangs running ldd

2024-05-24 Thread Jeremy Drake via Cygwin
On Sat, 25 May 2024, Takashi Yano wrote:

> Thanks for the report. However, I cannot reproduce the issue.
> If it always hangs in GetConsoleProcessList (), I doubt it is not a cygwin
> bug but a windows bug.
>
> By any chance, is the number of processes that attach to the same pty more
> than 32768 in your environment?
>

I doubt it, I was running a shell with this command:
find /usr/bin -name \*.dll -printf '%p:\n' -exec ldd '{}' \;

This was in Windows Terminal, msys2-runtime 3.5.3, on Windows 11
10.0.22631.3593.  (I realized I forgot to include these details).

I will attempt to reproduce this in Windows Sandbox with Cygwin next.

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


frequent hangs running ldd

2024-05-24 Thread Jeremy Drake via Cygwin
Seen on msys2, but doesn't seem specific to it.

Frequently, when running ldd in a loop, it will hang.  I managed to get a
backtrace from gdb with symbols:

(gdb) bt
#0  0x7ffecea0fa34 in ntdll!ZwDeviceIoControlFile ()
   from /c/Windows/SYSTEM32/ntdll.dll
#1  0x7ffecbead7a9 in KERNELBASE!GetConsoleScreenBufferInfoEx ()
   from /c/Windows/System32/KERNELBASE.dll
#2  0x7ffecbef58dc in KERNELBASE!GetConsoleTitleW ()
   from /c/Windows/System32/KERNELBASE.dll
#3  0x7ffecbfb8195 in KERNELBASE!GetConsoleProcessList ()
   from /c/Windows/System32/KERNELBASE.dll
#4  0x00018015851f in fhandler_pty_common::get_console_process_id (
pid=19348, match=match@entry=true, cygwin=cygwin@entry=false,
stub_only=stub_only@entry=false, nat=nat@entry=false)
at /C/_/msys2-runtime/src/msys2-runtime/winsup/cygwin/fhandler/pty.cc:95
#5  0x000180101e3b in fhandler_console::attach_console (
owner=, err=err@entry=0x0)
at /C/_/msys2-runtime/src/msys2-runtime/winsup/cygwin/fhandler/console.cc:76
#6  0x000180102d40 in fhandler_console::set_output_mode (m=tty::native,
t=0x1a0030028, p=0x800021d68)
at 
/C/_/msys2-runtime/src/msys2-runtime/winsup/cygwin/fhandler/console.cc:853
#7  0x00018010d6cc in fhandler_console::set_console_mode_to_native ()
at 
/C/_/msys2-runtime/src/msys2-runtime/winsup/cygwin/fhandler/console.cc:4189
#8  0x00018010d71d in ContinueDebugEvent_Hooked (p=26628, t=26656, s=65538)
at 
/C/_/msys2-runtime/src/msys2-runtime/winsup/cygwin/fhandler/console.cc:4238
#9  0x000100401bd7 in ?? ()
#10 0x00010040279f in ?? ()
#11 0x0001800483a7 in dll_crt0_1 ()
at /C/_/msys2-runtime/src/msys2-runtime/winsup/cygwin/dcrt0.cc:1015
#12 0x000180045f63 in _cygtls::call2 (this=0x7ce00,
func=0x180047240 , arg=0x0,
buf=buf@entry=0x7cdf0)
at /C/_/msys2-runtime/src/msys2-runtime/winsup/cygwin/cygtls.cc:41
#13 0x000180046014 in _cygtls::call (func=,
arg=)
at /C/_/msys2-runtime/src/msys2-runtime/winsup/cygwin/cygtls.cc:28
#14 0x in ?? ()

Other attempts without symbols showed the same Windows APIs at least.

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: double-fork issue on Windows on ARM64

2024-05-21 Thread Jeremy Drake via Cygwin-developers
On Mon, 20 May 2024, Jeremy Drake wrote:

> Today, I was attempting to look at the TerminateThread situation.  The
> call in question comes from the attempt to terminate the wait_thread of a
> chld_procs entry.  I noticed elsewhere in cygwin code (flock.cc) that
> CancelSynchronousIo was being called, and that stood out to me because
> chances are that the wait thread (if running) is going to be blocked in
> ReadFile.  I am testing with the following hack, and so far have not seen
> a hang


I left my reproducer running with this hack, and I did eventually get an
error exit from the intermediate subprocess, which seems to have been a
signal 11 (if I'm reading the status from waitpid correctly).

What I noticed today is that in pinfo.cc, near the end of proc_waiter, it
sets vchild.wait_thread = NULL;.  If my reading of this is correct, that
does nothing useful, because vchild is a stack variable there and the
function returns soon after.  I that what that *intended* to do was to
NULL out the wait_thread pointer that would be checked in proc_terminate,
but there's no guarantee that the entry in chld_procs is in the same place
at the end of proc_waiter as it was at the start (so arg may point to
some other pinfo entirely).

Does any of this make any sense, or am I barking up the wrong tree here?


Re: double-fork issue on Windows on ARM64

2024-05-20 Thread Jeremy Drake via Cygwin-developers
On Wed, 8 May 2024, Jeremy Drake wrote:

> (this is the same issue discussed in
> https://cygwin.com/pipermail/cygwin-patches/2024q1/012621.html)
>
> On MSYS2, running on Windows on ARM64 only, we've been plagued by issues
> with processes hanging up.  Usually pacman, when it is trying to validate
> signatures with gpgme.  When a process is hung in this way, no debugger
> seems to be able to attach properly.
>
> > anecdotally, the hang occurs when _exit() calls
> > proc_terminate() which is then blocked by a call to TerminateThread()
> > with an invalid thread handle (for more details, see
> > https://github.com/msys2/msys2-autobuild/issues/62#issuecomment-1951796327).


As a follow-up to this, that was from a proposed workaround of just
commenting out the double-fork behavior in gpgme.  After reading a comment
in the code and doing some research online, it seems the double-fork is an
accepted idiom on posix to avoid having to wait for the (grand)child,
without creating zombie processes.  I was unable to see zombie processes
in ps or /proc/, but I did see extra cygpid.* entries in
/proc/sys/BaseNamedObjects/cygwin* which seem to be much the same thing.

Today, I was attempting to look at the TerminateThread situation.  The
call in question comes from the attempt to terminate the wait_thread of a
chld_procs entry.  I noticed elsewhere in cygwin code (flock.cc) that
CancelSynchronousIo was being called, and that stood out to me because
chances are that the wait thread (if running) is going to be blocked in
ReadFile.  I am testing with the following hack, and so far have not seen
a hang:
diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
index 86e4e607ab..020906d797 100644
--- a/winsup/cygwin/sigproc.cc
+++ b/winsup/cygwin/sigproc.cc
@@ -410,7 +410,7 @@ proc_terminate ()
  if (!have_execed || !have_execed_cygwin)
chld_procs[i]->ppid = 1;
  if (chld_procs[i].wait_thread)
-   chld_procs[i].wait_thread->terminate_thread ();
+   CancelSynchronousIo (chld_procs[i].wait_thread->thread_handle ());
  /* Release memory associated with this process unless it is 'myself'.
 'myself' is only in the chld_procs table when we've execed.  We
 reach here when the next process has finished initializing but we


As a disclaimer, I am having a hard time wrapping my head around this
code, so I don't know what kind of side-effects this may have, but it does
seem to help the hang, without resulting in "zombie" cygpid entries.

(Note that I first tried
+ if (CancelSynchronousIo (chld_procs[i].wait_thread->thread_handle 
()))
+   chld_procs[i].wait_thread->detach ();
+ else
+   chld_procs[i].wait_thread->terminate_thread ();
but that resulted in a (debuggable) hang in detach, because the
cygthread::stub was waiting for thread_sync, while cygthread::detach was
waiting for *this.  That appears to be because this is an auto-releasing
cygthread.  It kind of bothers me that there is no synchronization to be
sure the wait_thread is done shutting down before moving on in
proc_terminate, but I don't see an obvious way in the current structure).


double-fork issue on Windows on ARM64

2024-05-08 Thread Jeremy Drake via Cygwin-developers
(this is the same issue discussed in
https://cygwin.com/pipermail/cygwin-patches/2024q1/012621.html)

On MSYS2, running on Windows on ARM64 only, we've been plagued by issues
with processes hanging up.  Usually pacman, when it is trying to validate
signatures with gpgme.  When a process is hung in this way, no debugger
seems to be able to attach properly.

After many months of off-and-on progress trying to debug this, we've
*finally* got an idea of what behavior is causing this, and a standalone
reproducer that runs on Cygwin.

> A common symptom is that the hanging process has a command-line that is
> identical to its parent process' command-line (indicating that it has
> been fork()ed), and anecdotally, the hang occurs when _exit() calls
> proc_terminate() which is then blocked by a call to TerminateThread()
> with an invalid thread handle (for more details, see
> https://github.com/msys2/msys2-autobuild/issues/62#issuecomment-1951796327).
>
> In my tests, I found that the hanging process is spawned from
> _gpgme_io_spawn() which lets the child process immediately spawn another
> child. That seems like a fantastic way to find timing-related bugs in
> the MSYS2/Cygwin runtime.
>
> As a work-around, it does seem to help if we avoid that double-fork.

That led me to make the attached reproducer, which is based on the code
from _gpgme_io_spawn.  I originally expected that this would require some
timing adjustment, hence the defines to change the binary and argument (I
expected to use /bin/sleep and different values).  It turns out, this
reproduces readily with /bin/true.

I build this with `gcc -ggdb -o testfork testfork.c`, and this reproduces:
* on a Raspberry PI 4 running Windows 10, with an i686 msys2 runtime
* on a QC710 running Windows 11 23H2, with x86_64 msys2 runtime (this
seems to reproduce it most readily).
* on a hyper-v virtual machine on Dev Kit 2023 running Windows 11 23H2,
with x86_64 msys2 runtime or Cygwin 3.5.3.  This seems to require running
two instances of testfork.exe at the same time.

When attaching to the hung process, gdb shows
(gdb) i thr
  Id   Target IdFrame
  1Thread 6516.0xbe8error return
/cygdrive/d/a/scallywag/gdb/gdb-13.2-1.x86_64/src/gdb-13.2/gdb/windows-nat.c:748
was 31: A device attached to the system is not functioning.
0x in ?? ()
  2Thread 6516.0x1b28 "sig" 0x7ff8051a8a64 in ?? ()
* 3Thread 6516.0x12b4   0x7ff8051b4374 in ?? ()


Let me know if I can provide any additional info, or anything else we can
try to help debug this.#include 
#include 
#include 

#ifndef BINARY
#define BINARY "/bin/true"
#endif

#ifndef ARG
#define ARG "0.1"
#endif

int main(int argc, char ** argv)
{
	while (1)
	{
		int pid;
		printf("Starting group of 100x " BINARY " " ARG "\n");
		for (int i = 0; i < 100; ++i)
		{
			pid = fork();
			if (pid == -1)
			{
perror("fork error");
return 1;
			}
			else if (pid == 0)
			{
if ((pid = fork()) == 0)
{
	char * const args[] = {BINARY, ARG, NULL};
	execv(BINARY, args);
	perror("execv failed");
	_exit(5);
}
if (pid == -1)
{
	perror("inner fork error");
	_exit(1);
}
else
{
	_exit(0);
}
			}
			else
			{
int status;
if (waitpid(pid, , 0) == -1)
{
	perror("waitpid error");
	return 2;
}
else if (status != 0)
{
	fprintf(stderr, "subprocess exited non-zero: %d\n", status);
	return WEXITSTATUS(status);
}
			}
		}
	}
	return 0;
}


Re: binutils >= 2.41 makes .rsrc section read-only

2024-02-20 Thread Jeremy Drake via Cygwin
On Tue, 20 Feb 2024, Corinna Vinschen wrote:

> On Feb 19 21:41, Jeremy Drake via Cygwin wrote:
> > 1) is there actually a good reason that _cygheap_start is in the .rsrc and
> > not the .cygheap section?
>
> As you know we got rid of this way to define the cygheap, but
> _cygheap_start was just defined this way so it's the address marking
> the start address of the cygheap.
>
> It was just the way it was.  It's probably just as well to move
>
>   _SYM (_cygheap_start) = .;
>
> to the begining of the .cygheap section.  Did you try?

I hadn't; I have now and it seems to work fine.  I basically assumed there
was some reason I wasn't aware of for putting it in the .rsrc section, so
focused more on trying to figure out how to get binutils to clear the
readonly section flag.  When I couldn't figure that out, I figured I'd ask
here both about the "institutional knowledge" if there was a known reason
why it needed to be in the .rsrc section and because I figured some people
knowledgable in binutils pe stuff might also read this and be able to give
me another idea on making the .rsrc section read/write again if it did
need to stay in that section.


-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


binutils >= 2.41 makes .rsrc section read-only

2024-02-19 Thread Jeremy Drake via Cygwin
This is probably the right thing to do, but breaks building msys2-runtime
(read: cygwin) 3.3, because the _cygheap_start symbol is actually in the
.rsrc section and code very early attempts to memset _cygheap_start.
Couple of questions:

1) is there actually a good reason that _cygheap_start is in the .rsrc and
not the .cygheap section?

2) how could I make the .rsrc section read/write again?  I attempted to
use objcopy --set-section-flags .rsrc=contents,alloc,load,data a.dll b.dll
(or a.o b.o with .o files straight out of windres), but it does not seem
to actually clear the readonly flag.  I was able to set the 'code' flag,
so it is actually finding the right section.


See also https://github.com/msys2/msys2-runtime/issues/200

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: [PATCH] Cygwin: console: Avoid slipping past disable_master_thread check.

2024-02-03 Thread Jeremy Drake via Cygwin-patches
Thanks for taking the time to write this up, this issue has been bugging
me for years... (see also:
https://cygwin.com/pipermail/cygwin-patches/2021q4/011638.html)

On Sat, 3 Feb 2024, Johannes Schindelin wrote:

> Concretely, the hangs occur typically when some `pacman` process (a
> package manager using the MSYS2 runtime, i.e. the Cygwin runtime with
> several dozen patches on top) calls a few non-Cygwin processes. Those
> processes seem to succeed, but there is an extra `pacman` process left
> hanging around (reported using the same command-line as its parent
> process, indicating a `fork()`ed child process or maybe that
> signal-handler that is spawned for every non-Cygwin child process) and at
> that point the program hangs indefinitely (or at least until the GitHub
> workflow run times out after 6 hours).

I don't think this requires running non-Cygwin children - I see this most
often when pacman is using GPGME to validate signatueres.  That
fork/exec's (Cygwin) gpg.

> I was not able to obtain any helpful stacktraces, they all seem to make no
> sense, I only vaguely remember that one thread was waiting for an object,
> but that could be a false flag.

My recollection when I tried to debug was that every debugger I tried got
an error trying to get the context of the main thread of the hung process.
There was another thread, which seemed to be for Cygwin signal handling,
which was blithely waiting for an object, but I kind of expect that's what
it was supposed to be doing.

> Stopping those hanging `pacman` processes via `wmic process ... delete`
> counter-intuitively fails to result in `pacman` to exit with a non-zero
> exit code. Instead, the program now runs to completion successfully!

> Do you have any idea what the bug could be? Or how I could diagnose this
> better? Attaching via `gdb` only produces unhelpful stacktraces (that may
> even be bogus, by the looks of it). Or do you think that your patch that I
> am replying to could potentially fix this problem? How could the code be
> improved to avoid those hangs altogether, or at least to make them easier
> to diagnose?



Re: Restore SEM_FAILCRITICALERRORS [was: Aren't Windows System Error popups meant to be disabled in Cygwin?]

2024-02-03 Thread Jeremy Drake via Cygwin
On Fri, 2 Feb 2024, Corinna Vinschen wrote:

> On Feb  2 09:43, David Allsopp via Cygwin wrote:
> > However, this patch came from MSYS2, and subsequently they seem to
> > have found it problematic for the same reason as me
> > (https://github.com/msys2/msys2-runtime/pull/18#issuecomment-810897624)
> > and have just recently reintroduced the flag
> > (https://github.com/msys2/msys2-runtime/commit/7616b8a2e0ffcf068b47e1a66bbb1dbd7d9b5c50)
> > to control it.
> >
> > The reasoning in
> > https://cygwin.com/pipermail/cygwin/2006-August/150081.html seems as
> > valid now as it did in 2006.
> >
> > Is it possible to revisit having the flag, or even just reverting the 
> > behaviour?
> >
>
> I'm sympathetic, and personally I would prefer to revert the patch and
> stick to SEM_FAILCRITICALERRORS by default.
>
> The question is this: Why does, apparently, everybody expect Cygwin to
> do the "right thing", with different definitions of "right", when in
> fact the executable in question can easily call SetErrorMode by itself?

The different definitions of "right" is the reason the flag/option was
re-added in MSYS2.  I think the most "right" thing Cygwin could do (if it
were to only do one thing, rather than having an option) would be to
somehow have native processes inherit the error mode as though Cygwin were
not in the mix.  The issue with that, as you've seen, is that there are
any number of Cygwin processes in the hierarchy.

As far as the executable being able to call SetErrorMode itself, that
would be OK except for when the error is coming from the loader, before
anything from the executable is run (such as for missing DLL or missing
export from DLL).

I do like the idea of a native Windows program like "nohup" that sets the
error mode and then runs a subprocess.  Why doesn't Windows come with
something like that? ;)

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: [PATCH] fchmodat/fstatat: fix regression with empty `pathname`

2023-06-28 Thread Jeremy Drake via Cygwin-patches
On Tue, 27 Jun 2023, Johannes Schindelin wrote:

> In 4b8222983f (Cygwin: fix errno values set by readlinkat, 2023-04-18)
> the code of `readlinkat()` was adjusted to align the `errno` with Linux'
> behavior.
>
>   I noticed this issue when one of my workflows failed consistently
>   while trying to untar an archive containing a symbolic link and
>   claiming this:
>
>   Cannot change mode to rwxr-xr-x: Not a directory
>

I wonder if this is related to the issue from the thread
https://cygwin.com/pipermail/cygwin/2023-May/253738.html (sounds like it).
If so, tar was
rebuilt to pick up the new behavior in 3.4.7 (presumably via configure
checks), it may need another rebuild to pick up the fixed behavior after
this fix.


Re: Installing/upgrading only NOARCH packages.

2023-05-01 Thread Jeremy Drake via Cygwin
On Wed, 19 Apr 2023, Brian Inglis wrote:

> As 32 bit Windows systems are no longer getting security updates,
> recommendations for similar legacy systems include running them in VMs with
> access to update executables and libraries blocked.

There is a 32-bit variant of Windows 10; Windows 10 is scheduled to be
supported until October 2025.

Personally, I have a tablet, which shipped with Windows 8 and has a
64-bit-capable Atom processor, that nonetheless has a 32-bit UEFI firmware
without legacy boot support, and is thus incapable of booting a 64-bit
edition of Windows.  I have gotten the free upgrades to 8.1 (as soon as
I bought it) and then 10 when it came out.

I just wanted to correct the record that 32-bit Windows is already out of
support, or the general implication that they don't really matter anymore.
Maybe they don't, but maybe people don't realize that systems of 64-bit
processor vintage doesn't mean that a system can actually boot a 64-bit
Windows, even though it could otherwise run it (and can in a virtual
machine, though it's painful due to lack of RAM, and now lack of
VM software that still supports 32-bit Windows ;) ).

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: Cygwin 3.4.3 and 3.5.0... hangs in make, top, procps, ls /proc/PID/...

2023-01-01 Thread Jeremy Drake via Cygwin
On Sat, 31 Dec 2022, Brian Inglis wrote:

> was also getting the messages below locally and still on GitHub scallywag:
>
>   cygcheck (6936) child_copy: cygheap read copy failed,
>
> ../curl/scallywag/1_x86_64  build.log:2022-12-26T00:39:35.6163236Z   0
> [main] cygcheck (6936) child_copy: cygheap read copy failed, 0x0..0x80003B5F0,
> done 0, windows pid 6936, Win32 error 299
> ../curl/scallywag/1_x86_64  build.log:2022-12-26T00:48:03.4525278Z   0
> [main] cygcheck (568) child_copy: cygheap read copy failed, 0x0..0x80003BA48,
> done 0, windows pid 568, Win32 error 299
> ../dialog/scallywag/1_x86_64  build.log:2022-12-31T18:42:37.0939902Z   0
> [main] cygcheck (6992) child_copy: cygheap read copy failed, 0x0..0x80003CB38,
> done 0, windows pid 6992, Win32 error 299


This feels ASLR related.  Maybe try what Corinna suggested in
https://cygwin.com/pipermail/cygwin/2022-December/252720.html for a
similar error in Docker?

> Try this:
>
>   cp /bin/cygwin1.dll ~/docker-cygwin1.dll
>   peflags -d0 ~/docker-cygwin1.dll
>
> Then copy that DLL over to /bin/cygwin1.dll in your docker image
> and try again.

Though of course disregard 'docker' there.

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: [PATCH] Cygwin: pty: Fix 'Bad address' error when running 'cmd.exe /c dir'

2022-11-19 Thread Jeremy Drake via Cygwin-patches
On Fri, 18 Nov 2022, Johannes Schindelin wrote:

> Hi Corinna,
>
> On Mon, 24 Oct 2022, Corinna Vinschen wrote:
>
> > However, two points:
> >
> > - I'm wondering if the patch (both of yours) doesn't actually just cover
> >   a problem in child_info_spawn::worker().  Different runpath values,
> >   depending on the app path being "cmd" or "cmd.exe"?  That sounds like
> >   worker() is doing weird stuff.  And it does in line 400ff.
> >
> >   So, if the else branch of this code is apparently working fine for
> >   "cmd" per Takashi's observation in
> >   https://cygwin.com/pipermail/cygwin-patches/2022q4/012032.html, how
> >   much sense is in the if branch handling "command.com" and "cmd.exe"
> >   specially?  Wouldn't a better patch get rid of this extra if and
> >   the null_app_name variable instead?
>
> FWIW I would be in favor of getting rid of this special handling (unless
> it causes a regression). Given the recent experience, I expect Takashi to
> want to work on this without any interference from my side.


I was thinking maybe this check was intended to handle the, umm, "special"
quoting rules that "cmd /c" has (especially without "/s").  I don't know
why that would have anything to do with pcon though, so I may be totally
off the mark.


Re: [PATCH] Cygwin: Improve FAQ on early breakpoint for ASLR

2022-11-03 Thread Jeremy Drake via Cygwin-patches
On Thu, 3 Nov 2022, Jon Turney wrote:

> gdb supports 'set disable-randomization off' on Windows since [1]
> (included in gdb 13).
>
> https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=bcb9251f029da8dcf360a4f5acfa3b4211c87bb0;hp=8fea1a81c7d9279a6f91e49ebacfb61e0f8ce008

Is it really *disable*-randomization *off*?  The double-negative seems to
suggest that in that case ASLR would be left *on*.


Re: Re: Deadlock of the process tree when running make

2022-04-11 Thread Jeremy Drake via Cygwin
On Mon, 11 Apr 2022, Alexey Izbyshev wrote:

> Yes, sshd is running as a service, but I'm not sure that patch is relevant. In
> my case, the problematic pipe that the hanging conhost.exe is waiting on is
> probably created for that specific conhost.exe process within the process tree
> rooted at "make", which runs as an ordinary user. Also, wouldn't the hang be
> deterministic if the problem were in the pipe ownership?

Yes it would.  I just noticed some of the evidence pointing that way - a
presumably native javac.exe, an anonymous "named pipe" handle, and then
when I saw sshd involved the last piece required for that scenario -
running as a service.  But Takashi's reply sounds like sshd drops the
well-known service sid when it switches to the logged-on user's token
anyway.

This is both good and bad, I guess.  Bad because your problem may not be
solved yet (though maybe with the latest test dll, fingers crossed!).
Good because there's a mystery hang that's been plaguing me when running
(under emulation) on Windows on ARM64 that the circumstances of that
environment has made virtually impossible to debug, and every commit that
mentions fixing a deadlock gives me new hope that that will be the fix
that makes it go away.

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: Deadlock of the process tree when running make

2022-04-10 Thread Jeremy Drake via Cygwin
On Sat, 9 Apr 2022, Alexey Izbyshev wrote:

> I don't have mintty because "make" is run via an SSH session. I suppose
> I should look into sshd in this case?

Sshd wouldn't happen to be running as a service, would it?

https://cygwin.com/pipermail/cygwin-patches/2022q2/011867.html


-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: [PATCH v4] Cygwin: pipe: Avoid deadlock for non-cygwin writer.

2022-03-29 Thread Jeremy Drake via Cygwin-patches
On Tue, 29 Mar 2022, Takashi Yano wrote:

> diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
> index b87160edb..006c7b4bf 100644
> --- a/winsup/cygwin/fhandler.h
> +++ b/winsup/cygwin/fhandler.h
> @@ -1194,6 +1194,7 @@ private:
>HANDLE hdl_cnt_mtx;
>HANDLE query_hdl_proc;
>HANDLE query_hdl_value;
> +  HANDLE query_hdl_close_req_evt;
>uint64_t pipename_key;
>DWORD pipename_pid;
>LONG pipename_id;
> @@ -1258,6 +1259,16 @@ public:
>}
>bool reader_closed ();
>HANDLE temporary_query_hdl ();
> +  bool need_close_query_hdl ()
> +{
> +  return query_hdl_close_req_evt ?
> + IsEventSignalled (query_hdl_close_req_evt) : false;
> +}
> +  void request_close_query_hdl ()
> +{
> +  if (query_hdl_close_req_evt)
> + SetEvent (query_hdl_close_req_evt);
> +}
>  };
>
>  #define CYGWIN_FIFO_PIPE_NAME_LEN 47

Oh, a minor optimization: should close_query_handle also close (and NULL)
the query_hdl_close_req_evt?


Re: [PATCH v4] Cygwin: pipe: Avoid deadlock for non-cygwin writer.

2022-03-29 Thread Jeremy Drake via Cygwin-patches
On Tue, 29 Mar 2022, Takashi Yano wrote:

> diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
> index fb3d09d84..cd2d3a7ef 100644
> --- a/winsup/cygwin/spawn.cc
> +++ b/winsup/cygwin/spawn.cc
> @@ -645,8 +646,18 @@ child_info_spawn::worker (const char *prog_arg, const 
> char *const *argv,
>&& (fd == fileno_stdout || fd == fileno_stderr))
> {
>   fhandler_pipe *pipe = (fhandler_pipe *)(fhandler_base *) cfd;
> - pipe->close_query_handle ();
>   pipe->set_pipe_non_blocking (false);
> + pipe->request_close_query_hdl ();
> +
> + tty_min dummy_tty;
> + dummy_tty.ntty = (fh_devices) myself->ctty;
> + dummy_tty.pgid = myself->pgid;
> + tty_min *t = cygwin_shared->tty.get_cttyp ();
> + if (!t) /* If tty is not allocated, use dummy_tty instead. */
> +   t = _tty;
> + /* Emit __SIGNONCYGCHLD to let all processes in the
> +process group close query_hdl. */
> + t->kill_pgrp (__SIGNONCYGCHLD);
> }
>   else if (cfd->get_dev () == FH_PIPER && fd == fileno_stdin)
> {
>

This block seems to be inside a loop over handles.  Would it make sense to
move the `tty_min dummy_tty` through `t->kill_pgrp` lines outside the
loop, and set a flag in the loop instead, so the pgrp only needs to be
signaled (killed) once rather than for each handle that needs closing?


Re: pipe hang issue when running as SYSTEM

2022-03-25 Thread Jeremy Drake via Cygwin
On Fri, 25 Mar 2022, Takashi Yano wrote:

> I will submit v3 patch shortly.

I applied your v3 patch to 3.3.4 in
https://github.com/msys2/msys2-runtime/pull/88 and re-ran my test action,
and it worked as expected this time.

I've put out a call to test on the msys2-runtime issue to confirm that it
also solves the real-world cases.

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: pipe hang issue when running as SYSTEM

2022-03-25 Thread Jeremy Drake via Cygwin
On Fri, 25 Mar 2022, Takashi Yano wrote:

> I can confirm the sample script hangs indeed if the script is
> running as SYSTEM account. However, in my test, the patch solves
> the issue.
>
> It would be very helpfull if a simple github repository with
> github actions, which can reproduce the issue, is provided.

After much fighting with docker on windows,
https://github.com/jeremyd2019/msys2-pipe-hang-test/runs/5688176578?check_suite_focus=true

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: pipe hang issue when running as SYSTEM

2022-03-24 Thread Jeremy Drake via Cygwin
On Tue, 22 Mar 2022, Takashi Yano wrote:

> > Thanks for the report. This is expected problem as mentioned
> > in b531d6b commit message. However, I could not imagin the
> > situation that the service has multiple writer for the pipe
> > and one of them is a non-cygwin app.
> >
> > Question is: Does the docker invoke the command using SYSTEM
> > account? Or is the processes in docker determined as running
> > as a service?
>
> I confirmed the processes in Windows docker are running as
> well_known_service_sid. Let me consider a while.
>

Just got another report to MSYS2 of this behavior, this time from a Gitlab
CI runner that is running as a service and trying to pipe output from
Microsoft cl.exe while running configure.  This user reports that 3.3.4
with your "[PATCH v2] Cygwin: pipe: Avoid deadlock for non-cygwin writer."
applied to it does not solve the problem.

https://github.com/msys2/MSYS2-packages/issues/2893

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


pipe hang issue when running as SYSTEM

2022-03-21 Thread Jeremy Drake via Cygwin
This issue was reported to MSYS2 as a hang when trying to build libxml2 in
a Windows docker container.  Another user was able to come up with a
simple reproducer and a reasonable theory as to why it happens.  The msys2
issue is https://github.com/msys2/msys2-runtime/issues/77, and I will
quote the reproducer and analysis here:

I am having the same issue when building mingw. I managed to reduce the
issue to the following shell script:

#!/bin/sh
seq 1 9 > big_file
eval '$(eval cmd.exe /c "type big_file" | : )'

When running as a normal user this completes immediately, but when run as
a system service it hangs forever. The issue appears to be that when
running under the SYSTEM account, the third sh process holds open the read
end of the pipe. Since cmd has too much output to write all at once, it
waits until the pipe's buffer has room to write more, but since sh isn't
actually reading from the pipe, this hangs forever. When running as a
normal user the read end of the pipe is not kept open, and so cmd.exe gets
an error when attempting to write and exits immediately.

My suspicion is that this is caused by f79a461 (which keeps the read end
of the pipe open) and b531d6b (which changes the behavior depending on
whether or not the program is running as a service).

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: posix_spawn issues on i686

2022-01-12 Thread Jeremy Drake via Cygwin
> > > Sorry, I am not subscribed to the list so don't have the message to reply
> > > to for threading purposes

> New developer snapshot is up at https://cygwin.com/snapshots/
> Please test.

This works, and make's "make check" now gets the same results as it does
when built with --disable-posix-spawn in i686 cygwin.

Thanks

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: posix_spawn issues on i686

2022-01-11 Thread Jeremy Drake via Cygwin
On Mon, 10 Jan 2022, Jeremy Drake wrote:

> From https://github.com/msys2/MSYS2-packages/issues/2801
>
> MSYS2 recently rebuilt GNU make 4.3, and I found that after rebuilding, it
> broke rather horribly on i686, where any attempt to run a command resulted
> in "Invalid argument" errors.  Some debugging revealed that rebuilding
> make resulted in it using posix_spawn now instead of vfork.  Passing
> --disable-posix-spawn to make's configure script results in a working i686
> make.
>

> Can you create a simple, self-contained testcase in plain C?


Sorry, I am not subscribed to the list so don't have the message to reply
to for threading purposes, but attached please find a C reproducer that
works on x86_64 but fails on i686.  The particular issue seems to be the
POSIX_SPAWN_RESETIDS flag - not setting that allows i686 to succeed too.
#include 
#include 
#include 

#include 
#include 
#include 

extern char **environ;

int main()
{
pid_t pid;
char *argv[] = {"sh", "-c", "echo hi", NULL};
posix_spawnattr_t attr;
int status;
short flags = POSIX_SPAWN_RESETIDS;

if ((status = posix_spawnattr_init()) != 0) {
printf("posix_spawnattr_init: %s\n", strerror(status));
	return status;
}
if ((status = posix_spawnattr_setflags(, flags)) != 0) {
printf("posix_spawnattr_setflags: %s\n", strerror(status));
	return status;
}

status = posix_spawn(, "/bin/sh", NULL, , argv, environ);
if (status == 0) {
printf("Child pid: %i\n", pid);
do {
  if (waitpid(pid, , 0) != -1) {
printf("Child status %d\n", WEXITSTATUS(status));
  } else {
perror("waitpid");
return 1;
  }
} while (!WIFEXITED(status) && !WIFSIGNALED(status));
} else {
printf("posix_spawn: %s\n", strerror(status));
}
return status;
}

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


posix_spawn issues on i686

2022-01-10 Thread Jeremy Drake via Cygwin
>From https://github.com/msys2/MSYS2-packages/issues/2801

MSYS2 recently rebuilt GNU make 4.3, and I found that after rebuilding, it
broke rather horribly on i686, where any attempt to run a command resulted
in "Invalid argument" errors.  Some debugging revealed that rebuilding
make resulted in it using posix_spawn now instead of vfork.  Passing
--disable-posix-spawn to make's configure script results in a working i686
make.

>From the MSYS2 bug report:

"""
For reference, I tried to rebuild "make" in cygwin 32 bit and it has the
same problem:

rebuilding cygport make.cygport all results in a broken make
Adding CYGCONF_ARGS="--disable-posix-spawn" to the cygport file and
rebuilding again results in a good make
A Makefile to reproduce the issue:

all:
echo hi
"""

In addition, make check fails rather horribly as well.

I know that 32-bit is on the way out, but it is concerning to me that
there is some latent bug lurking in this code path that is apparently not
well exercised.

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: [PATCH] fhandler_pipe: add sanity limit to handle loops

2021-12-29 Thread Jeremy Drake via Cygwin-patches
On Wed, 29 Dec 2021, Ken Brown wrote:

> Takashi must be unavailable also, but it's a simple enough fix that I decided
> to go ahead and push it.

Thanks.  Regarding the other hang I'm seeing on ARM64, I tried gdb windbg
and lldb, and none of them seem able to read the 'context' of the main
thread when in this state so I can't get a stack trace.


Re: [PATCH] fhandler_pipe: add sanity limit to handle loops

2021-12-28 Thread Jeremy Drake via Cygwin-patches
On Mon, 27 Dec 2021, Jon Turney wrote:

> On 24/12/2021 00:29, Jeremy Drake via Cygwin-patches wrote:
> > again, so I can't confirm this.  I took a core with 'dumper' but gdb
> > doesn't want to load it (it says Core file format not supported, maybe
> > something with msys2's gdb?).
>
> I think you need gdb 11 (for this patch set [1], which is also in cygwin's
> gdb 10 package) to read x86_64 cygwin core dumps.
>
> [1] https://sourceware.org/pipermail/gdb-patches/2020-August/171232.html

Thanks, this was the problem.  But the core dump wasn't much help anyway,
the stuff I was interested in was pre-exception, and the backtrace
seemed to stop at the exception handling (unlike when 'live' debugging
when the stack trace continued).


Re: [PATCH] fhandler_pipe: add sanity limit to handle loops

2021-12-27 Thread Jeremy Drake via Cygwin-patches
On Sun, 26 Dec 2021, Ken Brown wrote:

> On 12/26/2021 6:23 PM, Jeremy Drake wrote:
> > On Sun, 26 Dec 2021, Ken Brown wrote:
> >
> > > On 12/26/2021 5:43 PM, Jeremy Drake wrote:
> > > > My loops are still going after an hour.  I know that ARM64 would have
> > > > hit
> > > > the assert before now.

x86_64 server 2022 is still going after 24 hours.  I'm going to stop it
now, and try to get an msys-2.0.dll with working PDB symbols to hopefully
see where ARM64 is hanging up.


Re: [PATCH] fhandler_pipe: add sanity limit to handle loops

2021-12-26 Thread Jeremy Drake via Cygwin-patches
On Sun, 26 Dec 2021, Ken Brown wrote:

> On 12/26/2021 5:43 PM, Jeremy Drake wrote:
> > My loops are still going after an hour.  I know that ARM64 would have hit
> > the assert before now.

Well, ARM64 hung up, but didn't hit the assert, so maybe there's some
*other* issue running around.  Unfortunately gdb doesn't work to attach to
a process there (it gets confused with the ARM64 DLLs loaded in the
process).  And WinDbg can't load the symbols for a cygwin dll (cv2pdb
generally works pretty well for this, but it seems to be confused by the
split .dbg file for msys-2.0.dll).


Re: [PATCH] fhandler_pipe: add sanity limit to handle loops

2021-12-26 Thread Jeremy Drake via Cygwin-patches
On Sun, 26 Dec 2021, Ken Brown wrote:

> +   /* NtQueryInformationProcess can return STATUS_SUCCESS with
> +  invalid handle data for certain processes.  See
> +  
> https://github.com/processhacker/processhacker/blob/master/phlib/native.c#L5754.

I would recommend using the "permalink" to the line, since future
commits could change both the line number and even the comment you are
referring to.

https://github.com/processhacker/processhacker/blob/05f5e9fa477dcaa1709d9518170d18e1b3b8330d/phlib/native.c#L5754


> +  We need to ensure that NumberOfHandles is zero in this
> +  case to avoid a crash in the loop below. */
> +   phi->NumberOfHandles = 0;
> status = NtQueryInformationProcess (proc, ProcessHandleInformation,
> phi, nbytes, );
> if (NT_SUCCESS (status))

Would it make sense to leave an assert (phi->NumberOfHandles <= n_handle)
before the for loop too just in case something odd happens in the
future?  That made it a lot easier to know what was going on.

My loops are still going after an hour.  I know that ARM64 would have hit
the assert before now.

Would this also be pushed to the 3.3 branch?  Or are there plans to make a
3.3.4 at some point?  I saw a pipe-related hang reported to MSYS2 (that I
didn't see this issue in the stack traces), but I am not sure if there are
any more pipe fixes pending post 3.3.3.


Re: [PATCH] fhandler_pipe: add sanity limit to handle loops

2021-12-26 Thread Jeremy Drake via Cygwin-patches
On Sun, 26 Dec 2021, Ken Brown wrote:

> On 12/26/2021 11:04 AM, Ken Brown wrote:
> > On 12/26/2021 10:09 AM, Ken Brown wrote:
> > > 1. For some processes, NtQueryInformationProcess(ProcessHandleInformation)
> > > can return STATUS_SUCCESS with invalid handle information.  See the
> > > comment starting at line 5754, where it is shown how to detect this.

I kind of thought something like this (that NumberOfHandles was
uninitialized memory).

> > If I'm right, the following patch should fix the problem:
> >
> > diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc
> > index ba6b70f55..4cef3e4ca 100644
> > --- a/winsup/cygwin/fhandler_pipe.cc
> > +++ b/winsup/cygwin/fhandler_pipe.cc
> > @@ -1228,6 +1228,7 @@ fhandler_pipe::get_query_hdl_per_process (WCHAR *name,
> >      HeapAlloc (GetProcessHeap (), 0, nbytes);
> >    if (!phi)
> >      goto close_proc;
> > + phi->NumberOfHandles = 0;
> >    status = NtQueryInformationProcess (proc,
> > ProcessHandleInformation,
> >    phi, nbytes, );
> >    if (NT_SUCCESS (status))
>
> Actually, this first hunk should suffice.
>
> > Jeremy, could you try this?
> >
> > Ken


I've built (leaving the assert in place too), and I've got 3 loops going
on server 2022 and 1 going on ARM64.  So far so good.  I don't know how
long before calling it good though.


Re: [PATCH] fhandler_pipe: add sanity limit to handle loops

2021-12-25 Thread Jeremy Drake via Cygwin-patches
I set up a windows server 2022 VM last night and went nuts stressing
pacman/GPGME.  I was able to reproduce the issue there:

status = 0x, phi->NumberOfHandles = 8261392, n_handle = 256
[#--]  14%
assertion "phi->NumberOfHandles <= n_handle" failed: file
"../../.././winsup/cygwin/fhandler_pipe.cc", line 1281, function: void*
fhandler_pipe::get_query_hdl_per_process(WCHAR*, OBJECT_NAME_INFORMATION*)

So it is not something inherent in the x86_64-on-ARM64 emulation but can
happen on native x86_64 also.


Re: [PATCH] fhandler_pipe: add sanity limit to handle loops

2021-12-25 Thread Jeremy Drake via Cygwin-patches
On Sat, 25 Dec 2021, Jeremy Drake via Cygwin-patches wrote:

> On Sun, 26 Dec 2021, Takashi Yano wrote:
>
> > Could you please check the result of the following test case
> > in that ARM64 platform?
> >
>

OK, on Windows 11 ARM64 (same machine as I was testing the assert on):
per_process: n_handle=52, NumberOfHandles=52
per_system: n_handle=65536, NumberOfHandles=35331

On GitHub "windows-2022" runner:
per_process: n_handle=63, NumberOfHandles=63
per_system: n_handle=65536, NumberOfHandles=34077

On Windows 10 x86_64 (can't remember if it was 21H1 or 21H2):
per_process: n_handle=37, NumberOfHandles=37
per_system: n_handle=131072, NumberOfHandles=76069


I was able to reproduce on that Windows 10 box *once*, when I got the
stack traces, but not since.


Re: [PATCH] fhandler_pipe: add sanity limit to handle loops

2021-12-25 Thread Jeremy Drake via Cygwin-patches
On Sun, 26 Dec 2021, Takashi Yano wrote:

> Could you please check the result of the following test case
> in that ARM64 platform?
>

I will probably not be able to get to this until tomorrow at the earliest.
But keep in mind the issue I'm seeing is not deterministic - I have to run
pacman in a loop validating files via GPGME and eventually it will hang
(or hit the assert I added in that version).  Most of the time, it's
perfectly fine.


Re: [PATCH] fhandler_pipe: add sanity limit to handle loops

2021-12-24 Thread Jeremy Drake via Cygwin-patches
On Sat, 25 Dec 2021, Takashi Yano wrote:

> On Fri, 24 Dec 2021 19:47:46 -0800 (PST)
> Jeremy Drake wrote:
> > phi->NumberOfHandles = 7999168, n_handle = 256
> > assertion "phi->NumberOfHandles <= n_handle" failed: file
> > "../../.././winsup/cygwin/fhandler_pipe.cc", line 1280, function: void*
> > fhandler_pipe::get_query_hdl_per_process(WCHAR*, OBJECT_NAME_INFORMATION*)
> > Aborted
>
> What!? Could you please check value of the "status" ?

status = 0x, phi->NumberOfHandles = 7286688, n_handle = 256
assertion "phi->NumberOfHandles <= n_handle" failed: file
"../../.././winsup/cygwin/fhandler_pipe.cc", line 1281, function: void*
fhandler_pipe::get_query_hdl_per_process(WCHAR*, OBJECT_NAME_INFORMATION*)
Aborted

> What version of windows do you use?

This was on Windows 11 (22000.376) on ARM64, but msys2 has started seeing
similar hangs on Github's "windows-2022" runner.  I don't have one of
those locally to test against however.  But if push came to shove, I think
I downloaded a Server 2022 evaluation ISO, I could set up a VM and see
what happens.


Re: [PATCH] fhandler_pipe: add sanity limit to handle loops

2021-12-24 Thread Jeremy Drake via Cygwin-patches
On Sat, 25 Dec 2021, Takashi Yano wrote:

> Could you please try
> assert(phi->NumberOfHandles <= n_handle)
> rather than
> assert(phi->NumberOfHandles < n_handle)
> ?

I thought of that when I was re-reading my email.  I also added a printf:

index 9ce140089..4d10451c1 100644
--- a/winsup/cygwin/fhandler_pipe.cc
+++ b/winsup/cygwin/fhandler_pipe.cc
@@ -11,6 +11,7 @@ details. */
 #include "winsup.h"
 #include 
 #include 
+#include 
 #include "cygerrno.h"
 #include "security.h"
 #include "path.h"
@@ -1271,6 +1272,13 @@ fhandler_pipe::get_query_hdl_per_process (WCHAR *name,
  if (!NT_SUCCESS (status))
goto close_proc;

+  if (phi->NumberOfHandles > n_handle)
+{
+  small_printf ("phi->NumberOfHandles = %lu, n_handle = %lu\n",
+  (unsigned long) phi->NumberOfHandles,
+  (unsigned long) n_handle);
+  assert(phi->NumberOfHandles <= n_handle);
+}
   for (ULONG j = 0; j < phi->NumberOfHandles; j++)
{
  /* Check for the peculiarity of cygwin read pipe */

phi->NumberOfHandles = 7999168, n_handle = 256
assertion "phi->NumberOfHandles <= n_handle" failed: file
"../../.././winsup/cygwin/fhandler_pipe.cc", line 1280, function: void*
fhandler_pipe::get_query_hdl_per_process(WCHAR*, OBJECT_NAME_INFORMATION*)
Aborted



Re: [PATCH] fhandler_pipe: add sanity limit to handle loops

2021-12-24 Thread Jeremy Drake via Cygwin-patches
On Fri, 24 Dec 2021, Ken Brown wrote:

> On 12/24/2021 2:42 PM, Jeremy Drake wrote:
> > It does seem to happen much more often on Windows on ARM64 (so much so
> > that at first I thought it was an issue with their emulation).  With this
> > patch I have not seen the issue again.
>
> So can you test your diagnosis by removing your patch and adding an assertion?

Done:
:: Starting core system upgrade...
 there is nothing to do
:: Starting full system upgrade...
 there is nothing to do
assertion "phi->NumberOfHandles < n_handle" failed: file
"../../.././winsup/cygwin/fhandler_pipe.cc", line 1275, function: void*
fhandler_pipe::get_query_hdl_per_process(WCHAR*, OBJECT_NAME_INFORMATION*)
Aborted



Re: [PATCH] fhandler_pipe: add sanity limit to handle loops

2021-12-24 Thread Jeremy Drake via Cygwin-patches
On Fri, 24 Dec 2021, Ken Brown wrote:

> So can you test your diagnosis by removing your patch and adding an assertion?

I will attempt to do this.  Do I need any special configure args or
anything for assertions to be enabled?

> > Also, it seems to have started cropping up in msys2's CI when the GHA
> > runner was switched from "windows-2019" to "windows-2022".
>
> And does your patch help here too?

I have not tried this.  This seemed to be pretty clearly a cygwin rather
than msys2-specific issue so I submitted the patch here first rather than
getting it integrated into msys2 so that it would then be used on their
runners.


Re: [PATCH] fhandler_pipe: add sanity limit to handle loops

2021-12-24 Thread Jeremy Drake via Cygwin-patches
On Fri, 24 Dec 2021, Ken Brown wrote:

> I agree that it's hard to see how the line you quoted could cause an
> exception.  But you were using an optimized build, so I'm not sure how
> reliable the line-number information is.
>
> Is it feasible to run your test under strace?  If so, you could add some
> debug_printf statements to examine the values of n_handle and
> phi->NumberOfHandles.  Or what about simply adding an assertion that
> phi->NumberOfHandles <= n_handle?
>
> Ken

This issue is not consistent, I was able to reproduce once on x64 by
running commands in a loop overnight, but the next time I tried to
reproduce I ran for over 24 hours without hitting it.

It does seem to happen much more often on Windows on ARM64 (so much so
that at first I thought it was an issue with their emulation).  With this
patch I have not seen the issue again.

Also, it seems to have started cropping up in msys2's CI when the GHA
runner was switched from "windows-2019" to "windows-2022".

I forgot to give a full link to the MSYS2 issue where I have been
investigating this:
https://github.com/msys2/MSYS2-packages/issues/2752



Re: [PATCH] fhandler_pipe: add sanity limit to handle loops

2021-12-23 Thread Jeremy Drake via Cygwin-patches
On Thu, 23 Dec 2021, Ken Brown wrote:

> > -  for (ULONG j = 0; j < phi->NumberOfHandles; j++)
> > +  for (ULONG j = 0; j < min(phi->NumberOfHandles, n_handle); j++)
>
> Reading the preceding code, I don't see how n_handle could be less than
> phi->NumberOfHandles.  Can you explain?
>

Not really.  I saw this stack trace:
...
#3  0x000180062f13 in exception::handle (e=0x14cc4f0, frame=, in=, dispatch=) at 
/c/S/msys2-runtime/src/msys2-runtime/winsup/cygwin/exceptions.cc:835
#4  0x7ff8abd320cf in ntdll!.chkstk () from /c/Windows/SYSTEM32/ntdll.dll
#5  0x7ff8abce1454 in ntdll!RtlRaiseException () from 
/c/Windows/SYSTEM32/ntdll.dll
#6  0x7ff8abd30bfe in ntdll!KiUserExceptionDispatcher () from 
/c/Windows/SYSTEM32/ntdll.dll
#7  0x000180092687 in fhandler_pipe::get_query_hdl_per_process 
(this=this@entry=0x1803700f8, name=name@entry=0x14cc820 
L"\\Device\\NamedPipe\\dd50a72ab4668b33-10348-pipe-nt-0x6E6", 
ntfn=ntfn@entry=0x8000c2ce0) at 
/c/S/msys2-runtime/src/msys2-runtime/winsup/cygwin/fhandler_pipe.cc:1281
#8  0x000180092bdb in fhandler_pipe::temporary_query_hdl 
(this=this@entry=0x1803700f8) at 
/c/S/msys2-runtime/src/msys2-runtime/winsup/cygwin/fhandler_pipe.cc:1190
...

Line 1281 of fhandler_pipe.cc was
  if (phi->Handles[j].GrantedAccess != access)

The only way I could see that causing an exception was if it was reading
past the end of the allocated memory, if j was greater than (or equal to)
n_handle.  Unfortunately, I haven't been able to catch it in a debugger
again, so I can't confirm this.  I took a core with 'dumper' but gdb
doesn't want to load it (it says Core file format not supported, maybe
something with msys2's gdb?).



Re: [HEADSUP] Phasing out old Windows versions and 32 bit support

2021-10-27 Thread Jeremy Drake via Cygwin
Oops, forgot to send to list.

On Wed, 27 Oct 2021, Takashi Yano wrote:

> On Tue, 26 Oct 2021 22:55:01 +0200
> Corinna Vinschen wrote:
> > We're also planning to drop Support for the 32 bit release of Cygwin in
> > 2022, thus Cygwin 3.4.0 won't come in 32 bit anymore, and the package
> > maintainers won't have to update 32 bit packages anymore.  If you're
> > still running Cygwin under WOW64, consider to move to 64 bit in the next
> > couple of months.
>
> I agree with you that 32 bit cygwin under WOW64 is not worth to
> support any more. However, 32 bit version of Windows 10 will be
> still supported at least until Oct. 2025. Personally, I think it
> would not be nice to exclude the supported windows version from
> cygwin support.

Please also note that Windows on ARM64 only just got the ability to run
x86_64 binaries in Windows 11.  Previously the only option was 32-bit
x86, as there is no native ARM port of Cygwin.  I don't know about Cygwin
directly, but I know there are users of Git for Windows on ARM.

I think a more gradual sunsetting of 32-bit might be a resonable
compromise, based on the original wording:

> 11:26  starting with 3.4.0, maintainers are not obliged to
> release packages in 32 bit

so as of 3.4.0 some packages that are problematic on 32 bit may stop
providing 32-bit versions.  But not being obliged to release packages is a
whole different thing than having the core code that allows *any* package
to run being ripped out in that version.

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: [PATCH v2] Cygwin: respect PC_SYM_FOLLOW and PC_SYM_NOFOLLOW_REP with inner links

2021-07-07 Thread Jeremy Drake via Cygwin-patches
On Mon, 31 May 2021, Corinna Vinschen wrote:

> So we have two contradict problems, one which is solved by following
> inner symlinks, one which is solved by not doing that...

I hesitate to suggest it, but maybe an option/setting in the CYGWIN
variable as to whether to use this new behavior?  I am pretty much out of
ideas on how to make it work with native programs where they expect to see
the subst or mapped drive, not the target or UNC path.  Then MSYS2 could
either patch to change the default, or else just tell the (probably few)
people who hit it how to change the setting.


Re: [PATCH v3] Cygwin: respect PC_SYM_FOLLOW and PC_SYM_NOFOLLOW_REP with inner links

2021-07-07 Thread Jeremy Drake via Cygwin-patches
On Tue, 6 Jul 2021, Corinna Vinschen wrote:

> This looks like here be dragons.  A good solution would change the
> used native tools to allow paths > MAX_PATH finally, or to use other,
> equivalent tools already allowing that.

BTW, this seems pretty unlikely.  Portable tools are most likely using C
or C++ standard library file IO, not native Windows APIs, and while MS did
add an option to remove MAX_PATH limits from normal Win32 file/directory
APIs without the '\\?\' passthrough to NT trick [1], there are a lot of
hoops to jump through, and it would be hard to guarantee a whole stack of
libraries that might be linked in would be able to handle it.

[1]:
https://docs.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation#enable-long-paths-in-windows-10-version-1607-and-later


[PATCH v4] Cygwin: respect PC_SYM_FOLLOW and PC_SYM_NOFOLLOW_REP with inner links

2021-07-06 Thread Jeremy Drake via Cygwin-patches
The new GetFinalPathNameW handling for native symlinks in inner path
components is disabled if caller doesn't want to follow symlinks, or
doesn't want to follow reparse points.
---
 winsup/cygwin/path.cc | 88 ++-
 1 file changed, 45 insertions(+), 43 deletions(-)

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index e62f8fe2b..1869fb8c8 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -722,9 +722,10 @@ path_conv::check (const char *src, unsigned opt,
  int symlen = 0;

  /* Make sure to check certain flags on last component only. */
- for (unsigned pc_flags = opt & (PC_NO_ACCESS_CHECK | PC_KEEP_HANDLE);
+ for (unsigned pc_flags = opt & (PC_NO_ACCESS_CHECK | PC_KEEP_HANDLE
+| PC_SYM_FOLLOW | PC_SYM_NOFOLLOW_REP);
   ;
-  pc_flags = 0)
+  pc_flags = opt & (PC_SYM_FOLLOW | PC_SYM_NOFOLLOW_REP))
{
  const suffix_info *suff;
  char *full_path;
@@ -3480,48 +3481,49 @@ restart:
goto file_not_symlink;
}
 #endif /* __i386__ */
-  {
-   PWCHAR fpbuf = tp.w_get ();
-   DWORD ret;
-
-   ret = GetFinalPathNameByHandleW (h, fpbuf, NT_MAX_PATH, 0);
-   if (ret)
- {
-   UNICODE_STRING fpath;
+  if ((pc_flags & (PC_SYM_FOLLOW | PC_SYM_NOFOLLOW_REP)) == PC_SYM_FOLLOW)
+   {
+ PWCHAR fpbuf = tp.w_get ();
+ DWORD ret;

-   RtlInitCountedUnicodeString (, fpbuf, ret * sizeof (WCHAR));
-   fpbuf[1] = L'?';/* \\?\ --> \??\ */
-   if (!RtlEqualUnicodeString (, , !!ci_flag))
- {
-   issymlink = true;
-   /* upath.Buffer is big enough and unused from this point on.
-  Reuse it here, avoiding yet another buffer allocation. */
-   char *nfpath = (char *) upath.Buffer;
-   sys_wcstombs (nfpath, NT_MAX_PATH, fpbuf);
-   res = posixify (nfpath);
-
-   /* If the incoming path consisted of a drive prefix only,
-  we just handle a virtual drive, created with, e.g.
-
-subst X: C:\foo\bar
-
-  Treat it like a symlink.  This is required to tell an
-  lstat caller that the "drive" is actually pointing
-  somewhere else, thus, it's a symlink in POSIX speak. */
-   if (upath.Length == 14) /* \??\X:\ */
- {
-   fileattr &= ~FILE_ATTRIBUTE_DIRECTORY;
-   path_flags |= PATH_SYMLINK;
- }
-   /* For final paths differing in inner path components return
-  length as negative value.  This informs path_conv::check
-  to skip realpath handling on the last path component. */
-   else
- res = -res;
-   break;
- }
- }
-  }
+ ret = GetFinalPathNameByHandleW (h, fpbuf, NT_MAX_PATH, 0);
+ if (ret)
+   {
+ UNICODE_STRING fpath;
+
+ RtlInitCountedUnicodeString (, fpbuf, ret * sizeof (WCHAR));
+ fpbuf[1] = L'?';  /* \\?\ --> \??\ */
+ if (!RtlEqualUnicodeString (, , !!ci_flag))
+   {
+ issymlink = true;
+ /* upath.Buffer is big enough and unused from this point on.
+Reuse it here, avoiding yet another buffer allocation. */
+ char *nfpath = (char *) upath.Buffer;
+ sys_wcstombs (nfpath, NT_MAX_PATH, fpbuf);
+ res = posixify (nfpath);
+
+ /* If the incoming path consisted of a drive prefix only,
+we just handle a virtual drive, created with, e.g.
+
+  subst X: C:\foo\bar
+
+Treat it like a symlink.  This is required to tell an
+lstat caller that the "drive" is actually pointing
+somewhere else, thus, it's a symlink in POSIX speak. */
+ if (upath.Length == 14)   /* \??\X:\ */
+   {
+ fileattr &= ~FILE_ATTRIBUTE_DIRECTORY;
+ path_flags |= PATH_SYMLINK;
+   }
+ /* For final paths differing in inner path components return
+length as negative value.  This informs path_conv::check
+to skip realpath handling on the last path component. */
+ else
+   res = -res;
+ break;
+   }
+   }
+   }

 /* Normal file. */
 file_not_symlink:
-- 
2.32.0.windows.1
From 67a276c35a3b48697c6b61caaf4ffea5a174c75b Mon Sep 17 00:00:00 2001
From: Jeremy Drake 
Date: Sat, 29 May 2021 11:48:11 -0700
Subject: [PATCH] Cygwin: respect PC_SYM_FOLLOW and PC_SYM_NOFOLLOW_REP with
 inner links.

The new 

Re: [PATCH v3] Cygwin: respect PC_SYM_FOLLOW and PC_SYM_NOFOLLOW_REP with inner links

2021-07-06 Thread Jeremy Drake via Cygwin-patches
On Tue, 6 Jul 2021, Corinna Vinschen wrote:

> This formatting is just ugly.  I suggest to move the PC_SYM_* test
> to the block after the 32 bit code and reuse the existing braces,
> just with adapted indentation, i. e.:

+1.  I was trying to avoid reformatting otherwise unchanged lines to
reduce patch size.

> > @@ -3704,7 +3708,8 @@ chdir (const char *in_dir)
> >
> >/* Convert path.  PC_NONULLEMPTY ensures that we don't check for
> >  NULL/empty/invalid again. */
> > -  path_conv path (in_dir, PC_SYM_FOLLOW | PC_POSIX | PC_NONULLEMPTY);
> > +  path_conv path (in_dir, PC_SYM_FOLLOW | PC_POSIX | PC_NONULLEMPTY
> > + | PC_SYM_NOFOLLOW_REP);
> >if (path.error)
> > {
> >   set_errno (path.error);
>
> I'm still not convinced that we should do this.  I'm pretty certain this
> will result in problems in Cygwin processes when you least expect them.
>
> Consider that the output of getcwd and realpath/readlink on the same
> path may differ after this patch.  Using PC_SYM_NOFOLLOW_REP like this
> also changes the normal sym follow handling for the last path component
> in path_copnv::check, potentially.
>
> This looks like here be dragons.  A good solution would change the
> used native tools to allow paths > MAX_PATH finally, or to use other,
> equivalent tools already allowing that.

I am not convinced that this even completely solved the issues I was
seeing, or some of the reports of issues with unc paths suddenly showing
up instead of mapped drives in native tools that weren't expecting them.

But, I do think respecting the PC_SYM_NOFOLLOW_REP flag for inner links is
correct, and I am sending a new version.


Re: [PATCH v2] Cygwin: respect PC_SYM_FOLLOW and PC_SYM_NOFOLLOW_REP with inner links

2021-07-03 Thread Jeremy Drake via Cygwin-patches
On Mon, 31 May 2021, Corinna Vinschen wrote:

> What bugs me here is that there's no guarantee that you can keep your
> path below MAX_PATH, independently of what you do here.  This is all
> a bit like patching up left and right just to keep dumb native tools
> running even in scenarios where they just fail otherwise.
>
> So we have two contradict problems, one which is solved by following
> inner symlinks, one which is solved by not doing that... I'm not overly
> keen to support this scenario.
>
> Wouldn't that be something more suited for an MSYS2-local patch?

I discussed this with the MSYS2 maintainers, and while they are open to
disabling this code in the short term, they would like to minimize the
patches against upstream Cygwin they carry.

For now I've proposed https://github.com/msys2/msys2-runtime/pull/54
there, and will test that is indeed the 'fix' for this (and a couple of
apparently related issues with mapped-network-drives turning into UNC for
Windows processes)

> Corinna



Re: realpath issue with native[strict] symlinks

2021-06-07 Thread Jeremy Drake via Cygwin
On Fri, 28 May 2021, Jeremy Drake wrote:

> On Thu, 27 May 2021, Jeremy Drake wrote:
>
> > > > Treating mapped/subst drives as though they were not symlinks, without
> > > > messing with intermedate symlinks.
> > >
> > > It was that simple, surprise, surprise...
> >
> > It turns out it wasn't, after all.  This only seems to work at the root of
> > a subst-ed drive.  I just got the following error:
>

There was another report of someone running into an issue with this
change, though not exactly the same situation as mine:
https://github.com/msys2/msys2-runtime/issues/41#issuecomment-854913283

It seems that a mapped network drive is being 'dereferenced', confusing
eclipse trying to find source files from debug info from an arm
cross-compiler.

Unfortunately I have not heard back whether or not my patch helps this
case.

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


[PATCH v3] Cygwin: respect PC_SYM_FOLLOW and PC_SYM_NOFOLLOW_REP with inner links

2021-06-03 Thread Jeremy Drake via Cygwin-patches
Just updated for formatting.

> Formatting should try to stick to 80 chars max line length, if possible.
> Kind of like this, just with TABs:
>
> -   for (unsigned pc_flags = opt & (PC_NO_ACCESS_CHECK | PC_KEEP_HANDLE);
> +   for (unsigned pc_flags = opt & (PC_NO_ACCESS_CHECK | PC_KEEP_HANDLE
> | PC_SYM_FOLLOW | 
> PC_SYM_NOFOLLOW_REP);
>From 4bb959b57606465d5a7abe7d3ae168db66f5f6fa Mon Sep 17 00:00:00 2001
From: Jeremy Drake 
Date: Sat, 29 May 2021 13:17:08 -0700
Subject: [PATCH 1/2] Revert "Cygwin: Handle virtual drives as non-symlinks"

This reverts commit c8949d04001e3dbc03651475b6cd1c5623400835.
---
 winsup/cygwin/path.cc | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index 6a07f0d06..e62f8fe2b 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -3505,9 +3505,14 @@ restart:
 
 subst X: C:\foo\bar
 
-  Treat it as a normal file. */
+  Treat it like a symlink.  This is required to tell an
+  lstat caller that the "drive" is actually pointing
+  somewhere else, thus, it's a symlink in POSIX speak. */
if (upath.Length == 14) /* \??\X:\ */
- goto file_not_symlink;
+ {
+   fileattr &= ~FILE_ATTRIBUTE_DIRECTORY;
+   path_flags |= PATH_SYMLINK;
+ }
/* For final paths differing in inner path components return
   length as negative value.  This informs path_conv::check
   to skip realpath handling on the last path component. */
-- 
2.31.1.windows.1

From ea36ccb13b2080663535d867e6fe8edf246efe83 Mon Sep 17 00:00:00 2001
From: Jeremy Drake 
Date: Sat, 29 May 2021 11:48:11 -0700
Subject: [PATCH 2/2] Cygwin: respect PC_SYM_FOLLOW and PC_SYM_NOFOLLOW_REP
 with inner links.

The new GetFinalPathNameW handling for native symlinks in inner path
components is disabled if caller doesn't want to follow symlinks, or
doesn't want to follow reparse points.  Set flag to not follow reparse
points in chdir, allowing native processes to see their cwd potentially
including native symlinks, rather than dereferencing them.
---
 winsup/cygwin/path.cc | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index e62f8fe2b..a6bb3aeff 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -722,9 +722,10 @@ path_conv::check (const char *src, unsigned opt,
  int symlen = 0;
 
  /* Make sure to check certain flags on last component only. */
- for (unsigned pc_flags = opt & (PC_NO_ACCESS_CHECK | PC_KEEP_HANDLE);
+ for (unsigned pc_flags = opt & (PC_NO_ACCESS_CHECK | PC_KEEP_HANDLE
+| PC_SYM_FOLLOW | PC_SYM_NOFOLLOW_REP);
   ;
-  pc_flags = 0)
+  pc_flags = opt & (PC_SYM_FOLLOW | PC_SYM_NOFOLLOW_REP))
{
  const suffix_info *suff;
  char *full_path;
@@ -3452,6 +3453,8 @@ restart:
break;
}
 
+  if ((pc_flags & (PC_SYM_FOLLOW | PC_SYM_NOFOLLOW_REP)) == PC_SYM_FOLLOW)
+  {
   /* Check if the inner path components contain native symlinks or
 junctions, or if the drive is a virtual drive.  Compare incoming
 path with path returned by GetFinalPathNameByHandleA.  If they
@@ -3522,6 +3525,7 @@ restart:
  }
  }
   }
+  }
 
 /* Normal file. */
 file_not_symlink:
@@ -3704,7 +3708,8 @@ chdir (const char *in_dir)
 
   /* Convert path.  PC_NONULLEMPTY ensures that we don't check for
 NULL/empty/invalid again. */
-  path_conv path (in_dir, PC_SYM_FOLLOW | PC_POSIX | PC_NONULLEMPTY);
+  path_conv path (in_dir, PC_SYM_FOLLOW | PC_POSIX | PC_NONULLEMPTY
+ | PC_SYM_NOFOLLOW_REP);
   if (path.error)
{
  set_errno (path.error);
-- 
2.31.1.windows.1



[PATCH v2] Cygwin: respect PC_SYM_FOLLOW and PC_SYM_NOFOLLOW_REP with inner links

2021-05-30 Thread Jeremy Drake via Cygwin-patches
First, revert the handling of virtual drives as non-symlinks.  This is no
longer necessary.

The new GetFinalPathNameW handling for native symlinks in inner path
components is disabled if caller doesn't want to follow symlinks, or
doesn't want to follow reparse points.  Set flag to not follow reparse
points in chdir, allowing native processes to see their cwd potentially
including native symlinks, rather than dereferencing them.
---

For v2, I realized the PC_SYM_NOFOLLOW_REP flag was supposed to do this,
and that lack of PC_SYM_FOLLOW was not being respected either.  With this,
and patching `pwd -P` to `pwd` in makepkg, the long-named package builds
successfully.  I did not re-indent the code for the addition of the if due
to having learned from my patch to rebase, but it looks kind of ugly.

 winsup/cygwin/path.cc | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index e62f8fe2b..2ce5aef81 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -722,9 +722,9 @@ path_conv::check (const char *src, unsigned opt,
  int symlen = 0;

  /* Make sure to check certain flags on last component only. */
- for (unsigned pc_flags = opt & (PC_NO_ACCESS_CHECK | PC_KEEP_HANDLE);
+ for (unsigned pc_flags = opt & (PC_NO_ACCESS_CHECK | PC_KEEP_HANDLE | 
PC_SYM_FOLLOW | PC_SYM_NOFOLLOW_REP);
   ;
-  pc_flags = 0)
+  pc_flags = opt & (PC_SYM_FOLLOW | PC_SYM_NOFOLLOW_REP))
{
  const suffix_info *suff;
  char *full_path;
@@ -3452,6 +3452,8 @@ restart:
break;
}

+  if ((pc_flags & (PC_SYM_FOLLOW | PC_SYM_NOFOLLOW_REP)) == PC_SYM_FOLLOW)
+  {
   /* Check if the inner path components contain native symlinks or
 junctions, or if the drive is a virtual drive.  Compare incoming
 path with path returned by GetFinalPathNameByHandleA.  If they
@@ -3522,6 +3524,7 @@ restart:
  }
  }
   }
+  }

 /* Normal file. */
 file_not_symlink:
@@ -3704,7 +3707,7 @@ chdir (const char *in_dir)

   /* Convert path.  PC_NONULLEMPTY ensures that we don't check for
 NULL/empty/invalid again. */
-  path_conv path (in_dir, PC_SYM_FOLLOW | PC_POSIX | PC_NONULLEMPTY);
+  path_conv path (in_dir, PC_SYM_FOLLOW | PC_POSIX | PC_NONULLEMPTY | 
PC_SYM_NOFOLLOW_REP);
   if (path.error)
{
  set_errno (path.error);
-- 
2.31.1.windows.1From 4bb959b57606465d5a7abe7d3ae168db66f5f6fa Mon Sep 17 00:00:00 2001
From: Jeremy Drake 
Date: Sat, 29 May 2021 13:17:08 -0700
Subject: [PATCH 1/2] Revert "Cygwin: Handle virtual drives as non-symlinks"

This reverts commit c8949d04001e3dbc03651475b6cd1c5623400835.
---
 winsup/cygwin/path.cc | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index 6a07f0d06..e62f8fe2b 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -3505,9 +3505,14 @@ restart:
 
 subst X: C:\foo\bar
 
-  Treat it as a normal file. */
+  Treat it like a symlink.  This is required to tell an
+  lstat caller that the "drive" is actually pointing
+  somewhere else, thus, it's a symlink in POSIX speak. */
if (upath.Length == 14) /* \??\X:\ */
- goto file_not_symlink;
+ {
+   fileattr &= ~FILE_ATTRIBUTE_DIRECTORY;
+   path_flags |= PATH_SYMLINK;
+ }
/* For final paths differing in inner path components return
   length as negative value.  This informs path_conv::check
   to skip realpath handling on the last path component. */
-- 
2.31.1.windows.1

From 9a1d868c3e027416876d9bd110161562f8b77b0a Mon Sep 17 00:00:00 2001
From: Jeremy Drake 
Date: Sat, 29 May 2021 11:48:11 -0700
Subject: [PATCH 2/2] Cygwin: respect PC_SYM_FOLLOW and PC_SYM_NOFOLLOW_REP
 with inner links.

The new GetFinalPathNameW handling for native symlinks in inner path
components is disabled if caller doesn't want to follow symlinks, or
doesn't want to follow reparse points.  Set flag to not follow reparse
points in chdir, allowing native processes to see their cwd potentially
including native symlinks, rather than dereferencing them.
---
 winsup/cygwin/path.cc | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index e62f8fe2b..2ce5aef81 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -722,9 +722,9 @@ path_conv::check (const char *src, unsigned opt,
  int symlen = 0;
 
  /* Make sure to check certain flags on last component only. */
- for (unsigned pc_flags = opt & (PC_NO_ACCESS_CHECK | PC_KEEP_HANDLE);
+ for (unsigned pc_flags = opt & (PC_NO_ACCESS_CHECK | 

Re: [PATCH] Cygwin: tweak handling of native symlinks from chdir

2021-05-30 Thread Jeremy Drake via Cygwin-patches
On Sat, 29 May 2021, Jeremy Drake via Cygwin-patches wrote:

> On Sat, 29 May 2021, Jeremy Drake wrote:
>
> > First, revert the handling of virtual drives as non-symlinks.  This is no
> > longer necessary.
> >
>
> Spoke too soon, it seems that `makepkg` uses `pwd -P` and that still
> dereferences symlinks, as does `pwd -W` and `cygpath -w .`  :(

With the two patches from the OP, plus patching `makepkg` to change all
`pwd -P` to straight `pwd`, AND patching msys2's msys2_path_conv.cc
`posix_to_win32_path` to use the new flag to path_conv, I was finally able
to build the problematic package with the long name...  I imagine
`cygwin_conv_path` should have that flag too, for cygpath and `pwd -W`.

Hopefully there's a better (less invasive) approach that I'm not seeing.


Re: [PATCH] Cygwin: tweak handling of native symlinks from chdir

2021-05-29 Thread Jeremy Drake via Cygwin-patches
On Sat, 29 May 2021, Jeremy Drake wrote:

> First, revert the handling of virtual drives as non-symlinks.  This is no
> longer necessary.
>

Spoke too soon, it seems that `makepkg` uses `pwd -P` and that still
dereferences symlinks, as does `pwd -W` and `cygpath -w .`  :(


[PATCH] Cygwin: tweak handling of native symlinks from chdir

2021-05-29 Thread Jeremy Drake via Cygwin-patches
First, revert the handling of virtual drives as non-symlinks.  This is no
longer necessary.

Then, change the handling that really matters for my situation:

The new GetFinalPathNameW handling for native symlinks in inner path
components is disabled in chdir, allowing native processes to see their
cwd potentially including native symlinks, rather than dereferencing
them.

This seems to work for virtual drives being the CWD for native proceses,
both for the root and subdirectories.

Thoughts?From 352de39c3474fdb73570121820b493bd81a73bb5 Mon Sep 17 00:00:00 2001
From: Jeremy Drake 
Date: Sat, 29 May 2021 11:48:11 -0700
Subject: [PATCH 2/2] Cygwin: tweak handling of native symlinks from chdir

The new GetFinalPathNameW handling for native symlinks in inner path
components is disabled in chdir, allowing native processes to see their
cwd potentially including native symlinks, rather than dereferencing
them.
---
 winsup/cygwin/path.cc | 5 +++--
 winsup/cygwin/path.h  | 1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index e62f8fe2b..130926cbc 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -751,7 +751,7 @@ path_conv::check (const char *src, unsigned opt,
  if (error)
return;
 
- sym.pc_flags = pc_flags;
+ sym.pc_flags = pc_flags | (opt & PC_NO_NATIVE_SYM_INNER);
 
  if (!dev.exists ())
{
@@ -3480,6 +3480,7 @@ restart:
goto file_not_symlink;
}
 #endif /* __i386__ */
+  if (!(pc_flags & PC_NO_NATIVE_SYM_INNER))
   {
PWCHAR fpbuf = tp.w_get ();
DWORD ret;
@@ -3704,7 +3705,7 @@ chdir (const char *in_dir)
 
   /* Convert path.  PC_NONULLEMPTY ensures that we don't check for
 NULL/empty/invalid again. */
-  path_conv path (in_dir, PC_SYM_FOLLOW | PC_POSIX | PC_NONULLEMPTY);
+  path_conv path (in_dir, PC_SYM_FOLLOW | PC_POSIX | PC_NONULLEMPTY | 
PC_NO_NATIVE_SYM_INNER);
   if (path.error)
{
  set_errno (path.error);
diff --git a/winsup/cygwin/path.h b/winsup/cygwin/path.h
index adb0ca11f..d3b14a40c 100644
--- a/winsup/cygwin/path.h
+++ b/winsup/cygwin/path.h
@@ -59,6 +59,7 @@ enum pathconv_arg
   PC_KEEP_HANDLE= _BIT (12),   /* keep handle for later stat calls */
   PC_NO_ACCESS_CHECK= _BIT (13),   /* helper flag for error check */
   PC_SYM_NOFOLLOW_DIR   = _BIT (14),   /* don't follow a trailing slash */
+  PC_NO_NATIVE_SYM_INNER = _BIT (15),  /* skip native symlink inner handling */
   PC_DONT_USE   = _BIT (31)/* conversion to signed happens. */
 };
 
-- 
2.31.1.windows.1

From 4bb959b57606465d5a7abe7d3ae168db66f5f6fa Mon Sep 17 00:00:00 2001
From: Jeremy Drake 
Date: Sat, 29 May 2021 13:17:08 -0700
Subject: [PATCH 1/2] Revert "Cygwin: Handle virtual drives as non-symlinks"

This reverts commit c8949d04001e3dbc03651475b6cd1c5623400835.
---
 winsup/cygwin/path.cc | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index 6a07f0d06..e62f8fe2b 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -3505,9 +3505,14 @@ restart:
 
 subst X: C:\foo\bar
 
-  Treat it as a normal file. */
+  Treat it like a symlink.  This is required to tell an
+  lstat caller that the "drive" is actually pointing
+  somewhere else, thus, it's a symlink in POSIX speak. */
if (upath.Length == 14) /* \??\X:\ */
- goto file_not_symlink;
+ {
+   fileattr &= ~FILE_ATTRIBUTE_DIRECTORY;
+   path_flags |= PATH_SYMLINK;
+ }
/* For final paths differing in inner path components return
   length as negative value.  This informs path_conv::check
   to skip realpath handling on the last path component. */
-- 
2.31.1.windows.1



Re: realpath issue with native[strict] symlinks

2021-05-28 Thread Jeremy Drake via Cygwin
On Thu, 27 May 2021, Jeremy Drake wrote:

> > > Treating mapped/subst drives as though they were not symlinks, without
> > > messing with intermedate symlinks.
> >
> > It was that simple, surprise, surprise...
>
> It turns out it wasn't, after all.  This only seems to work at the root of
> a subst-ed drive.  I just got the following error:

I was thinking about this more, and maybe I'm fixated on the wrong code
(since that code change "broke" it).  Perhaps SUBST drives *should* be
treated as native symlinks, and what needs to change is that native
symlinks should not be dereferenced when launching native programs.  I
assume the symlink-dereferencing there predates the native symlink
support, and was required since native programs wouldn't be able to
traverse a cygwin symlink or .lnk file.

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


realpath issue with native[strict] symlinks

2021-05-27 Thread Jeremy Drake via Cygwin
> > Treating mapped/subst drives as though they were not symlinks, without
> > messing with intermedate symlinks.
>
> It was that simple, surprise, surprise...

It turns out it wasn't, after all.  This only seems to work at the root of
a subst-ed drive.  I just got the following error:
error: [Errno 2] No such file or directory:
'C:/msys32/home/Administrator/MINGW-packages/mingw-w64-python-sphinxcontrib-serializinghtml/pkg/mingw-w64-clang-aarch64-python-sphinxcontrib-serializinghtml/clangarm64/lib/python3.8/site-packages/sphinxcontrib/serializinghtml/locales/sphinxcontrib.serializinghtml.pot'

Where T:\ -> C:\msys32\home\Administrator\MINGW-packages and current
directory was under /t/

I was under the impression that that code iterated through path
components, so simply claiming that it wasn't a symlink when it got to the
"symlink" at the root of the drive would suffice, but apparently it's not
so simple.

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


[PATCH] cygutils/cygdrop: fix return type of usageCore

2021-05-26 Thread Jeremy Drake via Cygwin-apps
Fixes a warning "no return statement in function returning non-void",
and solves a crash running --help.

Hopefully this is the right place for this now, since I am not interesting
in becoming a package maintainer as the list description says ;)From f4821db24d4e4feca16e4aea58843128e233ef4e Mon Sep 17 00:00:00 2001
From: Jeremy Drake 
Date: Tue, 25 May 2021 16:13:17 -0700
Subject: [PATCH] cygdrop: fix return type of usageCore

Fixes a warning "no return statement in function returning non-void",
and solves a crash running --help.
---
 src/cygdrop/cygdrop.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cygdrop/cygdrop.cc b/src/cygdrop/cygdrop.cc
index 35bcc19..dc403c9 100644
--- a/src/cygdrop/cygdrop.cc
+++ b/src/cygdrop/cygdrop.cc
@@ -39,7 +39,7 @@ static void help (FILE * f, const char *name);
 static void version (FILE * f, const char *name);
 static void license (FILE * f, const char *name);
 
-static int
+static void
 usageCore (FILE * f, const char * name)
 {
   fprintf (f,
-- 
2.31.1



[PATCH] cygdrop: fix return type of usageCore

2021-05-25 Thread Jeremy Drake via Cygwin-patches
Fixes a warning "no return statement in function returning non-void",
and solves a crash running --help.From f4821db24d4e4feca16e4aea58843128e233ef4e Mon Sep 17 00:00:00 2001
From: Jeremy Drake 
Date: Tue, 25 May 2021 16:13:17 -0700
Subject: [PATCH] cygdrop: fix return type of usageCore

Fixes a warning "no return statement in function returning non-void",
and solves a crash running --help.
---
 src/cygdrop/cygdrop.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cygdrop/cygdrop.cc b/src/cygdrop/cygdrop.cc
index 35bcc19..dc403c9 100644
--- a/src/cygdrop/cygdrop.cc
+++ b/src/cygdrop/cygdrop.cc
@@ -39,7 +39,7 @@ static void help (FILE * f, const char *name);
 static void version (FILE * f, const char *name);
 static void license (FILE * f, const char *name);
 
-static int
+static void
 usageCore (FILE * f, const char * name)
 {
   fprintf (f,
-- 
2.31.1



Re: [RFC] FAST_CWD warnings on ARM64 insider preview

2021-05-19 Thread Jeremy Drake via Cygwin-patches
On Wed, 19 May 2021, Corinna Vinschen wrote:

> > +#ifndef IMAGE_FILE_MACHINE_ARM64
> > +#define IMAGE_FILE_MACHINE_ARM64 0xAA64
> > +#endif
>
> IMAGE_FILE_MACHINE_ARM64 is already defined for some time in winnt.h
> so we won't need the ifdef.

OK.  Was just matching what was done with PROCESSOR_ARCHITECTURE_ARM64.

> > +  typedef BOOL (WINAPI * IsWow64Process2_t)(HANDLE hProcess, USHORT 
> > *pProcessMachine, USHORT *pNativeMachine);
> > +  IsWow64Process2_t pfnIsWow64Process2 = 
> > (IsWow64Process2_t)GetProcAddress(GetModuleHandle("kernel32.dll"), 
> > "IsWow64Process2");
> > +  if (pfnIsWow64Process2 && pfnIsWow64Process2(GetCurrentProcess(), 
> > , ) && nativemachine == IMAGE_FILE_MACHINE_ARM64)
>
> We're having the autoload mechanism for that, i. e., your patch can get
> rid of the GetModuleHandle/GetProcAddress preliminaries entirely.
>
> By using the LoadDLLfuncEx() expression, failure to load the function
> will result in a return value of FALSE with GetLastError ==
> ERROR_PROC_NOT_FOUND, so this is safe on older systems.

Nice.

> It's easier to understand with a full example, so I took the liberty to
> rewrite your patch accordingly.  Since the idea and the basic work is
> yours, I'd push this under your name, see below.

Looks good.  Thanks


Re: realpath issue with native[strict] symlinks

2021-05-18 Thread Jeremy Drake via Cygwin
Sorry ,forgot subject

On Tue, 18 May 2021, Jeremy Drake wrote:

> > Sorry, but there's only this or that, not both.  Either we revert the
> > entire change, including the native shortcut stuff, or we do it
> > completely and fully, including handling virtual drives as symlinks.
>
> I had success with just the following change (of course comments would
> also need to be updated):
>
> diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
> index 4ebaf4dc6..53534b8b6 100644
> --- a/winsup/cygwin/path.cc
> +++ b/winsup/cygwin/path.cc
> @@ -3670,8 +3670,7 @@ restart:
>  somewhere else, thus, it's a symlink in POSIX speak. */
>   if (upath.Length == 14) /* \??\X:\ */
> {
> - fileattr &= ~FILE_ATTRIBUTE_DIRECTORY;
> - path_flags |= PATH_SYMLINK;
> + goto file_not_symlink;
> }
>   /* For final paths differing in inner path components return
>  length as negative value.  This informs path_conv::check
>
> Treating mapped/subst drives as though they were not symlinks, without
> messing with intermedate symlinks.
>

-- 
Worst Response To A Crisis, 1985:
From a readers' Q and A column in TV GUIDE: "If we get involved
in a nuclear war, would the electromagnetic pulses from exploding
bombs damage my videotapes?"

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


[no subject]

2021-05-18 Thread Jeremy Drake via Cygwin
> Sorry, but there's only this or that, not both.  Either we revert the
> entire change, including the native shortcut stuff, or we do it
> completely and fully, including handling virtual drives as symlinks.

I had success with just the following change (of course comments would
also need to be updated):

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index 4ebaf4dc6..53534b8b6 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -3670,8 +3670,7 @@ restart:
   somewhere else, thus, it's a symlink in POSIX speak. */
if (upath.Length == 14) /* \??\X:\ */
  {
-   fileattr &= ~FILE_ATTRIBUTE_DIRECTORY;
-   path_flags |= PATH_SYMLINK;
+   goto file_not_symlink;
  }
/* For final paths differing in inner path components return
   length as negative value.  This informs path_conv::check

Treating mapped/subst drives as though they were not symlinks, without
messing with intermedate symlinks.

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


[RFC] FAST_CWD warnings on ARM64 insider preview

2021-05-18 Thread Jeremy Drake via Cygwin-patches
I have been trying out x86_64 msys2 (a fork of cygwin) on Windows insider
preview builds on ARM64, which added support for x86_64 emulation.  I was
getting the notorious FAST_CWD warnings, which were interfering with
MINGW CMake.  Last night late, I went looking at the cause, and found some
code that attempted to disable FAST_CWD warnings, but only on i686, and
trying to look at GetNativeSystemInfo, which was lying.  I quickly put
together this patch, and it seems to work.

Note I did this late at night, with no real regard to investigating or
matching code style.  This patch is currently more in an RFC state, if the
approach looks OK, and I'd be grateful for any pointers on getting it into
shape for evental acceptance.

Thanks,
JeremyFrom 346dfb78fc5522d5d7288571d635303c69a5270a Mon Sep 17 00:00:00 2001
From: Jeremy Drake 
Date: Tue, 18 May 2021 00:48:52 -0700
Subject: [PATCH] cygwin: suppress FAST_CWD warnings on ARM64.

The old check was insufficient: new insider preview builds of Windows
allow running x86_64 process on ARM64.  The IsWow64Process2 function
seems to be the intended way to figure this situation out.
---
 winsup/cygwin/path.cc | 30 ++
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index dd7048486..5c4adcd49 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -4708,29 +4708,19 @@ find_fast_cwd ()
 {
   bool warn = 1;
 
-#ifndef __x86_64__
-  #ifndef PROCESSOR_ARCHITECTURE_ARM64
-  #define PROCESSOR_ARCHITECTURE_ARM64 12
-  #endif
+#ifndef IMAGE_FILE_MACHINE_ARM64
+#define IMAGE_FILE_MACHINE_ARM64 0xAA64
+#endif
 
-  SYSTEM_INFO si;
+  USHORT procmachine, nativemachine;
 
   /* Check if we're running in WOW64 on ARM64.  Skip the warning as long as
-there's no solution for finding the FAST_CWD pointer on that system.
-
-2018-07-12: Apparently current ARM64 WOW64 has a bug:
-It's GetNativeSystemInfo returns PROCESSOR_ARCHITECTURE_INTEL in
-wProcessorArchitecture.  Since that's an invalid value (a 32 bit
-host system hosting a 32 bit emulator for itself?) we can use this
-value as an indicator to skip the message as well. */
-  if (wincap.is_wow64 ())
-   {
- GetNativeSystemInfo ();
- if (si.wProcessorArchitecture == PROCESSOR_ARCHITECTURE_ARM64
- || si.wProcessorArchitecture == PROCESSOR_ARCHITECTURE_INTEL)
-   warn = 0;
-   }
-#endif /* !__x86_64__ */
+there's no solution for finding the FAST_CWD pointer on that system. */
+
+  typedef BOOL (WINAPI * IsWow64Process2_t)(HANDLE hProcess, USHORT 
*pProcessMachine, USHORT *pNativeMachine);
+  IsWow64Process2_t pfnIsWow64Process2 = 
(IsWow64Process2_t)GetProcAddress(GetModuleHandle("kernel32.dll"), 
"IsWow64Process2");
+  if (pfnIsWow64Process2 && pfnIsWow64Process2(GetCurrentProcess(), 
, ) && nativemachine == IMAGE_FILE_MACHINE_ARM64)
+   warn = 0;
 
   if (warn)
small_printf ("Cygwin WARNING:\n"
-- 
2.31.1.windows.1



Re: [PATCH] Add support for high-entropy-va flag to peflags.

2021-05-17 Thread Jeremy Drake via Cygwin-patches
On Mon, 17 May 2021, Corinna Vinschen wrote:

> Hi Jeremy,
>
> Thanks for the patch, but I have two nits:
>
> - The patch doesn't apply cleanly with `git am'.  Please check again.

Probably got mangled in the mail.  Attached this time.

>
> - I would prefer a massively reduced patch size, by *not* changing
>   indentation on otherwise unaffected lines.
>

DoneFrom 26e7d716b5ecc49cc2e8d5ab05a1586c089c75fe Mon Sep 17 00:00:00 2001
From: Jeremy Drake 
Date: Sat, 15 May 2021 12:07:26 -0700
Subject: [PATCH] Add support for high-entropy-va flag to peflags.

This allows for setting, clearing, and displaying the value of the "high
entropy va" dll characteristics flag.

Signed-off-by: Jeremy Drake 
---
 peflags.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/peflags.c b/peflags.c
index 4d22e4a..bb333d7 100644
--- a/peflags.c
+++ b/peflags.c
@@ -112,7 +112,7 @@ static const symbolic_flags_t pe_symbolic_flags[] = {
 /*CF(0x0004, reserved_0x0004),*/
 /*CF(0x0008, reserved_0x0008),*/
 /*CF(0x0010, unspec_0x0010),*/
-/*CF(0x0020, unspec_0x0020),*/
+  CF(0x0020, high-entropy-va),
   CF(0x0040, dynamicbase),
   CF(0x0080, forceinteg),
   CF(0x0100, nxcompat),
@@ -181,6 +181,7 @@ sizeof_values_t sizeof_vals[5] = {
 
 static struct option long_options[] = {
   {"dynamicbase",  optional_argument, NULL, 'd'},
+  {"high-entropy-va", optional_argument, NULL, 'e'},
   {"forceinteg",   optional_argument, NULL, 'f'},
   {"nxcompat", optional_argument, NULL, 'n'},
   {"no-isolation", optional_argument, NULL, 'i'},
@@ -203,7 +204,7 @@ static struct option long_options[] = {
   {NULL, no_argument, NULL, 0}
 };
 static const char *short_options
-   = "d::f::n::i::s::b::W::t::w::l::S::x::X::y::Y::z::T:vhV";
+   = "d::e::f::n::i::s::b::W::t::w::l::S::x::X::y::Y::z::T:vhV";
 
 static void short_usage (FILE *f);
 static void help (FILE *f);
@@ -699,6 +700,11 @@ parse_args (int argc, char *argv[])
 optarg,
 IMAGE_DLLCHARACTERISTICS_DYNAMIC_BASE);
  break;
+   case 'e':
+ handle_pe_flag_option (long_options[option_index].name,
+optarg,
+IMAGE_DLLCHARACTERISTICS_HIGH_ENTROPY_VA);
+ break;
case 'n':
  handle_pe_flag_option (long_options[option_index].name,
 optarg,
@@ -1069,6 +1075,9 @@ help (FILE *f)
 "\n"
 "  -d, --dynamicbase  [BOOL]   Image base address may be relocated using\n"
 "  address space layout randomization (ASLR).\n"
+"  -e,\n"
+"  --high-entropy-va  [BOOL]   Image is compatible with 64-bit address space\n"
+"  layout randomization (ASLR).\n"
 "  -f, --forceinteg   [BOOL]   Code integrity checks are enforced.\n"
 "  -n, --nxcompat [BOOL]   Image is compatible with data execution\n"
 "  prevention (DEP).\n"
-- 
2.31.1.windows.1



[PATCH] Add support for high-entropy-va flag to peflags.

2021-05-15 Thread Jeremy Drake via Cygwin-patches
This allows for setting, clearing, and displaying the value of the "high
entropy va" dll characteristics flag.

Signed-off-by: Jeremy Drake 
---
I'm not sure this is the right list for this... I made this patch a while
back and it has been kicking around msys2's rebase package, figured I'd
submit it upstream.

 peflags.c | 130 +-
 1 file changed, 69 insertions(+), 61 deletions(-)

diff --git a/peflags.c b/peflags.c
index 4d22e4a..358199a 100644
--- a/peflags.c
+++ b/peflags.c
@@ -112,7 +112,7 @@ static const symbolic_flags_t pe_symbolic_flags[] = {
 /*CF(0x0004, reserved_0x0004),*/
 /*CF(0x0008, reserved_0x0008),*/
 /*CF(0x0010, unspec_0x0010),*/
-/*CF(0x0020, unspec_0x0020),*/
+  CF(0x0020, high-entropy-va),
   CF(0x0040, dynamicbase),
   CF(0x0080, forceinteg),
   CF(0x0100, nxcompat),
@@ -180,30 +180,31 @@ sizeof_values_t sizeof_vals[5] = {
 #define pulong(struct, offset) (PULONG)((PBYTE)(struct)+(offset))

 static struct option long_options[] = {
-  {"dynamicbase",  optional_argument, NULL, 'd'},
-  {"forceinteg",   optional_argument, NULL, 'f'},
-  {"nxcompat", optional_argument, NULL, 'n'},
-  {"no-isolation", optional_argument, NULL, 'i'},
-  {"no-seh",   optional_argument, NULL, 's'},
-  {"no-bind",  optional_argument, NULL, 'b'},
-  {"wdmdriver",optional_argument, NULL, 'W'},
-  {"tsaware",  optional_argument, NULL, 't'},
-  {"wstrim",   optional_argument, NULL, 'w'},
-  {"bigaddr",  optional_argument, NULL, 'l'},
-  {"sepdbg",   optional_argument, NULL, 'S'},
-  {"stack-reserve",optional_argument, NULL, 'x'},
-  {"stack-commit", optional_argument, NULL, 'X'},
-  {"heap-reserve", optional_argument, NULL, 'y'},
-  {"heap-commit",  optional_argument, NULL, 'Y'},
-  {"cygwin-heap",  optional_argument, NULL, 'z'},
-  {"filelist", no_argument, NULL, 'T'},
-  {"verbose",  no_argument, NULL, 'v'},
-  {"help", no_argument, NULL, 'h'},
-  {"version",  no_argument, NULL, 'V'},
+  {"dynamicbase", optional_argument, NULL, 'd'},
+  {"high-entropy-va", optional_argument, NULL, 'e'},
+  {"forceinteg",  optional_argument, NULL, 'f'},
+  {"nxcompat",optional_argument, NULL, 'n'},
+  {"no-isolation",optional_argument, NULL, 'i'},
+  {"no-seh",  optional_argument, NULL, 's'},
+  {"no-bind", optional_argument, NULL, 'b'},
+  {"wdmdriver",   optional_argument, NULL, 'W'},
+  {"tsaware", optional_argument, NULL, 't'},
+  {"wstrim",  optional_argument, NULL, 'w'},
+  {"bigaddr", optional_argument, NULL, 'l'},
+  {"sepdbg",  optional_argument, NULL, 'S'},
+  {"stack-reserve",   optional_argument, NULL, 'x'},
+  {"stack-commit",optional_argument, NULL, 'X'},
+  {"heap-reserve",optional_argument, NULL, 'y'},
+  {"heap-commit", optional_argument, NULL, 'Y'},
+  {"cygwin-heap", optional_argument, NULL, 'z'},
+  {"filelist",no_argument, NULL, 'T'},
+  {"verbose", no_argument, NULL, 'v'},
+  {"help",no_argument, NULL, 'h'},
+  {"version", no_argument, NULL, 'V'},
   {NULL, no_argument, NULL, 0}
 };
 static const char *short_options
-   = "d::f::n::i::s::b::W::t::w::l::S::x::X::y::Y::z::T:vhV";
+   = "d::e::f::n::i::s::b::W::t::w::l::S::x::X::y::Y::z::T:vhV";

 static void short_usage (FILE *f);
 static void help (FILE *f);
@@ -699,6 +700,11 @@ parse_args (int argc, char *argv[])
 optarg,
 IMAGE_DLLCHARACTERISTICS_DYNAMIC_BASE);
  break;
+   case 'e':
+ handle_pe_flag_option (long_options[option_index].name,
+optarg,
+IMAGE_DLLCHARACTERISTICS_HIGH_ENTROPY_VA);
+ break;
case 'n':
  handle_pe_flag_option (long_options[option_index].name,
 optarg,
@@ -1067,45 +1073,47 @@ help (FILE *f)
 "given, the specified value will be overwritten; if no argument is given, 
the\n"
 "numerical value will be displayed in decimal and hexadecimal notation.\n"
 "\n"
-"  -d, --dynamicbase  [BOOL]   Image base address may be relocated using\n"
-"  address space layout randomization (ASLR).\n"
-"  -f, --forceinteg   [BOOL]   Code integrity checks are enforced.\n"
-"  -n, --nxcompat [BOOL]   Image is compatible with data execution\n"
-"  prevention (DEP).\n"
-"  -i, --no-isolation [BOOL]   Image understands isolation but do not 
isolate\n"
-"  the image.\n"
-"  -s, --no-seh   [BOOL]   Image does not use structured exception 
handling\n"
-"  (SEH). No SE handler may be called in this 
image.\n"
-"  -b, --no-bind  [BOOL]   Do not bind this image.\n"
-"  -W, --wdmdriver[BOOL]   Driver uses the WDM model.\n"
-"  -t, --tsaware  [BOOL]   Image is Terminal Server aware.\n"
-"  -w, --wstrim  

realpath issue with native[strict] symlinks

2021-05-14 Thread Jeremy Drake via Cygwin
I apologize for not replying to the message properly, I am not subscribed
and am copy-pasting from web archive.

On Sat, May 8, 2021 at 12:20 AM Corinna Vinschen wrote:
> The changes have a behavioral change, but I think this is for the
> better: Virtual drives are not treated as drives anymore, but as
> symlinks.  Given they are just pointers to other drives or directories,
> tha't much closer to reality.  I. e., in case of my above virtual drive
> T:, what you'll see in the /cygdrive dir (unless your cygdrive prefix is
> / only) is this:

I am concerned about this behavior.  The reason I was using subst to begin
with was that some build tools encode the full path in their filenames,
resulting in hitting MAX_PATH-related issues for not horribly long/deep
paths.  With this change, running a native Windows process (MinGW-w64)
from a subst drive results in what it sees as the CWD being 'dereferenced'
as though subst was not used, defeating the path-shortening purpose.

For instance, using your example mapping:
> $ ls -lG /cygdrive
> total 16
> d---r-x---+ 1 TrustedInstaller  0 Apr 29 21:07 c
> drwxr-xr-x  1 corinna   0 Dec 31  1979 e
> lrwxrwxrwx  1 corinna  32 May  6 20:43 t -> 
> /cygdrive/c/cygwin64/home/corinna/tmp

Prior to these changes would show
$ cd /cygdrive/t && cmd /c cd
T:\

But after these changes would show
$ cd /cygdrive/t && cmd /c cd
C:\cygwin64\home\corinna\tmp

This path could well be long enough to trigger build issues for certain
MINGW-packages.

Sorry to be a nuisance...

-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Cannot access subst drives after commit 456c3a46

2021-05-04 Thread Jeremy Drake via Cygwin
Sort of like the system32 issue, but still present after the fix for that.

Previously I was able to shorten paths for both native and cygwin
processes by using SUBST to map a drive letter.  After this commit was
applied, this stopped working:

$ subst
T:\: -> C:\blah\blah\MINGW-packages

$ ls /t/
ls: cannot access '/t/': No such file or directory

$ mount
...
T: on /t type ntfs (binary,noacl,posix=0,user,noumount,auto)



-- 
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


Re: cygwin + binutils 2.36 + ASLR/dynamicbase defaults

2021-02-28 Thread Jeremy Drake via Cygwin
On Sun, 28 Feb 2021, ASSI wrote:

> > Is this still problematic for cygwin?
>
> Yes it is and I'm currently figuring out how to best get rid of it in
> order to be able to update binutils (why this was ever allowed in
> without an accompanying configure option is a mystery to me).

Well, Microsoft's LINK.EXE and LLVM's LLD have already been using these
new defaults for some time.  But I was surprised how quickly my patch was
accepted/merged.

> I've
> already nixed it for Cygwin, but I'm not yet sure what to do for the
> cross compilation toolchain.  While it should in principle work there,
> I'm pretty sure that there will be problems when it comes to the nitty
> gritty details.  It's already transpired that some of the linker scripts
> can't deal with the larger base addresses this change does generate
> eventually.

To clarify, default base addresses should not have changed for cygwin
targets, they were already above 4GB.

I have a prelimiary patch that I plan to send upstream once I get some
testing done on it, which reverts the default dll characteristics for
cygwin targets.  I don't know if what you've done to 'nix it' for Cygwin
was similar.

I have not seen anything one way or the other on the NXCOMPAT flag.  Does
that also needs to be reverted for Cygwin?

> > The reason I'm asking is because we updated to 2.36 in MSYS2 and are
> > wondering if we need to patch this out (or change the defaults) It
> > seems to work as is right now, but maybe we are just lucky(?).
>
> You are just lucky and need to test more. :-)

I have seen the issues you described on 32-bit, but my understanding of
how ASLR works suggested that it should be very rare on 64-bit.

> Note that the change does not only affect DLL as the commit message
> would want you to believe and you will eventually end up with a
> situation where ASLR tries moves the stack of an executable, at which
> point you can no longer fork.

... but now that you mention the stack moving, yes, I could see that
being an issue.

Yes, the specific field where these flags are stored is called "DLL
Characteristics" so that is how it was referred to, but they do not
exclusively apply to DLLs.

https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#dll-characteristicsFrom 39bde2f11a1eb97503bae9cf15f5fc05640e5251 Mon Sep 17 00:00:00 2001
From: Jeremy Drake 
Date: Sun, 28 Feb 2021 15:49:08 -0800
Subject: [PATCH] ld: revert default dll characteristics for Cygwin.

Mail thread from
https://cygwin.com/pipermail/cygwin/2021-February/247922.html suggests
these flags will NOT work for Cygwin, which relies on stable address
layouts for their fork() emulation.

In the process, renamed move_default_addr_high shell variable to
cygwin_beahior, as the old name wasn't quite accurate anymore and I
wanted to use it choose which dll characteristics flags to use by
default.

Also copied that switch to pe.em, as it was only in pep.em before but
32-bit also needed to switch defaults for Cygwin.
---
 ld/emultempl/pe.em  | 13 -
 ld/emultempl/pep.em | 40 +---
 2 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/ld/emultempl/pe.em b/ld/emultempl/pe.em
index 748a6b49412..9f757cc31dc 100644
--- a/ld/emultempl/pe.em
+++ b/ld/emultempl/pe.em
@@ -5,6 +5,16 @@ if [ -z "$MACHINE" ]; then
 else
   OUTPUT_ARCH=${ARCH}:${MACHINE}
 fi
+
+case ${target} in
+  *-*-cygwin*)
+cygwin_behavior=1
+;;
+  *)
+cygwin_behavior=0;
+;;
+esac
+
 rm -f e${EMULATION_NAME}.c
 (echo;echo;echo;echo;echo)>e${EMULATION_NAME}.c # there, now line numbers 
match ;-)
 fragment <--
Problem reports:  https://cygwin.com/problems.html
FAQ:  https://cygwin.com/faq/
Documentation:https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple


[PATCH v2] cygwin: use CREATE_DEFAULT_ERROR_MODE in spawn

2020-12-09 Thread Jeremy Drake via Cygwin-patches
This allows native processes to get Windows-default error handling
behavior (such as invoking the registered JIT debugger).

---
 winsup/cygwin/spawn.cc | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 92d190d1a..83b216f52 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -431,6 +431,13 @@ child_info_spawn::worker (const char *prog_arg, const char 
*const *argv,

   c_flags |= CREATE_SEPARATE_WOW_VDM | CREATE_UNICODE_ENVIRONMENT;

+  /* Add CREATE_DEFAULT_ERROR_MODE flag for non-Cygwin processes so they
+get the default error mode instead of inheriting the mode Cygwin
+uses.  This allows things like Windows Error Reporting/JIT debugging
+to work with processes launched from a Cygwin shell. */
+  if (!real_path.iscygexec ())
+   c_flags |= CREATE_DEFAULT_ERROR_MODE;
+
   /* We're adding the CREATE_BREAKAWAY_FROM_JOB flag here to workaround
 issues with the "Program Compatibility Assistant (PCA) Service".
 For some reason, when starting long running sessions from mintty(*),
-- 
2.29.2.windows.3



Re: [PATCH 1/1] cygwin: use CREATE_DEFAULT_ERROR_MODE in spawn

2020-12-08 Thread Jeremy Drake via Cygwin-patches
On Tue, 8 Dec 2020, Jon Turney wrote:
> On 07/12/2020 09:43, Corinna Vinschen wrote:
> > On Dec  4 10:35, Jeremy Drake via Cygwin-patches wrote:
> > > On Fri, 4 Dec 2020, Corinna Vinschen via Cygwin-patches wrote:
> > >
> > > > I'm not happy about a new CYGWIN option.
> > > >
> > > > Wouldn't it make sense, perhaps, to switch to CREATE_DEFAULT_ERROR_MODE
> > > > for all non-Cygwin processes by default instead?
>
> I agree.
>
> Cygwin calls SetErrorMode(), so we need to use this flag to prevent that
> non-default behaviour being inherited by processes started with
> CreateProcess().
>

In that case, here's my initial, much simpler patch

-- >8 --
Subject: [PATCH] cygwin: use CREATE_DEFAULT_ERROR_MODE in spawn

This allows native processes to get Windows-default error handling
behavior (such as invoking the registered JIT debugger), while cygwin
processes would quickly set their error mode back to what they expect.
---
 winsup/cygwin/spawn.cc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index c77d62984..f5952d53b 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -429,7 +429,8 @@ child_info_spawn::worker (const char *prog_arg, const char 
*const *argv,
   c_flags = GetPriorityClass (GetCurrentProcess ());
   sigproc_printf ("priority class %d", c_flags);

-  c_flags |= CREATE_SEPARATE_WOW_VDM | CREATE_UNICODE_ENVIRONMENT;
+  c_flags |= CREATE_SEPARATE_WOW_VDM | CREATE_UNICODE_ENVIRONMENT
+ | CREATE_DEFAULT_ERROR_MODE;

   /* We're adding the CREATE_BREAKAWAY_FROM_JOB flag here to workaround
 issues with the "Program Compatibility Assistant (PCA) Service".


Re: [PATCH 1/1] cygwin: use CREATE_DEFAULT_ERROR_MODE in spawn

2020-12-04 Thread Jeremy Drake via Cygwin-patches
On Fri, 4 Dec 2020, Corinna Vinschen via Cygwin-patches wrote:

> I'm not happy about a new CYGWIN option.
>
> Wouldn't it make sense, perhaps, to switch to CREATE_DEFAULT_ERROR_MODE
> for all non-Cygwin processes by default instead?

In fact, my first iteration was to set that flag unconditionally (relying
on the fact that SetErrorMode is called extremely early in Cygwin process
startup rather than only setting it for non-Cygwin processes), but I
received feedback that it would be better to put it behind an option:

https://github.com/msys2/msys2-runtime/pull/18#issuecomment-723683606



[PATCH 1/1] cygwin: use CREATE_DEFAULT_ERROR_MODE in spawn

2020-12-03 Thread Jeremy Drake via Cygwin-patches
if a new CYGWIN/MSYS environment option `winjitdebug` is true, allowing
native subprocesses to get Windows-default error handling behavior (such
as invoking the registered JIT debugger).  Cygwin processes will quickly
set their error mode on start, so getting JIT debugging for them will
still require setting `error_start`.

This patch was previously submitted to MSYS2
(https://github.com/msys2/msys2-runtime/pull/18) but it was suggested I
should try sending it upstream.

---
 winsup/cygwin/environ.cc | 1 +
 winsup/cygwin/globals.cc | 1 +
 winsup/cygwin/spawn.cc   | 2 ++
 winsup/doc/cygwinenv.xml | 9 +
 4 files changed, 13 insertions(+)

diff --git a/winsup/cygwin/environ.cc b/winsup/cygwin/environ.cc
index 3a03657db..fa47f4b31 100644
--- a/winsup/cygwin/environ.cc
+++ b/winsup/cygwin/environ.cc
@@ -120,6 +120,7 @@ static struct parse_thing
   {"wincmdln", {}, setbool, NULL, {{false}, {true}}},
   {"winsymlinks", {func: set_winsymlinks}, isfunc, NULL, {{0}, {0}}},
   {"disable_pcon", {_pcon}, setbool, NULL, {{false}, {true}}},
+  {"winjitdebug", {_default_errmode}, setbool, NULL, {{false}, {true}}},
   {NULL, {0}, setdword, 0, {{0}, {0}}}
 };

diff --git a/winsup/cygwin/globals.cc b/winsup/cygwin/globals.cc
index 942bd1c83..2d2ac0949 100644
--- a/winsup/cygwin/globals.cc
+++ b/winsup/cygwin/globals.cc
@@ -71,6 +71,7 @@ bool reset_com;
 bool wincmdln;
 winsym_t allow_winsymlinks = WSYM_sysfile;
 bool disable_pcon;
+bool spawn_default_errmode;

 bool NO_COPY in_forkee;

diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 92d190d1a..6239e3539 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -430,6 +430,8 @@ child_info_spawn::worker (const char *prog_arg, const char 
*const *argv,
   sigproc_printf ("priority class %d", c_flags);

   c_flags |= CREATE_SEPARATE_WOW_VDM | CREATE_UNICODE_ENVIRONMENT;
+  if (spawn_default_errmode)
+   c_flags |= CREATE_DEFAULT_ERROR_MODE;

   /* We're adding the CREATE_BREAKAWAY_FROM_JOB flag here to workaround
 issues with the "Program Compatibility Assistant (PCA) Service".
diff --git a/winsup/doc/cygwinenv.xml b/winsup/doc/cygwinenv.xml
index a52b6ac19..7137edcb9 100644
--- a/winsup/doc/cygwinenv.xml
+++ b/winsup/doc/cygwinenv.xml
@@ -103,6 +103,15 @@ pty will be disabled.  This is for programs which do not 
work properly
 under pty with pseudo console enabled.  Defaults to not set.
 

+
+winjitdebug - if set, the
+CREATE_DEFAULT_ERROR_MODE flag is passed to
+CreateProcess in spawn.  This prevents
+cygwin-set error mode flags from being inherited by the new process, allowing
+native processes to invoke any system-registered JIT debugger, and/or invoke
+Windows Error Reporting.  Defaults to not set.
+
+
 

 
-- 
2.29.2.windows.2