Message-ID: <49614F62.5070006@kernel.org>
Date: Sun, 04 Jan 2009 16:08:02 -0800
From: "H. Peter Anvin" <hpa@kernel.org>
Organization: Linux Kernel Organization, Inc.
User-Agent: Thunderbird 2.0.0.14 (X11/20080501)
MIME-Version: 1.0
To: Andi Kleen <andi@firstfloor.org>
CC: akpm@linux-foundation.org, x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] [4/5] Use asm stubs for 32bit sigreturn codes
References: <200901051236.281008835@firstfloor.org> <20090104233641.6F5723E6652@basil.firstfloor.org>
In-Reply-To: <20090104233641.6F5723E6652@basil.firstfloor.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1860
Lines: 59

Andi Kleen wrote:
> 32bit sigreturn/rt_sigreturn currently uses some magic casting
> with assumptions about the stack layout
> to simulate call by reference for the pt_regs structure on the 
> stack. This is fragile, non standard and according to reports 
> breaks with LLVM at least. I suppose it could break in
> future gcc versions too when their stack layout changes.
> 
> So instead of having this magic in C do it in small
> assembler stubs similar to what x86-64 does (which
> always passes a pointer to pt_regs for such functions)
> 
> This also leads to cleaner code.
> 

It's wrong, however:

#define asmlinkage CPP_ASMLINKAGE __attribute__((regparm(0)))

> +ENTRY(stub_sigreturn)
> +	movl %esp,%eax
> +	jmp sys_sigreturn
> +END(stub_sigreturn)

This assumes regparm=3, i.e !asmlinkage.

> -asmlinkage unsigned long sys_sigreturn(unsigned long __unused)
> +asmlinkage unsigned long sys_sigreturn(struct pt_regs *regs)
>  {
>  	struct sigframe __user *frame;
> -	struct pt_regs *regs;
>  	unsigned long ax;
>  	sigset_t set;
>  
> -	regs = (struct pt_regs *) &__unused;
>  	frame = (struct sigframe __user *)(regs->sp - 8);
>  
>  	if (!access_ok(VERIFY_READ, frame, sizeof(*frame)))
> @@ -212,7 +210,7 @@
>  	return 0;
>  }

I think we already have a patch in the tree that changes this to:

asmlinkage unsigned long sys_sigreturn(struct pt_regs regs)

... although doing it as a pointer is better, but requires a change to
the asmlinkage thing.

Getting rid of __attribute__((regparm(0))) for asmlinkage on i386 would
definitely be good, but is a bigger thing than only these couple of
functions.

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