Bug 862343 - assembled code at __restore_rt wastes space
assembled code at __restore_rt wastes space
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: glibc (Show other bugs)
17
x86_64 Linux
unspecified Severity unspecified
: ---
: ---
Assigned To: Jeff Law
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-02 13:04 EDT by John Reiser
Modified: 2016-11-24 10:50 EST (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-10-02 13:12:04 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description John Reiser 2012-10-02 13:04:49 EDT
Description of problem: On x86_64 the assembled instructions at __restore_rt are functionally correct, but waste space.
  (gdb) x/4i __restore_rt
     0x3e8f8359a0 <__restore_rt>:	mov    $0xf,%rax
     0x3e8f8359a7 <__restore_rt+7>:	syscall 
     0x3e8f8359a9 <__restore_rt+9>:	nopl   0x0(%rax)
     0x3e8f8359b0 <__GI___libc_sigaction>:	sub    $0xd0,%rsp

The first instruction "mov $0xf,%rax" takes seven bytes:
  (gdb) x/xg __restore_rt
  0x3e8f8359a0 <__restore_rt>:	0x0f0000000fc0c748
using a REX prefix byte of 0x48 to force 64-bit width, and the 0xC7 opcode which is "mov 32-bit immediate to any destination".  Five bytes would be sufficient:
        movl $0xf,%eax   ###  B8 0F 00 00 00
which uses the specific "mov 32-bit immediate to register EAX" and the implicit zero extension from 32 bits into a 64-bit register.

Because of the two extra bytes, then the 8-byte alignment for the next routine
__GI___libc_sigaction requires an additional 7 bytes for the nopl.  Thus __restore_rt occupies 16 bytes when 8 would suffice.

[How many other instances are there of such waste?]


Version-Release number of selected component (if applicable):
glibc-2.15-56.fc17.x86_64


How reproducible: always


Steps to Reproduce:
1. $ gdb /lib64/libc.so.6
2. (gdb) x/4i __restore_rt
3. (gdb) x/xg __restore_rt
  
Actual results: The first instruction "mov $0xf,%rax" occupies 7 bytes using REX prefix and 0xC7 opcode.  Alignment of the next routine requires a 7-byte NOP.


Expected results: The first instruction "movl $0xf,%eax" achieves the same effect in only 5 bytes.  Alignment of the next routine requires only a 1-byte NOP.


Additional info: The source file is sysdeps/unix/sysv/linux/x86_64/sigaction.c
Comment 1 Jakub Jelinek 2012-10-02 13:12:04 EDT
Changing it would break libgcc and unwinding (at least on kernels that don't provide vDSO, but if they provide vDSO, __restore_rt isn't used at all anyway).
gcc/libgcc/config/i386/linux-unwind.h relies on the exact insn sequence:

  /* movq $__NR_rt_sigreturn, %rax ; syscall.  */
#ifdef __LP64__
#define RT_SIGRETURN_SYSCALL    0x050f0000000fc0c7ULL
#else
#define RT_SIGRETURN_SYSCALL    0x050f40000201c0c7ULL
#endif
  if (*(unsigned char *)(pc+0) == 0x48
      && *(unsigned long long *)(pc+1) == RT_SIGRETURN_SYSCALL)
Comment 2 John Reiser 2012-10-02 13:32:27 EDT
(In reply to comment #1)
> if [the kernel] provides vDSO, [then] __restore_rt isn't used at all anyway.

That is not true in
  kernel-3.5.3-1.fc17.x86_64
  glibc-2.15-56.fc17.x86_64

__restore_rt is used for many signal returns, even when [vdso] is present and used.

Here's a test case:
----- my_test.c
#include <signal.h>
#include <stdio.h>
#include <string.h>

void my_handler(int signo, siginfo_t *si, void *v)
{
	struct ucontext *uc = v;
	printf("signo=%d  si=%p  uc=%p\n", signo, si, uc);
}

main()
{
	struct sigaction act;
	struct sigaction old;

	act.sa_sigaction = my_handler;
	memset(&act.sa_mask, 0, sizeof(act.sa_mask));
	act.sa_flags = 0;

	sigaction (SIGRTMIN, &act, &old);
	kill(getpid(), SIGRTMIN);
	return 0;
}
-----
$ gcc -o my_test -g my_test.c
$ gdb ./my_test
(gdb) b main
(gdb) run

Breakpoint 1, main () at my_test.c:16

(gdb) x/i __restore_rt
   0x3e8f8359a0 <__restore_rt>:	mov    $0xf,%rax
(gdb) b *0x3e8f8359a0
(gdb) c

Program received signal SIG34, Real-time event 34.

(gdb) c
Continuing.
signo=34  si=0x7fffffffdb30  uc=0x7fffffffda00

Breakpoint 2, <signal handler called>

(gdb) x/i $pc
=> 0x3e8f8359a0 <__restore_rt>:	mov    $0xf,%rax   <<<<< __restore_rt is used
(gdb) info proc
process 4312
(gdb) shell cat /proc/4312/maps
7ffff7ffe000-7ffff7fff000 r-xp 00000000 00:00 0                          [vdso]
7ffffffde000-7ffffffff000 rw-p 00000000 00:00 0                          [stack]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]
gdb) shell od -Ax -tx4 /proc/4312/auxv
000000 00000021 00000000 f7ffe000 00007fff   <<<<< [vdso] is present at 0x7ffff7ffe000

$ grep 'AT_.*33' /usr/include/elf.h
#define AT_SYSINFO_EHDR	33
$

Note You need to log in before you can comment on or make changes to this bug.