Re: [PATCH] Cygwin: mmap: fix mapping beyond EOF on 64 bit
On 2020-07-20 15:08, Ken Brown via Cygwin-patches wrote: > On 7/20/2020 4:29 PM, Brian Inglis wrote: >> On 2020-07-20 09:41, Corinna Vinschen wrote: >>> Ultimately, I wonder if we really should keep all the 32 bit OS stuff >>> in. The number of real 32 bit systems (not WOW64) is dwindling fast. >>> Keeping all the AT_ROUND_TO_PAGE stuff in just for what? 2%? of the >>> systems is really not worth it, I guess. > [...] >> If you don't want to ask and perhaps reconsider the impact of your approach, >> maybe give folks a heads up on the mailing list that the current release >> will be >> the last to support 32 bit Windows? > > Corinna didn't propose dropping support for 32 bit Windows. She proposed > dropping a feature that works *only* on 32 bit Windows. Sorry if I misunderstood, but it sounded more drastic than that, and required for the code to work compatibly on Windows 32 bit, rather than supporting a different feature that only worked on Windows 32 bit, as anything non-Linux/BSD/POSIX can be considered non-essential if support is not widespread, nor essential for critical packages. -- Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada This email may be disturbing to some readers as it contains too much technical detail. Reader discretion is advised. [Data in IEC units and prefixes, physical quantities in SI.]
Re: [PATCH] Cygwin: mmap: Remove AT_ROUND_TO_PAGE workaround
Hi Corinna,
On 7/20/2020 2:55 PM, Corinna Vinschen wrote:
From: Corinna Vinschen
It's working on 32 bit OSes only anyway. It even fails on WOW64.
Signed-off-by: Corinna Vinschen
---
Notes:
Hi Ken,
can you please review this patch and check if it doesn't break
your testcase again?
Thanks,
Corinna
winsup/cygwin/mmap.cc | 117 --
1 file changed, 34 insertions(+), 83 deletions(-)
diff --git a/winsup/cygwin/mmap.cc b/winsup/cygwin/mmap.cc
index 1fccc6c58ee9..8ac96606c2e6 100644
--- a/winsup/cygwin/mmap.cc
+++ b/winsup/cygwin/mmap.cc
@@ -195,12 +195,7 @@ MapView (HANDLE h, void *addr, size_t len, DWORD openflags,
DWORD protect = gen_create_protect (openflags, flags);
void *base = addr;
SIZE_T viewsize = len;
-#ifdef __x86_64__ /* AT_ROUND_TO_PAGE isn't supported on 64 bit systems. */
ULONG alloc_type = MEM_TOP_DOWN;
-#else
- ULONG alloc_type = (base && !wincap.is_wow64 () ? AT_ROUND_TO_PAGE : 0)
-| MEM_TOP_DOWN;
-#endif
#ifdef __x86_64__
/* Don't call NtMapViewOfSectionEx during fork. It requires autoloading
@@ -878,6 +873,10 @@ mmap64 (void *addr, size_t len, int prot, int flags, int
fd, off_t off)
if (!anonymous (flags) && fd != -1)
{
+ UNICODE_STRING fname;
+ IO_STATUS_BLOCK io;
+ FILE_STANDARD_INFORMATION fsi;
+
/* Ensure that fd is open */
cygheap_fdget cfd (fd);
if (cfd < 0)
@@ -896,19 +895,16 @@ mmap64 (void *addr, size_t len, int prot, int flags, int
fd, off_t off)
/* The autoconf mmap test maps a file of size 1 byte. It then tests
every byte of the entire mapped page of 64K for 0-bytes since that's
-what POSIX requires. The problem is, we can't create that mapping on
-64 bit systems. The file mapping will be only a single page, 4K, and
-since 64 bit systems don't support the AT_ROUND_TO_PAGE flag, the
-remainder of the 64K slot will result in a SEGV when accessed.
-
-So, what we do here is cheating for the sake of the autoconf test
-on 64 bit systems. The justification is that there's very likely
-no application actually utilizing the map beyond EOF, and we know that
-all bytes beyond EOF are set to 0 anyway. If this test doesn't work
-on 64 bit systems, it will result in not using mmap at all in a
-package. But we want that mmap is treated as usable by autoconf,
-regardless whether the autoconf test runs on a 32 bit or a 64 bit
-system.
+what POSIX requires. The problem is, we can't create that mapping.
+The file mapping will be only a single page, 4K, and the remainder
+of the 64K slot will result in a SEGV when accessed.
+
+So, what we do here is cheating for the sake of the autoconf test.
+The justification is that there's very likely no application actually
+utilizing the map beyond EOF, and we know that all bytes beyond EOF
+are set to 0 anyway. If this test doesn't work, it will result in
+not using mmap at all in a package. But we want mmap being treated
+as usable by autoconf.
Ok, so we know exactly what autoconf is doing. The file is called
"conftest.txt", it has a size of 1 byte, the mapping size is the
@@ -916,31 +912,19 @@ mmap64 (void *addr, size_t len, int prot, int flags, int
fd, off_t off)
mapping is MAP_SHARED, the offset is 0.
If all these requirements are given, we just return an anonymous map.
-This will help to get over the autoconf test even on 64 bit systems.
The tests are ordered for speed. */
-#ifdef __x86_64__
- if (1)
-#else
- if (wincap.is_wow64 ())
-#endif
- {
- UNICODE_STRING fname;
- IO_STATUS_BLOCK io;
- FILE_STANDARD_INFORMATION fsi;
-
- if (len == pagesize
- && prot == (PROT_READ | PROT_WRITE)
- && flags == MAP_SHARED
- && off == 0
- && (RtlSplitUnicodePath (fh->pc.get_nt_native_path (), NULL,
- ),
- wcscmp (fname.Buffer, L"conftest.txt") == 0)
- && NT_SUCCESS (NtQueryInformationFile (fh->get_handle (), ,
-, sizeof fsi,
-FileStandardInformation))
- && fsi.EndOfFile.QuadPart == 1LL)
- flags |= MAP_ANONYMOUS;
- }
+ if (len == pagesize
+ && prot == (PROT_READ | PROT_WRITE)
+ && flags == MAP_SHARED
+ && off == 0
+ && (RtlSplitUnicodePath (fh->pc.get_nt_native_path (), NULL,
+ ),
+ wcscmp (fname.Buffer, L"conftest.txt") == 0)
+ && NT_SUCCESS (NtQueryInformationFile (fh->get_handle (), ,
+, sizeof fsi,
+
Re: [PATCH] Cygwin: mmap: fix mapping beyond EOF on 64 bit
On 7/20/2020 4:29 PM, Brian Inglis wrote: On 2020-07-20 09:41, Corinna Vinschen wrote: Ultimately, I wonder if we really should keep all the 32 bit OS stuff in. The number of real 32 bit systems (not WOW64) is dwindling fast. Keeping all the AT_ROUND_TO_PAGE stuff in just for what? 2%? of the systems is really not worth it, I guess. [...] If you don't want to ask and perhaps reconsider the impact of your approach, maybe give folks a heads up on the mailing list that the current release will be the last to support 32 bit Windows? Corinna didn't propose dropping support for 32 bit Windows. She proposed dropping a feature that works *only* on 32 bit Windows. Ken
Re: [PATCH] Cygwin: mmap: fix mapping beyond EOF on 64 bit
On 2020-07-20 09:41, Corinna Vinschen wrote: > Ultimately, I wonder if we really should keep all the 32 bit OS stuff > in. The number of real 32 bit systems (not WOW64) is dwindling fast. > Keeping all the AT_ROUND_TO_PAGE stuff in just for what? 2%? of the > systems is really not worth it, I guess. A lot of older/smaller laptops, especially low cost budget models for lower income groups including students, seniors and for developing countries, Windows (Surface, etc.) tablets, and VMs supporting legacy systems and applications, run Windows 32 bit, and MS still sells W10 and supports 32 bit for that reason, except in OEM channels going forward from the 2020-04 release[1]. About 40% of W7 systems sold were 32 bit, with about 25% of systems still running W7, so 10% of systems could still be 32-bit. If you don't want to ask and perhaps reconsider the impact of your approach, maybe give folks a heads up on the mailing list that the current release will be the last to support 32 bit Windows? -- Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada This email may be disturbing to some readers as it contains too much technical detail. Reader discretion is advised. [Data in IEC units and prefixes, physical quantities in SI.] [1] 2004 looks like it is 16 years old - delete software still using 2 digit years for anything, as we will have many ambiguities until 2032!
[PATCH] Cygwin: mmap: Remove AT_ROUND_TO_PAGE workaround
From: Corinna Vinschen
It's working on 32 bit OSes only anyway. It even fails on WOW64.
Signed-off-by: Corinna Vinschen
---
Notes:
Hi Ken,
can you please review this patch and check if it doesn't break
your testcase again?
Thanks,
Corinna
winsup/cygwin/mmap.cc | 117 --
1 file changed, 34 insertions(+), 83 deletions(-)
diff --git a/winsup/cygwin/mmap.cc b/winsup/cygwin/mmap.cc
index 1fccc6c58ee9..8ac96606c2e6 100644
--- a/winsup/cygwin/mmap.cc
+++ b/winsup/cygwin/mmap.cc
@@ -195,12 +195,7 @@ MapView (HANDLE h, void *addr, size_t len, DWORD openflags,
DWORD protect = gen_create_protect (openflags, flags);
void *base = addr;
SIZE_T viewsize = len;
-#ifdef __x86_64__ /* AT_ROUND_TO_PAGE isn't supported on 64 bit systems. */
ULONG alloc_type = MEM_TOP_DOWN;
-#else
- ULONG alloc_type = (base && !wincap.is_wow64 () ? AT_ROUND_TO_PAGE : 0)
-| MEM_TOP_DOWN;
-#endif
#ifdef __x86_64__
/* Don't call NtMapViewOfSectionEx during fork. It requires autoloading
@@ -878,6 +873,10 @@ mmap64 (void *addr, size_t len, int prot, int flags, int
fd, off_t off)
if (!anonymous (flags) && fd != -1)
{
+ UNICODE_STRING fname;
+ IO_STATUS_BLOCK io;
+ FILE_STANDARD_INFORMATION fsi;
+
/* Ensure that fd is open */
cygheap_fdget cfd (fd);
if (cfd < 0)
@@ -896,19 +895,16 @@ mmap64 (void *addr, size_t len, int prot, int flags, int
fd, off_t off)
/* The autoconf mmap test maps a file of size 1 byte. It then tests
every byte of the entire mapped page of 64K for 0-bytes since that's
-what POSIX requires. The problem is, we can't create that mapping on
-64 bit systems. The file mapping will be only a single page, 4K, and
-since 64 bit systems don't support the AT_ROUND_TO_PAGE flag, the
-remainder of the 64K slot will result in a SEGV when accessed.
-
-So, what we do here is cheating for the sake of the autoconf test
-on 64 bit systems. The justification is that there's very likely
-no application actually utilizing the map beyond EOF, and we know that
-all bytes beyond EOF are set to 0 anyway. If this test doesn't work
-on 64 bit systems, it will result in not using mmap at all in a
-package. But we want that mmap is treated as usable by autoconf,
-regardless whether the autoconf test runs on a 32 bit or a 64 bit
-system.
+what POSIX requires. The problem is, we can't create that mapping.
+The file mapping will be only a single page, 4K, and the remainder
+of the 64K slot will result in a SEGV when accessed.
+
+So, what we do here is cheating for the sake of the autoconf test.
+The justification is that there's very likely no application actually
+utilizing the map beyond EOF, and we know that all bytes beyond EOF
+are set to 0 anyway. If this test doesn't work, it will result in
+not using mmap at all in a package. But we want mmap being treated
+as usable by autoconf.
Ok, so we know exactly what autoconf is doing. The file is called
"conftest.txt", it has a size of 1 byte, the mapping size is the
@@ -916,31 +912,19 @@ mmap64 (void *addr, size_t len, int prot, int flags, int
fd, off_t off)
mapping is MAP_SHARED, the offset is 0.
If all these requirements are given, we just return an anonymous map.
-This will help to get over the autoconf test even on 64 bit systems.
The tests are ordered for speed. */
-#ifdef __x86_64__
- if (1)
-#else
- if (wincap.is_wow64 ())
-#endif
- {
- UNICODE_STRING fname;
- IO_STATUS_BLOCK io;
- FILE_STANDARD_INFORMATION fsi;
-
- if (len == pagesize
- && prot == (PROT_READ | PROT_WRITE)
- && flags == MAP_SHARED
- && off == 0
- && (RtlSplitUnicodePath (fh->pc.get_nt_native_path (), NULL,
- ),
- wcscmp (fname.Buffer, L"conftest.txt") == 0)
- && NT_SUCCESS (NtQueryInformationFile (fh->get_handle (), ,
-, sizeof fsi,
-FileStandardInformation))
- && fsi.EndOfFile.QuadPart == 1LL)
- flags |= MAP_ANONYMOUS;
- }
+ if (len == pagesize
+ && prot == (PROT_READ | PROT_WRITE)
+ && flags == MAP_SHARED
+ && off == 0
+ && (RtlSplitUnicodePath (fh->pc.get_nt_native_path (), NULL,
+ ),
+ wcscmp (fname.Buffer, L"conftest.txt") == 0)
+ && NT_SUCCESS (NtQueryInformationFile (fh->get_handle (), ,
+, sizeof fsi,
+FileStandardInformation))
+ &&
Re: [PATCH] Cygwin: mmap: fix mapping beyond EOF on 64 bit
On 7/20/2020 11:41 AM, Corinna Vinschen wrote: On Jul 20 10:58, Ken Brown via Cygwin-patches wrote: On 7/20/2020 10:49 AM, Ken Brown via Cygwin-patches wrote: On 7/20/2020 10:43 AM, Ken Brown via Cygwin-patches wrote: On 7/20/2020 10:23 AM, Corinna Vinschen wrote: On Jul 20 09:34, Ken Brown via Cygwin-patches wrote: Commit 605bdcd410384dda6db66b9b8cd19e863702e1bb enabled mapping beyond EOF in 64 bit environments. But the variable 'orig_len' did not get rounded up to a multiple of 64K. This rounding was done on 32 bit only. Fix this by rounding up orig_len on 64 bit, in the same place where 'len' is rounded up. One consequence of this bug is that orig_len could be slightly smaller than len. Since these are both unsigned values, the statement 'orig_len -= len' would then cause orig_len to be huge, and mmap would fail with errno EFBIG. I observed this failure while debugging the problem reported in https://sourceware.org/pipermail/cygwin/2020-July/245557.html. The failure can be seen by running the test case in that report under gdb or strace. --- winsup/cygwin/mmap.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/winsup/cygwin/mmap.cc b/winsup/cygwin/mmap.cc index feb9e5d0e..a08d00f83 100644 --- a/winsup/cygwin/mmap.cc +++ b/winsup/cygwin/mmap.cc @@ -1144,6 +1144,7 @@ go_ahead: ends in, but there's nothing at all we can do about that. */ #ifdef __x86_64__ len = roundup2 (len, wincap.allocation_granularity ()); + orig_len = roundup2 (orig_len, wincap.allocation_granularity ()); Wouldn't it be simpler to just check for - if (orig_len - len) + if (orig_len > len) in the code following this #if/#else/#endif snippet? I don't think so, because we also want to use the rounded-up value of orig_len further down when we set sigbus_page_len Actually we first modify orig_len in 'orig_len -= len;' and then use it to set sigbus_page_len. In any case, I think it needs to be rounded up before being used. Oh, right. Now I see what you mean. At this point orig_len is still the actual exact size of the file. We only can create the SIGBUS pages starting the next allocation granularity, so, yeah, it makes sense to align orig_size to allocation granularity here. If you agree, maybe I should modify the commit message to make this point clear. Might make sense, yeah. While looking into this, I found another bug. The valid_page_len is wrong on 32 bit systems as well. That was supposed to be the remainder of the allocation granularity sized block the file's EOF is part of, but valid_page_len = orig_len % pagesize; is the size of the file's map within that 64K block, not the size of the remainder. That should have been valid_page_len = pagesize - orig_len % pagesize; so this didn't work correctly either. Ultimately, I wonder if we really should keep all the 32 bit OS stuff in. The number of real 32 bit systems (not WOW64) is dwindling fast. Keeping all the AT_ROUND_TO_PAGE stuff in just for what? 2%? of the systems is really not worth it, I guess. I agree. It complicates the code with very little benefit. Ken
Re: [PATCH] Cygwin: mmap: fix mapping beyond EOF on 64 bit
On Jul 20 10:58, Ken Brown via Cygwin-patches wrote: > On 7/20/2020 10:49 AM, Ken Brown via Cygwin-patches wrote: > > On 7/20/2020 10:43 AM, Ken Brown via Cygwin-patches wrote: > > > On 7/20/2020 10:23 AM, Corinna Vinschen wrote: > > > > On Jul 20 09:34, Ken Brown via Cygwin-patches wrote: > > > > > Commit 605bdcd410384dda6db66b9b8cd19e863702e1bb enabled mapping beyond > > > > > EOF in 64 bit environments. But the variable 'orig_len' did not get > > > > > rounded up to a multiple of 64K. This rounding was done on 32 bit > > > > > only. Fix this by rounding up orig_len on 64 bit, in the same place > > > > > where 'len' is rounded up. > > > > > > > > > > One consequence of this bug is that orig_len could be slightly smaller > > > > > than len. Since these are both unsigned values, the statement > > > > > 'orig_len -= len' would then cause orig_len to be huge, and mmap would > > > > > fail with errno EFBIG. > > > > > > > > > > I observed this failure while debugging the problem reported in > > > > > > > > > > https://sourceware.org/pipermail/cygwin/2020-July/245557.html. > > > > > > > > > > The failure can be seen by running the test case in that report under > > > > > gdb or strace. > > > > > --- > > > > > winsup/cygwin/mmap.cc | 1 + > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > diff --git a/winsup/cygwin/mmap.cc b/winsup/cygwin/mmap.cc > > > > > index feb9e5d0e..a08d00f83 100644 > > > > > --- a/winsup/cygwin/mmap.cc > > > > > +++ b/winsup/cygwin/mmap.cc > > > > > @@ -1144,6 +1144,7 @@ go_ahead: > > > > > ends in, but there's nothing at all we can do about that. */ > > > > > #ifdef __x86_64__ > > > > > len = roundup2 (len, wincap.allocation_granularity ()); > > > > > + orig_len = roundup2 (orig_len, wincap.allocation_granularity > > > > > ()); > > > > > > > > Wouldn't it be simpler to just check for > > > > > > > > - if (orig_len - len) > > > > + if (orig_len > len) > > > > > > > > in the code following this #if/#else/#endif snippet? > > > > > > I don't think so, because we also want to use the rounded-up value > > > of orig_len further down when we set sigbus_page_len > > > > Actually we first modify orig_len in 'orig_len -= len;' and then use it > > to set sigbus_page_len. In any case, I think it needs to be rounded up > > before being used. Oh, right. Now I see what you mean. At this point orig_len is still the actual exact size of the file. We only can create the SIGBUS pages starting the next allocation granularity, so, yeah, it makes sense to align orig_size to allocation granularity here. > If you agree, maybe I should modify the commit message to make this point > clear. Might make sense, yeah. While looking into this, I found another bug. The valid_page_len is wrong on 32 bit systems as well. That was supposed to be the remainder of the allocation granularity sized block the file's EOF is part of, but valid_page_len = orig_len % pagesize; is the size of the file's map within that 64K block, not the size of the remainder. That should have been valid_page_len = pagesize - orig_len % pagesize; so this didn't work correctly either. Ultimately, I wonder if we really should keep all the 32 bit OS stuff in. The number of real 32 bit systems (not WOW64) is dwindling fast. Keeping all the AT_ROUND_TO_PAGE stuff in just for what? 2%? of the systems is really not worth it, I guess. Feel free to apply this patch. Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer
Re: [PATCH] Cygwin: mmap: fix mapping beyond EOF on 64 bit
On 7/20/2020 10:49 AM, Ken Brown via Cygwin-patches wrote: On 7/20/2020 10:43 AM, Ken Brown via Cygwin-patches wrote: On 7/20/2020 10:23 AM, Corinna Vinschen wrote: On Jul 20 09:34, Ken Brown via Cygwin-patches wrote: Commit 605bdcd410384dda6db66b9b8cd19e863702e1bb enabled mapping beyond EOF in 64 bit environments. But the variable 'orig_len' did not get rounded up to a multiple of 64K. This rounding was done on 32 bit only. Fix this by rounding up orig_len on 64 bit, in the same place where 'len' is rounded up. One consequence of this bug is that orig_len could be slightly smaller than len. Since these are both unsigned values, the statement 'orig_len -= len' would then cause orig_len to be huge, and mmap would fail with errno EFBIG. I observed this failure while debugging the problem reported in https://sourceware.org/pipermail/cygwin/2020-July/245557.html. The failure can be seen by running the test case in that report under gdb or strace. --- winsup/cygwin/mmap.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/winsup/cygwin/mmap.cc b/winsup/cygwin/mmap.cc index feb9e5d0e..a08d00f83 100644 --- a/winsup/cygwin/mmap.cc +++ b/winsup/cygwin/mmap.cc @@ -1144,6 +1144,7 @@ go_ahead: ends in, but there's nothing at all we can do about that. */ #ifdef __x86_64__ len = roundup2 (len, wincap.allocation_granularity ()); + orig_len = roundup2 (orig_len, wincap.allocation_granularity ()); Wouldn't it be simpler to just check for - if (orig_len - len) + if (orig_len > len) in the code following this #if/#else/#endif snippet? I don't think so, because we also want to use the rounded-up value of orig_len further down when we set sigbus_page_len Actually we first modify orig_len in 'orig_len -= len;' and then use it to set sigbus_page_len. In any case, I think it needs to be rounded up before being used. If you agree, maybe I should modify the commit message to make this point clear. Ken
Re: [PATCH] Cygwin: mmap: fix mapping beyond EOF on 64 bit
On 7/20/2020 10:43 AM, Ken Brown via Cygwin-patches wrote: On 7/20/2020 10:23 AM, Corinna Vinschen wrote: On Jul 20 09:34, Ken Brown via Cygwin-patches wrote: Commit 605bdcd410384dda6db66b9b8cd19e863702e1bb enabled mapping beyond EOF in 64 bit environments. But the variable 'orig_len' did not get rounded up to a multiple of 64K. This rounding was done on 32 bit only. Fix this by rounding up orig_len on 64 bit, in the same place where 'len' is rounded up. One consequence of this bug is that orig_len could be slightly smaller than len. Since these are both unsigned values, the statement 'orig_len -= len' would then cause orig_len to be huge, and mmap would fail with errno EFBIG. I observed this failure while debugging the problem reported in https://sourceware.org/pipermail/cygwin/2020-July/245557.html. The failure can be seen by running the test case in that report under gdb or strace. --- winsup/cygwin/mmap.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/winsup/cygwin/mmap.cc b/winsup/cygwin/mmap.cc index feb9e5d0e..a08d00f83 100644 --- a/winsup/cygwin/mmap.cc +++ b/winsup/cygwin/mmap.cc @@ -1144,6 +1144,7 @@ go_ahead: ends in, but there's nothing at all we can do about that. */ #ifdef __x86_64__ len = roundup2 (len, wincap.allocation_granularity ()); + orig_len = roundup2 (orig_len, wincap.allocation_granularity ()); Wouldn't it be simpler to just check for - if (orig_len - len) + if (orig_len > len) in the code following this #if/#else/#endif snippet? I don't think so, because we also want to use the rounded-up value of orig_len further down when we set sigbus_page_len Actually we first modify orig_len in 'orig_len -= len;' and then use it to set sigbus_page_len. In any case, I think it needs to be rounded up before being used. Ken
Re: [PATCH] Cygwin: mmap: fix mapping beyond EOF on 64 bit
On 7/20/2020 10:23 AM, Corinna Vinschen wrote: On Jul 20 09:34, Ken Brown via Cygwin-patches wrote: Commit 605bdcd410384dda6db66b9b8cd19e863702e1bb enabled mapping beyond EOF in 64 bit environments. But the variable 'orig_len' did not get rounded up to a multiple of 64K. This rounding was done on 32 bit only. Fix this by rounding up orig_len on 64 bit, in the same place where 'len' is rounded up. One consequence of this bug is that orig_len could be slightly smaller than len. Since these are both unsigned values, the statement 'orig_len -= len' would then cause orig_len to be huge, and mmap would fail with errno EFBIG. I observed this failure while debugging the problem reported in https://sourceware.org/pipermail/cygwin/2020-July/245557.html. The failure can be seen by running the test case in that report under gdb or strace. --- winsup/cygwin/mmap.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/winsup/cygwin/mmap.cc b/winsup/cygwin/mmap.cc index feb9e5d0e..a08d00f83 100644 --- a/winsup/cygwin/mmap.cc +++ b/winsup/cygwin/mmap.cc @@ -1144,6 +1144,7 @@ go_ahead: ends in, but there's nothing at all we can do about that. */ #ifdef __x86_64__ len = roundup2 (len, wincap.allocation_granularity ()); + orig_len = roundup2 (orig_len, wincap.allocation_granularity ()); Wouldn't it be simpler to just check for - if (orig_len - len) + if (orig_len > len) in the code following this #if/#else/#endif snippet? I don't think so, because we also want to use the rounded-up value of orig_len further down when we set sigbus_page_len. Ken
Re: [PATCH] Cygwin: mmap: fix mapping beyond EOF on 64 bit
On Jul 20 09:34, Ken Brown via Cygwin-patches wrote: > Commit 605bdcd410384dda6db66b9b8cd19e863702e1bb enabled mapping beyond > EOF in 64 bit environments. But the variable 'orig_len' did not get > rounded up to a multiple of 64K. This rounding was done on 32 bit > only. Fix this by rounding up orig_len on 64 bit, in the same place > where 'len' is rounded up. > > One consequence of this bug is that orig_len could be slightly smaller > than len. Since these are both unsigned values, the statement > 'orig_len -= len' would then cause orig_len to be huge, and mmap would > fail with errno EFBIG. > > I observed this failure while debugging the problem reported in > > https://sourceware.org/pipermail/cygwin/2020-July/245557.html. > > The failure can be seen by running the test case in that report under > gdb or strace. > --- > winsup/cygwin/mmap.cc | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/winsup/cygwin/mmap.cc b/winsup/cygwin/mmap.cc > index feb9e5d0e..a08d00f83 100644 > --- a/winsup/cygwin/mmap.cc > +++ b/winsup/cygwin/mmap.cc > @@ -1144,6 +1144,7 @@ go_ahead: >ends in, but there's nothing at all we can do about that. */ > #ifdef __x86_64__ >len = roundup2 (len, wincap.allocation_granularity ()); > + orig_len = roundup2 (orig_len, wincap.allocation_granularity ()); Wouldn't it be simpler to just check for - if (orig_len - len) + if (orig_len > len) in the code following this #if/#else/#endif snippet? Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer
[PATCH] Cygwin: mmap: fix mapping beyond EOF on 64 bit
Commit 605bdcd410384dda6db66b9b8cd19e863702e1bb enabled mapping beyond EOF in 64 bit environments. But the variable 'orig_len' did not get rounded up to a multiple of 64K. This rounding was done on 32 bit only. Fix this by rounding up orig_len on 64 bit, in the same place where 'len' is rounded up. One consequence of this bug is that orig_len could be slightly smaller than len. Since these are both unsigned values, the statement 'orig_len -= len' would then cause orig_len to be huge, and mmap would fail with errno EFBIG. I observed this failure while debugging the problem reported in https://sourceware.org/pipermail/cygwin/2020-July/245557.html. The failure can be seen by running the test case in that report under gdb or strace. --- winsup/cygwin/mmap.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/winsup/cygwin/mmap.cc b/winsup/cygwin/mmap.cc index feb9e5d0e..a08d00f83 100644 --- a/winsup/cygwin/mmap.cc +++ b/winsup/cygwin/mmap.cc @@ -1144,6 +1144,7 @@ go_ahead: ends in, but there's nothing at all we can do about that. */ #ifdef __x86_64__ len = roundup2 (len, wincap.allocation_granularity ()); + orig_len = roundup2 (orig_len, wincap.allocation_granularity ()); #else len = roundup2 (len, wincap.is_wow64 () ? wincap.allocation_granularity () : wincap.page_size ()); -- 2.27.0
Re: [PATCH 5/5] Cygwin: Use MEMORY_WORKING_SET_EX_INFORMATION in dumper
On Jul 18 16:00, Jon Turney wrote: > Use the (undocumented) MEMORY_WORKING_SET_EX_INFORMATION in dumper to > determine if a MEM_IMAGE region is unsharable, and hence has been > modified. Great! Corinna -- Corinna Vinschen Cygwin Maintainer
Re: [PATCH] Cygwin: pty: Fix a bug on redirecting something to /dev/pty*.
On Jul 18 13:48, Takashi Yano via Cygwin-patches wrote: > - After commit 0365031ce1347600d854a23f30f1355745a1765c, key input > becomes not working by following steps. >1) Start cmd.exe in mintty. >2) Open another mintty. >3) Execute "echo AAA > /dev/pty*" (pty* is the pty opened in 1.) > This patch fixes the issue. > --- > winsup/cygwin/fhandler_tty.cc | 5 - > 1 file changed, 5 deletions(-) > > diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc > index a61167116..6a004f3a5 100644 > --- a/winsup/cygwin/fhandler_tty.cc > +++ b/winsup/cygwin/fhandler_tty.cc > @@ -969,11 +969,6 @@ fhandler_pty_slave::open (int flags, mode_t) >init_console_handler (true); > } > > - isHybrid = false; > - get_ttyp ()->pcon_pid = 0; > - get_ttyp ()->switch_to_pcon_in = false; > - get_ttyp ()->switch_to_pcon_out = false; > - >set_open_status (); >return 1; > > -- > 2.27.0 Pushed. Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer
