Re: munmap slowness; IsBadReadPtr considered harmful

[Brian Ford <ford at vss dot fsi dot com>]

On Tue, 24 Feb 2004, Corinna Vinschen wrote:
> On Feb 23 18:04, Brian Ford wrote:
> > Ok, no response.  That means either:
> >
[snip]
> > 4.) No one likes me :( ...
> >
> 5.) No one has enough spare time currently.
>
Yeah, I hoped that was it.  But, I saw enough activity from you and Chris
on the cygwin list about, IMHO, less interesting issues, that I wondered.

I'll be more patient now, I promise :D.

> > FWIW, the following replacement for IsBadReadPtr appears to have the
> > same functionality without the associated page fault overhead.
> >
[snip]
> > So, any comments about the function above, where to put it, or what to
> > name it?
> >
> IMHO, the name's ok.  It could perhaps get its place in miscfuncs.cc.
>
Here is my current version based on others in miscfuncs.cc:

int __stdcall
__check_invalid_addr_range(const void *addr, size_t len)
{
  if (addr == NULL || len == 0)
    return EINVAL;

  MEMORY_BASIC_INFORMATION mbuf;
  const void *end;

  for (end = (char *)addr + len, addr = (void *)((DWORD)addr % getpagesize());
       addr < end;
       addr = (char *)addr + mbuf.RegionSize)
    {
      if (!VirtualQuery (addr, &mbuf, sizeof (mbuf))
          || mbuf.State != MEM_COMMIT
          || mbuf.AllocationProtect & (PAGE_NOACCESS|PAGE_GUARD))
        return EINVAL;
    }

  return 0;
}

> > I see in the archives that the IsBadXPtr vs. VirtualQuery debate has
> > happened before in different contexts.  It seams to me that VirtualQuery
> > wins here hands down, though.
> >
> Did you check on 9x and NT?
>
No.  I'll try, but that is difficult around here.

> Is that really correctly implemented?
>
Probably not, but I think it is better now.

> AFAIU VirtualQuery, it returns information about consecutive pages
> which share the same attributes.  So the length returned must be >=
> the len parameter or you must check the actual attributes returned
> by VirtualQuery.
>
Um, why must it be >= len?  I agree about checking the attributes, though.

> One possible attribute would be PAGE_NOACCESS.
> In that case you would return true, even though the pages are not
> accessible, right?
>
Fixed.

I guess it all depends on your interpretation of the following lines from:

http://www.opengroup.org/onlinepubs/007904975/functions/munmap.html

ERRORS:

[EINVAL]
    Addresses in the range [addr,addr+len) are outside the valid range for
the address space of a process.

What does that *really* mean, especially in terms of
COMMIT/RESERVE/FREE and NOACCESS/GUARD?

Thanks again for your volunteer time.

-- 
Brian Ford
Senior Realtime Software Engineer
VITAL - Visual Simulation Systems
FlightSafety International
Phone: 314-551-8460
Fax:   314-551-8444