release9.2/bugs/rtheap_mmap_only (#76)

* remove mallocs from the RTlib's os_malloc call when we're on the mmap path

* Fix mmap flags: MAP_32BIT is x86_64 only, and MAP_FIXED is inappropriate for this application

* Cleanup: remove dangling free() on malloc path, and adjust heap allocations such that:

1) Usable size >= requested size
2) Heap data structure at end, not at beginning, of heap (so that user heaps start aligned, and so that we aren't grabbing a whole previous page regardless of requested size).

* Node should go at the end of the returned heap

The node data structure should, in the case where the actual heap and the returned heap differ, go at the end of the returned heap so that the returned heap's size is correct.

dyninstAPI/src/dynProcess.C

@@ -1587,7 +1587,7 @@ bool PCProcess::getAllActiveFrames(pdvector<Frame> &activeFrames) {
 #define HEAP_DYN_BUF_SIZE (0x100000)
 #endif
 
-static const Address ADDRESS_LO = ((Address)0);
+static const Address ADDRESS_LO = ((Address)0x10000);
 static const Address ADDRESS_HI = ((Address)~((Address)0));
 
 Address PCProcess::inferiorMalloc(unsigned size, inferiorHeapType type,

dyninstAPI_RT/src/RTheap-linux.c

@@ -43,13 +43,12 @@
 #include <sys/mman.h>                 /* mmap() */
 #include "RTheap.h"
 
-#define MAX_MAP_SIZE (1<<20)
-#if defined(MUTATEE64)
+#if 1 //defined(MUTATEE64)
 
 int     DYNINSTheap_align = 4; /* heaps are word-aligned */
 
 Address DYNINSTheap_loAddr = 0x10000; /* Bump to 64k to make SELinux happier */
-Address DYNINSTheap_hiAddr = ~0x0;
+Address DYNINSTheap_hiAddr = ~(Address)0x0;
 #elif defined(arch_power)
 int     DYNINSTheap_align  = 4; /* heaps are word-aligned */
 Address DYNINSTheap_loAddr = ~(Address)0; // should be defined by getpagesize() when used.
@@ -61,7 +60,7 @@ Address DYNINSTheap_loAddr = 0x50000000;
 Address DYNINSTheap_hiAddr = 0xb0000000;
 #endif
 
-int     DYNINSTheap_mmapFlags = MAP_FIXED | MAP_PRIVATE;
+int     DYNINSTheap_mmapFlags = MAP_ANONYMOUS | MAP_PRIVATE;
 
 
 RT_Boolean DYNINSTheap_useMalloc(void *lo, void *hi)
@@ -84,28 +83,36 @@ void DYNINSTheap_mmapFdClose(int fd)
   close(fd);
 }
 
-/* Linux /proc/PID/maps is unreliable when it is read with more than one
-read call (it can show pages that are not actually allocated).  We
-read it all in one call into this buffer.
 
-linux-2.4: reading /proc/PID/maps now returns after each line in the maps,
-so we must loop to get everything.
-*/
-
-// Static so we dont have to do the exponential backoff each time.
-static size_t mapSize = 1 << 15;
 
 int
 DYNINSTgetMemoryMap(unsigned *nump, dyninstmm_t **mapp)
 {
-   int fd, done;
-   ssize_t ret;
-   size_t length;
-   char *p;
-   dyninstmm_t *ms;
-   unsigned i, num;
-   char *procAsciiMap;
-
+    FILE* procmaps;
+    Address saddr = 0, eaddr = 0;
+    int num_matches;
+    procmaps = fopen("/proc/self/maps", "r");
+    char ch;
+    dyninstmm_t* maps = *mapp;
+    if(procmaps == NULL) return -1;
+    *nump = 0;
+    while(((num_matches = fscanf(procmaps, "%lx-%lx", &saddr, &eaddr)) != EOF) && (*nump < 1024)) {
+        if (num_matches == 2) {
+            maps[*nump].pr_vaddr = saddr;
+            maps[*nump].pr_size = eaddr - saddr;
+            (*nump)++;
+            // skip to next line
+            while ((ch = fgetc(procmaps)) != '\n' && ch != EOF) {
+                if (ch == EOF) break;
+            }
+        }
+        else
+        {
+            break;
+        }
+    }
+    fclose(procmaps);
+    return *nump < 1024;
 
    /* 
       Here are two lines from 'cat /proc/self/maps' on Linux 2.2.  Each
@@ -117,111 +124,4 @@ DYNINSTgetMemoryMap(unsigned *nump, dyninstmm_t **mapp)
       0804a000-0804c000 rw-p 00001000 08:09 12089      /bin/cat
       0804c000-0804f000 rwxp 00000000 00:00 0
    */
-   procAsciiMap = NULL;
-   done = 0;
-   while (1)
-   {
-       if(procAsciiMap == NULL) {
-           procAsciiMap = malloc(mapSize);
-           if (!procAsciiMap) {
-               fprintf(stderr, "DYNINSTgetMemoryMap: Out of memory\n");
-               goto end;
-           }
-       }
-       else
-       {
-           void *newMap = realloc(procAsciiMap, mapSize);
-           if (!newMap) {
-               fprintf(stderr, "DYNINSTgetMemoryMap: Out of memory\n");
-               goto freeBuffer;
-           }
-           procAsciiMap = newMap;
-       }
-       fd = open("/proc/self/maps", O_RDONLY);
-       if (0 > fd) {
-           perror("open /proc");
-           goto freeBuffer;
-       }
-       length = 0;
-       while (1)
-       {
-           ret = read(fd, procAsciiMap + length, mapSize - length);
-           length += ret;
-           if (0 == ret) {
-               done = 1;
-               break;
-           }
-           if (0 > ret) {
-               close(fd);
-               perror("read /proc");
-               goto freeBuffer;
-           }
-           /* Check if the buffer was to small and exponentially
-              increase its size */
-           if (length >= mapSize) {
-               close(fd);
-               mapSize = mapSize << 1;
-
-                  /* Return error if we reached the max size for
-                     the buffer*/
-                  if(mapSize > MAX_MAP_SIZE) {
-                      fprintf(stderr, "DYNINSTgetMemoryMap: memory map buffer \
-                                       is larger than the max size. max size=%d\n", MAX_MAP_SIZE);
-                      goto freeBuffer;
-                  }
-               break;
-           }
-       }
-       if(done) {
-           break;
-       }
-   }
-   procAsciiMap[length] = '\0'; /* Now string processing works */
-
-   /* Count lines, which is the same as the number of segments.
-      Newline characters separating lines are converted to nulls. */
-   for (num = 0, p = strtok(procAsciiMap, "\n");
-        p != NULL;
-        num++, p = strtok(NULL, "\n"))
-      ;
-
-   ms = (dyninstmm_t *) malloc(num * sizeof(dyninstmm_t));
-   if (! ms) {
-      fprintf(stderr, "DYNINSTgetMemoryMap: Out of memory\n");
-      goto freeBuffer;
-   }
-
-   p = procAsciiMap;
-   for (i = 0; i < num; i++) {
-      char *next = p + strlen(p) + 1; /* start of next line */
-      Address saddr, eaddr;
-
-      /* parse start address */
-      p = strtok(p, "-");
-      if (! p) goto parseerr;
-      saddr = strtoul(p, &p, 16);
-      ++p; /* skip '-' */
-
-      /* parse end address */
-      p = strtok(NULL, " ");
-      if (! p) goto parseerr;
-      eaddr = strtoul(p, NULL, 16);
-
-      ms[i].pr_vaddr = saddr;
-      ms[i].pr_size = eaddr - saddr;
-
-      p = next;
-   }
-
-   *nump = num;
-   *mapp = ms;
-   free(procAsciiMap);
-   return 0;
- parseerr:
-   free(ms);
-   fprintf(stderr, "DYNINSTgetMemoryMap: /proc/self/maps parse error\n");
- freeBuffer:
-   free(procAsciiMap);
- end:
-   return -1;
 }

dyninstAPI_RT/src/RTheap.c

@@ -249,25 +249,22 @@ void *DYNINSTos_malloc(size_t nbytes, void *lo_addr, void *hi_addr)
 {
   void *heap;
   size_t size = nbytes;
-  heapList_t *node = (heapList_t *)malloc(sizeof(heapList_t));
-
-  /* initialize page size */
+  heapList_t *node = NULL;
+    /* initialize page size */
   if (psize == -1) psize = getpagesize();
 
   /* buffer size must be aligned */
   if (size % DYNINSTheap_align != 0) {
-    free(node);
     return ((void *)-1);
   }
 
   /* use malloc() if appropriate */
   if (DYNINSTheap_useMalloc(lo_addr, hi_addr)) {
 
     Address ret_heap;
-    int size_heap = size + DYNINSTheap_align;
+    int size_heap = size + DYNINSTheap_align + sizeof(heapList_t);
     heap = malloc(size_heap);
     if (heap == NULL) {
-      free(node);
 #ifdef DEBUG
       fprintf(stderr, "Failed to MALLOC\n");      
 #endif 
@@ -279,75 +276,31 @@ void *DYNINSTos_malloc(size_t nbytes, void *lo_addr, void *hi_addr)
     if (ret_heap < (Address)lo_addr ||
         ret_heap + size - 1 > (Address)hi_addr) {
       free(heap);
-      free(node);
 #ifdef DEBUG
       fprintf(stderr, "MALLOC'd area fails range constraints\n");
 #endif 
       return NULL;
     }
 
     /* define new heap */
+    node = ret_heap + size;
     node->heap.ret_addr = (void *)ret_heap;
     node->heap.addr = heap;
     node->heap.len = size_heap;
     node->heap.type = HEAP_TYPE_MALLOC;
 
 
   } else { /* use mmap() for allocation */
-    Address lo = (Address) lo_addr;
+    Address lo = (Address) heap_alignUp(lo_addr, psize);
     Address hi = (Address) hi_addr;
-    int fd;
-    unsigned nmaps;
-    dyninstmm_t *maps;
-
-    /* What if we need to allocate memory not in the area we can mmap? */
-#if defined (os_linux)  && defined(arch_power)
-   DYNINSTheap_loAddr = getpagesize();
-#endif
-    if ((hi < DYNINSTheap_loAddr) || (lo > DYNINSTheap_hiAddr)) {
-      free(node);
-#ifdef DEBUG
-      fprintf(stderr, "CAN'T MMAP IN RANGE GIVEN\n");
-#endif 
-      return NULL;
-    }
-
-
-    /* Get memory map and sort it.  maps will point to malloc'd memory
-       that we must free. */
-    if (0 > DYNINSTgetMemoryMap(&nmaps, &maps)) {
-      free(node);
-#ifdef DEBUG
-      fprintf(stderr, "failed MMAP\n");
-#endif 
-      return NULL;
-    }
-    qsort(maps, (size_t)nmaps, (size_t)sizeof(dyninstmm_t), &heap_memmapCompare);
-    heap_checkMappings(nmaps, maps); /* sanity check */
-
-    /*DYNINSTheap_printMappings(nmaps, maps);*/
-
-    fd = DYNINSTheap_mmapFdOpen();
-    if (0 > fd) {
-      free(node);
-      free(maps);
-      return NULL;
-    }
-    heap = (void*) constrained_mmap(size, lo, hi, maps, nmaps, fd);
-    free(maps);
-    DYNINSTheap_mmapFdClose(fd);
-    if (!heap) {
-       free(node);
-#ifdef DEBUG
-       fprintf(stderr, "failed MMAP(2)\n");
-#endif 
-       return NULL;
-    }
+      heap = trymmap(size + sizeof(struct heapList_t), lo, hi, psize, -1);
+      if(!heap) return NULL;
+    node = heap + size;
 
     /* define new heap */
-    node->heap.ret_addr = heap;
     node->heap.addr = heap;
-    node->heap.len = size;
+    node->heap.ret_addr = heap;
+    node->heap.len = size + sizeof(struct heapList_t);
     node->heap.type = HEAP_TYPE_MMAP;
   }
 
@@ -356,7 +309,9 @@ void *DYNINSTos_malloc(size_t nbytes, void *lo_addr, void *hi_addr)
   node->next = Heaps;
   if (Heaps) Heaps->prev = node;
   Heaps = node;
-
+#ifdef DEBUG
+  fprintf(stderr, "new heap at %lx, size %lx\n", node->heap.ret_addr, node->heap.len);
+#endif
   return node->heap.ret_addr;
 }
 
@@ -394,8 +349,6 @@ int DYNINSTos_free(void *buf)
       break;
     }
 
-    /* free list element */
-    free(t);
     break;
   }
 

dyninstAPI_RT/src/RTposix.c

@@ -242,10 +242,19 @@ int DYNINSTwriteEvent(void *ev, size_t sz)
   return 0;
 }
 
+// Important note: addr will be zero in two cases here
+// One is the case where we're doing a constrained low mmap, in which case MAP_32BIT
+// is precisely correct. The other is the case where our
+// constrained map attempts have failed, and we're doing a scan for first available
+// mappable page. In that case, MAP_32BIT does no harm.
 void *map_region(void *addr, int len, int fd) {
      void *result;
-     result = mmap(addr, len, PROT_READ|PROT_WRITE|PROT_EXEC, 
-                   DYNINSTheap_mmapFlags, fd, 0);
+    int flags = DYNINSTheap_mmapFlags;
+#if defined(arch_x86_64)
+    if(addr == 0) flags |= MAP_32BIT;
+#endif
+     result = mmap(addr, len, PROT_READ|PROT_WRITE|PROT_EXEC,
+                   flags, fd, 0);
      if (result == MAP_FAILED)
          return NULL;
      return result;