manual: Document getcontext uc_stack value on Linux [BZ #759]

Message ID 1494961861-30734-1-git-send-email-adhemerval.zanella@linaro.org
State Dropped
Headers

Commit Message

Adhemerval Zanella May 16, 2017, 7:11 p.m. UTC
  As decribed in BZ#759, Linux getcontext implementation on Linux does
differs from other SysV system about the returned uc_stack.  This is
true not only for i386, but for all the architecture I could actually
check (aarch64, arm, alpha, hppa, m68k, mips, mips64, mips64n32,
powerpc, powerpc64, powerpc64le, s390x, sh, sparc, sparc64, and x86).

And I think we should not change current behavior for some reasons:

1. POSIX 2008 removed this SySV interface for a good reason and changing
   this behavior adds nothing for current portable code.  POSIX 2001
   specification does states that stack should be saved [1] and current
   GLIBC code does in a arch-specific manner (inside the mcontext_t)
   which allows the setcontext to work correctly.

2. Changing this behavior would potentially require compat symbols and
   I see no gain in adding compat symbols for deprecated interfaces.

3. Also, for comment #2 in BZ#759, it is up to kernel do setup the contents
   for ucontext_t and currently it does not provide the stack information
   as well.  Trying to change it is also another fix that does not worth
   the possible gains.

Instead my proposal is to make it clear the current interface behavior in
glibc documentation and close this bug as invalid.

	[BZ #759]
	* manual/setjmp.texi (getcontex): Document uc_stack value on Linux.

[1] http://pubs.opengroup.org/onlinepubs/009695399/functions/getcontext.html
---
 ChangeLog          | 5 +++++
 manual/setjmp.texi | 5 +++++
 2 files changed, 10 insertions(+)
  

Comments

Adhemerval Zanella June 23, 2017, 6:52 p.m. UTC | #1
Ping.

On 16/05/2017 16:11, Adhemerval Zanella wrote:
> As decribed in BZ#759, Linux getcontext implementation on Linux does
> differs from other SysV system about the returned uc_stack.  This is
> true not only for i386, but for all the architecture I could actually
> check (aarch64, arm, alpha, hppa, m68k, mips, mips64, mips64n32,
> powerpc, powerpc64, powerpc64le, s390x, sh, sparc, sparc64, and x86).
> 
> And I think we should not change current behavior for some reasons:
> 
> 1. POSIX 2008 removed this SySV interface for a good reason and changing
>    this behavior adds nothing for current portable code.  POSIX 2001
>    specification does states that stack should be saved [1] and current
>    GLIBC code does in a arch-specific manner (inside the mcontext_t)
>    which allows the setcontext to work correctly.
> 
> 2. Changing this behavior would potentially require compat symbols and
>    I see no gain in adding compat symbols for deprecated interfaces.
> 
> 3. Also, for comment #2 in BZ#759, it is up to kernel do setup the contents
>    for ucontext_t and currently it does not provide the stack information
>    as well.  Trying to change it is also another fix that does not worth
>    the possible gains.
> 
> Instead my proposal is to make it clear the current interface behavior in
> glibc documentation and close this bug as invalid.
> 
> 	[BZ #759]
> 	* manual/setjmp.texi (getcontex): Document uc_stack value on Linux.
> 
> [1] http://pubs.opengroup.org/onlinepubs/009695399/functions/getcontext.html
> ---
>  ChangeLog          | 5 +++++
>  manual/setjmp.texi | 5 +++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/manual/setjmp.texi b/manual/setjmp.texi
> index 94d16be..07cc7bf 100644
> --- a/manual/setjmp.texi
> +++ b/manual/setjmp.texi
> @@ -302,6 +302,11 @@ the content of the registers, the signal mask, and the current stack.
>  Executing the contents would start at the point where the
>  @code{getcontext} call just returned.
>  
> +On Linux the stack information return on @code{uc_stack} is empty by
> +default.  It also the case for signal handling information through
> +@code{sigaction} with @code{SA_SIGINFO}.  It could be obtained using
> +architecture specific field from @code{uc_mcontext} member.
> +
>  The function returns @code{0} if successful.  Otherwise it returns
>  @code{-1} and sets @var{errno} accordingly.
>  @end deftypefun
>
  
Adhemerval Zanella July 11, 2017, 5:56 p.m. UTC | #2
Ping (x2). I think something we won't bother to emulate solaris behaviour,
so in a short we should document it and close it.

On 23/06/2017 15:52, Adhemerval Zanella wrote:
> Ping.
> 
> On 16/05/2017 16:11, Adhemerval Zanella wrote:
>> As decribed in BZ#759, Linux getcontext implementation on Linux does
>> differs from other SysV system about the returned uc_stack.  This is
>> true not only for i386, but for all the architecture I could actually
>> check (aarch64, arm, alpha, hppa, m68k, mips, mips64, mips64n32,
>> powerpc, powerpc64, powerpc64le, s390x, sh, sparc, sparc64, and x86).
>>
>> And I think we should not change current behavior for some reasons:
>>
>> 1. POSIX 2008 removed this SySV interface for a good reason and changing
>>    this behavior adds nothing for current portable code.  POSIX 2001
>>    specification does states that stack should be saved [1] and current
>>    GLIBC code does in a arch-specific manner (inside the mcontext_t)
>>    which allows the setcontext to work correctly.
>>
>> 2. Changing this behavior would potentially require compat symbols and
>>    I see no gain in adding compat symbols for deprecated interfaces.
>>
>> 3. Also, for comment #2 in BZ#759, it is up to kernel do setup the contents
>>    for ucontext_t and currently it does not provide the stack information
>>    as well.  Trying to change it is also another fix that does not worth
>>    the possible gains.
>>
>> Instead my proposal is to make it clear the current interface behavior in
>> glibc documentation and close this bug as invalid.
>>
>> 	[BZ #759]
>> 	* manual/setjmp.texi (getcontex): Document uc_stack value on Linux.
>>
>> [1] http://pubs.opengroup.org/onlinepubs/009695399/functions/getcontext.html
>> ---
>>  ChangeLog          | 5 +++++
>>  manual/setjmp.texi | 5 +++++
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/manual/setjmp.texi b/manual/setjmp.texi
>> index 94d16be..07cc7bf 100644
>> --- a/manual/setjmp.texi
>> +++ b/manual/setjmp.texi
>> @@ -302,6 +302,11 @@ the content of the registers, the signal mask, and the current stack.
>>  Executing the contents would start at the point where the
>>  @code{getcontext} call just returned.
>>  
>> +On Linux the stack information return on @code{uc_stack} is empty by
>> +default.  It also the case for signal handling information through
>> +@code{sigaction} with @code{SA_SIGINFO}.  It could be obtained using
>> +architecture specific field from @code{uc_mcontext} member.
>> +
>>  The function returns @code{0} if successful.  Otherwise it returns
>>  @code{-1} and sets @var{errno} accordingly.
>>  @end deftypefun
>>
  
Zack Weinberg July 11, 2017, 6:23 p.m. UTC | #3
On Tue, Jul 11, 2017 at 1:56 PM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> Ping (x2). I think something we won't bother to emulate solaris behaviour,
> so in a short we should document it and close it.

I agree that existing behavior should not be changed, but the text you
proposed for the manual doesn't make a lot of sense unless you already
know what the problem is.

>>> +On Linux the stack information return on @code{uc_stack} is empty by
>>> +default.  It also the case for signal handling information through
>>> +@code{sigaction} with @code{SA_SIGINFO}.  It could be obtained using
>>> +architecture specific field from @code{uc_mcontext} member.

@strong{Compatibility Note:} Depending on the operating system,
information about the current context's stack may be in the
@code{uc_stack} field of @var{ucp}, or it may instead instead be in
architecture-specific subfields of the @code{uc_mcontext} field.

The detail regarding 'sigaction' belongs in the documentation for
'sigaction', and we currently don't document SA_SIGINFO at all, so I
would just leave that bit out.

zw
  
Adhemerval Zanella July 11, 2017, 6:28 p.m. UTC | #4
On 11/07/2017 15:23, Zack Weinberg wrote:
> On Tue, Jul 11, 2017 at 1:56 PM, Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>> Ping (x2). I think something we won't bother to emulate solaris behaviour,
>> so in a short we should document it and close it.
> 
> I agree that existing behavior should not be changed, but the text you
> proposed for the manual doesn't make a lot of sense unless you already
> know what the problem is.
> 
>>>> +On Linux the stack information return on @code{uc_stack} is empty by
>>>> +default.  It also the case for signal handling information through
>>>> +@code{sigaction} with @code{SA_SIGINFO}.  It could be obtained using
>>>> +architecture specific field from @code{uc_mcontext} member.
> 
> @strong{Compatibility Note:} Depending on the operating system,
> information about the current context's stack may be in the
> @code{uc_stack} field of @var{ucp}, or it may instead instead be in
> architecture-specific subfields of the @code{uc_mcontext} field.
> 
> The detail regarding 'sigaction' belongs in the documentation for
> 'sigaction', and we currently don't document SA_SIGINFO at all, so I
> would just leave that bit out.

Right, I do not have a preference here about the wording but I do think
we should at least document Linux behaviour (which originated the bug
report). What about:

@strong{Compatibility Note:} Depending on the operating system,
information about the current context's stack may be in the
@code{uc_stack} field of @var{ucp}, or it may instead instead be in
architecture-specific subfields of the @code{uc_mcontext} field.
On Linux the current context's stack returned on @code{uc_stack}
is empty by default.
  
Zack Weinberg July 11, 2017, 8:25 p.m. UTC | #5
On Tue, Jul 11, 2017 at 2:28 PM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> Right, I do not have a preference here about the wording but I do think
> we should at least document Linux behaviour (which originated the bug
> report). What about:
>
> @strong{Compatibility Note:} Depending on the operating system,
> information about the current context's stack may be in the
> @code{uc_stack} field of @var{ucp}, or it may instead instead be in
> architecture-specific subfields of the @code{uc_mcontext} field.
> On Linux the current context's stack returned on @code{uc_stack}
> is empty by default.

I actually think we _shouldn't_ document Linux's behavior in this
case.  Without a great deal more verbiage about how to write code that
portably examines the result of getcontext, it's better to just say
"this information could be in two different places" than "this
information could be in two different places, and on system X it's in
this one."  Good code has to be prepared to look in both places.  That
is the best _short_ advice.

zw
  
Adhemerval Zanella July 11, 2017, 9:43 p.m. UTC | #6
On 11/07/2017 17:25, Zack Weinberg wrote:
> On Tue, Jul 11, 2017 at 2:28 PM, Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>> Right, I do not have a preference here about the wording but I do think
>> we should at least document Linux behaviour (which originated the bug
>> report). What about:
>>
>> @strong{Compatibility Note:} Depending on the operating system,
>> information about the current context's stack may be in the
>> @code{uc_stack} field of @var{ucp}, or it may instead instead be in
>> architecture-specific subfields of the @code{uc_mcontext} field.
>> On Linux the current context's stack returned on @code{uc_stack}
>> is empty by default.
> 
> I actually think we _shouldn't_ document Linux's behavior in this
> case.  Without a great deal more verbiage about how to write code that
> portably examines the result of getcontext, it's better to just say
> "this information could be in two different places" than "this
> information could be in two different places, and on system X it's in
> this one."  Good code has to be prepared to look in both places.  That
> is the best _short_ advice.

Taking in consideration getcontext is deprecated and this is specific
implementation detail I think it is still worth to mention Linux behaviour
without the need of extending how to proper use this API (which should
not be used in newer programs anyway and in my experience the program
that still rely on such *context, libgo for instance, already have a 
lot of hacks for different systems). The manual imho should mainly 
document the current system supported and its idiosyncrasies.
  
Adhemerval Zanella Aug. 8, 2017, 6:27 p.m. UTC | #7
On 11/07/2017 18:43, Adhemerval Zanella wrote:
> 
> 
> On 11/07/2017 17:25, Zack Weinberg wrote:
>> On Tue, Jul 11, 2017 at 2:28 PM, Adhemerval Zanella
>> <adhemerval.zanella@linaro.org> wrote:
>>>
>>> Right, I do not have a preference here about the wording but I do think
>>> we should at least document Linux behaviour (which originated the bug
>>> report). What about:
>>>
>>> @strong{Compatibility Note:} Depending on the operating system,
>>> information about the current context's stack may be in the
>>> @code{uc_stack} field of @var{ucp}, or it may instead instead be in
>>> architecture-specific subfields of the @code{uc_mcontext} field.
>>> On Linux the current context's stack returned on @code{uc_stack}
>>> is empty by default.
>>
>> I actually think we _shouldn't_ document Linux's behavior in this
>> case.  Without a great deal more verbiage about how to write code that
>> portably examines the result of getcontext, it's better to just say
>> "this information could be in two different places" than "this
>> information could be in two different places, and on system X it's in
>> this one."  Good code has to be prepared to look in both places.  That
>> is the best _short_ advice.
> 
> Taking in consideration getcontext is deprecated and this is specific
> implementation detail I think it is still worth to mention Linux behaviour
> without the need of extending how to proper use this API (which should
> not be used in newer programs anyway and in my experience the program
> that still rely on such *context, libgo for instance, already have a 
> lot of hacks for different systems). The manual imho should mainly 
> document the current system supported and its idiosyncrasies.
> 

Zack do you still think we should not document Linux behaviour? I
understand your point that for document getcontext in a portable
way it would require more explanation, but I think the goal here
is just document this specific Linux difference for this deprecated
symbol.
  
Zack Weinberg Aug. 8, 2017, 6:30 p.m. UTC | #8
On Tue, Aug 8, 2017 at 2:27 PM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> On 11/07/2017 18:43, Adhemerval Zanella wrote:
>> On 11/07/2017 17:25, Zack Weinberg wrote:
>>> On Tue, Jul 11, 2017 at 2:28 PM, Adhemerval Zanella
>>> <adhemerval.zanella@linaro.org> wrote:
>>>>
>>>> Right, I do not have a preference here about the wording but I do think
>>>> we should at least document Linux behaviour (which originated the bug
>>>> report). What about:
>>>>
>>>> @strong{Compatibility Note:} Depending on the operating system,
>>>> information about the current context's stack may be in the
>>>> @code{uc_stack} field of @var{ucp}, or it may instead instead be in
>>>> architecture-specific subfields of the @code{uc_mcontext} field.
>>>> On Linux the current context's stack returned on @code{uc_stack}
>>>> is empty by default.
>>>
>>> I actually think we _shouldn't_ document Linux's behavior in this
>>> case.  Without a great deal more verbiage about how to write code that
>>> portably examines the result of getcontext, it's better to just say
>>> "this information could be in two different places" than "this
>>> information could be in two different places, and on system X it's in
>>> this one."  Good code has to be prepared to look in both places.  That
>>> is the best _short_ advice.
>>
>> Taking in consideration getcontext is deprecated and this is specific
>> implementation detail I think it is still worth to mention Linux behaviour
>> without the need of extending how to proper use this API (which should
>> not be used in newer programs anyway and in my experience the program
>> that still rely on such *context, libgo for instance, already have a
>> lot of hacks for different systems). The manual imho should mainly
>> document the current system supported and its idiosyncrasies.
>>
>
> Zack do you still think we should not document Linux behaviour? I
> understand your point that for document getcontext in a portable
> way it would require more explanation, but I think the goal here
> is just document this specific Linux difference for this deprecated
> symbol.

I would still prefer to just say

>>>> @strong{Compatibility Note:} Depending on the operating system,
>>>> information about the current context's stack may be in the
>>>> @code{uc_stack} field of @var{ucp}, or it may instead be in
>>>> architecture-specific subfields of the @code{uc_mcontext} field.

and no more.

zw
  
Adhemerval Zanella Aug. 8, 2017, 6:40 p.m. UTC | #9
On 08/08/2017 15:30, Zack Weinberg wrote:
> On Tue, Aug 8, 2017 at 2:27 PM, Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>> On 11/07/2017 18:43, Adhemerval Zanella wrote:
>>> On 11/07/2017 17:25, Zack Weinberg wrote:
>>>> On Tue, Jul 11, 2017 at 2:28 PM, Adhemerval Zanella
>>>> <adhemerval.zanella@linaro.org> wrote:
>>>>>
>>>>> Right, I do not have a preference here about the wording but I do think
>>>>> we should at least document Linux behaviour (which originated the bug
>>>>> report). What about:
>>>>>
>>>>> @strong{Compatibility Note:} Depending on the operating system,
>>>>> information about the current context's stack may be in the
>>>>> @code{uc_stack} field of @var{ucp}, or it may instead instead be in
>>>>> architecture-specific subfields of the @code{uc_mcontext} field.
>>>>> On Linux the current context's stack returned on @code{uc_stack}
>>>>> is empty by default.
>>>>
>>>> I actually think we _shouldn't_ document Linux's behavior in this
>>>> case.  Without a great deal more verbiage about how to write code that
>>>> portably examines the result of getcontext, it's better to just say
>>>> "this information could be in two different places" than "this
>>>> information could be in two different places, and on system X it's in
>>>> this one."  Good code has to be prepared to look in both places.  That
>>>> is the best _short_ advice.
>>>
>>> Taking in consideration getcontext is deprecated and this is specific
>>> implementation detail I think it is still worth to mention Linux behaviour
>>> without the need of extending how to proper use this API (which should
>>> not be used in newer programs anyway and in my experience the program
>>> that still rely on such *context, libgo for instance, already have a
>>> lot of hacks for different systems). The manual imho should mainly
>>> document the current system supported and its idiosyncrasies.
>>>
>>
>> Zack do you still think we should not document Linux behaviour? I
>> understand your point that for document getcontext in a portable
>> way it would require more explanation, but I think the goal here
>> is just document this specific Linux difference for this deprecated
>> symbol.
> 
> I would still prefer to just say
> 
>>>>> @strong{Compatibility Note:} Depending on the operating system,
>>>>> information about the current context's stack may be in the
>>>>> @code{uc_stack} field of @var{ucp}, or it may instead be in
>>>>> architecture-specific subfields of the @code{uc_mcontext} field.
> 
> and no more.
> 
> zw
> 

Alright, I will use your suggestion then.
  

Patch

diff --git a/manual/setjmp.texi b/manual/setjmp.texi
index 94d16be..07cc7bf 100644
--- a/manual/setjmp.texi
+++ b/manual/setjmp.texi
@@ -302,6 +302,11 @@  the content of the registers, the signal mask, and the current stack.
 Executing the contents would start at the point where the
 @code{getcontext} call just returned.
 
+On Linux the stack information return on @code{uc_stack} is empty by
+default.  It also the case for signal handling information through
+@code{sigaction} with @code{SA_SIGINFO}.  It could be obtained using
+architecture specific field from @code{uc_mcontext} member.
+
 The function returns @code{0} if successful.  Otherwise it returns
 @code{-1} and sets @var{errno} accordingly.
 @end deftypefun