Skip to contentSkip to conversation

work around duff ABIs

PATCH In review Matthew Wilcox 5 participants 11 messages Oct 20, 2002 – Nov 4, 2002

lore
Share & tools
lore

Matthew Wilcox

author
#0 lore raw

PATCH 1/1 work around duff ABIs

*sigh*. i hate this kind of bullshit. please, don't anyone ever try to pass 64-bit args through the syscall interface again.

Index: fs/read_write.c
===================================================================
RCS file: /var/cvs/linux-2.5/fs/read_write.c,v
retrieving revision 1.5

read_write.c +17 -4
diff -u -p -r1.5 read_write.c
@@ -14,6 +14,7 @@
 #include <linux/security.h>
 #include <linux/module.h>
 
+#include <asm/byteorder.h>
 #include <asm/uaccess.h>
 
 struct file_operations generic_ro_fops = {

@@ -285,9 +286,15 @@ asmlinkage ssize_t sys_write(unsigned in
 	return ret;
 }
 
-asmlinkage ssize_t sys_pread64(unsigned int fd, char *buf,
-			     size_t count, loff_t pos)
+#ifdef __BIG_ENDIAN
+asmlinkage ssize_t sys_pread64(unsigned int fd, char *buf, size_t count,
+				unsigned int high, unsigned int low)
+#else
+asmlinkage ssize_t sys_pread64(unsigned int fd, char *buf, size_t count,
+				unsigned int low, unsigned int high)
+#endif
 {
+	loff_t pos = (loff_t)high << 32 | low;
 	struct file *file;
 	ssize_t ret = -EBADF;
 

@@ -303,9 +310,15 @@ asmlinkage ssize_t sys_pread64(unsigned 
 	return ret;
 }
 
-asmlinkage ssize_t sys_pwrite64(unsigned int fd, const char *buf,
-			      size_t count, loff_t pos)
+#ifdef __BIG_ENDIAN
+asmlinkage ssize_t sys_pwrite64(unsigned int fd, const char *buf, size_t count,
+				unsigned int high, unsigned int low)
+#else
+asmlinkage ssize_t sys_pwrite64(unsigned int fd, const char *buf, size_t count,
+				unsigned int low, unsigned int high)
+#endif
 {
+	loff_t pos = (loff_t)high << 32 | low;
 	struct file *file;
 	ssize_t ret = -EBADF;

Index: fs/open.c
===================================================================
RCS file: /var/cvs/linux-2.5/fs/open.c,v
retrieving revision 1.4

open.c +23 -6
diff -u -p -r1.4 open.c
@@ -18,6 +18,7 @@
 #include <linux/backing-dev.h>
 #include <linux/security.h>
 
+#include <asm/byteorder.h>
 #include <asm/uaccess.h>
 
 #define special_file(m) (S_ISCHR(m)||S_ISBLK(m)||S_ISFIFO(m)||S_ISSOCK(m))

@@ -205,18 +206,34 @@ asmlinkage long sys_ftruncate(unsigned i
 	return do_sys_ftruncate(fd, length, 1);
 }
 
-/* LFS versions of truncate are only needed on 32 bit machines */
+/*
+ * LFS versions of truncate are only needed on 32 bit machines.  PA-RISC
+ * and MIPS ABIs specify 64-bit alignment for 64-bit quantities, but glibc
+ * ignores this and passes 64-bit quantities in misaligned registers.
+ */
 #if BITS_PER_LONG == 32
-asmlinkage long sys_truncate64(const char * path, loff_t length)
+#ifdef __BIG_ENDIAN
+asmlinkage long sys_truncate64(const char * path, unsigned int high, unsigned int low)
 {
-	return do_sys_truncate(path, length);
+	return do_sys_truncate(path, (loff_t)high << 32 | low);
 }
 
-asmlinkage long sys_ftruncate64(unsigned int fd, loff_t length)
+asmlinkage long sys_ftruncate64(unsigned int fd, unsigned int high, unsigned int low)
 {
-	return do_sys_ftruncate(fd, length, 0);
+	return do_sys_ftruncate(fd, (loff_t)high << 32 | low, 0);
 }
-#endif
+#else	/* !__BIG_ENDIAN */
+asmlinkage long sys_truncate64(const char * path, unsigned int low, unsigned int high)
+{
+	return do_sys_truncate(path, (loff_t)high << 32 | low);
+}
+
+asmlinkage long sys_ftruncate64(unsigned int fd, unsigned int low, unsigned int high)
+{
+	return do_sys_ftruncate(fd, (loff_t)high << 32 | low, 0);
+}
+#endif	/* !__BIG_ENDIAN */
+#endif	/* BITS_PER_LONG == 32 */
 
 #if !(defined(__alpha__) || defined(__ia64__))
Signature · 1 line
Revolutions do not require corporate support.
#1 lore raw
Quoted 3 lines · Matthew Wilcox

*sigh*. i hate this kind of bullshit. please, don't anyone ever try to pass 64-bit args through the syscall interface again.

I agree it can be a pain.

[-----------snip-------------]

read_write.c asmlinkage ssize_t sys_write(unsigned in · line 295

Quoted 9 lines
read_write.c +7 -2
-asmlinkage ssize_t sys_pread64(unsigned int fd, char *buf,
-			     size_t count, loff_t pos)
+#ifdef __BIG_ENDIAN
+asmlinkage ssize_t sys_pread64(unsigned int fd, char *buf, size_t count,
+				unsigned int high, unsigned int low)
+#else
+asmlinkage ssize_t sys_pread64(unsigned int fd, char *buf, size_t count,
+				unsigned int low, unsigned int high)
+#endif

Nonono. Please see __LONG_LONG_PAIR in /usr/include/endian.h. Your user space code should be doing something like this:

    static inline _syscall5(ssize_t, __syscall_pread, int, fd, void *, buf,
	    size_t, count, off_t, offset_hi, off_t, offset_lo);

    ssize_t __libc_pread(int fd, void *buf, size_t count, off_t offset)
    {
	return(__syscall_pread(fd,buf,count,__LONG_LONG_PAIR((off_t)0,offset)));
    }

    ssize_t __libc_pread64(int fd, void *buf, size_t count, off64_t offset)
    {
	return(__syscall_pread(fd, buf, count,
		    __LONG_LONG_PAIR((off_t)(offset>>32),
		    (off_t)(offset&0xffffffff))));
    }

Your patch is going to break GNU libc, uClibc, and anyone else in userspace that is doing pread and pread64 correctly....

-Erik

--
Erik B. Andersen http://codepoet-consulting.com/
--This message was written using 73% post-consumer electrons--

author
#2 lore raw
Quoted 18 lines · Erik Andersen

Nonono. Please see __LONG_LONG_PAIR in /usr/include/endian.h. Your user space code should be doing something like this:

    static inline _syscall5(ssize_t, __syscall_pread, int, fd, void *, buf,
	    size_t, count, off_t, offset_hi, off_t, offset_lo);

    ssize_t __libc_pread(int fd, void *buf, size_t count, off_t offset)
    {
	return(__syscall_pread(fd,buf,count,__LONG_LONG_PAIR((off_t)0,offset)));
    }

    ssize_t __libc_pread64(int fd, void *buf, size_t count, off64_t offset)
    {
	return(__syscall_pread(fd, buf, count,
		    __LONG_LONG_PAIR((off_t)(offset>>32),
		    (off_t)(offset&0xffffffff))));
    }
Quoted 2 lines

Your patch is going to break GNU libc, uClibc, and anyone else in userspace that is doing pread and pread64 correctly....

Well... since you just proved that you don't understand the problem, I'd hazard a guess that uClibc is also broken, as well as glibc.

Here's how the ABI specifies that pread() shall take its arguments:

asmlinkage ssize_t sys_pread64(unsigned int fd, char *buf, size_t count,
loff_t pos)

fd arg0
buf arg1
count arg2
pos arg4 & arg5

Here's what __LONG_LONG_PAIR does:

fd arg0
buf arg1
count arg2
pos(HI) arg3
pos(LO) arg4

Signature · 1 line
Revolutions do not require corporate support.
#3 lore raw
Quoted 3 lines · Matthew Wilcox

asmlinkage ssize_t sys_pread64(unsigned int fd, char *buf, size_t count,
loff_t pos)

Which ABI. If its the hppa ABI then its however you specified it and however your call setup/return code handles it. If that needs glue and your own syscall vectors calling sys_pread64 indirectly BFD.

#4 lore raw
In reply to Matthew Wilcox
Quoted 19 lines · Matthew Wilcox
Quoted 18 lines · Erik Andersen

Nonono. Please see __LONG_LONG_PAIR in /usr/include/endian.h. Your user space code should be doing something like this:

    static inline _syscall5(ssize_t, __syscall_pread, int, fd, void *, buf,
	    size_t, count, off_t, offset_hi, off_t, offset_lo);

    ssize_t __libc_pread(int fd, void *buf, size_t count, off_t offset)
    {
	return(__syscall_pread(fd,buf,count,__LONG_LONG_PAIR((off_t)0,offset)));
    }

    ssize_t __libc_pread64(int fd, void *buf, size_t count, off64_t offset)
    {
	return(__syscall_pread(fd, buf, count,
		    __LONG_LONG_PAIR((off_t)(offset>>32),
		    (off_t)(offset&0xffffffff))));
    }
Quoted 5 lines
Quoted 2 lines

Your patch is going to break GNU libc, uClibc, and anyone else in userspace that is doing pread and pread64 correctly....

Well... since you just proved that you don't understand the problem, I'd hazard a guess that uClibc is also broken, as well as glibc.

I understand the problem very well. Passing 64 bit stuff via syscalls is a major pain in the butt. But your patch is not just changing hppa and mips -- you are breaking the ABI on x86, arm, powerpc, etc, etc. etc where it is currently working. Look very closely at your patch. See those endianness ifdefs? You are adding endianness specific ifdefs into pread, truncate, and ftruncate to switch the argument order. User space is already doing that too. At no time on any architecture is the low stuff passed into arg3. Ergo, your patch is going to break userspace where pread and pread64 are now working correctly....

If you want to change the kernel to passing eliminate 64 bit stuff via syscalls, and instead pass pairs of 32bit entities -- I'm all for that as that would make explicit what user space is doing anyways. But don't break binary compatibility for no reason. Why make both user-space _and_ kernel space add ifdefs for endianness? Make arg3 _always_ contain the hi-bits.

-Erik

--
Erik B. Andersen http://codepoet-consulting.com/
--This message was written using 73% post-consumer electrons--

author
#5 lore raw
Quoted 10 lines · Erik Andersen

I understand the problem very well. Passing 64 bit stuff via syscalls is a major pain in the butt. But your patch is not just changing hppa and mips -- you are breaking the ABI on x86, arm, powerpc, etc, etc. etc where it is currently working. Look very closely at your patch. See those endianness ifdefs? You are adding endianness specific ifdefs into pread, truncate, and ftruncate to switch the argument order. User space is already doing that too. At no time on any architecture is the low stuff passed into arg3. Ergo, your patch is going to break userspace where pread and pread64 are now working correctly....

Nope. Some architectures _do not_ pad 64-bit arguments, others _do_. On ARM/x86, we currently do use arg3 for the low part of the argument, but on PPC we use it for the high part because of this sexswapping crap being done in userspace.

Quoted 6 lines

If you want to change the kernel to passing eliminate 64 bit stuff via syscalls, and instead pass pairs of 32bit entities -- I'm all for that as that would make explicit what user space is doing anyways. But don't break binary compatibility for no reason. Why make both user-space _and_ kernel space add ifdefs for endianness? Make arg3 _always_ contain the hi-bits.

I'd love to move to that model. But I suspect we need a consensus to _never_ pass 64-bit quantities across the syscall boundary, and we aren't going to get it. So we're going to add more crap every time someone adds a loff_t ;-(

Signature · 1 line
Revolutions do not require corporate support.
#6 lore raw
In reply to Matthew Wilcox
Quoted 2 lines · Matthew Wilcox

*sigh*. i hate this kind of bullshit. please, don't anyone ever try to pass 64-bit args through the syscall interface again.

I was about to do exactly that ...

Quoted 3 lines

+ * LFS versions of truncate are only needed on 32 bit machines. PA-RISC
+ * and MIPS ABIs specify 64-bit alignment for 64-bit quantities, but glibc
+ * ignores this and passes 64-bit quantities in misaligned registers.

Isn't glibc broken ?

regards
john

Signature · 3 lines
"It's a cardboard universe ... and if you lean too hard against it, you fall
 through." 
	- Philip K. Dick
#7 lore raw
In reply to Matthew Wilcox
Quoted 3 lines · Matthew Wilcox

*sigh*. i hate this kind of bullshit. please, don't anyone ever try to pass 64-bit args through the syscall interface again.

Please bury this crap in arch/hppa/

author
#8 lore raw
Quoted 6 lines · Alan Cox
Quoted 3 lines · Matthew Wilcox

*sigh*. i hate this kind of bullshit. please, don't anyone ever try to pass 64-bit args through the syscall interface again.

Please bury this crap in arch/hppa/

and arch/mips?

Signature · 1 line
Revolutions do not require corporate support.
#9 lore raw
Quoted 9 lines · Matthew Wilcox
Quoted 6 lines · Alan Cox
Quoted 3 lines · Matthew Wilcox

*sigh*. i hate this kind of bullshit. please, don't anyone ever try to pass 64-bit args through the syscall interface again.

Please bury this crap in arch/hppa/

and arch/mips?

IMHO yes

#10 lore raw
In reply to Matthew Wilcox

read_write.c asmlinkage ssize_t sys_write(unsigned in · line 295

Quoted 9 lines · Matthew Wilcox
read_write.c +7 -2
-asmlinkage ssize_t sys_pread64(unsigned int fd, char *buf,
-			     size_t count, loff_t pos)
+#ifdef __BIG_ENDIAN
+asmlinkage ssize_t sys_pread64(unsigned int fd, char *buf, size_t count,
+				unsigned int high, unsigned int low)
+#else
+asmlinkage ssize_t sys_pread64(unsigned int fd, char *buf, size_t count,
+				unsigned int low, unsigned int high)
+#endif

Broken for actual 64-bit targets.

I'd suggest frobbing the arguments in arch specific code for those 32-bit targets that do pad 64-bit arguments to the next even/odd register. I disbelieve you can do anything cleanly here.

r~