From 626fa9d31e6d97a7cea9d7297235ae6b9b7b06ab Mon Sep 17 00:00:00 2001 From: Jay Krell Date: Fri, 2 Apr 2021 00:07:49 -0700 Subject: [PATCH] mono_breakpoint_clean_code always returns TRUE. (#16603) --- mono/mini/mini-amd64.c | 2 +- mono/mini/mini-x86.c | 2 +- mono/mini/tramp-amd64.c | 22 ++++++++-------------- mono/mini/tramp-x86.c | 13 +++++-------- 4 files changed, 15 insertions(+), 24 deletions(-) diff --git a/mono/mini/mini-amd64.c b/mono/mini/mini-amd64.c index 2e25c138757..cfdb1b19c9d 100644 --- a/mono/mini/mini-amd64.c +++ b/mono/mini/mini-amd64.c @@ -8323,7 +8323,7 @@ mono_arch_get_patch_offset (guint8 *code) } /** - * \return TRUE if no sw breakpoint was present. + * \return TRUE if no sw breakpoint was present (always). * * Copy \p size bytes from \p code - \p offset to the buffer \p buf. If the debugger inserted software * breakpoints in the original code, they are removed in the copy. diff --git a/mono/mini/mini-x86.c b/mono/mini/mini-x86.c index d3cdf7f0c43..cc4c69619b8 100644 --- a/mono/mini/mini-x86.c +++ b/mono/mini/mini-x86.c @@ -5769,7 +5769,7 @@ mono_arch_get_patch_offset (guint8 *code) } /** - * \return TRUE if no sw breakpoint was present. + * \return TRUE if no sw breakpoint was present (always). * * Copy \p size bytes from \p code - \p offset to the buffer \p buf. If the debugger inserted software * breakpoints in the original code, they are removed in the copy. diff --git a/mono/mini/tramp-amd64.c b/mono/mini/tramp-amd64.c index 2dae6161b9d..8b0f4123de3 100644 --- a/mono/mini/tramp-amd64.c +++ b/mono/mini/tramp-amd64.c @@ -148,18 +148,16 @@ mono_arch_patch_callsite (guint8 *method_start, guint8 *orig_code, guint8 *addr) // last instruction of a function is the call (due to OP_NOT_REACHED) instruction and then directly followed by a // different method. In that case current orig_code points into next method and method_start will also point into // next method, not the method including the call to patch. For this specific case, fallback to using a method_start of NULL. - gboolean can_write = mono_breakpoint_clean_code (method_start != orig_code ? method_start : NULL, orig_code, 14, buf, sizeof (buf)); + mono_breakpoint_clean_code (method_start != orig_code ? method_start : NULL, orig_code, 14, buf, sizeof (buf)); code = buf + 14; /* mov 64-bit imm into r11 (followed by call reg?) or direct call*/ if (((code [-13] == 0x49) && (code [-12] == 0xbb)) || (code [-5] == 0xe8)) { if (code [-5] != 0xe8) { - if (can_write) { - g_assert ((guint64)(orig_code - 11) % 8 == 0); - mono_atomic_xchg_ptr ((gpointer*)(orig_code - 11), addr); - VALGRIND_DISCARD_TRANSLATIONS (orig_code - 11, sizeof (gpointer)); - } + g_assert ((guint64)(orig_code - 11) % 8 == 0); + mono_atomic_xchg_ptr ((gpointer*)(orig_code - 11), addr); + VALGRIND_DISCARD_TRANSLATIONS (orig_code - 11, sizeof (gpointer)); } else { gboolean disp_32bit = ((((gint64)addr - (gint64)orig_code)) < (1 << 30)) && ((((gint64)addr - (gint64)orig_code)) > -(1 << 30)); @@ -178,19 +176,15 @@ mono_arch_patch_callsite (guint8 *method_start, guint8 *orig_code, guint8 *addr) mono_arch_flush_icache (thunk_start, thunk_code - thunk_start); MONO_PROFILER_RAISE (jit_code_buffer, (thunk_start, thunk_code - thunk_start, MONO_PROFILER_CODE_BUFFER_HELPER, NULL)); } - if (can_write) { - mono_atomic_xchg_i32 ((gint32*)(orig_code - 4), ((gint64)addr - (gint64)orig_code)); - VALGRIND_DISCARD_TRANSLATIONS (orig_code - 5, 4); - } + mono_atomic_xchg_i32 ((gint32*)(orig_code - 4), ((gint64)addr - (gint64)orig_code)); + VALGRIND_DISCARD_TRANSLATIONS (orig_code - 5, 4); } } else if ((code [-7] == 0x41) && (code [-6] == 0xff) && (code [-5] == 0x15)) { /* call *(%rip) */ gpointer *got_entry = (gpointer*)((guint8*)orig_code + (*(guint32*)(orig_code - 4))); - if (can_write) { - mono_atomic_xchg_ptr (got_entry, addr); - VALGRIND_DISCARD_TRANSLATIONS (orig_code - 5, sizeof (gpointer)); - } + mono_atomic_xchg_ptr (got_entry, addr); + VALGRIND_DISCARD_TRANSLATIONS (orig_code - 5, sizeof (gpointer)); } } diff --git a/mono/mini/tramp-x86.c b/mono/mini/tramp-x86.c index 7b5386f1a4a..24881c439a1 100644 --- a/mono/mini/tramp-x86.c +++ b/mono/mini/tramp-x86.c @@ -97,7 +97,7 @@ mono_arch_patch_callsite (guint8 *method_start, guint8 *orig_code, guint8 *addr) // last instruction of a function is the call (due to OP_NOT_REACHED) instruction and then directly followed by a // different method. In that case current orig_code points into next method and method_start will also point into // next method, not the method including the call to patch. For this specific case, fallback to using a method_start of NULL. - gboolean can_write = mono_breakpoint_clean_code (method_start != orig_code ? method_start : NULL, orig_code, 8, buf, sizeof (buf)); + mono_breakpoint_clean_code (method_start != orig_code ? method_start : NULL, orig_code, 8, buf, sizeof (buf)); code = buf + 8; @@ -111,16 +111,13 @@ mono_arch_patch_callsite (guint8 *method_start, guint8 *orig_code, guint8 *addr) code -= 6; orig_code -= 6; if (code [1] == 0xe8) { - if (can_write) { - mono_atomic_xchg_i32 ((gint32*)(orig_code + 2), (gsize)addr - ((gsize)orig_code + 1) - 5); + mono_atomic_xchg_i32 ((gint32*)(orig_code + 2), (gsize)addr - ((gsize)orig_code + 1) - 5); - /* Tell valgrind to recompile the patched code */ - VALGRIND_DISCARD_TRANSLATIONS (orig_code + 2, 4); - } + /* Tell valgrind to recompile the patched code */ + VALGRIND_DISCARD_TRANSLATIONS (orig_code + 2, 4); } else if (code [1] == 0xe9) { /* A PLT entry: jmp */ - if (can_write) - mono_atomic_xchg_i32 ((gint32*)(orig_code + 2), (gsize)addr - ((gsize)orig_code + 1) - 5); + mono_atomic_xchg_i32 ((gint32*)(orig_code + 2), (gsize)addr - ((gsize)orig_code + 1) - 5); } else { printf ("Invalid trampoline sequence: %x %x %x %x %x %x n", code [0], code [1], code [2], code [3], code [4], code [5]); -- 2.11.4.GIT