From owner-freebsd-bugs  Fri May 12 08:00:25 1995
Return-Path: bugs-owner
Received: (from majordom@localhost)
          by freefall.cdrom.com (8.6.10/8.6.6) id IAA06303
          for bugs-outgoing; Fri, 12 May 1995 08:00:25 -0700
Received: from hutcs.cs.hut.fi (hutcs.cs.hut.fi [130.233.192.2])
          by freefall.cdrom.com (8.6.10/8.6.6) with SMTP id IAA06283
          for <freebsd-bugs@freefall.cdrom.com>; Fri, 12 May 1995 08:00:18 -0700
Received: from shadows.cs.hut.fi by hutcs.cs.hut.fi with SMTP id AA22175
  (5.65c8/HUTCS-S 1.4 for <freebsd-bugs@freefall.cdrom.com>); Fri, 12 May 1995 17:59:23 +0300
Received: (ylo@localhost) by shadows.cs.hut.fi (8.6.10/8.6.10) id RAA03674; Fri, 12 May 1995 17:59:24 +0300
Date: Fri, 12 May 1995 17:59:24 +0300
Message-Id: <199505121459.RAA03674@shadows.cs.hut.fi>
From: Tatu Ylonen <ylo@cs.hut.fi>
To: bde@zeta.org.au
Cc: maddox@CS.Berkeley.EDU, freebsd-bugs@freefall.cdrom.com,
        maddox@redwood.CS.Berkeley.EDU, shadows-tech@cs.hut.fi
In-Reply-To: <199505120932.TAA20816@godzilla.zeta.org.au> (bde@zeta.org.au)
Subject: Re: i386/395: CRITICAL PROBLEM: spl functions implemented incorrectly
Organization: Helsinki University of Technology, Finland
Sender: bugs-owner@FreeBSD.org
Precedence: bulk

> No it can't.  `static inline' functions have the same semantics as
> ordinary static functions.  They aren't equivalent to macros.  Reordering
> is severely limited by the C Standard.  The ISO version section 5.1.2.3
> says:
> 
> "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".
> 
> Note that static [inline] functions are not special.  spl*() all modify
> the (non-volatile) object `cpl', so the above applies to them.  Making
> `cpl' volatile wouldn't affect this and would be wrong (it isn't volatile
> as far as C routines can tell since interrupt handlers preserve it).

You don't say anything about what can be assumed about values of non-volatile
expressions.

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

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.

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
expectable things here.  It is keeping the non-volatile value "value"
in a register across a call it knows will not modify "value".  This
sounds very reasonable.

    Tatu Ylonen <ylo@cs.hut.fi>