Hi Corinna,
On 7/20/2020 2:55 PM, Corinna Vinschen wrote:
From: Corinna Vinschen <cori...@vinschen.de>
It's working on 32 bit OSes only anyway. It even fails on WOW64.
Signed-off-by: Corinna Vinschen <cori...@vinschen.de>
---
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,
- &fname),
- wcscmp (fname.Buffer, L"conftest.txt") == 0)
- && NT_SUCCESS (NtQueryInformationFile (fh->get_handle (), &io,
- &fsi, 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,
+ &fname),
+ wcscmp (fname.Buffer, L"conftest.txt") == 0)
+ && NT_SUCCESS (NtQueryInformationFile (fh->get_handle (), &io,
+ &fsi, sizeof fsi,
+ FileStandardInformation))
+ && fsi.EndOfFile.QuadPart == 1LL)
+ flags |= MAP_ANONYMOUS;
}
if (anonymous (flags) || fd == -1)
@@ -1089,6 +1073,7 @@ go_ahead:
}
#ifdef __x86_64__
+ orig_len = roundup2 (orig_len, pagesize);
if (!wincap.has_extended_mem_api ())
addr = mmap_alloc.alloc (addr, orig_len ?: len, fixed (flags));
#else
@@ -1099,7 +1084,6 @@ go_ahead:
deallocated and the address we got is used as base address for the
subsequent real mappings. This ensures that we have enough space
for the whole thing. */
- orig_len = roundup2 (orig_len, pagesize);
PVOID newaddr = VirtualAlloc (addr, orig_len, MEM_TOP_DOWN |
MEM_RESERVE,
PAGE_READWRITE);
if (!newaddr)
@@ -1132,51 +1116,18 @@ go_ahead:
if (orig_len)
{
/* If the requested length is bigger than the file size, the
- remainder is created as anonymous mapping. Actually two
- mappings are created, first the remainder from the file end to
- the next 64K boundary as accessible pages with the same
- protection as the file's pages, then as much pages as necessary
- to accomodate the requested length, but as reserved pages which
- raise a SIGBUS when trying to access them. AT_ROUND_TO_PAGE
- and page protection on shared pages is only supported by the
- 32 bit environment, so don't even try on 64 bit or even WOW64.
- This results in an allocation gap in the first 64K block the file
- 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 ());
-#endif
+ remainder is created as anonymous mapping, as reserved pages which
+ raise a SIGBUS when trying to access them. This results in an
+ allocation gap in the first 64K block the file ends in, but there's
+ nothing at all we can do about that. */
+ len = roundup2 (len, pagesize);
if (orig_len - len)
{
- orig_len -= len;
- size_t valid_page_len = 0;
-#ifndef __x86_64__
- if (!wincap.is_wow64 ())
- valid_page_len = orig_len % pagesize;
-#endif
- size_t sigbus_page_len = orig_len - valid_page_len;
+ size_t sigbus_page_len = orig_len - len;
- caddr_t at_base = base + len;
- if (valid_page_len)
- {
- prot |= __PROT_FILLER;
- flags &= MAP_SHARED | MAP_PRIVATE;
- flags |= MAP_ANONYMOUS | MAP_FIXED;
- at_base = mmap_worker (NULL, &fh_anonymous, at_base,
- valid_page_len, prot, flags, -1, 0, NULL);
- if (!at_base)
- {
- fh->munmap (fh->get_handle (), base, len);
- set_errno (ENOMEM);
- goto out_with_unlock;
- }
- at_base += valid_page_len;
- }
if (sigbus_page_len)
{
+ caddr_t at_base = base + len;
prot = PROT_READ | PROT_WRITE | __PROT_ATTACH;
flags = MAP_ANONYMOUS | MAP_NORESERVE | MAP_FIXED;
at_base = mmap_worker (NULL, &fh_anonymous, at_base,
I think you still left in some 32 bit code that should be removed, and also
orig_len now doesn't get rounded up on 32 bit. Here's an additional diff that I
think is needed beyond your patch:
diff --git a/winsup/cygwin/mmap.cc b/winsup/cygwin/mmap.cc
index 8ac96606c..fa9266825 100644
--- a/winsup/cygwin/mmap.cc
+++ b/winsup/cygwin/mmap.cc
@@ -1009,20 +1009,8 @@ mmap64 (void *addr, size_t len, int prot, int flags, int
fd, off_t off)
goto go_ahead;
}
fsiz -= off;
- /* We're creating the pages beyond EOF as reserved, anonymous pages.
- Note that 64 bit environments don't support the AT_ROUND_TO_PAGE
- flag, which is required to get this right for the remainder of
- the first 64K block the file ends in. We perform the workaround
- nevertheless to support expectations that the range mapped beyond
- EOF can be safely munmap'ed instead of being taken by another,
- totally unrelated mapping. */
- if ((off_t) len > fsiz && !autogrow (flags))
- orig_len = len;
-#ifdef __i386__
- else if (!wincap.is_wow64 () && roundup2 (len, wincap.page_size ())
- < roundup2 (len, pagesize))
- orig_len = len;
-#endif
+ /* We're creating the pages beyond EOF as reserved, anonymous
+ pages if MAP_AUTOGROW is not set. */
if ((off_t) len > fsiz)
{
if (autogrow (flags))
@@ -1037,9 +1025,12 @@ mmap64 (void *addr, size_t len, int prot, int flags, int
fd, off_t off)
}
}
else
- /* Otherwise, don't map beyond EOF, since Windows would change
- the file to the new length, in contrast to POSIX. */
- len = fsiz;
+ {
+ /* Otherwise, don't map beyond EOF, since Windows would change
+ the file to the new length, in contrast to POSIX. */
+ orig_len = len;
+ len = fsiz;
+ }
}
/* If the requested offset + len is <= file size, drop MAP_AUTOGROW.
@@ -1072,8 +1063,8 @@ go_ahead:
}
}
-#ifdef __x86_64__
orig_len = roundup2 (orig_len, pagesize);
+#ifdef __x86_64__
if (!wincap.has_extended_mem_api ())
addr = mmap_alloc.alloc (addr, orig_len ?: len, fixed (flags));
#else
I'm attaching an amended commit.
I could easily have missed something, and I don't have a 32 bit OS to test on,
so just ignore my changes if I'm wrong.
But I've retested the php test case, and it's still OK with this patch.
Ken
>From 6c26194304efa42394fdb509a94d618931ba8279 Mon Sep 17 00:00:00 2001
From: Corinna Vinschen <cori...@vinschen.de>
Date: Mon, 20 Jul 2020 20:55:43 +0200
Subject: [PATCH] Cygwin: mmap: Remove AT_ROUND_TO_PAGE workaround
It's working on 32 bit OSes only anyway. It even fails on WOW64.
Signed-off-by: Corinna Vinschen <cori...@vinschen.de>
---
winsup/cygwin/mmap.cc | 142 +++++++++++++-----------------------------
1 file changed, 42 insertions(+), 100 deletions(-)
diff --git a/winsup/cygwin/mmap.cc b/winsup/cygwin/mmap.cc
index 1fccc6c58..fa9266825 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,
- &fname),
- wcscmp (fname.Buffer, L"conftest.txt") == 0)
- && NT_SUCCESS (NtQueryInformationFile (fh->get_handle (), &io,
- &fsi, 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,
+ &fname),
+ wcscmp (fname.Buffer, L"conftest.txt") == 0)
+ && NT_SUCCESS (NtQueryInformationFile (fh->get_handle (), &io,
+ &fsi, sizeof fsi,
+ FileStandardInformation))
+ && fsi.EndOfFile.QuadPart == 1LL)
+ flags |= MAP_ANONYMOUS;
}
if (anonymous (flags) || fd == -1)
@@ -1025,20 +1009,8 @@ mmap64 (void *addr, size_t len, int prot, int flags, int
fd, off_t off)
goto go_ahead;
}
fsiz -= off;
- /* We're creating the pages beyond EOF as reserved, anonymous pages.
- Note that 64 bit environments don't support the AT_ROUND_TO_PAGE
- flag, which is required to get this right for the remainder of
- the first 64K block the file ends in. We perform the workaround
- nevertheless to support expectations that the range mapped beyond
- EOF can be safely munmap'ed instead of being taken by another,
- totally unrelated mapping. */
- if ((off_t) len > fsiz && !autogrow (flags))
- orig_len = len;
-#ifdef __i386__
- else if (!wincap.is_wow64 () && roundup2 (len, wincap.page_size ())
- < roundup2 (len, pagesize))
- orig_len = len;
-#endif
+ /* We're creating the pages beyond EOF as reserved, anonymous
+ pages if MAP_AUTOGROW is not set. */
if ((off_t) len > fsiz)
{
if (autogrow (flags))
@@ -1053,9 +1025,12 @@ mmap64 (void *addr, size_t len, int prot, int flags, int
fd, off_t off)
}
}
else
- /* Otherwise, don't map beyond EOF, since Windows would change
- the file to the new length, in contrast to POSIX. */
- len = fsiz;
+ {
+ /* Otherwise, don't map beyond EOF, since Windows would change
+ the file to the new length, in contrast to POSIX. */
+ orig_len = len;
+ len = fsiz;
+ }
}
/* If the requested offset + len is <= file size, drop MAP_AUTOGROW.
@@ -1088,6 +1063,7 @@ go_ahead:
}
}
+ orig_len = roundup2 (orig_len, pagesize);
#ifdef __x86_64__
if (!wincap.has_extended_mem_api ())
addr = mmap_alloc.alloc (addr, orig_len ?: len, fixed (flags));
@@ -1099,7 +1075,6 @@ go_ahead:
deallocated and the address we got is used as base address for the
subsequent real mappings. This ensures that we have enough space
for the whole thing. */
- orig_len = roundup2 (orig_len, pagesize);
PVOID newaddr = VirtualAlloc (addr, orig_len, MEM_TOP_DOWN | MEM_RESERVE,
PAGE_READWRITE);
if (!newaddr)
@@ -1132,51 +1107,18 @@ go_ahead:
if (orig_len)
{
/* If the requested length is bigger than the file size, the
- remainder is created as anonymous mapping. Actually two
- mappings are created, first the remainder from the file end to
- the next 64K boundary as accessible pages with the same
- protection as the file's pages, then as much pages as necessary
- to accomodate the requested length, but as reserved pages which
- raise a SIGBUS when trying to access them. AT_ROUND_TO_PAGE
- and page protection on shared pages is only supported by the
- 32 bit environment, so don't even try on 64 bit or even WOW64.
- This results in an allocation gap in the first 64K block the file
- 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 ());
-#endif
+ remainder is created as anonymous mapping, as reserved pages which
+ raise a SIGBUS when trying to access them. This results in an
+ allocation gap in the first 64K block the file ends in, but there's
+ nothing at all we can do about that. */
+ len = roundup2 (len, pagesize);
if (orig_len - len)
{
- orig_len -= len;
- size_t valid_page_len = 0;
-#ifndef __x86_64__
- if (!wincap.is_wow64 ())
- valid_page_len = orig_len % pagesize;
-#endif
- size_t sigbus_page_len = orig_len - valid_page_len;
+ size_t sigbus_page_len = orig_len - len;
- caddr_t at_base = base + len;
- if (valid_page_len)
- {
- prot |= __PROT_FILLER;
- flags &= MAP_SHARED | MAP_PRIVATE;
- flags |= MAP_ANONYMOUS | MAP_FIXED;
- at_base = mmap_worker (NULL, &fh_anonymous, at_base,
- valid_page_len, prot, flags, -1, 0, NULL);
- if (!at_base)
- {
- fh->munmap (fh->get_handle (), base, len);
- set_errno (ENOMEM);
- goto out_with_unlock;
- }
- at_base += valid_page_len;
- }
if (sigbus_page_len)
{
+ caddr_t at_base = base + len;
prot = PROT_READ | PROT_WRITE | __PROT_ATTACH;
flags = MAP_ANONYMOUS | MAP_NORESERVE | MAP_FIXED;
at_base = mmap_worker (NULL, &fh_anonymous, at_base,
--
2.27.0