Hi Ben,

On Fri, Dec 23, 2016 at 8:03 PM, Ben Coman <btc@openinworld.com> wrote:

On Sat, Dec 24, 2016 at 2:33 AM, Holger Freyther <holger@freyther.de> wrote:
> > On 23 Dec 2016, at 01:46, Ben Coman <btc@openinworld.com> wrote:
> >
> > Just curious about the behaviour and performance of mprotect and VirtualProtect.  Would it be feasible to use on each FFI callout so that aberrant memory access from C code cannot corrupt the Smalltalk Image, but generate an exception that could be handled in-Image?
> Hi Ben,
> * syscall overhead (e.g. mprotect is not part of the VDSO)
> * lock/unlock of Page Table Entries (PTEs)
> * walking page table entries
> * modifying page table entries
> * merging/combining virtual memory areas now (allocation)
> * cache/tlb being invalidated
> E.g. for FreeBSD the chain is like this with a lot of details on the way
> * sys_mprotect[1] the syscall after the dispatch
> * vm_map_protect[2] for the VM subsystem
> * AMD64's pmap_protect[3] implementation various ways to invalidate the TLB in it
> holger
> [1] https://github.com/freebsd/freebsd/blob/releng/10.3/sys/vm/vm_mmap.c#L657
> [2] https://github.com/freebsd/freebsd/blob/releng/10.3/sys/vm/vm_map.c#L1924
> [3] https://github.com/freebsd/freebsd/blob/releng/10.3/sys/amd64/amd64/pmap.c#L3906

Thanks Holger.  A bit hard to soak up in a first sitting, but
interesting to get a feel for how it works under the covers.
I summarised the loops, so it seems it needs to modify every page table entry.


      /* Make a first pass to check for protection violations.*/
      while ((current != &map->header) && (current->start < end))
      {... current = current->next; ...}

      /* Do an accounting pass for private read-only mappings that now
will do cow due to allowed write (e.g. debugger sets breakpoint on
text segment) */
     for (current = entry; (current != &map->header) &&
(current->start < end); current = current->next)

     /* Go back and fix up protections. [Note that clipping is not
necessary the second time.] */
     current = entry;
     while ((current != &map->header) && (current->start < end))
              for (; sva < eva; sva = va_next) {
                 va_next = (sva + NBPDR) & ~PDRMASK;
                 for (pte = pmap_pde_to_pte(pde, sva); sva != va_next;
pte++,   sva += PAGE_SIZE)
                    {...flip page table bits...}
         current = current->next;

The documentation of variables in the code there is not great.  I'm
guessing at variable purposes...
sva = start virtual address
eva = end virtual address
pte = page table entry
NBPDR=number bytes / page directory

So maybe useful for debugging, but not practical for every FFI call (??)

Well, even if it was only switched on for debugging one would need to /not/ protect pages containing objects passed through the FFI to be written to.  I don't see an easy way of doing this.  One might think one only had to scan the objects in the current FFI call and not protect them.  But how would one distinguish between those just read from and those written to?

Also Spur's support for pinned objects means that one can pass pinned objects out to foreign code and allow the foreign code to access the object after the call returns.  I don't know how to not protect these objects.  If one starts to make exceptions for all objects in the current call and all pinned objects pretty soon one has lots of the heap unprotected.

But there's another principle here.  While a debugging idea may seem really useful its best not to implement it until one really needs it.  Debugging features add complexity and if done wrong can introduce differences between normal and debug behaviour.  There's an opportunity cost I'n implementing them.  So let's not add debug features until we really need them.

Happy solstice!
best, Eliot