x86: Fixup some nits in longjmp asm implementation

Message ID 20240105191240.3646771-1-goldstein.w.n@gmail.com
State Changes Requested
Headers
Series x86: Fixup some nits in longjmp asm implementation |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch fail Patch failed to apply to master at the time it was sent
redhat-pt-bot/TryBot-32bit fail Patch series failed to apply

Commit Message

Noah Goldstein Jan. 5, 2024, 7:12 p.m. UTC
  Replace a stray `nop` with a `.p2align` directive.
---
 .../unix/sysv/linux/x86_64/____longjmp_chk.S  |  2 +-
 sysdeps/x86_64/dl-machine.h                   | 31 +++++++++----------
 2 files changed, 16 insertions(+), 17 deletions(-)
  

Comments

H.J. Lu Jan. 5, 2024, 8:09 p.m. UTC | #1
On Fri, Jan 5, 2024 at 11:12 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> Replace a stray `nop` with a `.p2align` directive.
> ---
>  .../unix/sysv/linux/x86_64/____longjmp_chk.S  |  2 +-
>  sysdeps/x86_64/dl-machine.h                   | 31 +++++++++----------
>  2 files changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S b/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S
> index 9aa24620b9..9d9732afdc 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S
> +++ b/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S
> @@ -57,8 +57,8 @@ longjmp_msg:
>         cfi_def_cfa_offset(16);                                         \
>         LOAD_MSG;                                                       \
>         call    HIDDEN_JUMPTARGET(__fortify_fail);                      \
> -       nop;                                                            \
>         cfi_restore_state;                                              \
> +       .p2align 3, 5;                                                          \
>  .Lok2:                                                                 \
>         movq    %r10, %rdi;                                             \
>         cfi_restore (%rdi);                                             \
> diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
> index adcc0f7113..7ef6e24eb5 100644
> --- a/sysdeps/x86_64/dl-machine.h
> +++ b/sysdeps/x86_64/dl-machine.h
> @@ -618,25 +618,24 @@ x86_64_rewrite_plt (struct link_map *map, ElfW(Addr) plt_rewrite)
>         if (((uint64_t) disp + (uint64_t) ((uint32_t) INT32_MIN))
>             <= (uint64_t) UINT32_MAX)
>           {
> -           pad = branch_start + JMP32_INSN_SIZE;
> +         pad = branch_start + JMP32_INSN_SIZE;

These changes are unrelated.

> -           if (__glibc_unlikely (pad > plt_end))
> -             continue;
> +         if (__glibc_unlikely (pad > plt_end))
> +           continue;
>
> -           /* If the target branch can be reached with a direct branch,
> -              rewrite the PLT entry with a direct branch.  */
> -           if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_BINDINGS))
> -             {
> -               const char *sym_name = x86_64_reloc_symbol_name (map,
> -                                                                reloc);
> -               _dl_debug_printf ("changing '%s' PLT entry in '%s' to "
> -                                 "direct branch\n", sym_name,
> -                                 DSO_FILENAME (map->l_name));
> -             }
> +         /* If the target branch can be reached with a direct branch,
> +            rewrite the PLT entry with a direct branch.  */
> +         if (__glibc_unlikely (GLRO (dl_debug_mask) & DL_DEBUG_BINDINGS))
> +           {
> +             const char *sym_name = x86_64_reloc_symbol_name (map, reloc);
> +             _dl_debug_printf ("changing '%s' PLT entry in '%s' to "
> +                               "direct branch\n",
> +                               sym_name, DSO_FILENAME (map->l_name));
> +           }
>
> -           /* Write out direct branch.  */
> -           *(uint8_t *) branch_start = JMP32_INSN_OPCODE;
> -           *((uint32_t *) (branch_start + 1)) = disp;
> +         /* Write out direct branch.  */
> +         *(uint8_t *) branch_start = JMP32_INSN_OPCODE;
> +         *((uint32_t *) (branch_start + 1)) = disp;
>           }
>         else
>           {
> --
> 2.34.1
>
  

Patch

diff --git a/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S b/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S
index 9aa24620b9..9d9732afdc 100644
--- a/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S
+++ b/sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S
@@ -57,8 +57,8 @@  longjmp_msg:
 	cfi_def_cfa_offset(16);						\
 	LOAD_MSG;							\
 	call	HIDDEN_JUMPTARGET(__fortify_fail);			\
-	nop;								\
 	cfi_restore_state;						\
+	.p2align 3, 5;								\
 .Lok2:									\
 	movq	%r10, %rdi;						\
 	cfi_restore (%rdi);						\
diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
index adcc0f7113..7ef6e24eb5 100644
--- a/sysdeps/x86_64/dl-machine.h
+++ b/sysdeps/x86_64/dl-machine.h
@@ -618,25 +618,24 @@  x86_64_rewrite_plt (struct link_map *map, ElfW(Addr) plt_rewrite)
 	if (((uint64_t) disp + (uint64_t) ((uint32_t) INT32_MIN))
 	    <= (uint64_t) UINT32_MAX)
 	  {
-	    pad = branch_start + JMP32_INSN_SIZE;
+	  pad = branch_start + JMP32_INSN_SIZE;
 
-	    if (__glibc_unlikely (pad > plt_end))
-	      continue;
+	  if (__glibc_unlikely (pad > plt_end))
+	    continue;
 
-	    /* If the target branch can be reached with a direct branch,
-	       rewrite the PLT entry with a direct branch.  */
-	    if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_BINDINGS))
-	      {
-		const char *sym_name = x86_64_reloc_symbol_name (map,
-								 reloc);
-		_dl_debug_printf ("changing '%s' PLT entry in '%s' to "
-				  "direct branch\n", sym_name,
-				  DSO_FILENAME (map->l_name));
-	      }
+	  /* If the target branch can be reached with a direct branch,
+	     rewrite the PLT entry with a direct branch.  */
+	  if (__glibc_unlikely (GLRO (dl_debug_mask) & DL_DEBUG_BINDINGS))
+	    {
+	      const char *sym_name = x86_64_reloc_symbol_name (map, reloc);
+	      _dl_debug_printf ("changing '%s' PLT entry in '%s' to "
+				"direct branch\n",
+				sym_name, DSO_FILENAME (map->l_name));
+	    }
 
-	    /* Write out direct branch.  */
-	    *(uint8_t *) branch_start = JMP32_INSN_OPCODE;
-	    *((uint32_t *) (branch_start + 1)) = disp;
+	  /* Write out direct branch.  */
+	  *(uint8_t *) branch_start = JMP32_INSN_OPCODE;
+	  *((uint32_t *) (branch_start + 1)) = disp;
 	  }
 	else
 	  {