Date: Thu, 1 Mar 2012 23:47:32 +0100 From: Tijl Coosemans <tijl@freebsd.org> To: Bruce Evans <brde@optusnet.com.au> 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 Message-ID: <201203012347.32984.tijl@freebsd.org> In-Reply-To: <20120229151223.K2273@besplex.bde.org> References: <201202282217.q1SMHrIk094780@svn.freebsd.org> <20120229151223.K2273@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--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 <setjmp.h> 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 <setjmp.h> 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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201203012347.32984.tijl>