From owner-freebsd-bugs Fri May 12 09:20:41 1995 Return-Path: bugs-owner Received: (from majordom@localhost) by freefall.cdrom.com (8.6.10/8.6.6) id JAA10890 for bugs-outgoing; Fri, 12 May 1995 09:20:41 -0700 Received: from godzilla.zeta.org.au (godzilla.zeta.org.au [203.2.228.34]) by freefall.cdrom.com (8.6.10/8.6.6) with ESMTP id JAA10854 for ; Fri, 12 May 1995 09:19:57 -0700 Received: (from bde@localhost) by godzilla.zeta.org.au (8.6.9/8.6.9) id CAA01072; Sat, 13 May 1995 02:19:01 +1000 Date: Sat, 13 May 1995 02:19:01 +1000 From: Bruce Evans Message-Id: <199505121619.CAA01072@godzilla.zeta.org.au> To: bde@zeta.org.au, ylo@cs.hut.fi Subject: Re: i386/395: CRITICAL PROBLEM: spl functions implemented incorrectly Cc: freebsd-bugs@freefall.cdrom.com, maddox@CS.Berkeley.EDU, maddox@redwood.CS.Berkeley.EDU, shadows-tech@cs.hut.fi Sender: bugs-owner@FreeBSD.org Precedence: bulk >> "Accessing a volatile object, modifying an object, modifying a file, or >> calling a function that does any of these operations are all _side effects_. >> At ... _sequence points_, all side effects of previous evaluations shall >> be complete and no side effects of subsequent evaluations shall have >> taken place". >You don't say anything about what can be assumed about values of non-volatile >expressions. They can be assumed to be non-volatile :-). >I checked with gcc-2.6.3 (with patches for i386 bugs - I get same >results with plain 2.6.3). The following code: >extern int cpl; >extern int value; >static __inline int spl1() >{ > int old = cpl; > cpl |= 1; > return old; >} >static __inline void splx(int x) >{ > cpl = x; >} >void foo() >{ > int x, y; > value *= 2; > x = spl1(); > value *= 3; > splx(x); >} >When compiled with "gcc -O -S koe.c" this gives: > .file "koe.c" >gcc2_compiled.: >___gnu_compiled_c: >.text > .align 2 >.globl _foo > .type _foo,@function >_foo: > pushl %ebp > movl %esp,%ebp > movl _value,%eax > leal 0(,%eax,2),%edx > movl %edx,_value > movl _cpl,%ecx > orb $1,_cpl > leal (%edx,%eax,4),%eax <-- note: "value" kept in register across > movl %eax,_value call to spl1. > movl %ecx,_cpl > leave > ret >Lfe1: > .size _foo,Lfe1-_foo Note that cpl is stored to in the desired order. >Thus, the spl code did not do what it was supposed to do: we cannot >make assumptions about values computed before entering the critical >section being still valid in the critical section. spl isn't supposed to turn non-volatile variables into volatile ones. It just happens to have that effect if you hide it sufficiently from the compiler. For gcc-2.3.3 on the i386, any non-static function is hidden sufficiently. >There definitely is a serious problem here. I cannot be absolutely >sure whether it is in the compiler or in the kernel as I do not have >the standard. In my opinion the compiler is doing only reasonable and The problem is actually that many variables are declared as /*nonvolatile*/ when they are actually volatile. Almost any device driver contains dozens of examples. Consider a buffer that is filled by interrupt handlers, e.g, the clists in struct tty. The pointers for the buffer and the buffer contents are all volatile but none are declared volatile. A sufficiently good optimizer might cache some of the values across function calls. Non-static spl()s just require a better optimizer for there to be a problem. Note that the locking feature of spl*() isn't important here. Any call to an external function together with a call to a static inline spl*() would work just as well. Any write through a pointer together with a static inline spl*() would work just as well... On second thoughts, the optimizer doesn't need to be much better for an external function to not help. Consider the case where the critical variables and all pointers to them are static. Then the external function can't legitimately modify the values. I don't know of a good way to avoid this problem. Declaring all critical variables as volatile would be slow. It would be nice if the variables were volatile inside the critical region and volatile outside it, but C doesn't provide this feature directly. You could probably get it by declaring the variables as volatile and accessing them as *(/* nonvolatile */ foo_t *)&foovar inside the critical region but I don't want that. I seem to remember some bug reports for gcc under Linux related to this problem. I think it was expected that __asm("" : : : "memory"); would flush all cached variables from registers. It should do this because "memory" in the clobber list says that arbitrary memory might be written to. I think it didn't work as expected. If/when it works, it may be the best way to handle the problem. Bruce