Removed the 'place-holder' behaviour of VG_(mmap). Previously, VG_(mmap) would

add a segment mapping to the segment skip-list, and then often the caller of
VG_(mmap) would do another one for the same segment, just to change the SF_*
flags.  Now VG_(mmap) gets passed the appropriate SF_* flags so it can do it
directly.   This results in shorter, simpler code, and less work at runtime.

Also, strengthened checking in VG_(mmap), POST(mmap), POST(mmap2) -- now if the
result is not in the right place, it aborts rather than unmapping and
continuing.  This is because if it's not in the right place, something has
gone badly wrong.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@2466

diff --git a/coregrind/vg_include.h b/coregrind/vg_include.h
index 01820dc47f8f0f3ead8d65c700d0e0227d696e06..a5e154bc5dd297a35d5ff9819fd9312a8d575ec1 100644 (file)
--- a/coregrind/vg_include.h
+++ b/coregrind/vg_include.h
@@ -1150,8 +1150,8 @@ extern Int VG_(nanosleep)( const struct vki_timespec *req,
                            struct vki_timespec *rem );
 
 /* system/mman.h */
-extern void* VG_(mmap)( void* start, UInt length,
-                        UInt prot, UInt flags, UInt fd, UInt offset );
+extern void* VG_(mmap)( void* start, UInt length, UInt prot, UInt flags,
+                        UInt sf_flags, UInt fd, UInt offset );
 extern Int  VG_(munmap)( void* start, Int length );
 extern Int  VG_(mprotect)( void *start, Int length, UInt prot );
 

diff --git a/coregrind/vg_memory.c b/coregrind/vg_memory.c
index 29e664147343a371d8c2225ef22205e2195f8f28..5bf681fd58349a883dc92335c039b612ecd5a653 100644 (file)
--- a/coregrind/vg_memory.c
+++ b/coregrind/vg_memory.c
@@ -677,23 +677,21 @@ Bool VG_(is_addressable)(Addr p, Int size)
 /*--------------------------------------------------------------------*/
 
 // Returns 0 on failure.
-Addr VG_(client_alloc)(Addr addr, UInt len, UInt prot, UInt flags)
+Addr VG_(client_alloc)(Addr addr, UInt len, UInt prot, UInt sf_flags)
 {
    len = PGROUNDUP(len);
 
-   if (!(flags & SF_FIXED))
+   if (!(sf_flags & SF_FIXED))
       addr = VG_(find_map_space)(addr, len, True);
 
    // Don't do the mapping if we couldn't find space!
    if (0 == addr)
       return 0;
 
-   flags |= SF_CORE;
-
    if (VG_(mmap)((void *)addr, len, prot,
                 VKI_MAP_FIXED | VKI_MAP_PRIVATE | VKI_MAP_ANONYMOUS | VKI_MAP_CLIENT,
-                -1, 0) == (void *)addr) {
-      VG_(map_segment)(addr, len, prot, flags);
+                 sf_flags | SF_CORE, -1, 0) == (void *)addr) 
+   {
       return addr;
    }
 

diff --git a/coregrind/vg_mylibc.c b/coregrind/vg_mylibc.c
index e243bbb2aaba4397be50694b47b872b5f49f2802..e241c4081873019eccf7cf4a5571abbc6efc1acf 100644 (file)
--- a/coregrind/vg_mylibc.c
+++ b/coregrind/vg_mylibc.c
@@ -264,52 +264,40 @@ static Addr mmap_inner(void *start, UInt length, UInt prot, UInt flags, UInt fd,
 }
 
 /* Returns -1 on failure. */
-void* VG_(mmap)( void* start, UInt length, 
-                 UInt prot, UInt flags, UInt fd, UInt offset)
+void* VG_(mmap)( void* start, UInt length,
+                 UInt prot, UInt flags, UInt sf_flags, UInt fd, UInt offset)
 {
    Addr  res;
 
    if (!(flags & VKI_MAP_FIXED)) {
       start = (void *)VG_(find_map_space)((Addr)start, length, !!(flags & VKI_MAP_CLIENT));
-      if (start == 0)
-        return (void *)-1;
 
       flags |= VKI_MAP_FIXED;
    }
+   if (start == 0)
+      return (void *)-1;
 
    res = mmap_inner(start, length, prot, flags, fd, offset);
 
+   // Check it ended up in the right place.
    if (!VG_(is_kerror)(res)) {
-      UInt sf_flags = SF_MMAP;
-
-      if (flags & VKI_MAP_FIXED)
-        sf_flags |= SF_FIXED;
-      if (flags & VKI_MAP_SHARED)
-        sf_flags |= SF_SHARED;
-      if (!(flags & VKI_MAP_ANONYMOUS))
-        sf_flags |= SF_FILE;
-      if (!(flags & VKI_MAP_CLIENT))
-        sf_flags |= SF_VALGRIND;
-      if (flags & VKI_MAP_NOSYMS)
-        sf_flags |= SF_NOSYMS;
-
-      /* placeholder - caller will update flags etc if they want */
-      VG_(map_fd_segment)(res, length, prot, sf_flags, fd, offset, NULL);
-
       if (flags & VKI_MAP_CLIENT) {
-        if (res < VG_(client_base) || res >= VG_(client_end)) {
-           VG_(munmap)((void *)res, length);
-           res = -1;
-        }
+         vg_assert(VG_(client_base) <= res && res+length < VG_(client_end));
       } else {
-        if (res < VG_(valgrind_base) || res >= VG_(valgrind_end)) {
-           VG_(munmap)((void *)res, length);
-           res = -1;
-        }
+         vg_assert(VG_(valgrind_base) <= res && res+length < VG_(valgrind_end));
       }
    }
 
-   
+   if (!VG_(is_kerror)(res)) {
+      sf_flags |= SF_MMAP;
+      if (  flags & VKI_MAP_FIXED)      sf_flags |= SF_FIXED;
+      if (  flags & VKI_MAP_SHARED)     sf_flags |= SF_SHARED;
+      if (!(flags & VKI_MAP_ANONYMOUS)) sf_flags |= SF_FILE;
+      if (!(flags & VKI_MAP_CLIENT))    sf_flags |= SF_VALGRIND;
+      if (  flags & VKI_MAP_NOSYMS)     sf_flags |= SF_NOSYMS;
+
+      VG_(map_fd_segment)(res, length, prot, sf_flags, fd, offset, NULL);
+   }
 
    return VG_(is_kerror)(res) ? ((void*)(-1)) : (void*)res;
 }

diff --git a/coregrind/vg_signals.c b/coregrind/vg_signals.c
index 203d8973edde046b04963cd897458fdb11cea16b..dbb730fdfbfb193796af0483af666878bd9f0934 100644 (file)
--- a/coregrind/vg_signals.c
+++ b/coregrind/vg_signals.c
@@ -2141,15 +2141,13 @@ void vg_sync_signalhandler ( Int sigNo, vki_ksiginfo_t *info, struct vki_ucontex
            then extend the stack segment. 
         */
         Addr base = PGROUNDDN(esp);
-        Char *ret = VG_(mmap)((Char *)base, seg->addr - base, 
-                              VKI_PROT_READ | VKI_PROT_WRITE | VKI_PROT_EXEC,
-                              VKI_MAP_PRIVATE | VKI_MAP_FIXED | VKI_MAP_ANONYMOUS | VKI_MAP_CLIENT,
-                              -1, 0);
-        if ((Addr)ret == base) {
-           VG_(map_segment)(base, seg->addr - base,
-                            VKI_PROT_READ|VKI_PROT_WRITE|VKI_PROT_EXEC,
-                            SF_STACK|SF_GROWDOWN);
-           return;             /* restart instruction */
+        if ((void*)-1 != VG_(mmap)((Char *)base, seg->addr - base,
+                              VKI_PROT_READ|VKI_PROT_WRITE|VKI_PROT_EXEC,
+                              VKI_MAP_PRIVATE|VKI_MAP_FIXED|VKI_MAP_ANONYMOUS|VKI_MAP_CLIENT,
+                              SF_STACK|SF_GROWDOWN,
+                              -1, 0))
+         {
+           return;             // extension succeeded, restart instruction
         }
         /* Otherwise fall into normal signal handling */
       } else if (info->si_code == 2 && /* SEGV_ACCERR */

diff --git a/coregrind/vg_symtab2.c b/coregrind/vg_symtab2.c
index 6dcf26b463f85dc1f8a738d06e4f5c7a9ebf1b5c..2fafa8d2c76b7e793317ff605b5d0af17da19028 100644 (file)
--- a/coregrind/vg_symtab2.c
+++ b/coregrind/vg_symtab2.c
@@ -1095,7 +1095,9 @@ Addr open_debug_file( Char* name, UInt crc, UInt* size )
    *size = stat_buf.st_size;
    
    if ((addr = (Addr)VG_(mmap)(NULL, *size, VKI_PROT_READ,
-                               VKI_MAP_PRIVATE|VKI_MAP_NOSYMS, fd, 0)) == (Addr)-1) {
+                               VKI_MAP_PRIVATE|VKI_MAP_NOSYMS, 
+                               0, fd, 0)) == (Addr)-1) 
+   {
       VG_(close)(fd);
       return 0;
    }
@@ -1182,7 +1184,8 @@ Bool vg_read_lib_symbols ( SegInfo* si )
    }
 
    oimage = (Addr)VG_(mmap)( NULL, n_oimage, 
-                             VKI_PROT_READ, VKI_MAP_PRIVATE|VKI_MAP_NOSYMS, fd, 0 );
+                             VKI_PROT_READ, VKI_MAP_PRIVATE|VKI_MAP_NOSYMS, 
+                             0, fd, 0 );
 
    VG_(close)(fd);
 

diff --git a/coregrind/vg_syscalls.c b/coregrind/vg_syscalls.c
index 39cadd41d1f4cbc2194682c38a038c40728bb6d9..cec1c15f27b416d92c834ef2b73dbc02b7ac500c 100644 (file)
--- a/coregrind/vg_syscalls.c
+++ b/coregrind/vg_syscalls.c
@@ -1020,12 +1020,11 @@ static Addr do_brk(Addr newbrk)
 
         if (newaddr == current) {
            ret = newbrk;
-        } else if (VG_(mmap)((void *)current , newaddr-current,
-                             VKI_PROT_READ | VKI_PROT_WRITE | VKI_PROT_EXEC,
-                             VKI_MAP_PRIVATE | VKI_MAP_ANONYMOUS | VKI_MAP_FIXED | VKI_MAP_CLIENT,
-                             -1, 0) >= 0) {
-           VG_(map_segment)(current, newaddr-current, VKI_PROT_READ|VKI_PROT_WRITE|VKI_PROT_EXEC,
-                            SF_FIXED|SF_BRK);
+         } else if ((void*)-1 != VG_(mmap)((void*)current, newaddr-current,
+               VKI_PROT_READ|VKI_PROT_WRITE|VKI_PROT_EXEC,
+               VKI_MAP_PRIVATE|VKI_MAP_ANONYMOUS|VKI_MAP_FIXED|VKI_MAP_CLIENT,
+               SF_FIXED|SF_BRK, -1, 0)) 
+         {
            ret = newbrk;
         }
       } else {
@@ -3992,11 +3991,8 @@ PRE(mmap2)
 POST(mmap2)
 {
    if (!VG_(is_kerror)(res)) {
-      if (!valid_client_addr(res, arg2, tid, "mmap2")) {
-         VG_(munmap)((void *)res, arg2);
-         res = -VKI_ENOMEM;
-      } else
-         mmap_segment( (Addr)res, arg2, arg3, arg4, arg5, arg6 * (ULong)VKI_BYTES_PER_PAGE );
+      vg_assert(valid_client_addr(res, arg2, tid, "mmap2"));
+      mmap_segment( (Addr)res, arg2, arg3, arg4, arg5, arg6 * (ULong)VKI_BYTES_PER_PAGE );
    }
 }
 
@@ -4046,11 +4042,8 @@ PRE(mmap)
       res = VG_(do_syscall)(__NR_mmap, new_arg_block);
 
       if (!VG_(is_kerror)(res)) {
-         if (!valid_client_addr(res, a2, tid, "mmap")) {
-           VG_(munmap)((void *)res, a2);
-           res = -VKI_ENOMEM;
-         } else
-           mmap_segment( (Addr)res, a2, a3, a4, a5, a6 );
+         vg_assert(valid_client_addr(res, a2, tid, "mmap"));
+         mmap_segment( (Addr)res, a2, a3, a4, a5, a6 );
       }
    }
 }