[PATCH 1/1] Apply transparent_union attribute to union semun

[PATCH 1/1] Apply transparent_union attribute to union semun

[jonas]

From: Jonas Bonn <jonas@southpole.se>

The syscall handler for semctl is written under the assumption that the
toolchain will pass "small" unions as function parameters directly instead
of by reference.  The union semun is "small" and thus fits this description.

Since it is assumed that the union will be passed directly and not by
reference, it is safe to access the union members without going via
get_user.

The OpenRISC architecture, however, passes all unions by reference, thus
breaking the above assumption.

The technically correct fix here is to mark the union as being transparent
so that the ABI of the union's first element determines the parameter
passing method and thus make explicit what's already implied in the function
definition.

Signed-off-by: Jonas Bonn <jonas@southpole.se>
---
 include/linux/sem.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/sem.h b/include/linux/sem.h
index f2961af..6985c7d 100644
--- a/include/linux/sem.h
+++ b/include/linux/sem.h
@@ -48,7 +48,7 @@ union semun {
 	unsigned short __user *array;	/* array for GETALL & SETALL */
 	struct seminfo __user *__buf;	/* buffer for IPC_INFO */
 	void __user *__pad;
-};
+} __attribute__ ((transparent_union));
 
 struct  seminfo {
 	int semmap;
-- 
1.7.4.1

Re: [ORLinux] [PATCH 1/1] Apply transparent_union attribute to union semun

[Jeremy Bennett]

On Fri, 2011-05-27 at 00:27 +0200, jonas@southpole.se wrote: 
> From: Jonas Bonn <jonas@southpole.se>
> 
> The syscall handler for semctl is written under the assumption that the
> toolchain will pass "small" unions as function parameters directly instead
> of by reference.  The union semun is "small" and thus fits this description.
> 
> Since it is assumed that the union will be passed directly and not by
> reference, it is safe to access the union members without going via
> get_user.
> 
> The OpenRISC architecture, however, passes all unions by reference, thus
> breaking the above assumption.

Hi Jonas

Joern and I were discussing this - there is a case for changing the ABI.
It would certainly fix a debugging issue.

> The technically correct fix here is to mark the union as being transparent
> so that the ABI of the union's first element determines the parameter
> passing method and thus make explicit what's already implied in the function
> definition.
> 
> Signed-off-by: Jonas Bonn <jonas@southpole.se>
> ---
>  include/linux/sem.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/sem.h b/include/linux/sem.h
> index f2961af..6985c7d 100644
> --- a/include/linux/sem.h
> +++ b/include/linux/sem.h
> @@ -48,7 +48,7 @@ union semun {
>  	unsigned short __user *array;	/* array for GETALL & SETALL */
>  	struct seminfo __user *__buf;	/* buffer for IPC_INFO */
>  	void __user *__pad;
> -};
> +} __attribute__ ((transparent_union));
>  
>  struct  seminfo {
>  	int semmap;

Excellent. Semaphore test now passes. I look forward to rerunning
regression - we may be down to a single failure now!

Best wishes,


Jeremy

-- 
Tel:      +44 (1590) 610184
Cell:     +44 (7970) 676050
SkypeID: jeremybennett
Email:   jeremy.bennett@embecosm.com
Web:     www.embecosm.com

Re: [PATCH 1/1] Apply transparent_union attribute to union semun

[Jonas Bonn]

Hi,
I never got any feedback on this patch... is this an acceptable solution
for upstream?  ...or is the usage of this GCC extension discouraged?
/Jonas

On Fri, 2011-05-27 at 00:27 +0200, jonas@southpole.se wrote:
> From: Jonas Bonn <jonas@southpole.se>
> 
> The syscall handler for semctl is written under the assumption that the
> toolchain will pass "small" unions as function parameters directly instead
> of by reference.  The union semun is "small" and thus fits this description.
> 
> Since it is assumed that the union will be passed directly and not by
> reference, it is safe to access the union members without going via
> get_user.
> 
> The OpenRISC architecture, however, passes all unions by reference, thus
> breaking the above assumption.
> 
> The technically correct fix here is to mark the union as being transparent
> so that the ABI of the union's first element determines the parameter
> passing method and thus make explicit what's already implied in the function
> definition.
> 
> Signed-off-by: Jonas Bonn <jonas@southpole.se>
> ---
>  include/linux/sem.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/sem.h b/include/linux/sem.h
> index f2961af..6985c7d 100644
> --- a/include/linux/sem.h
> +++ b/include/linux/sem.h
> @@ -48,7 +48,7 @@ union semun {
>  	unsigned short __user *array;	/* array for GETALL & SETALL */
>  	struct seminfo __user *__buf;	/* buffer for IPC_INFO */
>  	void __user *__pad;
> -};
> +} __attribute__ ((transparent_union));
>  
>  struct  seminfo {
>  	int semmap;