On Wednesday 29 February 2012 06:01:36 Bruce Evans wrote: > 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.
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.
signature.asc
Description: This is a digitally signed message part.