Re: fs/binfmt_aout.o, Error: suffix or operands invalid for `cmp'[was Re: 2.6.1

[Steven Rostedt]

I have no idea why I have binfmt_aout turned on but I did and hit this
too.

On Thu, 22 Jun 2006, H. Peter Anvin wrote:

> Chuck Ebbert wrote:
> >>>>
> >>> It's complaining about this:
> >>>
> >>> #APP
> >>>         addl %ecx,%eax ; sbbl %edx,%edx; cmpl %eax,$-1073741824; sbbl $0,%edx   # dump.u_dsize, sum, flag,
> >>> #NO_APP
> >> The cmpl should have its arguments reversed.  It's quite possible some versions of the
> >> assembler accepts the form given, but they're wrong (and doubly confusing when used as
> >> input to sbb.)
> >
> > This was built with gcc 4.0.4 20060507 (prerelease).
> >
> > I don't normally build a.out support, but I just tried and it compiled
> > fine with gcc 4.1.1.  SO this is probably a compiler bug (almost certainly
> > given that it generated illegal assembler code.)
> >
>
> It's not (it's #APP, i.e. inline assembly); rather, it's an illegal
> constraint.
>

It's GCC optimizing a little too much.

The problem code is this (in binfmt_aout.c):

/* make sure we actually have a data and stack area to dump */
	set_fs(USER_DS);
#ifdef __sparc__
	if (!access_ok(VERIFY_READ, (void __user *)START_DATA(dump), dump.u_dsize))
		dump.u_dsize = 0;
	if (!access_ok(VERIFY_READ, (void __user *)START_STACK(dump), dump.u_ssize))
		dump.u_ssize = 0;
#else
	if (!access_ok(VERIFY_READ, (void __user *)START_DATA(dump), dump.u_dsize << PAGE_SHIFT))
		dump.u_dsize = 0;
	if (!access_ok(VERIFY_READ, (void __user *)START_STACK(dump), dump.u_ssize << PAGE_SHIFT))
		dump.u_ssize = 0;
#endif


Previously it did a set_fs(KERNEL_DS) so it needs to do the
set_fs(USER_DS) now.

But that is:

  #define set_fs(x)	(current_thread_info()->addr_limit = (x))

and access_ok is:

  #define access_ok(type,addr,size) (likely(__range_ok(addr,size) == 0))

where __range_ok is:

#define __range_ok(addr,size) ({ \
	unsigned long flag,sum; \
	__chk_user_ptr(addr); \
	asm("addl %3,%1 ; sbbl %0,%0; cmpl %1,%4; sbbl $0,%0" \
		:"=&r" (flag), "=r" (sum) \
		:"1" (addr),"g" ((int)(size)),"g" (current_thread_info()->addr_limit.seg)); \
	flag; })

What happened was that gcc optimized the
(current_thread_info()->addre_limit.seg to be a constant. Thus cmpl
failed.

The following patch fixes this, although I don't know the intel
constraints very well, but this does compile.

-- Steve

Signed-off-by: Steven Rostedt <rostedt@xxxxxxxxxxx>

Index: linux-2.6.17-mm1/include/asm-i386/uaccess.h
===================================================================
--- linux-2.6.17-mm1.orig/include/asm-i386/uaccess.h	2006-06-23 05:02:15.000000000 -0400
+++ linux-2.6.17-mm1/include/asm-i386/uaccess.h	2006-06-23 05:02:29.000000000 -0400
@@ -58,7 +58,7 @@ extern struct movsl_mask {
 	__chk_user_ptr(addr); \
 	asm("addl %3,%1 ; sbbl %0,%0; cmpl %1,%4; sbbl $0,%0" \
 		:"=&r" (flag), "=r" (sum) \
-		:"1" (addr),"g" ((int)(size)),"g" (current_thread_info()->addr_limit.seg)); \
+		:"1" (addr),"g" ((int)(size)),"r" (current_thread_info()->addr_limit.seg)); \
 	flag; })

 /**
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/