Fix MIPS waitid build [committed]
diff mbox

Message ID alpine.DEB.2.10.1412101850280.32166@digraph.polyomino.org.uk
State New
Headers show

Commit Message

Joseph Myers Dec. 10, 2014, 6:50 p.m. UTC
As previously discussed in
<https://sourceware.org/ml/libc-alpha/2012-11/msg00798.html>, MIPS (o32)
waitid has build warnings (now errors) because a function is declared
inline but functions with five-argument syscalls cannot be inlined for
MIPS o32.

This patch disables the -Winline warnings for waitid.c using a
MIPS-specific wrapper file.  As it's whole-file disabling, there's no
point in using push and pop, so just DIAG_IGNORE_NEEDS_COMMENT is
used.

Committed.

2014-12-10  Joseph Myers  <joseph@codesourcery.com>

	* sysdeps/unix/sysv/linux/mips/mips32/waitid.c: New file.

Comments

Rich Felker Dec. 10, 2014, 7:32 p.m. UTC | #1
On Wed, Dec 10, 2014 at 06:50:45PM +0000, Joseph Myers wrote:
> As previously discussed in
> <https://sourceware.org/ml/libc-alpha/2012-11/msg00798.html>, MIPS (o32)
> waitid has build warnings (now errors) because a function is declared
> inline but functions with five-argument syscalls cannot be inlined for
> MIPS o32.
> 
> This patch disables the -Winline warnings for waitid.c using a
> MIPS-specific wrapper file.  As it's whole-file disabling, there's no
> point in using push and pop, so just DIAG_IGNORE_NEEDS_COMMENT is
> used.
> 
> Committed.
> 
> 2014-12-10  Joseph Myers  <joseph@codesourcery.com>
> 
> 	* sysdeps/unix/sysv/linux/mips/mips32/waitid.c: New file.
> 
> diff --git a/sysdeps/unix/sysv/linux/mips/mips32/waitid.c b/sysdeps/unix/sysv/linux/mips/mips32/waitid.c
> new file mode 100644
> index 0000000..c18a57c
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/mips/mips32/waitid.c
> @@ -0,0 +1,7 @@
> +#include <libc-internal.h>
> +
> +/* MIPS forces a frame pointer for five-argument syscalls using
> +   alloca, so resulting in "inlining failed in call to 'do_waitid':
> +   function not inlinable".  */
> +DIAG_IGNORE_NEEDS_COMMENT (4.9, "-Winline");
> +#include <sysdeps/unix/sysv/linux/waitid.c>

Wouldn't it be preferable just to fix whatever the underlying cause
is? It sounds like some ugly and fragile hack is being used to
position the syscall arguments on the stack and I wouldn't be
surprised if it's only working by chance. I've seen other bad code
like this in the mips port that's only working by chance, for example
in setjmp/longjmp.

Rich
Joseph Myers Dec. 10, 2014, 9:28 p.m. UTC | #2
On Wed, 10 Dec 2014, Rich Felker wrote:

> Wouldn't it be preferable just to fix whatever the underlying cause
> is? It sounds like some ugly and fragile hack is being used to
> position the syscall arguments on the stack and I wouldn't be
> surprised if it's only working by chance. I've seen other bad code

A single asm adjusts the stack, puts arguments there, does the syscall and 
restores the stack.  The frame pointer is needed to avoid the stack 
pointer adjustment breaking unwind info.
Roland McGrath Dec. 11, 2014, 6 p.m. UTC | #3
> A single asm adjusts the stack, puts arguments there, does the syscall and 
> restores the stack.  The frame pointer is needed to avoid the stack 
> pointer adjustment breaking unwind info.

Or it could use .cfi_* directives inside the asm.

Patch
diff mbox

diff --git a/sysdeps/unix/sysv/linux/mips/mips32/waitid.c b/sysdeps/unix/sysv/linux/mips/mips32/waitid.c
new file mode 100644
index 0000000..c18a57c
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/mips/mips32/waitid.c
@@ -0,0 +1,7 @@ 
+#include <libc-internal.h>
+
+/* MIPS forces a frame pointer for five-argument syscalls using
+   alloca, so resulting in "inlining failed in call to 'do_waitid':
+   function not inlinable".  */
+DIAG_IGNORE_NEEDS_COMMENT (4.9, "-Winline");
+#include <sysdeps/unix/sysv/linux/waitid.c>