svn commit: r295561 - in head: include sys/mips/include sys/powerpc/include sys/sparc64/include sys/sys sys/x86/include

Bruce Evans brde at optusnet.com.au
Fri Feb 12 16:56:45 UTC 2016


On Fri, 12 Feb 2016, Konstantin Belousov wrote:

> On Sat, Feb 13, 2016 at 01:08:55AM +1100, Bruce Evans wrote:
>> On Fri, 12 Feb 2016, Konstantin Belousov wrote:
> ...
>>> Modified: head/include/signal.h
>>> ==============================================================================
>>> --- head/include/signal.h	Fri Feb 12 07:27:24 2016	(r295560)
>>> +++ head/include/signal.h	Fri Feb 12 07:38:19 2016	(r295561)
>>> @@ -36,6 +36,8 @@
>>> #include <sys/cdefs.h>
>>> #include <sys/_types.h>
>>> #include <sys/signal.h>
>>> +#include <machine/ucontext.h>
>>> +#include <sys/_ucontext.h>
>>
>> This pollutes even the non-POSIX case with POSIX and FreeBSD names.
>>
>> The correct ifdefs for this are messy.  No names should be added for
>> non-POSIX and old versions of POSIX.  2001 POSIX needs an XSI ifdef.  Later
>> versions of POSIX don't need an ifdef.  Normally it is better to hide
>> the ifdefs in the included headers, but _ucontext.h should always
>> provide ucontext_t and uc_*.
> Our non-POSIX namespace is strictly superset of the POSIX space.
> I would not hide {m,u}context_t from the default visibility, and this
> would defeat the goal of the change.

Our Standard C namespace is a subset of the POSIX namespace.  Most
Standard C headers are (were) careful about this.  There aren't many
Standard C headers or versions of Standard C or large variations in the
versions, so this is relatively easy to do.

We also attempt to use POSIX visibility ifdefs.  This is not done so
carefully, and has more bugs due to more variations.  But <signal.h>
does it fairly carefully.  It has ifdefs for XSI, POSIX >= 2001,
any-POSIX (this is now very broken by unconditional declarations for
pthreads), POSIX >= 1995 (realtime POSIX stuff which I think was
standardized in POSIX.4 (.1b?) a couple of years before 1995, but we
don't have ifdefs for that), POSIX >= 2008 (psignal).  That is just
in the top-level header.  Almost half of that is only under XSI or
BSD, and it is quite likely that some newer or older POSIX or XSI
things are under the wrong ifdefed but are usually visible because
everything is visible by default.  sys/signal.h has ifdefs for
POSIX >= 1993, more-delicate version ifdefs for XSI and POSIX <= 2001.
Most of the POSIX >= 1993 ifdefs are for realtime POSIX stuff.  An
enormous number of definitions for things like trap codes for FP errors
are misplaced under the POSIX >= 1993 || XSI ifdef.  These weren't in
POSIX.1-2006.  This contrasts with the fine-grained ifdefs for signal
numbers.

I find the ifdefs useful for seeing when POSIX introduced a feature
but not for actual use to compile under an old standard.

>>> Modified: head/sys/mips/include/ucontext.h
>>> ==============================================================================
>>> --- head/sys/mips/include/ucontext.h	Fri Feb 12 07:27:24 2016	(r295560)
>>> +++ head/sys/mips/include/ucontext.h	Fri Feb 12 07:38:19 2016	(r295561)
>>> @@ -50,13 +50,13 @@ typedef struct	__mcontext {
>>> 	 * struct sigcontext and ucontext_t at the same time.
>>> 	 */
>>> 	int		mc_onstack;	/* sigstack state to restore */
>>> -	register_t	mc_pc;		/* pc at time of signal */
>>> -	register_t	mc_regs[32];	/* processor regs 0 to 31 */
>>> -	register_t	sr;		/* status register */
>>> -	register_t	mullo, mulhi;	/* mullo and mulhi registers... */
>>> +	__register_t	mc_pc;		/* pc at time of signal */
>>> +	__register_t	mc_regs[32];	/* processor regs 0 to 31 */
>>> +	__register_t	sr;		/* status register */
>>> +	__register_t	mullo, mulhi;	/* mullo and mulhi registers... */
>>> 	int		mc_fpused;	/* fp has been used */
>>> 	f_register_t	mc_fpregs[33];	/* fp regs 0 to 31 and csr */
>>> -	register_t	mc_fpc_eir;	/* fp exception instruction reg */
>>> +	__register_t	mc_fpc_eir;	/* fp exception instruction reg */
>>> 	void		*mc_tls;	/* pointer to TLS area */
>>> 	int		__spare__[8];	/* XXX reserved */
>>> } mcontext_t;
>>>
>>
>> These mc_* names always polluted <ucontext.h> but no one cared since it
>> was an XSI extension.
>>
>> sr, mullo and mulhi have worse names.  These names don't even use the
>> same style as the others.  They now pollute <signal.h> of course.
>>
>> __spare__ is bogusly named and has a banal comment.  The names of spares
>> should be in the normal (reserved) namespace for the struct.  No namespace
>> is reserved here, bug mc_spare would be no worse than the other mc_'s.
>>
>> i386 was missing all of these bugs except the mc_* pollution.
> The members names for the structures are private per the structure
> namespace.  The names of the members cannot pollute signal.h.

No, they aren't private.  Applications can #define them to <syntax error>,
and this is valid since they are not reserved.  Standards have to reserve
names of all struct members to make this invalid.  POSIX does this for
uc_*.

>>> Copied and modified: head/sys/sys/_ucontext.h (from r295560, head/sys/sys/ucontext.h)
>>
>> sys/ucontext.h (== <POSIX <ucontext.h>) remains polluted.  I point out its
>> old bugs here since most of it is all quoted.
>
> I am not sure what do you mean by 'quoted' (enough quotes ?). I

Just quoted in the mail.

> carefully left everything not related to the ucontext_t definition out
> of sys/_ucontext.h. This way, there is no pollution of signal.h from
> the symbols you list below as contaminating. Since ucontext.h is not in
> POSIX, there is no compliance issues.

Yes, removal of ucontext.h in POSIX reduces the bugs.  The main remaining
ones are:

- <signal.h> includes <machine/ucontext.h> and <sys/_ucontext.h> and picks
   up any pollution there.  The main pollution there is:
   - mc_* in <machine/ucontext.h> (much more on arm64)
   - uc_* in <sys/_ucontext.h>.  This is only reserved in certain POSIX cases.
     Approximately POSIX >= 2008, or POSIX >= 2001 && XSI, or just certain
     XSI.
- POSIX between about 2001 and 2008 (and/or certain XSI) does have
   <ucontext.h>.  This seems to have just the old bugs, since any inclusion
   of this header means that you are using a version that supports it.  The
   old bugs are:
   - mc_* in <machine/ucontext.h> (much more on arm64)
   - a couple of nonstandard function.s

>>> --- head/sys/sys/ucontext.h	Fri Feb 12 07:27:24 2016	(r295560, copy source)
>>> +++ head/sys/sys/_ucontext.h	Fri Feb 12 07:38:19 2016	(r295561)
> ...
>>> -#ifndef _KERNEL
>>> -
>>> -__BEGIN_DECLS
>>> -
>>> -int	getcontext(ucontext_t *) __returns_twice;
>>> -ucontext_t *getcontextx(void);
>>
> Nothing of the listed functions is in any POSIX header.

They are in <ucontext.h>, which is in certain versions of POSIX.

>>> +/*
>>> + * Structure used in sigaltstack call.  Its definition is always
>>> + * needed for __ucontext.  If __BSD_VISIBLE is defined, the structure
>>> + * tag is actually sigaltstack.
>>> + */
>>> +struct __stack_t {
>>> +	void	*ss_sp;			/* signal stack base */
>>> +	__size_t ss_size;		/* signal stack length */
>>> +	int	ss_flags;		/* SS_DISABLE and/or SS_ONSTACK */
>>> +};
>>
>> This is now broken since it is outside of all ifdefs.  Its ss_* names are
>> in the application namespace for the non-POSIX case and POSIX before about
>> 2001.
> It only consumes the __stack_t tag which is both in the implementation name
> space (__ prefix) and in the POSIX reserved namespace (_t suffix).

#if _POSIX_VISIBLE < 200112
#define ss_sp  ss_sp is in the application namespace in versions of POSIX \
                before 2001.
#endif

BTW, the '<= 200112' ifdef for sigmask() is broken.  2001 is not before 2001.
(sigmask() was removed in 2001.)

>>> Modified: head/sys/x86/include/ucontext.h
>>> ==============================================================================
>>> --- head/sys/x86/include/ucontext.h	Fri Feb 12 07:27:24 2016	(r295560)
>>> +++ head/sys/x86/include/ucontext.h	Fri Feb 12 07:38:19 2016	(r295561)
>>> @@ -162,4 +162,9 @@ typedef struct __mcontext {
>>> } mcontext_t;
>>> #endif /* __amd64__ */
> The x86 ucontext.h is very accurate in not conflicting with the application
> namespace, all constants are defined in implementation-reserved namespace
> (have _M prefix).  I did not verified other arches, I only noted that MIPS
> has f_register_t bug.

Except for mc_*.  It started fairly clean and we cleaned it further a
few years ago.

>> I don't like the combined headers for x86, and this file is an especially
>> good bad example.  It consists of an __amd64__ ifdef and a __i386__ ifdef
>> with nothing shared outside except the copyright notice and the
>> idempotency ifdef.  This gives the lint bug.
>
> And I do like them.  More, I consider -m32 feature that was completed by
> the merging, an essential feature of the modern OS on x86_64.

Lots of the merge is a bad way to work around the design error of
<machine>.  -m32 should select the includes using -I, but it is hard
to think of a method less suited to that than <machine>.  I have no
good solution.

This reminds me that -m32 worked for about a month on freefall, but
has been broken by a missing -lgcc and possibly other libraries for a
couple of years.  More recently, old dynamically-linked executables
stopped working on freefall due to missing libraries at runtime.  I
should have known better than to use dynamic linkage.

Bruce


More information about the svn-src-head mailing list