Fix invalid sigprocmask call

  The POSIX document says

    The pthread_sigmask() and sigprocmask() functions shall fail if:

    [EINVAL]
    The value of the how argument is not equal to one of the defined values.

and this is how musl-libc is currently doing.  Fix the call to be safe
and correct

 [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_sigmask.html

gdb/ChangeLog:
2017-03-24  Yousong Zhou  <yszhou4tech@gmail.com>

    * common/signals-state-save-restore.c (save_original_signals_state):
    Fix invalid sigprocmask call.
---
 gdb/ChangeLog                           | 5 +++++
 gdb/common/signals-state-save-restore.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)
  

On 03/24/2017 03:01 AM, Yousong Zhou wrote:
> The POSIX document says
> 
>     The pthread_sigmask() and sigprocmask() functions shall fail if:
> 
>     [EINVAL]
>     The value of the how argument is not equal to one of the defined values.
> 
> and this is how musl-libc is currently doing.  Fix the call to be safe
> and correct
> 
>  [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_sigmask.html
> 

I don't agree.  It's a musl bug.  Please fix it / file a musl bug.

Note that that document also says (emphasis mine):

  If the argument set is not a null pointer, it points to a set of signals to
  be used to CHANGE the currently blocked set.

  The argument how indicates the way in which the set is CHANGED, and the
  application shall ensure it consists of one of the following values:

So the requirement to pass in one of SIG_BLOCK/SIG_SETMASK/SIG_UNBLOCK
only applies when we're CHANGING the set.

The Linux man pages also say (emphasis mine):

   If set is NULL, then the signal mask is unchanged (i.e., how is IGNORED), but the
   current value of the signal mask is nevertheless returned in oldset (if it is not NULL).

And this is AFAIK the historical Unix behavior.  For example,
"Advanced Programming in the UNIX Environment, 3rd edition"
says the same, and explicitly gives such an
example (section 10.13, page 347).  See:

https://books.google.pt/books?id=kCTMFpEcIOwC&pg=PA347&lpg=PA347&dq=sigprocmask+with+0+how&source=bl&ots=zvMvUNTumH&sig=u3b_iklV_mOcZRt0mNKkbj3dRFU&hl=en&sa=X&ved=0ahUKEwjl5b_Q9u7SAhUEPhQKHVsBCrM4ChDoAQguMAU#v=onepage&q=sigprocmask%20with%200%20how&f=false

Thanks,
Pedro Alves
  

On 24 March 2017 at 18:47, Pedro Alves <palves@redhat.com> wrote:
> On 03/24/2017 03:01 AM, Yousong Zhou wrote:
>> The POSIX document says
>>
>>     The pthread_sigmask() and sigprocmask() functions shall fail if:
>>
>>     [EINVAL]
>>     The value of the how argument is not equal to one of the defined values.
>>
>> and this is how musl-libc is currently doing.  Fix the call to be safe
>> and correct
>>
>>  [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_sigmask.html
>>
>
> I don't agree.  It's a musl bug.  Please fix it / file a musl bug.

I already did that before sending to gdb-patches

  http://www.openwall.com/lists/musl/2017/03/24/1

I am aware of the fact that the current code works with glibc and mac
osx 10.11.6.  The Linux kernel code at the moment also accepts the
call with how==0

But this is more about interpretation of POSIX document itself.  And
it says, clearly without pre-condition words or ambiguity in the
ERRORS section of that page, to return EINVAL if how is not equal to
one of the defined values.

I also tried to find some posix-compliant testsuite and to search the
github code for samples of pthread_sigmask call.  The first I came
across was the following code snippet at link
https://github.com/juj/posixtestsuite/blob/master/conformance/interfaces/pthread_sigmask/8-1.c#L57

        pthread_sigmask(SIG_BLOCK, NULL, &oactl);


Regards,
               yousong
  

yousong


On 24 March 2017 at 20:23, Yousong Zhou <yszhou4tech@gmail.com> wrote:
> On 24 March 2017 at 18:47, Pedro Alves <palves@redhat.com> wrote:
>> On 03/24/2017 03:01 AM, Yousong Zhou wrote:
>>> The POSIX document says
>>>
>>>     The pthread_sigmask() and sigprocmask() functions shall fail if:
>>>
>>>     [EINVAL]
>>>     The value of the how argument is not equal to one of the defined values.
>>>
>>> and this is how musl-libc is currently doing.  Fix the call to be safe
>>> and correct
>>>
>>>  [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_sigmask.html
>>>
>>
>> I don't agree.  It's a musl bug.  Please fix it / file a musl bug.
>
> I already did that before sending to gdb-patches
>
>   http://www.openwall.com/lists/musl/2017/03/24/1
>
> I am aware of the fact that the current code works with glibc and mac
> osx 10.11.6.  The Linux kernel code at the moment also accepts the
> call with how==0
>
> But this is more about interpretation of POSIX document itself.  And
> it says, clearly without pre-condition words or ambiguity in the
> ERRORS section of that page, to return EINVAL if how is not equal to
> one of the defined values.
>
> I also tried to find some posix-compliant testsuite and to search the
> github code for samples of pthread_sigmask call.  The first I came
> across was the following code snippet at link
> https://github.com/juj/posixtestsuite/blob/master/conformance/interfaces/pthread_sigmask/8-1.c#L57
>
>         pthread_sigmask(SIG_BLOCK, NULL, &oactl);
>
>
> Regards,
>                yousong

The following test case will also say for itself.

        https://github.com/juj/posixtestsuite/blob/master/conformance/interfaces/pthread_sigmask/16-1.c

Regards,
               yousong
  

On 03/24/2017 12:23 PM, Yousong Zhou wrote:
> On 24 March 2017 at 18:47, Pedro Alves <palves@redhat.com> wrote:
>> On 03/24/2017 03:01 AM, Yousong Zhou wrote:
>>> The POSIX document says
>>>
>>>     The pthread_sigmask() and sigprocmask() functions shall fail if:
>>>
>>>     [EINVAL]
>>>     The value of the how argument is not equal to one of the defined values.
>>>
>>> and this is how musl-libc is currently doing.  Fix the call to be safe
>>> and correct
>>>
>>>  [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_sigmask.html
>>>
>>
>> I don't agree.  It's a musl bug.  Please fix it / file a musl bug.
> 
> I already did that before sending to gdb-patches
> 
>   http://www.openwall.com/lists/musl/2017/03/24/1
> 
> I am aware of the fact that the current code works with glibc and mac
> osx 10.11.6.  The Linux kernel code at the moment also accepts the
> call with how==0

Cool.

> 
> But this is more about interpretation of POSIX document itself.  And
> it says, clearly without pre-condition words or ambiguity in the
> ERRORS section of that page, to return EINVAL if how is not equal to
> one of the defined values.

The standard wasn't built on a vacuum.  It starts by ratifying common
implementation behavior.  If no historical implementation behaves like what
you're suggesting, what's the point of enforcing that, when it's clearly
NOT the intent?  You're just causing porting pain for no good reason.
Please file a bug against the standard to have the error section clarified instead.

> 
> I also tried to find some posix-compliant testsuite and to search the
> github code for samples of pthread_sigmask call.  The first I came
> across was the following code snippet at link
> https://github.com/juj/posixtestsuite/blob/master/conformance/interfaces/pthread_sigmask/8-1.c#L57
> 
>         pthread_sigmask(SIG_BLOCK, NULL, &oactl);

The fact that that call includes SIG_BLOCK doesn't say whether
passing 0 should be rejected.

So I cloned that repo, and did a quick grep.  And lo:

https://github.com/juj/posixtestsuite/blob/26372421f53aeeeeeb4b23561c417886f1930ef6/conformance/interfaces/fork/12-1.c#L187

		/* Examine the current blocked signal set. USR1 & USR2 shall be present */
		ret = sigprocmask( 0, NULL, &mask );

		if ( ret != 0 )
		{
			UNRESOLVED( errno, "Sigprocmask failed in child" );
		}

Thanks,
Pedro Alves
  

On 03/24/2017 12:39 PM, Yousong Zhou wrote:

> The following test case will also say for itself.
> 
>         https://github.com/juj/posixtestsuite/blob/master/conformance/interfaces/pthread_sigmask/16-1.c

It does not.  The pthread_sigmask call there does NOT pass a NULL
as "set mask" argument:

	if (pthread_sigmask(r, &actl, NULL) != EINVAL) {
		perror("pthread_sigmask() did not fail even though invalid how parameter was passed to it.\n");
		pthread_exit((void*)-1);
	}

That case _should_ fail, of course.

Please just fix musl.

Thanks,
Pedro Alves
  

On 24 March 2017 at 20:55, Pedro Alves <palves@redhat.com> wrote:
> On 03/24/2017 12:23 PM, Yousong Zhou wrote:
>> On 24 March 2017 at 18:47, Pedro Alves <palves@redhat.com> wrote:
>>> On 03/24/2017 03:01 AM, Yousong Zhou wrote:
>>>> The POSIX document says
>>>>
>>>>     The pthread_sigmask() and sigprocmask() functions shall fail if:
>>>>
>>>>     [EINVAL]
>>>>     The value of the how argument is not equal to one of the defined values.
>>>>
>>>> and this is how musl-libc is currently doing.  Fix the call to be safe
>>>> and correct
>>>>
>>>>  [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_sigmask.html
>>>>
>>>
>>> I don't agree.  It's a musl bug.  Please fix it / file a musl bug.
>>
>> I already did that before sending to gdb-patches
>>
>>   http://www.openwall.com/lists/musl/2017/03/24/1
>>
>> I am aware of the fact that the current code works with glibc and mac
>> osx 10.11.6.  The Linux kernel code at the moment also accepts the
>> call with how==0
>
> Cool.
>
>>
>> But this is more about interpretation of POSIX document itself.  And
>> it says, clearly without pre-condition words or ambiguity in the
>> ERRORS section of that page, to return EINVAL if how is not equal to
>> one of the defined values.
>
> The standard wasn't built on a vacuum.  It starts by ratifying common
> implementation behavior.  If no historical implementation behaves like what
> you're suggesting, what's the point of enforcing that, when it's clearly
> NOT the intent?  You're just causing porting pain for no good reason.
> Please file a bug against the standard to have the error section clarified instead.

Lol, now I will consider the idea of bumping the door of POSIX committee ;)

>
>>
>> I also tried to find some posix-compliant testsuite and to search the
>> github code for samples of pthread_sigmask call.  The first I came
>> across was the following code snippet at link
>> https://github.com/juj/posixtestsuite/blob/master/conformance/interfaces/pthread_sigmask/8-1.c#L57
>>
>>         pthread_sigmask(SIG_BLOCK, NULL, &oactl);
>
> The fact that that call includes SIG_BLOCK doesn't say whether
> passing 0 should be rejected.
>
> So I cloned that repo, and did a quick grep.  And lo:
>
> https://github.com/juj/posixtestsuite/blob/26372421f53aeeeeeb4b23561c417886f1930ef6/conformance/interfaces/fork/12-1.c#L187
>
>                 /* Examine the current blocked signal set. USR1 & USR2 shall be present */
>                 ret = sigprocmask( 0, NULL, &mask );
>
>                 if ( ret != 0 )
>                 {
>                         UNRESOLVED( errno, "Sigprocmask failed in child" );
>                 }
>
> Thanks,
> Pedro Alves
>

Okay, then another fact that the posixtestsuite project also expects
to accept how==0

I am cc-ing musl-libc list now.

Regards,
                yousong
  

On 03/24/2017 01:05 PM, Yousong Zhou wrote:
> On 24 March 2017 at 20:55, Pedro Alves <palves@redhat.com> wrote:
>> The standard wasn't built on a vacuum.  It starts by ratifying common
>> implementation behavior.  If no historical implementation behaves like what
>> you're suggesting, what's the point of enforcing that, when it's clearly
>> NOT the intent?  You're just causing porting pain for no good reason.
>> Please file a bug against the standard to have the error section clarified instead.
> 
> Lol, now I will consider the idea of bumping the door of POSIX committee ;)

No need.  Go here:

 http://austingroupbugs.net/

> I am cc-ing musl-libc list now.

Original thread here:

 https://sourceware.org/ml/gdb-patches/2017-03/msg00426.html

Thanks,
Pedro Alves
  

On Mär 24 2017, Pedro Alves <palves@redhat.com> wrote:

> On 03/24/2017 03:01 AM, Yousong Zhou wrote:
>> The POSIX document says
>> 
>>     The pthread_sigmask() and sigprocmask() functions shall fail if:
>> 
>>     [EINVAL]
>>     The value of the how argument is not equal to one of the defined values.
>> 
>> and this is how musl-libc is currently doing.  Fix the call to be safe
>> and correct
>> 
>>  [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_sigmask.html
>> 
>
> I don't agree.  It's a musl bug.  Please fix it / file a musl bug.
>
> Note that that document also says (emphasis mine):
>
>   If the argument set is not a null pointer, it points to a set of signals to
>   be used to CHANGE the currently blocked set.
>
>   The argument how indicates the way in which the set is CHANGED, and the
>   application shall ensure it consists of one of the following values:

Later on it says:

    If set is a null pointer, the value of the argument how is not
    significant ...

Andreas.
  

On Fri, Mar 24, 2017 at 09:05:15PM +0800, Yousong Zhou wrote:
> On 24 March 2017 at 20:55, Pedro Alves <palves@redhat.com> wrote:
> > On 03/24/2017 12:23 PM, Yousong Zhou wrote:
> >> On 24 March 2017 at 18:47, Pedro Alves <palves@redhat.com> wrote:
> >>> On 03/24/2017 03:01 AM, Yousong Zhou wrote:
> >>>> The POSIX document says
> >>>>
> >>>>     The pthread_sigmask() and sigprocmask() functions shall fail if:
> >>>>
> >>>>     [EINVAL]
> >>>>     The value of the how argument is not equal to one of the defined values.
> >>>>
> >>>> and this is how musl-libc is currently doing.  Fix the call to be safe
> >>>> and correct
> >>>>
> >>>>  [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_sigmask.html
> >>>>
> >>>
> >>> I don't agree.  It's a musl bug.  Please fix it / file a musl bug.
> >>
> >> I already did that before sending to gdb-patches
> >>
> >>   http://www.openwall.com/lists/musl/2017/03/24/1
> >>
> >> I am aware of the fact that the current code works with glibc and mac
> >> osx 10.11.6.  The Linux kernel code at the moment also accepts the
> >> call with how==0
> >
> > Cool.
> >
> >>
> >> But this is more about interpretation of POSIX document itself.  And
> >> it says, clearly without pre-condition words or ambiguity in the
> >> ERRORS section of that page, to return EINVAL if how is not equal to
> >> one of the defined values.
> >
> > The standard wasn't built on a vacuum.  It starts by ratifying common
> > implementation behavior.  If no historical implementation behaves like what
> > you're suggesting, what's the point of enforcing that, when it's clearly
> > NOT the intent?  You're just causing porting pain for no good reason.
> > Please file a bug against the standard to have the error section clarified instead.
> 
> Lol, now I will consider the idea of bumping the door of POSIX committee ;)

If you file a report and it's deemed a bug in the standard and
changed, I'm happy to change this on the musl side. Just keep me
posted on what happens. I don't have any preference on what the
behavior "should" be here (IMO imposing a behavior when the caller has
violated constraints like passed a wrong value for how is pointless
anyway) but I do want to conform to the standard.

> >> I also tried to find some posix-compliant testsuite and to search the
> >> github code for samples of pthread_sigmask call.  The first I came
> >> across was the following code snippet at link
> >> https://github.com/juj/posixtestsuite/blob/master/conformance/interfaces/pthread_sigmask/8-1.c#L57
> >>
> >>         pthread_sigmask(SIG_BLOCK, NULL, &oactl);
> >
> > The fact that that call includes SIG_BLOCK doesn't say whether
> > passing 0 should be rejected.
> >
> > So I cloned that repo, and did a quick grep.  And lo:
> >
> > https://github.com/juj/posixtestsuite/blob/26372421f53aeeeeeb4b23561c417886f1930ef6/conformance/interfaces/fork/12-1.c#L187
> >
> >                 /* Examine the current blocked signal set. USR1 & USR2 shall be present */
> >                 ret = sigprocmask( 0, NULL, &mask );
> >
> >                 if ( ret != 0 )
> >                 {
> >                         UNRESOLVED( errno, "Sigprocmask failed in child" );
> >                 }
> >
> > Thanks,
> > Pedro Alves
> >
> 
> Okay, then another fact that the posixtestsuite project also expects
> to accept how==0
> 
> I am cc-ing musl-libc list now.

If you're talking about "Open POSIX Test Suite", which the above link
seems to point to a fork of, the majority of its tests are invalid,
invoking undefined behavior then asserting that the error/effect they
wrongly expect happens. Without a thorough audit of its test validity
it's less than worthless.

Rich
  

On 03/24/2017 03:35 PM, Rich Felker wrote:
> If you file a report and it's deemed a bug in the standard and
> changed, I'm happy to change this on the musl side. Just keep me
> posted on what happens. 

Keep me posted as well.

> I don't have any preference on what the
> behavior "should" be here (IMO imposing a behavior when the caller has
> violated constraints like passed a wrong value for how is pointless
> anyway) but I do want to conform to the standard.

IMO, no constrains were violated.

Thanks,
Pedro Alves
  

On Fri, Mar 24, 2017 at 03:52:08PM +0000, Pedro Alves wrote:
> On 03/24/2017 03:35 PM, Rich Felker wrote:
> > If you file a report and it's deemed a bug in the standard and
> > changed, I'm happy to change this on the musl side. Just keep me
> > posted on what happens. 
> 
> Keep me posted as well.
> 
> > I don't have any preference on what the
> > behavior "should" be here (IMO imposing a behavior when the caller has
> > violated constraints like passed a wrong value for how is pointless
> > anyway) but I do want to conform to the standard.
> 
> IMO, no constrains were violated.

I don't mean a constraint in the formal sense. Rather I'm talking
about the whole class of errors that are "programming mistake caused a
wrong/nonsensical value to be passed for an argument" as opposed to
errors that can legitimately happen in a non-buggy program (out of
memory, no such file, limit exceeded, invalid input, etc.). Sorry I
wasn't clear.

Rich
  

On Mär 24 2017, Rich Felker <dalias@aerifal.cx> wrote:

> If you file a report and it's deemed a bug in the standard and
> changed, I'm happy to change this on the musl side. Just keep me
> posted on what happens. I don't have any preference on what the
> behavior "should" be here (IMO imposing a behavior when the caller has
> violated constraints like passed a wrong value for how is pointless
> anyway) but I do want to conform to the standard.

The standard says that how must be ignored if set is NULL.

Andreas.
  

On Fri, Mar 24, 2017 at 06:33:55PM +0100, Andreas Schwab wrote:
> On Mär 24 2017, Rich Felker <dalias@aerifal.cx> wrote:
> 
> > If you file a report and it's deemed a bug in the standard and
> > changed, I'm happy to change this on the musl side. Just keep me
> > posted on what happens. I don't have any preference on what the
> > behavior "should" be here (IMO imposing a behavior when the caller has
> > violated constraints like passed a wrong value for how is pointless
> > anyway) but I do want to conform to the standard.
> 
> The standard says that how must be ignored if set is NULL.

The standard says in one place that it's "not significant" if "set is
a null pointer", but then in another that the call "shall fail" if
"the how argument is not equal to one of the defined values". The
former could be interpreted either as allowing any of the 3 defined
values (doesn't matter which) for how when set is null, or allowing
any value at all; the latter interpretation conflicts with the
normative errors section.

Anyway I don't think language-lawyering this offline is productive. If
anyone really cares about the behavior one way or the other, take it
to the Austin Group tracker where the decision-makers will see it.

Rich
  

On 24 March 2017 at 21:37, Pedro Alves <palves@redhat.com> wrote:
> On 03/24/2017 01:05 PM, Yousong Zhou wrote:
>> On 24 March 2017 at 20:55, Pedro Alves <palves@redhat.com> wrote:
>>> The standard wasn't built on a vacuum.  It starts by ratifying common
>>> implementation behavior.  If no historical implementation behaves like what
>>> you're suggesting, what's the point of enforcing that, when it's clearly
>>> NOT the intent?  You're just causing porting pain for no good reason.
>>> Please file a bug against the standard to have the error section clarified instead.
>>
>> Lol, now I will consider the idea of bumping the door of POSIX committee ;)
>
> No need.  Go here:
>
>  http://austingroupbugs.net/

Thanks for the pointer.  Here is the issue link

    http://austingroupbugs.net/view.php?id=1132


Regards,
                yousong