Re: [uml-devel] [PATCH 06/14] uml: make UML_SETJMP always safe

[Jeff Dike]

On Thu, Oct 05, 2006 at 11:38:52PM +0200, Paolo 'Blaisorblade' Giarrusso wrote:
> From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@xxxxxxxx>
> 
> If enable is moved by GCC in a register its value may not be preserved after
> coming back there with longjmp(). So, mark it as volatile to prevent this; this
> is suggested (it seems) in info gcc, when it talks about -Wuninitialized. I
> re-read this and it seems to say something different, but I still believe this
> may be needed.
> 
> Signed-off-by: Paolo 'Blaisorblade' Giarrusso <blaisorblade@xxxxxxxx>
> ---
> 
>  arch/um/include/longjmp.h |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/um/include/longjmp.h b/arch/um/include/longjmp.h
> index e93c6d3..e860bc5 100644
> --- a/arch/um/include/longjmp.h
> +++ b/arch/um/include/longjmp.h
> @@ -12,7 +12,8 @@ #define UML_LONGJMP(buf, val) do { \
>  } while(0)
>  
>  #define UML_SETJMP(buf) ({ \
> -	int n, enable;	   \
> +	int n;	   \
> +	volatile int enable;	\
>  	enable = get_signals(); \
>  	n = setjmp(*buf); \
>  	if(n != 0) \

I agree with this, but not entirely with your reasoning.  The
-Wuninitialized documentation just talks about when gcc emits a
warning.

What we want is a guarantee that enable is not cached in a register,
but is stored in memory.  What documentation I can find seems to imply
that is the case ("accesses to volatile objects must have settled
before the next sequence point").

However, given the prevailing opinion that essentially all volatile
declarations are hiding bugs, I wouldn't mind a bit of review of this
from someone holding this opinion.

				Jeff
-
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/