From 13560bdb6b1a360c1bb9329e4a08ae70d3522f41 Mon Sep 17 00:00:00 2001
From: Robert Nishihara <robertnishihara@gmail.com>
Date: Wed, 14 Sep 2016 17:45:18 -0700
Subject: [PATCH] Fix offset in get_malloc_mapinfo. (#24)

* Fix offset in get_malloc_mapinfo.

* Don't add offset inside mmap_record

* make clang-format happy
---
 src/malloc.c        | 24 ++++++++++++++----------
 src/malloc.h        |  2 +-
 src/plasma_client.c |  4 ++++
 3 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/src/malloc.c b/src/malloc.c
index d83423844..f18535437 100644
--- a/src/malloc.c
+++ b/src/malloc.c
@@ -36,6 +36,7 @@ struct mmap_record {
   UT_hash_handle hh_pointer;
 };
 
+/* TODO(rshin): Don't have two hash tables. */
 struct mmap_record *records_by_fd = NULL;
 struct mmap_record *records_by_pointer = NULL;
 
@@ -65,16 +66,16 @@ int create_buffer(int64_t size) {
 }
 
 void *fake_mmap(size_t size) {
-  // Add sizeof(size_t) so that the returned pointer is deliberately not
-  // page-aligned. This ensures that the segments of memory returned by
-  // fake_mmap are never contiguous.
-  int fd = create_buffer(size + sizeof(size_t));
-  void *pointer = mmap(NULL, size + sizeof(size_t), PROT_READ | PROT_WRITE,
-                       MAP_SHARED, fd, 0);
+  /* Add sizeof(size_t) so that the returned pointer is deliberately not
+   * page-aligned. This ensures that the segments of memory returned by
+   * fake_mmap are never contiguous. */
+  size += sizeof(size_t);
+
+  int fd = create_buffer(size);
+  void *pointer = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
   if (pointer == MAP_FAILED) {
     return pointer;
   }
-  pointer += sizeof(size_t);
 
   struct mmap_record *record = malloc(sizeof(struct mmap_record));
   record->fd = fd;
@@ -83,16 +84,19 @@ void *fake_mmap(size_t size) {
   HASH_ADD(hh_fd, records_by_fd, fd, sizeof(fd), record);
   HASH_ADD(hh_pointer, records_by_pointer, pointer, sizeof(pointer), record);
 
+  /* We lie to dlmalloc about where mapped memory actually lives. */
+  pointer += sizeof(size_t);
   LOG_DEBUG("%p = fake_mmap(%lu)", pointer, size);
   return pointer;
 }
 
 int fake_munmap(void *addr, size_t size) {
   LOG_DEBUG("fake_munmap(%p, %lu)", addr, size);
+  addr -= sizeof(size_t);
+  size += sizeof(size_t);
 
   struct mmap_record *record;
 
-  addr -= sizeof(size_t);
   HASH_FIND(hh_pointer, records_by_pointer, &addr, sizeof(addr), record);
   assert(record != NULL);
   close(record->fd);
@@ -100,7 +104,7 @@ int fake_munmap(void *addr, size_t size) {
   HASH_DELETE(hh_fd, records_by_fd, record);
   HASH_DELETE(hh_pointer, records_by_pointer, record);
 
-  return munmap(addr, size + sizeof(size_t));
+  return munmap(addr, size);
 }
 
 void get_malloc_mapinfo(void *addr,
@@ -108,7 +112,7 @@ void get_malloc_mapinfo(void *addr,
                         int64_t *map_size,
                         ptrdiff_t *offset) {
   struct mmap_record *record;
-  // TODO(rshin): Implement a more efficient search through records_by_fd.
+  /* TODO(rshin): Implement a more efficient search through records_by_fd. */
   for (record = records_by_fd; record != NULL; record = record->hh_fd.next) {
     if (addr >= record->pointer && addr < record->pointer + record->size) {
       *fd = record->fd;
diff --git a/src/malloc.h b/src/malloc.h
index 2b7395eba..9fc1f48bb 100644
--- a/src/malloc.h
+++ b/src/malloc.h
@@ -6,4 +6,4 @@ void get_malloc_mapinfo(void *addr,
                         int64_t *map_length,
                         ptrdiff_t *offset);
 
-#endif  // MALLOC_H
+#endif /* MALLOC_H */
diff --git a/src/plasma_client.c b/src/plasma_client.c
index e236f3378..20e5e087f 100644
--- a/src/plasma_client.c
+++ b/src/plasma_client.c
@@ -44,6 +44,8 @@ void plasma_create(int conn,
   assert(reply.metadata_size == metadata_size);
   /* The metadata should come right after the data. */
   assert(reply.metadata_offset == reply.data_offset + data_size);
+
+  // TOOD(rshin): Don't call mmap if this fd has already been mapepd.
   *data = ((uint8_t *) mmap(NULL, reply.map_size, PROT_READ | PROT_WRITE,
                             MAP_SHARED, fd, 0)) +
           reply.data_offset;
@@ -73,6 +75,8 @@ void plasma_get(int conn,
   plasma_reply reply;
   /* The following loop is run at most twice. */
   int fd = recv_fd(conn, (char *) &reply, sizeof(plasma_reply));
+
+  // TOOD(rshin): Don't call mmap if this fd has already been mapepd.
   *data =
       ((uint8_t *) mmap(NULL, reply.map_size, PROT_READ, MAP_SHARED, fd, 0)) +
       reply.data_offset;