clarify memory module to memmap module

CMakeLists.txt

@@ -25,7 +25,7 @@ add_library(hiload SHARED
   src/hiload.c
   src/symbols.c
   src/files.c
-  src/memory.c
+  src/memmap.c
   src/logger.c
   src/moduler.c
   src/hielf.c

src/common.h

@@ -1,5 +1,4 @@
-#ifndef COMMON_H_
+#pragma once
-#define COMMON_H_
 
 #include "logger.h"
 #include "types.h"
@@ -11,4 +10,3 @@ static inline u32 has_mask(u32 flags, u32 mask) { return flags & mask; }
 #define ARRLEN(A) (sizeof((A)) / (sizeof((A)[0])))
 #define UNUSED(v) (void)(v)
 
-#endif // COMMON_H_

src/filewatcher/filewatch_context.h

@@ -2,7 +2,6 @@
 #define FILEWATCH_CONTEXT_H_
 
 #include "vector.h"
-#include "filewatcher/filewatch.h"
 #include "types.h"
 
 /**

src/filewatcher/filewatcher.h

@@ -1,12 +1,17 @@
 #ifndef HI_FILEWATCHER_H_
 #define HI_FILEWATCHER_H_
 
+#include "filewatch_context.h"
 #include "filewatch_type.h"
 #include "types.h"
 
 /**
  * File event watcher using inotify.
  *
+ * Create a FileWatcher and give it files to monitor. FileWatcher creates a thread
+ * for listening system notifications. It then creates events for each filechange
+ * for those files under scrutiny.
+ *
  * Paths are stored as absolute, and this doesn't handle symlinks in watched
  * files. So if you have e.g. a symlink 'my-proj' to your project folder in
  * '/workspace/code/my-proj' paths might exist from either side and they are

src/hielf.h

@@ -1,5 +1,4 @@
-#ifndef ELF_H_
+#pragma once
-#define ELF_H_
 
 #include <elf.h>
 #include <libelf.h>
@@ -10,4 +9,3 @@ void *hi_elf_find_dynamic_segment(void *module_base, size_t n, Elf **elf);
 const char *hi_elf_dyntostr(unsigned type);
 const char *hi_elf_segtostr(unsigned type);
 
-#endif

src/hiload.c

@@ -2,12 +2,11 @@
 
 #include "vector.h"
 #include "common.h"
-#include "filewatcher/filewatch_context.h"
 #include "filewatcher/filewatcher.h"
 #include "logger.h"
-#include "memory.h"
+#include "memmap.h"
 #include "moduler.h"
-#include "string.h"
+#include "histring.h"
 #include "symbols.h"
 
 #include <assert.h>
@@ -30,7 +29,7 @@ static const char *module_exclude_filter[] = {
 
 
 typedef struct {
-  VectorMemoryRegion memory_regions;
+  VectorMemoryMap memory_regions;
   VectorStr enabled_modules;
   FileWatcher *filewatcher;
   VectorModuleData modules;
@@ -225,7 +224,7 @@ int hi_init(size_t n, const char **enabled_modules) {
   }
 
   sc_array_init(&context.memory_regions);
-  if (read_memory_maps_self(&context.memory_regions) != HI_OK) {
+  if (memmaps_from_process(&context.memory_regions) != HI_OK) {
     log_error("Could not populate program memory maps.\n");
     return HI_FAIL;
   }

src/histring.h

@@ -1,5 +1,4 @@
-#ifndef HI_STRING_H_
+#pragma once
-#define HI_STRING_H_
 
 #include <stddef.h>
 
@@ -49,4 +48,3 @@ const char *hi_str_starts_with(const char path[static 1], size_t listn,
  */
 int hi_path_has_filename(const char *path, const char *filename);
 
-#endif // HI_STRING_H_

src/logger.h

@@ -1,5 +1,4 @@
-#ifndef LOGGER_H_
+#pragma once
-#define LOGGER_H_
 
 #include "../3rd/sc/logger/sc_log.h"
 
@@ -24,4 +23,3 @@ bool log_get_verbose();
 #define log_errorv(...) do { if (log_get_verbose()) {   \
             log_error(__VA_ARGS__); } } while(0)
 
-#endif // LOGGER_H_

src/memmap.c

@@ -1,4 +1,4 @@
-#include "memory.h"
+#include "memmap.h"
 
 #include "files.h"
 #include "logger.h"
@@ -8,20 +8,17 @@ static inline int ptr_in_range(uptr ptr, uptr start, uptr end) {
   return ptr >= start && ptr <= end;
 }
 
-HiResult memory_find_region(uptr ptr, struct sc_array_memreg *const regions,
+HiResult memmaps_find_by_ptr(uptr ptr, VectorMemoryMap *const maps,
                              size_t *index) {
-  for (size_t i = 0; i < sc_array_size(regions); i++) {
+  for (size_t i = 0; i < sc_array_size(maps); i++) {
-    uptr start = regions->elems[i].region_start;
+    uptr start = maps->elems[i].span.start;
     // we assume a sorted region by start address, so we can do a quick discard
-    // here. very useful for relative vs absolute address checks
     if (ptr < start)
       return HI_FAIL;
-    uptr end = regions->elems[i].region_end;
+    uptr end = maps->elems[i].span.end;
     if (ptr_in_range(ptr, start, end)) {
       if (index)
         *index = i;
-      sc_log_debug("Pointer match (%p) found in index: %u, range: %p - %p\n",
-                   ptr, i, start, end);
       return HI_OK;
     }
   }
@@ -29,8 +26,8 @@ HiResult memory_find_region(uptr ptr, struct sc_array_memreg *const regions,
   return HI_FAIL;
 }
 
-HiResult read_memory_maps_self(struct sc_array_memreg *regions) {
+HiResult memmaps_from_process(VectorMemoryMap *outmaps) {
-  memory_clear_memregs(regions);
+  memmaps_clear(outmaps);
 
   char *maps_str = file_to_str_dyn("/proc/self/maps");
   if (!maps_str)
@@ -42,7 +39,7 @@ HiResult read_memory_maps_self(struct sc_array_memreg *regions) {
 
   char *line = strtok(strptr, "\n");
   while (line) {
-    MemoryRegion reg = {0};
+    MemoryMap reg = {0};
 
     char pathname[256];
     char perm_str[5];
@@ -53,8 +50,8 @@ HiResult read_memory_maps_self(struct sc_array_memreg *regions) {
     // Format example from the file:
     // 7fa0b66ca000-7fa0b66cc000 rw-p 00033000 fe:02 28063911 /usr/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2
     int result = sscanf(line, "%lx-%lx %4s %ld %x:%x %lu %255s",
-                        &reg.region_start,
+                        &reg.span.start,
-                        &reg.region_end,
+                        &reg.span.end,
                         perm_str,
                         &reg.offset,
                         &dev_major,
@@ -74,30 +71,28 @@ HiResult read_memory_maps_self(struct sc_array_memreg *regions) {
       reg.pathname = strdup(pathname);
     }
 
-    sc_array_add(regions, reg);
+    sc_array_add(outmaps, reg);
     line = strtok(NULL, "\n");
   }
 
   free(maps_str);
-
   return HI_OK;
 }
 
-MemoryRegionSpan
+MemorySpan memmaps_find_by_name(const char region_name[static 1],
-memory_get_module_span(const struct sc_array_memreg *const regions,
+                                  const VectorMemoryMap *const maps) {
-                       const char module_name[static 1]) {
   uptr start = ~0ull;
   uptr end = 0;
-  for (size_t i = 0; i < sc_array_size(regions); i++) {
+  for (size_t i = 0; i < sc_array_size(maps); i++) {
 
-    const MemoryRegion *reg = &sc_array_at(regions, i);
+    const MemoryMap *reg = &sc_array_at(maps, i);
 
-    if (reg->pathname && strcmp(reg->pathname, module_name) == 0) {
+    if (reg->pathname && strcmp(reg->pathname, region_name) == 0) {
-      if (reg->region_start < start) {
+      if (reg->span.start < start) {
-        start = reg->region_start;
+        start = reg->span.start;
       }
-      if (reg->region_end > end) {
+      if (reg->span.end > end) {
-        end = reg->region_end;
+        end = reg->span.end;
       }
     } else {
       // we passed the module regions
@@ -107,24 +102,24 @@ memory_get_module_span(const struct sc_array_memreg *const regions,
   }
 
   assert(start < ~0ull && end > 0);
-  return (MemoryRegionSpan){.region_start = start, .region_end = end};
+  return (MemorySpan){.start = start, .end = end};
 }
 
-void memory_clear_memregs(struct sc_array_memreg *regions) {
+void memmaps_clear(VectorMemoryMap *maps) {
-  for (size_t i = 0; i < sc_array_size(regions); i++) {
+  for (size_t i = 0; i < sc_array_size(maps); i++) {
-    memory_free_memreg(&sc_array_at(regions, i));
+    memmap_free(&sc_array_at(maps, i));
   }
-  sc_array_clear(regions);
+  sc_array_clear(maps);
 }
 
-void memory_term_memregs(struct sc_array_memreg *regions) {
+void memmaps_term(VectorMemoryMap *maps) {
-  for (size_t i = 0; i < sc_array_size(regions); i++) {
+  for (size_t i = 0; i < sc_array_size(maps); i++) {
-    memory_free_memreg(&sc_array_at(regions, i));
+    memmap_free(&sc_array_at(maps, i));
   }
-  sc_array_term(regions);
+  sc_array_term(maps);
 }
 
-void memory_free_memreg(MemoryRegion *reg) {
+void memmap_free(MemoryMap *reg) {
   if (reg) {
     free((char *)reg->pathname);
   }

src/memmap.h

@@ -0,0 +1,87 @@
+
+/**
+ * A module for reading and using data from /proc/pid/maps
+ */
+
+#pragma once
+
+#include "types.h"
+#include "vector.h"
+
+typedef enum {
+  HI_MEMORY_READ = (1 << 0),
+  HI_MEMORY_WRITE = (1 << 1),
+  HI_MEMORY_EXECUTE = (1 << 2),
+  HI_MEMORY_SHARED = (1 << 3),
+  HI_MEMORY_PRIVATE = (1 << 4)
+} MemoryPermissions;
+
+typedef struct MemorySpan {
+  uptr start;
+  uptr end;
+} MemorySpan;
+
+/**
+ * Holds information of a /proc/pid/maps entry.
+ *
+ * See `man proc(5)` for details.
+ */
+typedef struct MemoryMap {
+  /**
+   * Might be null as not all regions have a name or a path.
+   */
+  const char *pathname;
+  MemorySpan span;
+  ptrdiff offset;
+  u64 inode;
+  u32 permissions; // enum MemoryPermissions
+} MemoryMap;
+
+sc_array_def(MemoryMap, memmap);
+typedef struct sc_array_memmap VectorMemoryMap;
+
+/// Free memory and zero maps
+void memmaps_term(VectorMemoryMap *maps);
+
+/// Clear maps, but don't free the buffer
+void memmaps_clear(VectorMemoryMap *maps);
+
+/// Free memory for a single map
+void memmap_free(MemoryMap *map);
+
+/**
+ * Read allocated memory regions of the current process and fill @a outmaps.
+ *
+ * Resulting regions are ordered by start address and they don't overlap.
+ *
+ * @param outmaps Output map where data is placed. Cleared before use.
+ * @return HI_FAIL if reading the maps failed
+ */
+HiResult memmaps_from_process(VectorMemoryMap *outmaps);
+
+/**
+ * Search for a region that contains the given pointer.
+ *
+ * Assumes @a maps is sorted by start range.
+ *
+ * @param ptr The needle
+ * @param maps The haystack
+ * @param index If non-null, will contain index of the MemoryMap in @a maps
+ * @return if ptr was found from any region in maps
+ */
+HiResult memmaps_find_by_ptr(uptr ptr, VectorMemoryMap *const maps,
+                             size_t *index);
+
+/**
+ * Find the allocated memory region by a pathname.
+ *
+ * Assumes @a maps is ordered by start address and by path, i.e. all modules
+ * are allocated into consecutive memory addresses without gaps or other
+ * modules or named regions in between. This should be true when filled
+ * by @a memmaps_from_process.
+ *
+ * @param region_name Name for the region, e.g. absolute path to a module
+ * @param maps The haystack
+ */
+MemorySpan memmaps_find_by_name(const char region_name[static 1],
+                                  const VectorMemoryMap *const maps);

src/memory.h

@@ -1,49 +0,0 @@
-#pragma once
-
-#include "vector.h"
-#include "types.h"
-
-#include <assert.h>
-
-typedef enum {
-  HI_MEMORY_READ = 1 << 0,
-  HI_MEMORY_WRITE = 1 << 1,
-  HI_MEMORY_EXECUTE = 1 << 2,
-  HI_MEMORY_SHARED = 1 << 3,
-  HI_MEMORY_PRIVATE = 1 << 4
-} MemoryPermissions;
-
-typedef struct {
-  uptr region_start;
-  uptr region_end;
-} MemoryRegionSpan;
-
-typedef struct {
-  const char *pathname;
-  uptr region_start;
-  uptr region_end;
-  ptrdiff offset;
-  u64 inode;
-  u32 permissions; // enum MemoryPermissions
-} MemoryRegion;
-sc_array_def(MemoryRegion, memreg);
-typedef struct sc_array_memreg VectorMemoryRegion;
-
-// Free memory and init to zero
-void memory_term_memregs(VectorMemoryRegion *regions);
-// Doesn't free underlying array memory, only for each region
-void memory_clear_memregs(VectorMemoryRegion *regions);
-// Free child memory
-void memory_free_memreg(MemoryRegion *reg);
-
-/* A pointer that can be used to place the memory regions into. Clears regions
- * before use, but uses the same buffer. */
-HiResult read_memory_maps_self(VectorMemoryRegion *regions);
-
-/* Return index the pointer is found in */
-HiResult memory_find_pointer(uptr ptr,
-                                 VectorMemoryRegion *const regions,
-                                 size_t *index);
-
-MemoryRegionSpan memory_get_module_span(const VectorMemoryRegion *const regions,
-                                        const char module_name[static 1]);

src/moduler.c

@@ -5,7 +5,7 @@
 #include "hielf.h"
 #include "histring.h"
 #include "logger.h"
-#include "memory.h"
+#include "memmap.h"
 #include "symbols.h"
 #include "types.h"
 
@@ -22,7 +22,7 @@
 typedef struct {
   const char *filename;
   void *dlhandle;
-  MemoryRegionSpan memreg;
+  MemorySpan memreg;
 } PatchData;
 
 static void *adjust_if_relative(void *ptr, void *module_base) {
@@ -168,10 +168,10 @@ cleanup:
 }
 
 static HiResult moduler_apply_module_patch(VectorSymbol *psymbols,
-                                           MemoryRegionSpan module_memory) {
+                                           MemorySpan module_memory) {
 
-  void *module_base = (void *)module_memory.region_start;
+  void *module_base = (void *)module_memory.start;
-  size_t module_size = module_memory.region_end - module_memory.region_start;
+  size_t module_size = module_memory.end - module_memory.start;
 
   void *plt = NULL;
   ElfW(Rela) *jmprel = NULL;
@@ -296,7 +296,7 @@ PatchData moduler_create_patch(ModuleData *module) {
 }
 
 HiResult moduler_reload(VectorModuleData *modules, ModuleData *module,
-                        struct sc_array_memreg *memregs) {
+                        VectorMemoryMap *memmaps) {
 
   // Return OK because this isn't an error case. We just don't do it yet
   // because we only handle reloading a complete solib. Can't do that with
@@ -324,10 +324,10 @@ HiResult moduler_reload(VectorModuleData *modules, ModuleData *module,
   patch.dlhandle = new_handle;
 
   // refresh cache
-  read_memory_maps_self(memregs);
+  memmaps_from_process(memmaps);
 
-  patch.memreg = memory_get_module_span(memregs, patch.filename);
+  patch.memreg = memmaps_find_by_name(patch.filename, memmaps);
-  void *patch_base = (void *)patch.memreg.region_start;
+  void *patch_base = (void *)patch.memreg.start;
 
   VectorSymbol patch_symbols;
   symbol_init_symbols(&patch_symbols);
@@ -346,7 +346,7 @@ HiResult moduler_reload(VectorModuleData *modules, ModuleData *module,
       continue;
 
     log_debug("Patching: %s\n", mod.name);
-    MemoryRegionSpan module_memory = memory_get_module_span(memregs, mod.name);
+    MemorySpan module_memory = memmaps_find_by_name(mod.name, memmaps);
     moduler_apply_module_patch(&patch_symbols, module_memory);
 
     if (strncmp(mod.name, patch.filename, strlen(mod.name)) == 0) {

src/moduler.h

@@ -1,8 +1,7 @@
-#ifndef MODULER_H_
+#pragma once
-#define MODULER_H_
 
 #include "vector.h"
-#include "memory.h"
+#include "memmap.h"
 #include "symbols.h"
 #include "types.h"
 
@@ -41,6 +40,4 @@ static inline bool hi_modinfo_has(ModuleInfo flags, ModuleFlags flag) {
 #define HI_MODINFO_CLEAR(info, flag) ((info) &= ~flag)
 
 HiResult moduler_reload(VectorModuleData *modules, ModuleData *module,
-                        VectorMemoryRegion *memregs);
+                        VectorMemoryMap *memregs);
-
-#endif // MODULER_H_

src/types.h

@@ -1,5 +1,4 @@
-#ifndef TYPES_H_
+#pragma once
-#define TYPES_H_
 
 #include "hitypes.h"
 
@@ -22,5 +21,3 @@ typedef ptrdiff_t ptrdiff;
 
 typedef uint8_t bool8;
 typedef uint32_t bool32;
-
-#endif // TYPES_H_