diff --git a/include/hiload.h b/include/hiload.h index 501ce3c..df48d4f 100644 --- a/include/hiload.h +++ b/include/hiload.h @@ -3,7 +3,9 @@ * developing, significantly tightening the traditional feedback loop. * * Currently not supported: - * - modifying modules loaded at runtime + * - modifying he executable + * - modifying modules loaded at runtime, only those present at startup count + * - modifying two modules with depending changes at the same time */ #ifndef HILOAD_H_ diff --git a/src/files.c b/src/files.c index 9a9549c..5c2176a 100644 --- a/src/files.c +++ b/src/files.c @@ -21,7 +21,7 @@ char *file_to_str_dyn(const char *filename) { return s; } -static FileType file_interpret_as_type(const char *path) { +static FileType file_path_dir_or_file(const char *path) { const char *pathname_end = path + strlen(path); char last_char = *(pathname_end - 1); bool as_dir = (last_char == '/') || (last_char == '\\'); @@ -42,7 +42,7 @@ HiResult file_copy(const char *srcname, const char *destination) { const char *dstname = destination; - FileType dst_type = file_interpret_as_type(destination); + FileType dst_type = file_path_dir_or_file(destination); if (dst_type == HI_FILE_TYPE_DIR) { string_concat_buf(sizeof(buf), buf, destination, strpath_filename(srcname)); dstname = buf; diff --git a/src/hiload.c b/src/hiload.c index adba3b0..b11345d 100644 --- a/src/hiload.c +++ b/src/hiload.c @@ -63,7 +63,8 @@ static int initial_gather_callback(struct dl_phdr_info *info, size_t size, // Store the full name. Differs from dlpi_name for the executable only, // afaik const char *modpath = dl_info.dli_fname; - module.name = strdup(modpath); + module.original_name = strdup(modpath); + module.name = module.original_name; log_debugv("dli_fname: %s\n", dl_info.dli_fname); log_debugv("dli_sname: %s\n", dl_info.dli_sname); @@ -71,7 +72,8 @@ static int initial_gather_callback(struct dl_phdr_info *info, size_t size, } else { // I don't know when this could happen since we're passing the info straight // from the info struct - module.name = strdup(modname); + module.original_name = strdup(modname); + module.name = module.original_name; log_warn("Couldn't load dlinfo for %s: %s\n", module.name, dlerror()); } @@ -123,8 +125,8 @@ module_data_initial_gather(VectorModuleData *modules, size_t enabled_count, bool enable = false; - for (size_t i = 0; i < enabled_count; ++i) { - const char *enabled_module = enabled_modules[i]; + for (size_t j = 0; j < enabled_count; ++j) { + const char *enabled_module = enabled_modules[j]; if (string_is_empty(enabled_module)) { if (modinfo_has(module->info, HI_MODULE_STATE_EXEC)) { enable = true; @@ -193,15 +195,14 @@ static void handle_file_events(FileWatcher *fw, VectorModuleData *modules) { static HiResult reload_dirty_modules(HiloadContext *context) { HiResult ret = HI_OK; - for (u8 i = 0; i < vector_size(&context->modules); i++) { + for (size_t i = 0; i < vector_size(&context->modules); i++) { ModuleData *module = &vector_at(&context->modules, i); - // Operate on dirty modules only if (modinfo_has(module->info, HI_MODULE_STATE_DIRTY)) { - if (modinfo_has(module->info, HI_MODULE_STATE_ENABLED)) { + // Reload only enabled dirty modules log_info("Reloading %s...\n", module->name); - if (!HIOK(moduler_reload(&context->modules, module))) { + if (!HIOK(moduler_reload(&context->modules, i))) { ret = HI_FAIL; log_error("Failed loading: %s\n", module->name); } diff --git a/src/moduler.c b/src/moduler.c index 0b90b99..18bb771 100644 --- a/src/moduler.c +++ b/src/moduler.c @@ -33,9 +33,9 @@ static void *adjust_if_relative(void *ptr, void *module_base) { return (void *)p; } -static HiResult gather_patchable_symbols(VectorSymbol *symbols, - const char *module_name, - void *module_base) { +static HiResult moduler_collect_symbols(VectorSymbol *symbols, + const char *module_name, + void *module_base) { symbol_clear(symbols); HiResult ret = HI_FAIL; @@ -167,8 +167,8 @@ cleanup: return ret; } -static HiResult moduler_apply_module_patch(VectorSymbol *psymbols, - MemorySpan module_memory) { +static HiResult moduler_patch_functions(VectorSymbol *psymbols, + MemorySpan module_memory) { void *module_base = (void *)module_memory.start; size_t module_size = module_memory.end - module_memory.start; @@ -273,8 +273,8 @@ static PatchData moduler_create_patch(ModuleData *module) { } char file_append[32]; - if (strftime(file_append, sizeof(file_append), ".%Y%m%d%H%M%S.patch", t) == - 0) { + const char *patch_format = ".%Y%m%d%H%M%S.patch"; + if (strftime(file_append, sizeof(file_append), patch_format, t) == 0) { log_error("Failed to create patch filename.\n"); return (PatchData){0}; } @@ -294,8 +294,9 @@ static PatchData moduler_create_patch(ModuleData *module) { return data; } -HiResult moduler_reload(VectorModuleData *modules, ModuleData *module) { +HiResult moduler_reload(VectorModuleData *modules, size_t modindx) { + ModuleData *module = &vector_at(modules, modindx); // 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 // executables. @@ -335,7 +336,7 @@ HiResult moduler_reload(VectorModuleData *modules, ModuleData *module) { symbol_init(&patch_symbols); HiResult ret = - gather_patchable_symbols(&patch_symbols, patch.filename, patch_base); + moduler_collect_symbols(&patch_symbols, patch.filename, patch_base); if (!HIOK(ret)) { log_error("Failed to gather symbols for %s\n", patch.filename); return HI_FAIL; @@ -349,33 +350,32 @@ HiResult moduler_reload(VectorModuleData *modules, ModuleData *module) { log_debug("Patching: %s\n", mod.name); MemorySpan module_memory = memmaps_find_by_name(mod.name, &memmaps); - moduler_apply_module_patch(&patch_symbols, module_memory); + moduler_patch_functions(&patch_symbols, module_memory); + // This relies on the patch filename being different only by the append if (strncmp(mod.name, patch.filename, strlen(mod.name)) == 0) { // If patch is for the same module, also collect local object symbols for // coping those over. VectorSymbol module_symbols; symbol_init(&module_symbols); - ret = gather_patchable_symbols(&module_symbols, mod.name, - (void *)mod.address); + ret = moduler_collect_symbols(&module_symbols, mod.name, + (void *)mod.address); if (!HIOK(ret)) { log_error("Failed to gather symbols for %s\n", mod.name); symbol_term(&module_symbols); continue; } - // Copy old data to new data. Breaks with layout changes. - for (size_t i = 0; i < vector_size(&module_symbols); ++i) { - Symbol *sym = &vector_at(&module_symbols, i); + // Copy old data to new data location. Breaks if layout changes, + // e.g. struct fields are moved around + for (size_t j = 0; j < vector_size(&module_symbols); ++j) { + Symbol *sym = &vector_at(&module_symbols, j); if (sym->type == HI_SYMBOL_TYPE_OBJECT) { Symbol *ps = symbol_find(&patch_symbols, sym); if (ps) { - if (ps->size >= sym->size) { - memcpy(ps->address, sym->address, sym->size); - } else { - memcpy(ps->address, sym->address, ps->size); - } + size_t copy_size = MIN(sym->size, ps->size); + memcpy(ps->address, sym->address, copy_size); log_debug("Copied data for symbol: %s\n", sym->name); } } diff --git a/src/moduler.h b/src/moduler.h index 4440b3a..098dbed 100644 --- a/src/moduler.h +++ b/src/moduler.h @@ -16,8 +16,11 @@ typedef enum { typedef u32 ModuleInfo; typedef struct { - /// Owning. Filename for the module. + /// Filename for the module. Owning if not same as original_name; const char *name; + /// Owning. Original filename for the module. Used to track origins for + /// patches. + const char *original_name; /// Handle given by dlopen void *dlhandle; /// Start address for the module @@ -41,4 +44,4 @@ static inline bool modinfo_has(ModuleInfo flags, ModuleFlags flag) { #define HI_MODINFO_SET(info, flag) ((info) |= flag) #define HI_MODINFO_CLEAR(info, flag) ((info) &= ~flag) -HiResult moduler_reload(VectorModuleData *modules, ModuleData *module); +HiResult moduler_reload(VectorModuleData *modules, size_t modindx);