allow read into untouched noreserve mappings
Brian Ford
Brian.Ford@flightsafety.com
Mon Jul 17 22:54:00 GMT 2006
On Mon, 17 Jul 2006, Corinna Vinschen wrote:
> Sorry but... ERROR_INVALID_PARAMETER? When I debugged this I got
> ERROR_NOACCESS.
You are correct. The case fall through combined with the earlier EISDIR
error confused me. That, and the fact that the patch actually worked ;-).
> I would rather see the mmap_commit_noreserve_pages functionality folded
> into the existing mmap_is_attached_or_noreserve_page function (add
> parameter, see if len == 0 or > 0, yada yada yada) so that there's
> only one function which does the work, regardless from where it's called.
I was trying to be less intrusive and that looked messy. Oh, well...
Untested this time because I have to run to an appointment.
2006-07-17 Brian Ford <Brian.Ford@FlightSafety.com>
* winsup.h (mmap_region_status): New enum.
(mmap_is_attached_or_noreserve_page): Adjust prototype and rename
as below.
* mmap.cc (mmap_is_attached_or_noreserve_page): Rename
mmap_is_attached_or_noreserve. Add region length parameter.
Return enum above.
* exceptions.cc (_cygtls::handle_exceptions): Accomodate above.
* fhandler.cc (fhandler_base::raw_read): Call above for NOACCESS
errors and retry on success to allow reads into untouched
MAP_NORESERVE buffers.
--
Brian Ford
Lead Realtime Software Engineer
VITAL - Visual Simulation Systems
FlightSafety International
the best safety device in any aircraft is a well-trained crew...
.
-------------- next part --------------
Index: exceptions.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/exceptions.cc,v
retrieving revision 1.296
diff -u -p -u -p -r1.296 exceptions.cc
--- exceptions.cc 13 Jul 2006 08:33:34 -0000 1.296
+++ exceptions.cc 17 Jul 2006 22:31:11 -0000
@@ -526,12 +526,12 @@ _cygtls::handle_exceptions (EXCEPTION_RE
break;
case STATUS_ACCESS_VIOLATION:
- switch (mmap_is_attached_or_noreserve_page (e->ExceptionInformation[1]))
+ switch (mmap_is_attached_or_noreserve ((void *)e->ExceptionInformation[1],
+ 1))
{
- case 2: /* MAP_NORESERVE page, now commited. */
+ case MMAP_NORESERVE_COMMITED:
return 0;
- case 1: /* MAP_NORESERVE page, commit failed, or
- access to mmap page beyond EOF. */
+ case MMAP_RAISE_SIGBUS: /* commit failed for noreserve or beyond EOF. */
si.si_signo = SIGBUS;
si.si_code = BUS_OBJERR;
break;
Index: fhandler.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/fhandler.cc,v
retrieving revision 1.256
diff -u -p -u -p -r1.256 fhandler.cc
--- fhandler.cc 13 Jul 2006 20:56:24 -0000 1.256
+++ fhandler.cc 17 Jul 2006 22:31:11 -0000
@@ -223,8 +223,10 @@ fhandler_base::raw_read (void *ptr, size
HANDLE h = NULL; /* grumble */
int prio = 0; /* ditto */
+ int try_noreserve = 1;
DWORD len = ulen;
+retry:
ulen = (size_t) -1;
if (read_state)
{
@@ -259,6 +261,19 @@ fhandler_base::raw_read (void *ptr, size
bytes_read = 0;
break;
}
+ if (try_noreserve)
+ {
+ try_noreserve = 0;
+ switch (mmap_is_attached_or_noreserve (ptr, len))
+ {
+ case MMAP_NORESERVE_COMMITED:
+ goto retry;
+ case MMAP_RAISE_SIGBUS:
+ raise(SIGBUS);
+ case MMAP_NONE:
+ break;
+ }
+ }
case ERROR_INVALID_FUNCTION:
case ERROR_INVALID_PARAMETER:
case ERROR_INVALID_HANDLE:
Index: mmap.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/mmap.cc,v
retrieving revision 1.130
diff -u -p -u -p -r1.130 mmap.cc
--- mmap.cc 13 Jul 2006 10:29:21 -0000 1.130
+++ mmap.cc 17 Jul 2006 22:31:12 -0000
@@ -900,44 +900,59 @@ map::del_list (unsigned i)
}
/* This function is called from exception_handler when a segmentation
- violation has happened. We have two cases to check here.
+ violation has occurred. It should also be called from all Cygwin
+ functions that want to support passing noreserve mmap page addresses
+ to Windows system calls. In that case, it should be called only after
+ a system call indicates that the application buffer passed had an
+ invalid virtual address to avoid any performance impact in non-noreserve
+ cases.
- First, is it an address within "attached" mmap pages (indicated by
- the __PROT_ATTACH protection, see there)? In this case the function
- returns 1 and the exception_handler raises SIGBUS, as demanded by the
- memory protection extension described in SUSv3 (see the mmap man
- page).
-
- Second, check if the address is within "noreserve" mmap pages
- (indicated by MAP_NORESERVE flag). If so, the function calls
- VirtualAlloc to commit the page and returns 2. The exception handler
- then just returns with 0 and the affected application retries the
- failing memory access. If VirtualAlloc fails, the function returns
- 1, so that the exception handler raises a SIGBUS, as described in the
- MAP_NORESERVE man pages for Linux and Solaris.
-
- In any other case 0 is returned and a normal SIGSEGV is raised. */
-int
-mmap_is_attached_or_noreserve_page (ULONG_PTR addr)
+ Check if the address range is all within noreserve mmap regions. If so,
+ call VirtualAlloc to commit the pages and return MMAP_NORESERVE_COMMITED
+ on success. If the page has __PROT_ATTACH (SUSv3 memory protection
+ extension), or if VirutalAlloc fails, return MMAP_RAISE_SIGBUS.
+ Otherwise, return MMAP_NONE if the address range is not covered by an
+ attached or noreserve map.
+
+ On MAP_NORESERVE_COMMITED, the exeception handler should return 0 to
+ allow the application to retry the memory access, or the calling Cygwin
+ function should retry the Windows system call. */
+mmap_region_status
+mmap_is_attached_or_noreserve (void *addr, size_t len)
{
- list *map_list;
- long record_idx;
- caddr_t u_addr;
- DWORD u_len;
- DWORD pagesize = getsystempagesize ();
+ list *map_list = mmapped_areas.get_list_by_fd (-1);
- addr = rounddown (addr, pagesize);
- if (!(map_list = mmapped_areas.get_list_by_fd (-1)))
- return 0;
- if ((record_idx = map_list->search_record ((caddr_t)addr, pagesize,
- u_addr, u_len, -1)) < 0)
- return 0;
- if (map_list->get_record (record_idx)->attached ())
- return 1;
- if (!map_list->get_record (record_idx)->noreserve ())
- return 0;
- DWORD new_prot = map_list->get_record (record_idx)->gen_protect ();
- return VirtualAlloc ((void *)addr, pagesize, MEM_COMMIT, new_prot) ? 2 : 1;
+ if (map_list == NULL)
+ return MMAP_NONE;
+
+ while (len > 0)
+ {
+ caddr_t u_addr;
+ DWORD u_len;
+ long record_idx = map_list->search_record ((caddr_t)addr, 1,
+ u_addr, u_len, -1);
+ if (record_idx < 0)
+ return MMAP_NONE;
+
+ mmap_record *rec = map_list->get_record (record_idx);
+ if (rec->attached ())
+ return MMAP_RAISE_SIGBUS;
+ if (!rec->noreserve ())
+ return MMAP_NONE;
+
+ size_t commit_len = u_len - ((char *)addr - u_addr);
+ if (commit_len > len)
+ commit_len = len;
+
+ if (VirtualAlloc (addr, commit_len, MEM_COMMIT, rec->gen_protect ())
+ == NULL)
+ return MMAP_RAISE_SIGBUS;
+
+ addr = (char *)addr + commit_len;
+ len -= commit_len;
+ }
+
+ return MMAP_NORESERVE_COMMITED;
}
static caddr_t
Index: winsup.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/winsup.h,v
retrieving revision 1.189
diff -u -p -u -p -r1.189 winsup.h
--- winsup.h 13 Jul 2006 08:33:34 -0000 1.189
+++ winsup.h 17 Jul 2006 22:31:12 -0000
@@ -299,7 +299,13 @@ size_t getsystempagesize ();
/* mmap functions. */
void mmap_init ();
-int mmap_is_attached_or_noreserve_page (ULONG_PTR addr);
+enum mmap_region_status
+ {
+ MMAP_NONE,
+ MMAP_RAISE_SIGBUS,
+ MMAP_NORESERVE_COMMITED
+ };
+mmap_region_status mmap_is_attached_or_noreserve (void *addr, size_t len);
int winprio_to_nice (DWORD) __attribute__ ((regparm (1)));
DWORD nice_to_winprio (int &) __attribute__ ((regparm (1)));
More information about the Cygwin-patches
mailing list