[PATCH v3 0/1] MR3016: dbghelp: Allocate real_path with the CRT and copy it to the module heap.

[Alex Henrie (＠alexhenrie)]

Fixes both a memory leak and an alloc/free mismatch, and the module heap
is the most appropriate place to hold the variable long-term.

--
  v3: dbghelp: Allocate real_path with the CRT and copy it to the module heap.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3016

[PATCH v3 1/1] dbghelp: Allocate real_path with the CRT and copy it to the module heap.

[Alex Henrie]

From: Alex Henrie <alexhenrie24(a)gmail.com>

Fixes both a memory leak and an alloc/free mismatch, and the module heap
is the most appropriate place to hold the variable long-term.
---
 dlls/dbghelp/dbghelp_private.h |  1 +
 dlls/dbghelp/module.c          |  1 -
 dlls/dbghelp/pe_module.c       | 76 +++++++++++++++++-----------------
 dlls/dbghelp/storage.c         |  7 ++++
 4 files changed, 45 insertions(+), 40 deletions(-)

diff --git a/dlls/dbghelp/dbghelp_private.h b/dlls/dbghelp/dbghelp_private.h
index 7ba10447771..b74c83eb977 100644
--- a/dlls/dbghelp/dbghelp_private.h
+++ b/dlls/dbghelp/dbghelp_private.h
@@ -44,6 +44,7 @@ void     pool_destroy(struct pool* a) DECLSPEC_HIDDEN;
 void*    pool_alloc(struct pool* a, size_t len) __WINE_ALLOC_SIZE(2) __WINE_MALLOC DECLSPEC_HIDDEN;
 void*    pool_realloc(struct pool* a, void* ptr, size_t len) __WINE_ALLOC_SIZE(3) DECLSPEC_HIDDEN;
 char*    pool_strdup(struct pool* a, const char* str) __WINE_MALLOC DECLSPEC_HIDDEN;
+WCHAR*   pool_wcsdup(struct pool* a, const WCHAR* str) __WINE_MALLOC DECLSPEC_HIDDEN;
 
 struct vector
 {
diff --git a/dlls/dbghelp/module.c b/dlls/dbghelp/module.c
index bae4a77f16e..1493d3006cc 100644
--- a/dlls/dbghelp/module.c
+++ b/dlls/dbghelp/module.c
@@ -1068,7 +1068,6 @@ BOOL module_remove(struct process* pcs, struct module* module)
     hash_table_destroy(&module->ht_types);
     HeapFree(GetProcessHeap(), 0, module->sources);
     HeapFree(GetProcessHeap(), 0, module->addr_sorttab);
-    HeapFree(GetProcessHeap(), 0, module->real_path);
     pool_destroy(&module->pool);
     /* native dbghelp doesn't invoke registered callback(,CBA_SYMBOLS_UNLOADED,) here
      * so do we
diff --git a/dlls/dbghelp/pe_module.c b/dlls/dbghelp/pe_module.c
index 7aeb1cf8901..c697d4b7256 100644
--- a/dlls/dbghelp/pe_module.c
+++ b/dlls/dbghelp/pe_module.c
@@ -760,13 +760,10 @@ struct builtin_search
 static BOOL search_builtin_pe(void *param, HANDLE handle, const WCHAR *path)
 {
     struct builtin_search *search = param;
-    size_t size;
 
     if (!pe_map_file(handle, &search->fmap, DMT_PE)) return FALSE;
 
-    size = (lstrlenW(path) + 1) * sizeof(WCHAR);
-    if ((search->path = heap_alloc(size)))
-        memcpy(search->path, path, size);
+    search->path = wcsdup(path);
     return TRUE;
 }
 
@@ -818,47 +815,48 @@ struct module* pe_load_native_module(struct process* pcs, const WCHAR* name,
         }
         if (name) lstrcpyW(loaded_name, name);
     }
-    if (!(modfmt = HeapAlloc(GetProcessHeap(), 0, sizeof(struct module_format) + sizeof(struct pe_module_info))))
-        return NULL;
-    modfmt->u.pe_info = (struct pe_module_info*)(modfmt + 1);
-    if (pe_map_file(hFile, &modfmt->u.pe_info->fmap, DMT_PE))
+
+    if ((modfmt = HeapAlloc(GetProcessHeap(), 0, sizeof(struct module_format) + sizeof(struct pe_module_info))))
     {
-        struct builtin_search builtin = { NULL };
-        if (opened && modfmt->u.pe_info->fmap.u.pe.builtin &&
-            search_dll_path(pcs, loaded_name, modfmt->u.pe_info->fmap.u.pe.file_header.Machine, search_builtin_pe, &builtin))
-        {
-            TRACE("reloaded %s from %s\n", debugstr_w(loaded_name), debugstr_w(builtin.path));
-            image_unmap_file(&modfmt->u.pe_info->fmap);
-            modfmt->u.pe_info->fmap = builtin.fmap;
-            real_path = builtin.path;
-        }
-        if (!base) base = PE_FROM_OPTHDR(&modfmt->u.pe_info->fmap, ImageBase);
-        if (!size) size = PE_FROM_OPTHDR(&modfmt->u.pe_info->fmap, SizeOfImage);
-
-        module = module_new(pcs, loaded_name, DMT_PE, FALSE, base, size,
-                            modfmt->u.pe_info->fmap.u.pe.file_header.TimeDateStamp,
-                            PE_FROM_OPTHDR(&modfmt->u.pe_info->fmap, CheckSum),
-                            modfmt->u.pe_info->fmap.u.pe.file_header.Machine);
-        if (module)
-        {
-            module->real_path = real_path;
-            modfmt->module = module;
-            modfmt->remove = pe_module_remove;
-            modfmt->loc_compute = NULL;
-            module->format_info[DFI_PE] = modfmt;
-            module->reloc_delta = base - PE_FROM_OPTHDR(&modfmt->u.pe_info->fmap, ImageBase);
-        }
-        else
+        modfmt->u.pe_info = (struct pe_module_info*)(modfmt + 1);
+        if (pe_map_file(hFile, &modfmt->u.pe_info->fmap, DMT_PE))
         {
-            ERR("could not load the module '%s'\n", debugstr_w(loaded_name));
-            heap_free(real_path);
-            image_unmap_file(&modfmt->u.pe_info->fmap);
+            struct builtin_search builtin = { NULL };
+            if (opened && modfmt->u.pe_info->fmap.u.pe.builtin &&
+                search_dll_path(pcs, loaded_name, modfmt->u.pe_info->fmap.u.pe.file_header.Machine, search_builtin_pe, &builtin))
+            {
+                TRACE("reloaded %s from %s\n", debugstr_w(loaded_name), debugstr_w(builtin.path));
+                image_unmap_file(&modfmt->u.pe_info->fmap);
+                modfmt->u.pe_info->fmap = builtin.fmap;
+                real_path = builtin.path;
+            }
+            if (!base) base = PE_FROM_OPTHDR(&modfmt->u.pe_info->fmap, ImageBase);
+            if (!size) size = PE_FROM_OPTHDR(&modfmt->u.pe_info->fmap, SizeOfImage);
+
+            module = module_new(pcs, loaded_name, DMT_PE, FALSE, base, size,
+                                modfmt->u.pe_info->fmap.u.pe.file_header.TimeDateStamp,
+                                PE_FROM_OPTHDR(&modfmt->u.pe_info->fmap, CheckSum),
+                                modfmt->u.pe_info->fmap.u.pe.file_header.Machine);
+            if (module)
+            {
+                module->real_path = real_path ? pool_wcsdup(&module->pool, real_path) : NULL;
+                modfmt->module = module;
+                modfmt->remove = pe_module_remove;
+                modfmt->loc_compute = NULL;
+                module->format_info[DFI_PE] = modfmt;
+                module->reloc_delta = base - PE_FROM_OPTHDR(&modfmt->u.pe_info->fmap, ImageBase);
+            }
+            else
+            {
+                ERR("could not load the module '%s'\n", debugstr_w(loaded_name));
+                image_unmap_file(&modfmt->u.pe_info->fmap);
+            }
         }
+        if (!module) HeapFree(GetProcessHeap(), 0, modfmt);
     }
-    if (!module) HeapFree(GetProcessHeap(), 0, modfmt);
 
     if (opened) CloseHandle(hFile);
-
+    free(real_path);
     return module;
 }
 
diff --git a/dlls/dbghelp/storage.c b/dlls/dbghelp/storage.c
index daed3aa8d99..038b625cf1c 100644
--- a/dlls/dbghelp/storage.c
+++ b/dlls/dbghelp/storage.c
@@ -55,6 +55,13 @@ char* pool_strdup(struct pool* pool, const char* str)
     return ret;
 }
 
+WCHAR* pool_wcsdup(struct pool* pool, const WCHAR* str)
+{
+    WCHAR* ret;
+    if ((ret = pool_alloc(pool, (wcslen(str) + 1) * sizeof(WCHAR)))) wcscpy(ret, str);
+    return ret;
+}
+
 void vector_init(struct vector* v, unsigned esz, unsigned bucket_sz)
 {
     v->buckets = NULL;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/3016

[Alex Henrie (＠alexhenrie)]

On Tue Jun 27 02:51:01 2023 +0000, Alex Henrie wrote:
> changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/3016/diffs?diff_id=53948&start_sha=dc6cd8ee44ba9efee7f66663ccfebed4eb82d5d3#dce50b982cbff4a2112d3b8f7ca932ba110760b2_819_818)
> Done.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3016#note_36920

[PATCH v3 0/1] MR3016: dbghelp: Allocate real_path with the CRT and copy it to the module heap. - approved

[eric pouech (＠epo)]

This merge request was approved by eric pouech.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3016