Created attachment 9741 [details] Gentoo build log: sys-libs:glibc-2.23-r3:20170106-093416.log I am compiling all packages on my machines with -fno-omit-frame-pointer because it enables debuggers and performance measurement tools (gdb, perf record, valgrind memcheck, valgrind callgrind). However, glibc-2.23 fails to compile on my system: ../sysdeps/unix/sysv/linux/posix_fallocate.c: In function 'posix_fallocate': ../sysdeps/unix/sysv/linux/posix_fallocate.c:39:1: error: bp cannot be used in asm here ../sysdeps/unix/sysv/linux/posix_fallocate64.c: In function '__posix_fallocate64_l64': ../sysdeps/unix/sysv/linux/posix_fallocate64.c:42:1: error: bp cannot be used in asm here This bug is most likely caused by sysdeps/unix/sysv/linux/i386/sysdep.h:570: # define LOADREGS_6(arg1, arg2, arg3, arg4, arg5, arg6) \ register unsigned int _a6 asm ("ebp") = (unsigned int) (arg6); \ LOADREGS_5 (arg1, arg2, arg3, arg4, arg5)
AFAIK it is not an issue because 'ebp' is this context is used on kernel ABI to pass the 6th argument (the file sysdeps/unix/sysv/linux/i386/sysdep.h contains a comment about the linux convention for i686). Unless there is a compiler option to make easier to create syscall macros (so compiler can restore ebp somehow), you will need to explict disable -fno-omit-frame-pointer on these files. Another option would to code this syscalls in asm, but I will advise against since to make i386 to use the generic code is exactly to avoid the proliferation of such implementations.
Fixed in commit 6b1df8b27f. *** This bug has been marked as a duplicate of bug 20729 ***
(In reply to Adhemerval Zanella from comment #1) > AFAIK it is not an issue because 'ebp' is this context is used on kernel ABI > to pass the 6th argument (the file sysdeps/unix/sysv/linux/i386/sysdep.h > contains a comment about the linux convention for i686). It's an issue as long as it is fixable in glibc. The fact is, it *is* fixable in glibc. It's an issue as long as it is fixable in gcc. The fact is, it *is* fixable in gcc. It's an issue as long as it is fixable in the Linux kernel. The fact is, it *is* fixable in Linux kernel. It's an issue as long as it is fixable on my machine alone. The fact is, it *is* fixable on my machine alone. The question is: which software component is the easiest to change and will have the most impact on the world. > Unless there is a compiler option to make easier to create syscall macros > (so compiler can restore ebp somehow), you will need to explict disable > -fno-omit-frame-pointer on these files. > > Another option would to code this syscalls in asm, but I will advise against > since to make i386 to use the generic code is exactly to avoid the > proliferation of such implementations. The best solution is the update the definition of INTERNAL_SYSCALL with something like this: LOADREGS_##nr(args) asm volatile ( PUSHREGS_ASM_##nr(args) "call *%%gs:%P2" POPREGS_ASM_##nr : "=a" (resultvar) #define PUSHREGS_ASM_5(args...) "" // empty #define PUSHREGS_ASM_6(args...) "push %%ebp; mov arg6, %%ebp;" #define POPREGS_ASM_5 "" // empty #define POPREGS_ASM_6 "pop %%ebp"
(In reply to Andreas Schwab from comment #2) > Fixed in commit 6b1df8b27f. > > *** This bug has been marked as a duplicate of bug 20729 *** https://sourceware.org/git/gitweb.cgi?p=glibc.git;a=commitdiff;h=6b1df8b27f 6b1df8b27f is inferior to the solution I provided in my previous comment. Please revert 6b1df8b27f and commit a patch based on my proposal. Thanks.
(In reply to Jan Ziak (http://atom-symbol.net) from comment #3) > The best solution is the update the definition of INTERNAL_SYSCALL with > something like this: > > LOADREGS_##nr(args) > asm volatile ( > PUSHREGS_ASM_##nr(args) > "call *%%gs:%P2" > POPREGS_ASM_##nr > : "=a" (resultvar) > > #define PUSHREGS_ASM_5(args...) "" // empty > #define PUSHREGS_ASM_6(args...) "push %%ebp; mov arg6, %%ebp;" > > #define POPREGS_ASM_5 "" // empty > #define POPREGS_ASM_6 "pop %%ebp" #define PUSHREGS_ASM_6(args...) "pushl arg6; push %%ebp; mov 4(%%esp), %%ebp;" #define POPREGS_ASM_6 "pop %%ebp; add $4, %%esp"
(In reply to Jan Ziak (http://atom-symbol.net) from comment #3) > (In reply to Adhemerval Zanella from comment #1) > > AFAIK it is not an issue because 'ebp' is this context is used on kernel ABI > > to pass the 6th argument (the file sysdeps/unix/sysv/linux/i386/sysdep.h > > contains a comment about the linux convention for i686). > > It's an issue as long as it is fixable in glibc. The fact is, it *is* > fixable in glibc. > > It's an issue as long as it is fixable in gcc. The fact is, it *is* fixable > in gcc. > > It's an issue as long as it is fixable in the Linux kernel. The fact is, it > *is* fixable in Linux kernel. > > It's an issue as long as it is fixable on my machine alone. The fact is, it > *is* fixable on my machine alone. > > The question is: which software component is the easiest to change and will > have the most impact on the world. It was a bad wording from my part, my idea is since we already have a solution for correct 6-argument passing for compiler that might use the 6th argument passing it should not be an issue on GLIBC. However I wrongly assumed this could not be accomplished on this code, which it wrong. > > > Unless there is a compiler option to make easier to create syscall macros > > (so compiler can restore ebp somehow), you will need to explict disable > > -fno-omit-frame-pointer on these files. > > > > Another option would to code this syscalls in asm, but I will advise against > > since to make i386 to use the generic code is exactly to avoid the > > proliferation of such implementations. > > The best solution is the update the definition of INTERNAL_SYSCALL with > something like this: > > LOADREGS_##nr(args) > asm volatile ( > PUSHREGS_ASM_##nr(args) > "call *%%gs:%P2" > POPREGS_ASM_##nr > : "=a" (resultvar) > > #define PUSHREGS_ASM_5(args...) "" // empty > #define PUSHREGS_ASM_6(args...) "push %%ebp; mov arg6, %%ebp;" > > #define POPREGS_ASM_5 "" // empty > #define POPREGS_ASM_6 "pop %%ebp" Your solution is wrong and do not address the issue in question. This snippet is not related to C macros for syscall macros, but rather to auto-generated syscalls which is code and implemented directly in assembly. Six argument syscalls from C macros were fixed by a9fe4c5aa8e53ee3 and then optimized on 98ad631cd0a77. The optimization takes in consideration than if you are using GCC 5, the compiler an properly spill %ebx when needed, we can inline syscalls with 6 arguments. The problem is it does not take in consideration that for fomit-frame-pointer compiler option on i686 we can not use this optimization (as for PROF which was corrected by 95b097779a6). I could get a working build with the scratch patch: ----- diff --git a/sysdeps/unix/sysv/linux/i386/Makefile b/sysdeps/unix/sysv/linux/i386/Makefile index 9609752..cd5bb43 100644 --- a/sysdeps/unix/sysv/linux/i386/Makefile +++ b/sysdeps/unix/sysv/linux/i386/Makefile @@ -3,7 +3,7 @@ default-abi := 32 # %ebp is used to pass the 6th argument to system calls, so these # system calls are incompatible with a frame pointer. -uses-6-syscall-arguments = -fomit-frame-pointer +uses-6-syscall-arguments = -DNO_OPTIMIZE_FOR_GCC_5 ifeq ($(subdir),misc) sysdep_routines += ioperm iopl vm86 @@ -24,6 +24,7 @@ CFLAGS-semtimedop.os += $(uses-6-syscall-arguments) endif ifeq ($(subdir),elf) +sysdep-dl-routines += libc-do-syscall sysdep-others += lddlibc4 install-bin += lddlibc4 endif @@ -61,6 +62,8 @@ ifeq ($(subdir),nptl) # pull in __syscall_error routine libpthread-routines += sysdep libpthread-shared-only-routines += sysdep +CFLAGS-pthread_cond_wait.o += $(uses-6-syscall-arguments) +CFLAGS-pthread_cond_wait.os += $(uses-6-syscall-arguments) CFLAGS-pthread_rwlock_timedrdlock.o += $(uses-6-syscall-arguments) CFLAGS-pthread_rwlock_timedrdlock.os += $(uses-6-syscall-arguments) CFLAGS-pthread_rwlock_timedwrlock.o += $(uses-6-syscall-arguments) diff --git a/sysdeps/unix/sysv/linux/i386/libc-do-syscall.S b/sysdeps/unix/sysv/linux/i386/libc-do-syscall.S index f706c6d..ef9e513 100644 --- a/sysdeps/unix/sysv/linux/i386/libc-do-syscall.S +++ b/sysdeps/unix/sysv/linux/i386/libc-do-syscall.S @@ -18,7 +18,7 @@ #include <sysdep.h> -#ifndef OPTIMIZE_FOR_GCC_5 +//#ifndef OPTIMIZE_FOR_GCC_5 /* %eax, %ecx, %edx and %esi contain the values expected by the kernel. %edi points to a structure with the values of %ebx, %edi and %ebp. */ @@ -50,4 +50,4 @@ ENTRY (__libc_do_syscall) cfi_restore (ebx) ret END (__libc_do_syscall) -#endif +//#endif diff --git a/sysdeps/unix/sysv/linux/i386/sysdep.h b/sysdeps/unix/sysv/linux/i386/sysdep.h index baf4642..e35cc02 100644 --- a/sysdeps/unix/sysv/linux/i386/sysdep.h +++ b/sysdeps/unix/sysv/linux/i386/sysdep.h @@ -46,7 +46,7 @@ to compile glibc. Disable GCC 5 optimization when compiling for profiling since asm ("ebp") can't be used to put the 6th argument in %ebp for syscall. */ -#if __GNUC_PREREQ (5,0) && !defined PROF +#if __GNUC_PREREQ (5,0) && !defined PROF && !defined NO_OPTIMIZE_FOR_GCC_5 # define OPTIMIZE_FOR_GCC_5 #endif ----- It is not optimal since we want NO_OPTIMIZE_FOR_GCC_5 only when omit-frame-pointer is used and it will require most likely an configure check to do so. I will think in a better solution.
(In reply to Adhemerval Zanella from comment #6) > The optimization takes in consideration than if > you are using GCC 5, the compiler an properly spill %ebx when needed, we can > inline syscalls with 6 arguments. Just a note: stack frame pointer is stored in %ebp, not %ebx.
This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "GNU C Library master sources". The branch, master has been updated via 3b33d6ed6096c1d20d05a650b06026d673f7399a (commit) from 52ac22365a332cacf7aa97f1b41b3a0adfaff778 (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=3b33d6ed6096c1d20d05a650b06026d673f7399a commit 3b33d6ed6096c1d20d05a650b06026d673f7399a Author: Adhemerval Zanella <adhemerval.zanella@linaro.org> Date: Sun Jan 8 11:38:23 2017 -0200 Rework -fno-omit-frame-pointer support on i386 Commit 6b1df8b27f fixed the -OS build issue on i386 (BZ#20729) by expliciting disabling frame pointer (-fomit-frame-pointer) on the faulty objects. Although it does fix the issue, it is a subpar workaround that adds complexity in build process (a rule for each object to add the required compiler option and pontentially more rules for objects that call {INLINE,INTERNAL}_SYSCALL) and does not allow the implementations to get all the possible debug/calltrack information possible (used mainly in debuggers and performance measurement tools). This patch fixes it by adding an explicit configure check to see if -fno-omit-frame-pointer is set and to act accordingly (set or not OPTIMIZE_FOR_GCC_5). The make rules is simplified and only one is required: to add libc-do-syscall on loader due mmap (which will be empty anyway for default build with -fomit-frame-pointer). Checked on i386-linux-gnu with GCC 6.2.1 with CFLAGS sets as '-Os', '-O2 -fno-omit-frame-pointer', and '-O2 -fomit-frame-pointer'. For '-Os' the testsuite issues described by BZ#19463 and BZ#15105 still applied. It fixes BZ #21029, although it is marked as duplicated of #20729 (I reopened to track this cleanup). [BZ #21029] * config.h.in [CAN_USE_REGISTER_ASM_EBP]: New define. * sysdeps/unix/sysv/linux/i386/Makefile [$(subdir) = elf] (sysdep-dl-routines): Add libc-do-syscall. (uses-6-syscall-arguments): Remove. [$(subdir) = misc] (CFLAGS-epoll_pwait.o): Likewise. [$(subdir) = misc] (CFLAGS-epoll_pwait.os): Likewise. [$(subdir) = misc] (CFLAGS-mmap.o): Likewise. [$(subdir) = misc] (CFLAGS-mmap.os): Likewise. [$(subdir) = misc] (CFLAGS-mmap64.o): Likewise. [$(subdir) = misc] (CFLAGS-mmap64.os): Likewise. [$(subdir) = misc] (CFLAGS-pselect.o): Likewise. [$(subdir) = misc] (cflags-pselect.o): Likewise. [$(subdir) = misc] (cflags-pselect.os): Likewise. [$(subdir) = misc] (cflags-rtld-mmap.os): Likewise. [$(subdir) = sysvipc] (cflags-semtimedop.o): Likewise. [$(subdir) = sysvipc] (cflags-semtimedop.os): Likewise. [$(subdir) = io] (CFLAGS-posix_fadvise64.o): Likewise. [$(subdir) = io] (CFLAGS-posix_fadvise64.os): Likewise. [$(subdir) = io] (CFLAGS-posix_fallocate.o): Likewise. [$(subdir) = io] (CFLAGS-posix_fallocate.os): Likewise. [$(subdir) = io] (CFLAGS-posix_fallocate64.o): Likewise. [$(subdir) = io] (CFLAGS-posix_fallocate64.os): Likewise. [$(subdir) = io] (CFLAGS-sync_file_range.o): Likewise. [$(subdir) = io] (CFLAGS-sync_file_range.os): Likewise. [$(subdir) = io] (CFLAGS-fallocate.o): Likewise. [$(subdir) = io] (CFLAGS-fallocate.os): Likewise. [$(subdir) = io] (CFLAGS-fallocate64.o): Likewise. [$(subdir) = io] (CFLAGS-fallocate64.os): Likewise. [$(subdir) = nptl] (CFLAGS-pthread_rwlock_timedrdlock.o): Likewise. [$(subdir) = nptl] (CFLAGS-pthread_rwlock_timedrdlock.os): Likewise. [$(subdir) = nptl] (CFLAGS-pthread_rwlock_timedrwlock.o): Likewise. [$(subdir) = nptl] (CFLAGS-pthread_rwlock_timedrwlock.os): Likewise. [$(subdir) = nptl] (CFLAGS-sem_wait.o): Likewise. [$(subdir) = nptl] (CFLAGS-sem_wait.os): Likewise. [$(subdir) = nptl] (CFLAGS-sem_timedwait.o): Likewise. [$(subdir) = nptl] (CFLAGS-sem_timedwait.os): Likewise. * sysdeps/unix/sysv/linux/i386/configure.ac: Add check if compiler allows ebp on inline assembly. * sysdeps/unix/sysv/linux/i386/configure: Regenerate. * sysdeps/unix/sysv/linux/i386/sysdep.h (OPTIMIZE_FOR_GCC_5): Set if CAN_USE_REGISTER_ASM_EBP is set. (check_consistency): Likewise. ----------------------------------------------------------------------- Summary of changes: ChangeLog | 49 +++++++++++++++++++++++++++++ config.h.in | 4 ++ sysdeps/unix/sysv/linux/i386/Makefile | 39 +---------------------- sysdeps/unix/sysv/linux/i386/configure | 39 +++++++++++++++++++++++ sysdeps/unix/sysv/linux/i386/configure.ac | 17 ++++++++++ sysdeps/unix/sysv/linux/i386/sysdep.h | 6 ++-- 6 files changed, 113 insertions(+), 41 deletions(-)
Fixed by 3b33d6ed6096c1d.
(In reply to Adhemerval Zanella from comment #9) > Fixed by 3b33d6ed6096c1d. Thanks.
(In reply to Adhemerval Zanella from comment #9) > Fixed by 3b33d6ed6096c1d. The Changelog contains the text "2017-01-17 Adhemerval Zanella ...". Shouldn't the correct date be 2017-02-17?
I will fix it.