Re: [PATCH] Fix invalid sigprocmask call

Re: [PATCH] Fix invalid sigprocmask call

[Yousong Zhou]

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

Re: [PATCH] Fix invalid sigprocmask call

[Pedro Alves]

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

Re: [PATCH] Fix invalid sigprocmask call

[Yousong Zhou]

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

Re: [musl] Re: [PATCH] Fix invalid sigprocmask call

[Rich Felker]

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

Re: [musl] Re: [PATCH] Fix invalid sigprocmask call

[Pedro Alves]

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

Re: [musl] Re: [PATCH] Fix invalid sigprocmask call

[Rich Felker]

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

Re: [musl] Re: [PATCH] Fix invalid sigprocmask call

[Andreas Schwab]

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.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [musl] Re: [PATCH] Fix invalid sigprocmask call

[Rich Felker]

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