Patches attached.
Ken Brown (5):
Cygwin: mmap: refactor mmap_record::match
Cygwin: mmap: remove is_mmapped_region()
Cygwin: mmap: remove __PROT_FILLER and the associated methods
Cygwin: mmap_list::try_map: simplify
Cygwin: remove winsup/cygwin/local_includes/mmap_helper.h
winsup/cygwin/local_includes/mmap_helper.h | 89 ---------------
winsup/cygwin/local_includes/winsup.h | 1 -
winsup/cygwin/mm/mmap.cc | 126 +++++++--------------
3 files changed, 44 insertions(+), 172 deletions(-)
delete mode 100644 winsup/cygwin/local_includes/mmap_helper.h
--
2.45.1
From 4387a73d5593ddad4cf7592865b5b257e6a9d6de Mon Sep 17 00:00:00 2001
From: Ken Brown <kbr...@cornell.edu>
Date: Fri, 27 Dec 2024 15:30:12 -0500
Subject: [PATCH 1/5] Cygwin: mmap: refactor mmap_record::match
Slightly simplify the code and add comments to explain what the
function does. Add a new reference parameter "contains" that is set
to true if the chunk of this mmap_record contains the given address
range.
Signed-off-by: Ken Brown <kbr...@cornell.edu>
---
winsup/cygwin/mm/mmap.cc | 37 ++++++++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/winsup/cygwin/mm/mmap.cc b/winsup/cygwin/mm/mmap.cc
index 0224779458ef..acab85d19cf0 100644
--- a/winsup/cygwin/mm/mmap.cc
+++ b/winsup/cygwin/mm/mmap.cc
@@ -338,7 +338,8 @@ class mmap_record
void init_page_map (mmap_record &r);
SIZE_T find_unused_pages (SIZE_T pages) const;
- bool match (caddr_t addr, SIZE_T len, caddr_t &m_addr, SIZE_T &m_len);
+ bool match (caddr_t addr, SIZE_T len, caddr_t &m_addr, SIZE_T &m_len,
+ bool &contains);
off_t map_pages (SIZE_T len, int new_prot);
bool map_pages (caddr_t addr, SIZE_T len, int new_prot);
bool unmap_pages (caddr_t addr, SIZE_T len);
@@ -418,20 +419,30 @@ mmap_record::find_unused_pages (SIZE_T pages) const
return (SIZE_T) -1;
}
+/* Return true if the interval I from addr to addr + len intersects
+ the interval J of this mmap_record. The endpoint of the latter is
+ first rounded up to a page boundary. If there is an intersection,
+ then it is the interval from m_addr to m_addr + m_len. The
+ variable 'contains' is set to true if J contains I.
+*/
bool
-mmap_record::match (caddr_t addr, SIZE_T len, caddr_t &m_addr, SIZE_T &m_len)
+mmap_record::match (caddr_t addr, SIZE_T len, caddr_t &m_addr, SIZE_T &m_len,
+ bool &contains)
{
- caddr_t low = (addr >= get_address ()) ? addr : get_address ();
+ contains = false;
+ caddr_t low = MAX (addr, get_address ());
caddr_t high = get_address ();
if (filler ())
high += get_len ();
else
high += (PAGE_CNT (get_len ()) * wincap.page_size ());
- high = (addr + len < high) ? addr + len : high;
+ high = MIN (addr + len, high);
if (low < high)
{
m_addr = low;
m_len = high - low;
+ /* I is contained in J iff their intersection equals I. */
+ contains = (addr == m_addr && len == m_len);
return true;
}
return false;
@@ -653,14 +664,14 @@ mmap_list::try_map (void *addr, size_t len, int new_prot,
int flags, off_t off)
request, try to reset the protection on the requested area. */
caddr_t u_addr;
SIZE_T u_len;
+ bool contains;
LIST_FOREACH (rec, &recs, mr_next)
- if (rec->match ((caddr_t) addr, len, u_addr, u_len))
+ if (rec->match ((caddr_t) addr, len, u_addr, u_len, contains))
break;
if (rec)
{
- if (u_addr > (caddr_t) addr || u_addr + u_len < (caddr_t) addr + len
- || !rec->compatible_flags (flags))
+ if (!contains || !rec->compatible_flags (flags))
{
/* Partial match only, or access mode doesn't match. */
/* FIXME: Handle partial mappings gracefully if adjacent
@@ -730,11 +741,12 @@ is_mmapped_region (caddr_t start_addr, caddr_t
end_address)
mmap_record *rec;
caddr_t u_addr;
SIZE_T u_len;
+ bool contains;
bool ret = false;
LIST_FOREACH (rec, &map_list->recs, mr_next)
{
- if (rec->match (start_addr, len, u_addr, u_len))
+ if (rec->match (start_addr, len, u_addr, u_len, contains))
{
ret = true;
break;
@@ -786,10 +798,11 @@ mmap_is_attached_or_noreserve (void *addr, size_t len)
if (map_list == NULL)
goto out;
+ bool contains;
LIST_FOREACH (rec, &map_list->recs, mr_next)
{
- if (!rec->match (start_addr, len, u_addr, u_len))
+ if (!rec->match (start_addr, len, u_addr, u_len, contains))
continue;
if (rec->attached ())
{
@@ -1177,10 +1190,11 @@ munmap (void *addr, size_t len)
mmap_record *rec, *next_rec;
caddr_t u_addr;
SIZE_T u_len;
+ bool contains;
LIST_FOREACH_SAFE (rec, &map_list->recs, mr_next, next_rec)
{
- if (!rec->match ((caddr_t) addr, len, u_addr, u_len))
+ if (!rec->match ((caddr_t) addr, len, u_addr, u_len, contains))
continue;
if (rec->unmap_pages (u_addr, u_len))
{
@@ -1299,10 +1313,11 @@ mprotect (void *addr, size_t len, int prot)
mmap_record *rec;
caddr_t u_addr;
SIZE_T u_len;
+ bool contains;
LIST_FOREACH (rec, &map_list->recs, mr_next)
{
- if (!rec->match ((caddr_t) addr, len, u_addr, u_len))
+ if (!rec->match ((caddr_t) addr, len, u_addr, u_len, contains))
continue;
in_mapped = true;
if (rec->attached ())
--
2.45.1
From d3c62d48f87044d7607d81559c58ae06df5839af Mon Sep 17 00:00:00 2001
From: Ken Brown <kbr...@cornell.edu>
Date: Fri, 20 Dec 2024 12:17:34 -0500
Subject: [PATCH 2/5] Cygwin: mmap: remove is_mmapped_region()
The last use was removed in commit 29a126322783 ("Simplify stack
allocation code in child after fork").
Signed-off-by: Ken Brown <kbr...@cornell.edu>
---
winsup/cygwin/local_includes/winsup.h | 1 -
winsup/cygwin/mm/mmap.cc | 34 ---------------------------
2 files changed, 35 deletions(-)
diff --git a/winsup/cygwin/local_includes/winsup.h
b/winsup/cygwin/local_includes/winsup.h
index 38313962d96c..6841d4a59fb4 100644
--- a/winsup/cygwin/local_includes/winsup.h
+++ b/winsup/cygwin/local_includes/winsup.h
@@ -239,7 +239,6 @@ enum mmap_region_status
MMAP_NORESERVE_COMMITED
};
mmap_region_status mmap_is_attached_or_noreserve (void *addr, size_t len);
-bool is_mmapped_region (caddr_t start_addr, caddr_t end_address);
extern inline bool flush_file_buffers (HANDLE h)
{
diff --git a/winsup/cygwin/mm/mmap.cc b/winsup/cygwin/mm/mmap.cc
index acab85d19cf0..13e64c23256c 100644
--- a/winsup/cygwin/mm/mmap.cc
+++ b/winsup/cygwin/mm/mmap.cc
@@ -722,40 +722,6 @@ mmap_areas::del_list (mmap_list *ml)
cfree (ml);
}
-/* This function allows an external function to test if a given memory
- region is part of an mmapped memory region. */
-bool
-is_mmapped_region (caddr_t start_addr, caddr_t end_address)
-{
- size_t len = end_address - start_addr;
-
- LIST_READ_LOCK ();
- mmap_list *map_list = mmapped_areas.get_list_by_fd (-1, NULL);
-
- if (!map_list)
- {
- LIST_READ_UNLOCK ();
- return false;
- }
-
- mmap_record *rec;
- caddr_t u_addr;
- SIZE_T u_len;
- bool contains;
- bool ret = false;
-
- LIST_FOREACH (rec, &map_list->recs, mr_next)
- {
- if (rec->match (start_addr, len, u_addr, u_len, contains))
- {
- ret = true;
- break;
- }
- }
- LIST_READ_UNLOCK ();
- return ret;
-}
-
/* This function is called from exception_handler when a segmentation
violation has occurred. It should also be called from all Cygwin
functions that want to support passing noreserve (anonymous) mmap
--
2.45.1
From 87e07edb7c53f425c86579d013d29efd3f905203 Mon Sep 17 00:00:00 2001
From: Ken Brown <kbr...@cornell.edu>
Date: Fri, 20 Dec 2024 13:11:22 -0500
Subject: [PATCH 3/5] Cygwin: mmap: remove __PROT_FILLER and the associated
methods
This is left over from 32 bit Cygwin and is no longer used.
Signed-off-by: Ken Brown <kbr...@cornell.edu>
---
winsup/cygwin/mm/mmap.cc | 31 +++++++------------------------
1 file changed, 7 insertions(+), 24 deletions(-)
diff --git a/winsup/cygwin/mm/mmap.cc b/winsup/cygwin/mm/mmap.cc
index 13e64c23256c..9e6415de9951 100644
--- a/winsup/cygwin/mm/mmap.cc
+++ b/winsup/cygwin/mm/mmap.cc
@@ -27,14 +27,8 @@ details. */
is to support mappings longer than the file, without the file growing
to mapping length (POSIX semantics). */
#define __PROT_ATTACH 0x8000000
-/* Filler pages are the pages from the last file backed page to the next
- 64K boundary. These pages are created as anonymous pages, but with
- the same page protection as the file's pages, since POSIX applications
- expect to be able to access this part the same way as the file pages. */
-#define __PROT_FILLER 0x4000000
-
-/* Stick with 4K pages for bookkeeping, otherwise we just get confused
- when trying to do file mappings with trailing filler pages correctly. */
+
+/* Stick with 4K pages for bookkeeping. */
#define PAGE_CNT(bytes) howmany((bytes), wincap.page_size())
#define PGBITS (sizeof (DWORD)*8)
@@ -91,12 +85,6 @@ attached (int prot)
return (prot & __PROT_ATTACH) == __PROT_ATTACH;
}
-static inline bool
-filler (int prot)
-{
- return (prot & __PROT_FILLER) == __PROT_FILLER;
-}
-
static inline DWORD
gen_create_protect (DWORD openflags, int flags)
{
@@ -125,7 +113,7 @@ gen_protect (int prot, int flags)
return PAGE_EXECUTE_READWRITE;
if (prot & PROT_WRITE)
- ret = (priv (flags) && (!anonymous (flags) || filler (prot)))
+ ret = (priv (flags) && !anonymous (flags))
? PAGE_WRITECOPY : PAGE_READWRITE;
else if (prot & PROT_READ)
ret = PAGE_READONLY;
@@ -330,7 +318,6 @@ class mmap_record
bool noreserve () const { return ::noreserve (flags); }
bool autogrow () const { return ::autogrow (flags); }
bool attached () const { return ::attached (prot); }
- bool filler () const { return ::filler (prot); }
off_t get_offset () const { return offset; }
SIZE_T get_len () const { return len; }
caddr_t get_address () const { return base_address; }
@@ -430,13 +417,9 @@ mmap_record::match (caddr_t addr, SIZE_T len, caddr_t
&m_addr, SIZE_T &m_len,
bool &contains)
{
contains = false;
+ SIZE_T rec_len = PAGE_CNT (get_len ()) * wincap.page_size ();
caddr_t low = MAX (addr, get_address ());
- caddr_t high = get_address ();
- if (filler ())
- high += get_len ();
- else
- high += (PAGE_CNT (get_len ()) * wincap.page_size ());
- high = MIN (addr + len, high);
+ caddr_t high = MIN (addr + len, get_address () + rec_len);
if (low < high)
{
m_addr = low;
@@ -1569,7 +1552,7 @@ fhandler_dev_zero::mmap (caddr_t *addr, size_t len, int
prot,
HANDLE h;
void *base;
- if (priv (flags) && !filler (prot))
+ if (priv (flags))
{
/* Private anonymous maps are now implemented using VirtualAlloc.
This has two advantages:
@@ -1678,7 +1661,7 @@ fhandler_dev_zero::fixup_mmap_after_fork (HANDLE h, int
prot, int flags,
{
/* Re-create the map */
void *base;
- if (priv (flags) && !filler (prot))
+ if (priv (flags))
{
DWORD alloc_type = MEM_RESERVE | (noreserve (flags) ? 0 : MEM_COMMIT);
/* Always allocate R/W so that ReadProcessMemory doesn't fail
--
2.45.1
From c01da9db1e76869621b63f8075505fa49acf0d56 Mon Sep 17 00:00:00 2001
From: Ken Brown <kbr...@cornell.edu>
Date: Sun, 29 Dec 2024 18:20:07 -0500
Subject: [PATCH 4/5] Cygwin: mmap_list::try_map: simplify
Save the result of mmap_record::find_unused pages, and then pass that
result to the appropriate version of mmap_record::map_pages. Add a
new parameter of type off_t to the latter to make this possible, and
change its return from off_t to bool. This saves map_pages from
having to call find_unused_pages again.
Signed-off-by: Ken Brown <kbr...@cornell.edu>
---
winsup/cygwin/mm/mmap.cc | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/winsup/cygwin/mm/mmap.cc b/winsup/cygwin/mm/mmap.cc
index 9e6415de9951..9ed9131771d7 100644
--- a/winsup/cygwin/mm/mmap.cc
+++ b/winsup/cygwin/mm/mmap.cc
@@ -327,7 +327,7 @@ class mmap_record
SIZE_T find_unused_pages (SIZE_T pages) const;
bool match (caddr_t addr, SIZE_T len, caddr_t &m_addr, SIZE_T &m_len,
bool &contains);
- off_t map_pages (SIZE_T len, int new_prot);
+ bool map_pages (SIZE_T len, int new_prot, off_t off);
bool map_pages (caddr_t addr, SIZE_T len, int new_prot);
bool unmap_pages (caddr_t addr, SIZE_T len);
int access (caddr_t address);
@@ -449,21 +449,18 @@ mmap_record::init_page_map (mmap_record &r)
MAP_SET (len);
}
-off_t
-mmap_record::map_pages (SIZE_T len, int new_prot)
+bool
+mmap_record::map_pages (SIZE_T len, int new_prot, off_t off)
{
- /* Used ONLY if this mapping matches into the chunk of another already
- performed mapping in a special case of MAP_ANON|MAP_PRIVATE.
-
- Otherwise it's job is now done by init_page_map(). */
+ /* Used only in a MAP_ANON|MAP_PRIVATE request for len bytes, with
+ MAP_FIXED not given. Moreover, we know when this function is
+ called that this record contains enough unused pages starting at
+ off to satisfy the request. */
DWORD old_prot;
debug_printf ("map_pages (fd=%d, len=%lu, new_prot=%y)", get_fd (), len,
new_prot);
len = PAGE_CNT (len);
- off_t off = find_unused_pages (len);
- if (off == (off_t) -1)
- return (off_t) 0;
if (!noreserve ()
&& !VirtualProtect (get_address () + off * wincap.page_size (),
len * wincap.page_size (),
@@ -471,12 +468,12 @@ mmap_record::map_pages (SIZE_T len, int new_prot)
&old_prot))
{
__seterrno ();
- return (off_t) -1;
+ return false;
}
while (len-- > 0)
MAP_SET (off + len);
- return off * wincap.page_size ();
+ return true;
}
bool
@@ -630,13 +627,14 @@ mmap_list::try_map (void *addr, size_t len, int new_prot,
int flags, off_t off)
mapping. */
SIZE_T plen = PAGE_CNT (len);
LIST_FOREACH (rec, &recs, mr_next)
- if (rec->find_unused_pages (plen) != (SIZE_T) -1)
+ if ((off = rec->find_unused_pages (plen)) != (off_t) -1
+ && rec->compatible_flags (flags))
break;
- if (rec && rec->compatible_flags (flags))
+ if (rec)
{
- if ((off = rec->map_pages (len, new_prot)) == (off_t) -1)
+ if (!rec->map_pages (len, new_prot, off))
return (caddr_t) MAP_FAILED;
- return (caddr_t) rec->get_address () + off;
+ return (caddr_t) rec->get_address () + off * wincap.page_size ();
}
}
else if (fixed (flags))
--
2.45.1
From 8c3a918c346a9ae97e219679d780c9b716588fc8 Mon Sep 17 00:00:00 2001
From: Ken Brown <kbr...@cornell.edu>
Date: Sun, 29 Dec 2024 18:47:54 -0500
Subject: [PATCH 5/5] Cygwin: remove winsup/cygwin/local_includes/mmap_helper.h
None of its macros and functions are used anymore.
Signed-off-by: Ken Brown <kbr...@cornell.edu>
---
winsup/cygwin/local_includes/mmap_helper.h | 89 ----------------------
1 file changed, 89 deletions(-)
delete mode 100644 winsup/cygwin/local_includes/mmap_helper.h
diff --git a/winsup/cygwin/local_includes/mmap_helper.h
b/winsup/cygwin/local_includes/mmap_helper.h
deleted file mode 100644
index 645c5e3aac84..000000000000
--- a/winsup/cygwin/local_includes/mmap_helper.h
+++ /dev/null
@@ -1,89 +0,0 @@
-/* mmap_helper.h
-
-This file is part of Cygwin.
-
-This software is a copyrighted work licensed under the terms of the
-Cygwin license. Please consult the file "CYGWIN_LICENSE" for
-details. */
-
-#ifndef _MMAP_HELPER_H
-#define _MMAP_HELPER_H
-#define _MMIOWRAP(__ptr, __len, __func) \
-({ \
- BOOL __res; \
- for (int __i = 0; __i < 2; __i++) \
- { \
- __res = __func; \
- if (__res || __i > 0) \
- break; \
- DWORD __errcode = GetLastError (); \
- if (__errcode != ERROR_NOACCESS) \
- break; \
- switch (mmap_is_attached_or_noreserve (__ptr, __len)) \
- { \
- case MMAP_NORESERVE_COMMITED: \
- continue; \
- case MMAP_RAISE_SIGBUS: \
- raise(SIGBUS); \
- default: \
- break; \
- } \
- break; \
- } \
- __res; \
-})
-
-#define _MMSOCKWRAP(__ptr, __count, __func) \
-({ \
- int __res; \
- for (int __i = 0; __i < 2; __i++) \
- { \
- __res = __func; \
- if (!__res || __i > 0) \
- break; \
- DWORD __errcode = WSAGetLastError (); \
- if (__errcode != WSAEFAULT) \
- break; \
- for (unsigned __j = 0; __j < __count; __j++) \
- switch (mmap_is_attached_or_noreserve (__ptr[__j].buf, __ptr[__j].len))
\
- { \
- case MMAP_NORESERVE_COMMITED: \
- goto keeptrying; \
- case MMAP_RAISE_SIGBUS: \
- raise(SIGBUS); \
- default: \
- break; \
- } \
- break; \
- keeptrying: \
- continue; \
- } \
- __res; \
-})
-
-extern inline BOOL
-mmReadFile (HANDLE hFile, LPVOID lpBuffer, DWORD nNumberOfBytesToRead,
- LPDWORD lpNumberOfBytesRead, LPOVERLAPPED lpOverlapped)
-{
- return _MMIOWRAP (lpBuffer, nNumberOfBytesToRead,
- (ReadFile (hFile, lpBuffer, nNumberOfBytesToRead,
- lpNumberOfBytesRead, lpOverlapped)));
-}
-
-#ifdef _WINSOCK_H
-extern inline int
-mmWSARecvFrom (SOCKET s, LPWSABUF lpBuffers, DWORD dwBufferCount,
- LPDWORD lpNumberOfBytesRecvd, LPDWORD lpFlags,
- struct sockaddr* lpFrom,
- LPINT lpFromlen, LPWSAOVERLAPPED lpOverlapped,
- LPWSAOVERLAPPED_COMPLETION_ROUTINE lpCompletionRoutine)
-{
- return _MMSOCKWRAP (lpBuffers, dwBufferCount,
- (mmWSARecvFrom(s, lpBuffers, dwBufferCount,
- lpNumberOfBytesRecvd, lpFlags, lpFrom,
- lpFromlen, lpOverlapped,
- lpCompletionRoutine)));
-}
-#endif /*_WINSOCK_H*/
-
-#endif /*_MMAP_HELPER_H*/
--
2.45.1
