[PATCH] msvcrt: Add DECLSPEC_HOTPATCH to functions patched by libtcmalloc
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45199 Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> --- dlls/msvcrt/heap.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/dlls/msvcrt/heap.c b/dlls/msvcrt/heap.c index 7ea80a3a50..b4f11d8ef4 100644 --- a/dlls/msvcrt/heap.c +++ b/dlls/msvcrt/heap.c @@ -21,6 +21,7 @@ * handler and non atomic heap operations */ +#include "config.h" #include "msvcrt.h" #include "mtdll.h" #include "wine/debug.h" @@ -127,7 +128,7 @@ static MSVCRT_size_t msvcrt_heap_size(void *ptr) /********************************************************************* * ??2(a)YAPAXI@Z (MSVCRT.@) */ -void* CDECL MSVCRT_operator_new(MSVCRT_size_t size) +void* CDECL DECLSPEC_HOTPATCH MSVCRT_operator_new(MSVCRT_size_t size) { void *retval; int freed; @@ -169,7 +170,7 @@ void* CDECL MSVCRT_operator_new_dbg(MSVCRT_size_t size, int type, const char *fi /********************************************************************* * ??3(a)YAXPAX@Z (MSVCRT.@) */ -void CDECL MSVCRT_operator_delete(void *mem) +void CDECL DECLSPEC_HOTPATCH MSVCRT_operator_delete(void *mem) { TRACE("(%p)\n", mem); msvcrt_heap_free(mem); @@ -394,7 +395,7 @@ size_t CDECL _aligned_msize(void *p, MSVCRT_size_t alignment, MSVCRT_size_t offs /********************************************************************* * calloc (MSVCRT.@) */ -void* CDECL MSVCRT_calloc(MSVCRT_size_t count, MSVCRT_size_t size) +void* CDECL DECLSPEC_HOTPATCH MSVCRT_calloc(MSVCRT_size_t count, MSVCRT_size_t size) { MSVCRT_size_t bytes = count*size; @@ -420,7 +421,7 @@ void* CDECL _calloc_base(MSVCRT_size_t count, MSVCRT_size_t size) /********************************************************************* * free (MSVCRT.@) */ -void CDECL MSVCRT_free(void* ptr) +void CDECL DECLSPEC_HOTPATCH MSVCRT_free(void* ptr) { msvcrt_heap_free(ptr); } @@ -459,7 +460,7 @@ void* CDECL _malloc_base(MSVCRT_size_t size) /********************************************************************* * realloc (MSVCRT.@) */ -void* CDECL MSVCRT_realloc(void* ptr, MSVCRT_size_t size) +void* CDECL DECLSPEC_HOTPATCH MSVCRT_realloc(void* ptr, MSVCRT_size_t size) { if (!ptr) return MSVCRT_malloc(size); if (size) return msvcrt_heap_realloc(0, ptr, size); -- 2.18.0
On 07/07/18 00:16, Alex Henrie wrote:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45199 Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> --- dlls/msvcrt/heap.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
Can't we at least wait for a response on the GCC bug first?
On Sat, Jul 7, 2018 at 1:09 AM Zebediah Figura <z.figura12(a)gmail.com> wrote:
On 07/07/18 00:16, Alex Henrie wrote:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45199 Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> --- dlls/msvcrt/heap.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
Can't we at least wait for a response on the GCC bug first?
Even if the GCC bug were fixed immediately, we would still have to add DECLSPEC_HOTPATCH as needed until all major distributions adopt the next version of GCC. I agree that the situation isn't ideal, but on the plus side, even with the 12 functions identified in bug 45199, the functions that applications try to patch are still less than 1% of the total number of Windows functions. I also really want to see if there are still problems with applications patching 64-bit functions. There is no particular hotpatch prologue on x64 Windows. Instead, each application parses the first few bytes of the function, replaces them with a jump, and then reproduces the original opcodes at the location it jumps to before jumping back into the original function. The purpose of ms_hook_prologue on x64 is simply to provide some recognizable opcodes that are easy to remove and reproduce somewhere else. Until GCC 8.1, the instructions that GCC emitted by default happened to be recognizable by most 64-bit programs that do hotpatching. But the 8-byte NOP instruction currently emitted by ms_hook_prologue on x64 may still not be generally recognizable. The Steam Overlay, for example, can deal with eight 0x90 NOPs at the beginning of an x64 function but can't deal with the single 8-byte NOP that GCC currently emits. If other applications also can't deal with the 8-byte NOP then we may be able to convince the GCC developers to change the instructions emitted by ms_hook_prologue on x64 for improved compatibility. (The counterargument would be that since opcode parsing is atomic, if GCC switches from one 8-byte NOP to eight 1-byte NOPs then non-Windows applications that use ms_hook_prologue would have to start locking any thread that attempts to execute a function while it is being patched, to avoid reading from the middle of a recently inserted jump instruction.) But, we won't know what is needed until we get feedback from users about whether or not their 64-bit apps are still broken after adding DECLSPEC_HOTPATCH to them. -Alex
On 07/07/18 11:40, Alex Henrie wrote:
On Sat, Jul 7, 2018 at 1:09 AM Zebediah Figura <z.figura12(a)gmail.com> wrote:
On 07/07/18 00:16, Alex Henrie wrote:
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45199 Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> --- dlls/msvcrt/heap.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
Can't we at least wait for a response on the GCC bug first?
Even if the GCC bug were fixed immediately, we would still have to add DECLSPEC_HOTPATCH as needed until all major distributions adopt the next version of GCC. I agree that the situation isn't ideal, but on the plus side, even with the 12 functions identified in bug 45199, the functions that applications try to patch are still less than 1% of the total number of Windows functions.
Propagation may be slow, but I get the impression that we try to avoid even temporary workarounds if the proper place for the solution is upstream. Also, since bug 45199 only became really relevant after gcc 8.1, I'm not sure how concerned we need to be with waiting for "all major distributions".
participants (2)
-
Alex Henrie -
Zebediah Figura