Discussion:
sigprocmask and aliasing
(too old to reply)
Richard Kettlewell
2020-04-17 15:44:37 UTC
Permalink
https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigprocmask.html

int sigprocmask(int how, const sigset_t *restrict set,
sigset_t *restrict oset);

Can anyone justify the restrict-qualification of the set and oset
arguments, which forbids an otherwise perfectly reasonable idiom for
modifying the signal mask while also capturing its old value for later
restoration?

The change history says “The restrict keyword is added to the
pthread_sigmask() and sigprocmask() prototypes for alignment with the
ISO/IEC 9899:1999 standard.” but these functions do not appear in
standard C, and the C standard does not actually require one to apply
restrict qualifiers to function arguments willy-nilly, this doesn’t
really explain the change.
--
https://www.greenend.org.uk/rjk/
Scott Lurndal
2020-04-17 17:00:26 UTC
Permalink
Post by Richard Kettlewell
https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigprocmask.html
int sigprocmask(int how, const sigset_t *restrict set,
sigset_t *restrict oset);
Can anyone justify the restrict-qualification of the set and oset
arguments, which forbids an otherwise perfectly reasonable idiom for
modifying the signal mask while also capturing its old value for later
restoration?
Probably because some implementations access both at the same time,
e.g. when looping through the set of signals (test new and store old, bit by bit).
Richard Kettlewell
2020-04-17 17:02:44 UTC
Permalink
Post by Scott Lurndal
Post by Richard Kettlewell
https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigprocmask.html
int sigprocmask(int how, const sigset_t *restrict set,
sigset_t *restrict oset);
Can anyone justify the restrict-qualification of the set and oset
arguments, which forbids an otherwise perfectly reasonable idiom for
modifying the signal mask while also capturing its old value for later
restoration?
Probably because some implementations access both at the same time,
e.g. when looping through the set of signals (test new and store old, bit by bit).
Which ones?
--
https://www.greenend.org.uk/rjk/
Nicolas George
2020-04-17 17:09:26 UTC
Permalink
Richard Kettlewell , dans le message
Post by Richard Kettlewell
Post by Scott Lurndal
Probably
Which ones?
If he knew which ones, he would not have started with "probably", would
he?
Kaz Kylheku
2020-04-17 17:51:36 UTC
Permalink
Post by Scott Lurndal
Post by Richard Kettlewell
https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigprocmask.html
int sigprocmask(int how, const sigset_t *restrict set,
sigset_t *restrict oset);
Can anyone justify the restrict-qualification of the set and oset
arguments, which forbids an otherwise perfectly reasonable idiom for
modifying the signal mask while also capturing its old value for later
restoration?
Probably because some implementations access both at the same time,
e.g. when looping through the set of signals (test new and store old, bit by bit).
But

1) that bit-by-bit loop will still work fine if the objects overlap exactly!

2) you'd think POSIX would make specific remarks about this, rather than
some vague, nonsensical statement about ISO C conformance.
Scott Lurndal
2020-04-17 18:34:12 UTC
Permalink
Post by Kaz Kylheku
Post by Scott Lurndal
Post by Richard Kettlewell
https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigprocmask.html
int sigprocmask(int how, const sigset_t *restrict set,
sigset_t *restrict oset);
Can anyone justify the restrict-qualification of the set and oset
arguments, which forbids an otherwise perfectly reasonable idiom for
modifying the signal mask while also capturing its old value for later
restoration?
Probably because some implementations access both at the same time,
e.g. when looping through the set of signals (test new and store old, bit by bit).
But
1) that bit-by-bit loop will still work fine if the objects overlap exactly!
2) you'd think POSIX would make specific remarks about this, rather than
some vague, nonsensical statement about ISO C conformance.
As I recall it, we added restrict pretty generally to allow implementation
flexibility. But that was many years ago now. It's entirely likely that
it is related to the const qualifier on the input argument.
Kaz Kylheku
2020-04-17 17:33:33 UTC
Permalink
Post by Richard Kettlewell
https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigprocmask.html
int sigprocmask(int how, const sigset_t *restrict set,
sigset_t *restrict oset);
Can anyone justify the restrict-qualification of the set and oset
arguments, which forbids an otherwise perfectly reasonable idiom for
modifying the signal mask while also capturing its old value for later
restoration?
Wow, that's idiotic.

(So is restrict itself, but that's another discussion I'm prepared
to defend and win another time.)

(Phew, I'm not violating this anyhere.)

But you can adjust your idiom by allocating an extra set:

sigprocmask(SIG_BLOCK, &blocked_set, &saved_set);

...

sigprocmask(SIG_SETMASK, &saved_set, NULL);

Reviewing even a large code base for this shouldn't be a monstrous
undertaking.
Kaz Kylheku
2020-04-17 18:23:31 UTC
Permalink
Post by Kaz Kylheku
Post by Richard Kettlewell
https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigprocmask.html
int sigprocmask(int how, const sigset_t *restrict set,
sigset_t *restrict oset);
Can anyone justify the restrict-qualification of the set and oset
arguments, which forbids an otherwise perfectly reasonable idiom for
modifying the signal mask while also capturing its old value for later
restoration?
Wow, that's idiotic.
Or, is it? What if the implementation is structured like this?

{
if (oset)
*oset = the_process_mask;

if (set) switch (how) {
/* apply the *set bits to the_process_mask according to how */
}
}

That seems like a reasonable thing to do: before doing anything,
snapshot the old. If that obliterates the input, it's borked.
James Kuyper
2020-04-17 17:54:37 UTC
Permalink
Post by Richard Kettlewell
https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigprocmask.html
int sigprocmask(int how, const sigset_t *restrict set,
sigset_t *restrict oset);
Can anyone justify the restrict-qualification of the set and oset
arguments, which forbids an otherwise perfectly reasonable idiom for
modifying the signal mask while also capturing its old value for later
restoration?
The change history says “The restrict keyword is added to the
pthread_sigmask() and sigprocmask() prototypes for alignment with the
ISO/IEC 9899:1999 standard.” but these functions do not appear in
standard C, and the C standard does not actually require one to apply
restrict qualifiers to function arguments willy-nilly, this doesn’t
really explain the change.
Do you know specifically what it said in the version prior to that
change? One possibility would be to allow an implementation that fills
in *oset with the current mask values before setting them to the values
specified in *set.

If that's the case, it's possible that older versions of POSIX dealt
with the same issue by specifying that it was undefined behavior to set
both pointers to point at the same object; after C99 was approved, that
same restriction was conveyed by restrict-qualifying the the pointers.
Such changes did occur in the C standard. For instance, C90 said, about
memcpy(), that "If copying takes place between objects that overlap, the
behavior is undefined." (4.11.2.1). In C99, that statement was
discarded, because the new 'restrict' qualifiers conveyed the same
information.

It could also be a new restriction, added simply because someone was
reviewing all the places in POSIX in order to figure out where
'restrict' should be added. Again, there's precedent in the C standard -
no previous version of the C standard bothered specifying that it was
undefined behavior to call

fread(infile, 1, sizeof *infile, infile);

I suspect that it was felt so obvious that this wasn't a good idea, that
they didn't bother saying so. However, when they were reviewing to
determine where the new restrict keyword should be added to existing
library routines, fread() was an obvious one.
Richard Kettlewell
2020-04-17 18:37:22 UTC
Permalink
Post by James Kuyper
Post by Richard Kettlewell
https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigprocmask.html
int sigprocmask(int how, const sigset_t *restrict set,
sigset_t *restrict oset);
Can anyone justify the restrict-qualification of the set and oset
arguments, which forbids an otherwise perfectly reasonable idiom for
modifying the signal mask while also capturing its old value for later
restoration?
The change history says “The restrict keyword is added to the
pthread_sigmask() and sigprocmask() prototypes for alignment with the
ISO/IEC 9899:1999 standard.” but these functions do not appear in
standard C, and the C standard does not actually require one to apply
restrict qualifiers to function arguments willy-nilly, this doesn’t
really explain the change.
Do you know specifically what it said in the version prior to that
change?
https://pubs.opengroup.org/onlinepubs/007908799/xsh/sigprocmask.html
Post by James Kuyper
One possibility would be to allow an implementation that fills
in *oset with the current mask values before setting them to the values
specified in *set.
If so then it’s a shame that someone took the time to write down a
rationale for the change, but not the real one.
Post by James Kuyper
If that's the case, it's possible that older versions of POSIX dealt
with the same issue by specifying that it was undefined behavior to set
both pointers to point at the same object; after C99 was approved, that
same restriction was conveyed by restrict-qualifying the the pointers.
The old text contains no such restriction that I can see. Issue 7
forbids programs that were valid under issue 6.
Post by James Kuyper
Such changes did occur in the C standard. For instance, C90 said, about
memcpy(), that "If copying takes place between objects that overlap, the
behavior is undefined." (4.11.2.1). In C99, that statement was
discarded, because the new 'restrict' qualifiers conveyed the same
information.
It could also be a new restriction, added simply because someone was
reviewing all the places in POSIX in order to figure out where
'restrict' should be added. Again, there's precedent in the C standard -
no previous version of the C standard bothered specifying that it was
undefined behavior to call
fread(infile, 1, sizeof *infile, infile);
I suspect that it was felt so obvious that this wasn't a good idea, that
they didn't bother saying so. However, when they were reviewing to
determine where the new restrict keyword should be added to existing
library routines, fread() was an obvious one.
There’s no reasonable idiom in which the first and last args of fread
match; the same isn’t true of sigprocmask.
--
https://www.greenend.org.uk/rjk/
Kaz Kylheku
2020-04-17 23:15:09 UTC
Permalink
Post by Richard Kettlewell
Post by James Kuyper
One possibility would be to allow an implementation that fills
in *oset with the current mask values before setting them to the values
specified in *set.
If so then it’s a shame that someone took the time to write down a
rationale for the change, but not the real one.
Post by James Kuyper
If that's the case, it's possible that older versions of POSIX dealt
with the same issue by specifying that it was undefined behavior to set
both pointers to point at the same object; after C99 was approved, that
same restriction was conveyed by restrict-qualifying the the pointers.
The old text contains no such restriction that I can see. Issue 7
forbids programs that were valid under issue 6.
Even if it is so, if *implementations* did not accept those programs
in the first place, then they weren't *de facto* correct or portable;
Effectively 7 just codified it that way instead of requiring
implementors to support overlapping arguments.
Kaz Kylheku
2020-04-17 23:18:48 UTC
Permalink
Post by James Kuyper
Such changes did occur in the C standard. For instance, C90 said, about
memcpy(), that "If copying takes place between objects that overlap, the
behavior is undefined." (4.11.2.1). In C99, that statement was
discarded, because the new 'restrict' qualifiers conveyed the same
information.
I am looking at a copy of C99 (ISO 9899:1999, Second edition, 1999-12-01).
The wording is still there for memcpy.
Kaz Kylheku
2020-04-17 23:56:48 UTC
Permalink
Post by Richard Kettlewell
https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigprocmask.html
int sigprocmask(int how, const sigset_t *restrict set,
sigset_t *restrict oset);
Can anyone justify the restrict-qualification of the set and oset
arguments, which forbids an otherwise perfectly reasonable idiom for
modifying the signal mask while also capturing its old value for later
restoration?
1990-vintage 4.3BSD Reno had a sigprocmask:

The kernel code stores the returned mask through the "int *" pointer
before doing anything else.

https://www.tuhs.org/cgi-bin/utree.pl?file=4.3BSD-Reno/src/sys/kern/kern_sig.c

But that is deceptive. That "int *" retval mechanism is something
that seems to be part of the system call machinery; the word written
into that ends up in a register (%eax on i386).

The comment refers to additional behaviors handled in the library stub.
That stub is assembly code for various arches. Here is i386:

https://www.tuhs.org/cgi-bin/utree.pl?file=4.3BSD-Reno/src/lib/libc/i386/sys/sigprocmask.s

This assembly stub dereferences the new signals pointer, and converts it to a
by-value passage into the system call (all the signals in one 32 bit
register) confirming what the comment in the C kernel code says.

The system call returns the new signals in %eax, and the
assembly stub stores that through the old signals pointer if that is
not null.

So that looks like it will work fine with overlapping pointers on i386.

Here is the matching man page source:

https://www.tuhs.org/cgi-bin/utree.pl?file=4.3BSD-Reno/src/lib/libc/sys/sigprocmask.2
--
TXR Programming Lanuage: http://nongnu.org/txr
Music DIY Mailing List: http://www.kylheku.com/diy
ADA MP-1 Mailing List: http://www.kylheku.com/mp1
Kaz Kylheku
2020-04-18 00:15:14 UTC
Permalink
Post by Richard Kettlewell
https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigprocmask.html
int sigprocmask(int how, const sigset_t *restrict set,
sigset_t *restrict oset);
Can anyone justify the restrict-qualification of the set and oset
arguments, which forbids an otherwise perfectly reasonable idiom for
modifying the signal mask while also capturing its old value for later
restoration?
OpenSolaris b135 from 2010:

https://www.tuhs.org/cgi-bin/utree.pl?file=OpenSolaris_b135/uts/common/syscall/sigprocmask.c

Summary: totally fine.

sigprocmask copies the new signal mask from user space on entry into a
local buffer, and copies out the old one out to user space on exit. It
doesn't touch the user-space pointers beyond that, so is immune to
overlap issues.

Internally, the same buffer is used for preparing the returned old set
as was used for preparing the new set: that is to say, the signals
returned to user space are copied out of the same object that was used
to copy them in from the other argument, so even if the program uses
distinct objects, they are multiplexed onto one object.

The mask update according to "how" is done by a lower level function
lwp_sigmask which uses pass-by-value; it take two words' worth of
signals as two arguments and returns the old ones as a tuple.
Kaz Kylheku
2020-04-18 00:31:26 UTC
Permalink
Post by Kaz Kylheku
Post by Richard Kettlewell
https://pubs.opengroup.org/onlinepubs/9699919799/functions/sigprocmask.html
int sigprocmask(int how, const sigset_t *restrict set,
sigset_t *restrict oset);
Can anyone justify the restrict-qualification of the set and oset
arguments, which forbids an otherwise perfectly reasonable idiom for
modifying the signal mask while also capturing its old value for later
restoration?
https://www.tuhs.org/cgi-bin/utree.pl?file=OpenSolaris_b135/uts/common/syscall/sigprocmask.c
Summary: totally fine.
FreeBSD 5.3: fine

https://www.tuhs.org/cgi-bin/utree.pl?file=FreeBSD-5.3/sys/kern/kern_sig.c

The internal kern_sigprocmask is not fine against overlapping. It does
the obvious thing of saving the old one up-front. But that's actually
called from the real sig_procmask below which aims the pointers at two
distinct local variables. Those variables correspond with user space
by copying.

I can't so far be seem to be able to build a case for defending the
restrict stuff added by POSIX. Overlapping sigprocmask arguments are
supported by some classic operating systems.

(tuhs.org has is short on AT&T stuff, other than very old,
unsurprisingly. Would love to look into SVR4.)

(Man, so much of this code is so Mickey Mouse amateurish ... what was
all the legal wrangling about, you know? It's amazing this crud
actually booted, multi-tasked, and inspired zealotry in the user base.)
Scott Lurndal
2020-04-18 15:55:45 UTC
Permalink
Post by Kaz Kylheku
(tuhs.org has is short on AT&T stuff, other than very old,
unsurprisingly. Would love to look into SVR4.)
/*
*
* int
* sigprocmask(struct sigprocmaska *uap, rval_t *rvp)
* The sigprocmask() system call.
*
* Calling/Exit State:
* No locks to be held on entry and none will be held on return.
*
*/
/* ARGSUSED */
int
sigprocmask(struct sigprocmaska *uap, rval_t *rvp)
{
k_sigset_t kset;
k_sigset_t heldset;

/*
* User's oset and set might be the same address, so copyin first and
* save before copying out.
*/
if (uap->set) {
sigset_t set;
if (copyin((void *)uap->set, &set, sizeof set))
return EFAULT;
sigutok(&set, &kset);
}

if (uap->oset) {
sigset_t set;
/*
* Since we are merely reading the held mask for the
* current context and since this mask can only be
* changed by the owning context, we do not acquire
* the p_mutex lock.
*/

sigktou(&u.u_lwpp->l_sigheld, &set);
if (copyout(&set, (void *)uap->oset, sizeof set))
return EFAULT;
}

if (uap->set) {
sigdiffset(&kset, &sig_cantmask);
switch (uap->how) {
case SIG_BLOCK:
sigorset(&kset, &u.u_lwpp->l_sigheld);
lwpsigmask(kset);
break;
case SIG_UNBLOCK:
heldset = u.u_lwpp->l_sigheld;
sigdiffset(&heldset, &kset);
lwpsigmask(heldset);
break;
case SIG_SETMASK:
lwpsigmask(kset);
break;
default:
return EINVAL;
}
}

return 0;
}
Kaz Kylheku
2020-04-21 17:14:51 UTC
Permalink
Post by Scott Lurndal
Post by Kaz Kylheku
(tuhs.org has is short on AT&T stuff, other than very old,
unsurprisingly. Would love to look into SVR4.)
/*
*
* int
* sigprocmask(struct sigprocmaska *uap, rval_t *rvp)
* The sigprocmask() system call.
*
* No locks to be held on entry and none will be held on return.
*
*/
/* ARGSUSED */
int
sigprocmask(struct sigprocmaska *uap, rval_t *rvp)
{
k_sigset_t kset;
k_sigset_t heldset;
/*
* User's oset and set might be the same address, so copyin first and
* save before copying out.
*/
Thhanks for that, Scott. Really, it looks like someone should contact
the POSIX people (is it still "Austin Group"?) with a defect report.

The specification of sigpromask should encode what is looking to be
the predominant existing implementation practice.

A reasonable specification might be that "oset and newset may be
pointers to objects which overlap, if the overlap is exact: that is,
oset and newset may be the same pointer."

Copying implementations like this quoted one here, and also the others I
presented, would work with inexact overlap also, but the utility of that
seems vanishingly small.

An implementaton of sigprocmask can use "restrict" internally if it
wishes, after checkig the non-restricted pointers for overlap.


static int sigprocmask_ovlap(int how, const sigset_t *newset, sigset_t *oset);
static int sigprocmask_nolap(int how, const sigset_t *restrict newset,
sigset_t *restrict oset);

int sigprocmask(int how, const sigset_t *newset, sigset_t *oset)
{
return ((newset == oset) ?
sigprocmask_ovlap : sigprocmask_nolap)(how, newset, oset);
}

So then whatever optimization benefit comes from restrict is still
obtained, with a bit of penalty for the equality test.

Since implementations copy data across the kernel boundary, this is
all moot anyway.
--
TXR Programming Lanuage: http://nongnu.org/txr
Music DIY Mailing List: http://www.kylheku.com/diy
ADA MP-1 Mailing List: http://www.kylheku.com/mp1
James Kuyper
2020-04-21 19:04:40 UTC
Permalink
...
Post by Kaz Kylheku
An implementaton of sigprocmask can use "restrict" internally if it
wishes, after checkig the non-restricted pointers for overlap.
static int sigprocmask_ovlap(int how, const sigset_t *newset, sigset_t *oset);
static int sigprocmask_nolap(int how, const sigset_t *restrict newset,
sigset_t *restrict oset);
int sigprocmask(int how, const sigset_t *newset, sigset_t *oset)
{
return ((newset == oset) ?
sigprocmask_ovlap : sigprocmask_nolap)(how, newset, oset);
}
So then whatever optimization benefit comes from restrict is still
obtained, with a bit of penalty for the equality test.
I would expect that the optimization benefit that comes from the
restrict is entirely due to the fact that it allows you to skip the test
of whether newset == oset, so that approach wouldn't help.

If you changed those functions to have external linkage, I'd expect that
with a reasonably good optimizing compiler, the following code:

int ret = sigprocmask_nolap(how, &myset, &tset);
myset = tset;

would probably be at least as fast as

int ret = sigprocmask(how, &myset, &myset);

On the other hand, I'd expect sigprocmask_nolap(how, &myset, NULL) to be
marginally faster than either, while signprocmask(how, &myset, NULL)
would not be.

I suspect that's precisely the reason why the real sigprocmask()
function is declared the same way as your sigprocmask_nolap(), except
with external linkage: people who don't need a copy of the old mask pay
the price in their own code of making that possible, while those who
don't, don't have to pay that price.
Kaz Kylheku
2020-04-21 22:03:17 UTC
Permalink
Post by James Kuyper
...
Post by Kaz Kylheku
An implementaton of sigprocmask can use "restrict" internally if it
wishes, after checkig the non-restricted pointers for overlap.
static int sigprocmask_ovlap(int how, const sigset_t *newset, sigset_t *oset);
static int sigprocmask_nolap(int how, const sigset_t *restrict newset,
sigset_t *restrict oset);
int sigprocmask(int how, const sigset_t *newset, sigset_t *oset)
{
return ((newset == oset) ?
sigprocmask_ovlap : sigprocmask_nolap)(how, newset, oset);
}
So then whatever optimization benefit comes from restrict is still
obtained, with a bit of penalty for the equality test.
I would expect that the optimization benefit that comes from the
restrict is entirely due to the fact that it allows you to skip the test
of whether newset == oset, so that approach wouldn't help.
The newset == oset test doesn't represent all the cases that break
on the the nolap version.

If we don't use restrict at all, and simply write a function which
divides its implementation based on these two cases, the newset != oset
branch of the code cannot be optimized aggressively. It is still subject
to the suspiction that the objects might overlap inexactly.

By shunting that case to a restrict-decorated function, we turn that
suspicion into undefined behavior, allowing for the aggressive
treatment.
Post by James Kuyper
If you changed those functions to have external linkage, I'd expect that
int ret = sigprocmask_nolap(how, &myset, &tset);
myset = tset;
would probably be at least as fast as
int ret = sigprocmask(how, &myset, &myset);
The wrapper could be an inline function, so the comparison could be
elided in cases like this.
Post by James Kuyper
On the other hand, I'd expect sigprocmask_nolap(how, &myset, NULL) to be
marginally faster than either, while signprocmask(how, &myset, NULL)
would not be.
So with an inlined sigprocmask, the latter could reduce to the former
at compile time.

Loading...