From owner-svn-src-head@FreeBSD.ORG Thu Mar 1 22:47:38 2012 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id C39DD106566B; Thu, 1 Mar 2012 22:47:38 +0000 (UTC) (envelope-from tijl@freebsd.org) Received: from mailrelay007.isp.belgacom.be (mailrelay007.isp.belgacom.be [195.238.6.173]) by mx1.freebsd.org (Postfix) with ESMTP id 7D1CC8FC1C; Thu, 1 Mar 2012 22:47:37 +0000 (UTC) X-Belgacom-Dynamic: yes X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AgAFAE/7T09bsWst/2dsb2JhbABDsRmCcYEIgX0BAQVWIxALDgouOR4GiB25BooBgn0EBAIMAwUEAgwEAgI+FIUTCoQaBIgcoBKBUw Received: from 45.107-177-91.adsl-dyn.isp.belgacom.be (HELO kalimero.tijl.coosemans.org) ([91.177.107.45]) by relay.skynet.be with ESMTP; 01 Mar 2012 23:47:36 +0100 Received: from kalimero.tijl.coosemans.org (kalimero.tijl.coosemans.org [127.0.0.1]) by kalimero.tijl.coosemans.org (8.14.5/8.14.5) with ESMTP id q21MlZFu046337; Thu, 1 Mar 2012 23:47:35 +0100 (CET) (envelope-from tijl@freebsd.org) From: Tijl Coosemans To: Bruce Evans Date: Thu, 1 Mar 2012 23:47:32 +0100 User-Agent: KMail/1.13.7 (FreeBSD/10.0-CURRENT; KDE/4.7.3; i386; ; ) References: <201202282217.q1SMHrIk094780@svn.freebsd.org> <20120229151223.K2273@besplex.bde.org> In-Reply-To: <20120229151223.K2273@besplex.bde.org> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart1475720.AtHMusYaJp"; protocol="application/pgp-signature"; micalg=pgp-sha256 Content-Transfer-Encoding: 7bit Message-Id: <201203012347.32984.tijl@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r232275 - in head/sys: amd64/include i386/include pc98/include x86/include X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Mar 2012 22:47:38 -0000 --nextPart1475720.AtHMusYaJp Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable On Wednesday 29 February 2012 06:01:36 Bruce Evans wrote: > On Tue, 28 Feb 2012, Tijl Coosemans wrote: >=20 >> Log: >> Copy amd64 setjmp.h to x86 and replace amd64/i386/pc98 setjmp.h with st= ubs. >=20 > This may be correct (except for comment), but it is confusing. >=20 >> Added: >> head/sys/x86/include/setjmp.h >> - copied unchanged from r232268, head/sys/amd64/include/setjmp.h >> Modified: >> head/sys/amd64/include/setjmp.h >> head/sys/i386/include/setjmp.h >> head/sys/pc98/include/setjmp.h >> >> Modified: head/sys/amd64/include/setjmp.h >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D >> --- head/sys/amd64/include/setjmp.h Tue Feb 28 22:15:46 2012 (r232274) >> +++ head/sys/amd64/include/setjmp.h Tue Feb 28 22:17:52 2012 (r232275) >> @@ -1,50 +1,6 @@ >> ... >> -#define _JBLEN 12 /* Size of the jmp_buf on AMD64. */ >> - >> -/* >> - * jmp_buf and sigjmp_buf are encapsulated in different structs to force >> - * compile-time diagnostics for mismatches. The structs are the same >> - * internally to avoid some run-time errors for mismatches. >> - */ >> -#if __BSD_VISIBLE || __POSIX_VISIBLE || __XSI_VISIBLE >> -typedef struct _sigjmp_buf { long _sjb[_JBLEN]; } sigjmp_buf[1]; >> -#endif >> - >> -typedef struct _jmp_buf { long _jb[_JBLEN]; } jmp_buf[1]; >=20 > On amd64, the comment was specific to amd64 (but with amd64 misspelled > as AMD64), and actually matched the code. >=20 >> Modified: head/sys/i386/include/setjmp.h >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D >> --- head/sys/i386/include/setjmp.h Tue Feb 28 22:15:46 2012 (r232274) >> +++ head/sys/i386/include/setjmp.h Tue Feb 28 22:17:52 2012 (r232275) >> @@ -1,50 +1,6 @@ >> -#define _JBLEN 11 /* Size of the jmp_buf on x86. */ >> - >> -/* >> - * jmp_buf and sigjmp_buf are encapsulated in different structs to force >> - * compile-time diagnostics for mismatches. The structs are the same >> - * internally to avoid some run-time errors for mismatches. >> - */ >> -#if __BSD_VISIBLE || __POSIX_VISIBLE || __XSI_VISIBLE >> -typedef struct _sigjmp_buf { int _sjb[_JBLEN + 1]; } sigjmp_buf[1]; >> -#endif >> - >> -typedef struct _jmp_buf { int _jb[_JBLEN + 1]; } jmp_buf[1]; >=20 > On i386, the comment had a differently wrong name for the old code, and > mismatched the old code, but is correct for the new code. >=20 > _JBLEN was apparently 1 less than what it was claimed to be on i386. > I forget what the extra 1 is for. >=20 >> Copied: head/sys/x86/include/setjmp.h (from r232268, head/sys/amd64/incl= ude/setjmp.h) >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D >> --- /dev/null 00:00:00 1970 (empty, because file is newly added) >> +++ head/sys/x86/include/setjmp.h Tue Feb 28 22:17:52 2012 (r232275, cop= y of r232268, head/sys/amd64/include/setjmp.h) >> @@ -0,0 +1,50 @@ >> ... >> +#define _JBLEN 12 /* Size of the jmp_buf on AMD64. */ >=20 > This code looks wrong, since _JBLEN was only 11 for i386, but binary > compatibility of the jmp_buf is preserved by the extra 1 in it. >=20 > This comment is wrong, since the code is no longer just for AMD64 (sic), > and the comment makes it look more like the code has an off-by-1 error > for !AMD64. >=20 >> + >> +/* >> + * jmp_buf and sigjmp_buf are encapsulated in different structs to force >> + * compile-time diagnostics for mismatches. The structs are the same >> + * internally to avoid some run-time errors for mismatches. >> + */ >> +#if __BSD_VISIBLE || __POSIX_VISIBLE || __XSI_VISIBLE >> +typedef struct _sigjmp_buf { long _sjb[_JBLEN]; } sigjmp_buf[1]; >> +#endif >> + >> +typedef struct _jmp_buf { long _jb[_JBLEN]; } jmp_buf[1]; >=20 > No extra 1 for either now. >=20 > The extra 1 was apparently a placeholder for the signal mask and became > obsolete 15-20 years ago. 4.4BSD-Lite2 claims not to support the signal > mask, and doesn't have space for it in the plain jmp_buf. setjmp.h > was in src/include and was moved to MD files to clean up its ifdefs. > Now we're barely escaping putting the ifdefs back in the x86 version, > sigh. Ifdefs are really needed, since the i386 version is too small > to have space for mxcsr, and mxcsr is only hacked into the amd64 version > by packing it using the doubled space given by long entries. >=20 > From the 4.4BSD-Lite2 version: > % #if defined(hp300) || defined(__hp300__) || defined(luna68k) || defined= (__luna68k__) > % #define _JBLEN 17 > % #endif > %=20 > % #if defined(i386) || defined(__i386__) > % #define _JBLEN 10 > % #endif > %=20 > % #if defined(mips) || defined(__mips__) > % #define _JBLEN 83 > % #endif > %=20 > % #if defined(sparc) || defined(__sparc__) > % #define _JBLEN 10 > % #endif > %=20 > % #if defined(tahoe) || defined(__tahoe__) > % #define _JBLEN 10 > % #endif > %=20 > % #if defined(vax) || defined(__vax__) > % #define _JBLEN 10 > % #endif > %=20 > % #ifndef _ANSI_SOURCE > % /* > % * WARNING: sigsetjmp() isn't supported yet, this is a placeholder. > % */ > % typedef int sigjmp_buf[_JBLEN + 1]; > % #endif /* not ANSI */ > %=20 > % typedef int jmp_buf[_JBLEN]; >=20 > The extra 1 seems to have been nonsense there too. BSD has always (?) > saved the signal mask in the ordinary jmp_buf. 4.4BSD-Lite2 seems to > do this, and could hardly have worked without it. So _JBLEN was already > large enough and didn't need a placeholder for the signal mask. But > I now vaguely remember stealing this extra 1 for the FP control word. > It was sloppy not to update the comment. Later, jb left the extra 1 > and the comment alone. The commit log says that _JBLEN was left in > case something (mis)uses it. I think it should be removed. It should > only be used in the 2 jmp_buf structs, and now that sigsetjmp() has > been standard for so long, and since making the structure layouts > identical is best for any system which saves the signal mask in setjmp() > and probably also on any system that doesn't, I don't see any need for > separate jmp_buf structs (just use separate typedefs using the same > struct). >=20 > Next, I don't see why can't be MI except for the definition > of _JBLEN. _JBLEN can be declared in some harmless MD header that is > already usually included. Since we removed machine/ansi.h, > machine/types.h seems to be the only suitable one. >=20 > Here is what current arches have in their machine/setjmp.h: >=20 > amd64, i386: not much > arm: has lots of comments and register offsets. These are defined as > _JB_REG_* so they aren't pollution, but there is no reason to > export them to the application either. The actual > structs are the usual 2 arrays of ints, with the extra 1 for > both the comment not matching the code, as on i386. The extra > 1 is unused, or at least has no _JB_REG_* for it. > ia64: has lots of namespace-pollution definitions under a __BSD_VISIBLE > ifdef. The structs are arrays of long doubles! This defeats > my idea of using a MI array of register_t's. _JBLEN could be > expanded for long doubles, but __align() would be required too, > and it gets messier than a separate file. > mips: just the usual extra 1 (now 4 instances for 32/64 doubling) and > the usual comment not matching the code. > powerpc: like x86 > sparc64: just the usual extra 1. The comment is fixed by removing it. >=20 > So the extra 1 seems to be just a ~20-year old mistake, faithfully > propagated to all arches except amd64 i386, with unfaithful propagation > just fixed for i386. If we could add the returns_twice attribute to setjmp() then the compiler makes sure all registers are dead before calling it and jmp_buf wouldn't have to be that big. Also, from ISO C: "All accessible objects have values, and all other components of the abstract machine [249] have state, as of the time the longjmp function was called" "[249] This includes, but is not limited to, the floating-point status flags and the state of open files." So I think storing mxcsr in jmp_buf is incorrect. --nextPart1475720.AtHMusYaJp Content-Type: application/pgp-signature; name=signature.asc Content-Description: This is a digitally signed message part. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.18 (FreeBSD) iF4EABEIAAYFAk9P/IQACgkQfoCS2CCgtiu9jgEAgfuPYDxt5ThKBNCqUnEHFwk6 YlHVXn3NFICGgFN5lQEA/0/Gu9IBCYckJVzIbehov8bpQRsb7bKIjdzVa6addSop =0hlp -----END PGP SIGNATURE----- --nextPart1475720.AtHMusYaJp--