Date: Wed, 29 Feb 2012 16:01:36 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Tijl Coosemans <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 Message-ID: <20120229151223.K2273@besplex.bde.org> In-Reply-To: <201202282217.q1SMHrIk094780@svn.freebsd.org> References: <201202282217.q1SMHrIk094780@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 28 Feb 2012, Tijl Coosemans wrote:
> Log:
> Copy amd64 setjmp.h to x86 and replace amd64/i386/pc98 setjmp.h with stubs.
This may be correct (except for comment), but it is confusing.
> 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
> ==============================================================================
> --- 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];
On amd64, the comment was specific to amd64 (but with amd64 misspelled
as AMD64), and actually matched the code.
> Modified: head/sys/i386/include/setjmp.h
> ==============================================================================
> --- 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];
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.
_JBLEN was apparently 1 less than what it was claimed to be on i386.
I forget what the extra 1 is for.
> Copied: head/sys/x86/include/setjmp.h (from r232268, head/sys/amd64/include/setjmp.h)
> ==============================================================================
> --- /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, copy of r232268, head/sys/amd64/include/setjmp.h)
> @@ -0,0 +1,50 @@
> ...
> +#define _JBLEN 12 /* Size of the jmp_buf on AMD64. */
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.
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.
> +
> +/*
> + * 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];
No extra 1 for either now.
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.
>From the 4.4BSD-Lite2 version:
% #if defined(hp300) || defined(__hp300__) || defined(luna68k) || defined(__luna68k__)
% #define _JBLEN 17
% #endif
%
% #if defined(i386) || defined(__i386__)
% #define _JBLEN 10
% #endif
%
% #if defined(mips) || defined(__mips__)
% #define _JBLEN 83
% #endif
%
% #if defined(sparc) || defined(__sparc__)
% #define _JBLEN 10
% #endif
%
% #if defined(tahoe) || defined(__tahoe__)
% #define _JBLEN 10
% #endif
%
% #if defined(vax) || defined(__vax__)
% #define _JBLEN 10
% #endif
%
% #ifndef _ANSI_SOURCE
% /*
% * WARNING: sigsetjmp() isn't supported yet, this is a placeholder.
% */
% typedef int sigjmp_buf[_JBLEN + 1];
% #endif /* not ANSI */
%
% typedef int jmp_buf[_JBLEN];
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).
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.
Here is what current arches have in their machine/setjmp.h:
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.
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.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120229151223.K2273>