Date: Wed, 29 Feb 2012 15:05:26 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Alexander Best <arundel@FreeBSD.org> Cc: Jilles Tjoelker <jilles@FreeBSD.org>, svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r232183 - head/sys/kern Message-ID: <20120229141447.F2092@besplex.bde.org> In-Reply-To: <20120228194520.GA56030@freebsd.org> References: <201202261514.q1QFET0v070810@svn.freebsd.org> <20120228194520.GA56030@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 28 Feb 2012, Alexander Best wrote: > On Sun Feb 26 12, Jilles Tjoelker wrote: >> Log: >> Fix fchmod() and fchown() on fifos. >> >> The new fifo implementation in r232055 broke fchmod() and fchown() on fifos. >> Postfix needs this. > > clang seems to have a problem with this commit: > > usr/subversion-src/sys/kern/sys_pipe.c:1556:10: error: promoted type 'int' of K&R function parameter is not compatible with the parameter type 'mode_t' (aka 'unsigned short') declared in a previous prototype [-Werror] > mode_t mode; > ^ > /usr/subversion-src/sys/kern/sys_pipe.c:155:19: note: previous declaration is here > static fo_chmod_t pipe_chmod; > ^ > 1 error generated. > *** Error code 1 Indeed, sys_pipe.c still uses mostly K&R-style function definitions, and the recent change was compatible with this to a fault. It is an old bug in gcc that it accepts function definitions that don't match their prototype. The most common error is the one here -- the function definition is K&R and has a type smaller than its promotion; the prototype must give the promoted type to match, but this breaks it for use with function definitions that are not K&R and use the narrow type. It is an old design error to have APIs that pass narrowed types. Passing narrowed types is normally just a pessimization. The ABI normally requires passing a widened value. The widened value may have garbage in the extra bits, but for safety and to support missing prototypes, in practice callers must sign-extend or zero-extend the narrow value. The for safety and to support missing prototypes, in practice callees can't trust callers to have done this, so they force to a narrow value. Then to actually use the value, they often have to widen it again. The result is often a couple of extra instructions after optimizing away only half of the logical extra conversions for this. So the supposed optimization of passing narrow types is usually a pessimization. Maybe this is a good pessimization for private APIs, but it shouldn't be required for external ones. Narrow types in syscalls are especially bad. Now the C API doesn't really extend across the syscall boundary, so both sides have to force the above conversions or depend on the compiler doing them because it fears missing prototypes. In practice, FreeBSD depends on the compiler doing them for most or all arches. The narrow mode_t for open(2) is worst. It is the only narrow type in in a core syscall, and has further problems. The prototype for open() is "int open(const char *path, int flags, ...);", with the only option for "..." being a mode_t. open() is thus variadic, but variadic functions really can't be passed a narrowed type like mode_t, and syscalls can't really pass this either. FreeBSD used to depend on various type puns near here, and still has my XXX comments in syscalls.master about them, but some of them have been fixed. The largest one is that the kernel assumes that the mode_t arg is always there. It copies stack garbage if it is not there, and only later checks the flags and avoids using the stack garbage when there is no arg there. This is not too bad if done intentionally. It must be arranged that the stack garbage is in accessible memory, but it always is in practice. The kernel is carefully to access the arg as an int and not as a mode_t, and to pass it down through too many layers as an int. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120229141447.F2092>