Re: [PATCH] mips: Fix store/load gp registers to/from ucontext_t

["Maciej W. Rozycki" <macro at imgtec dot com>]

On Wed, 17 May 2017, Joseph Myers wrote:

> > More details, not to be included in the commit message:
> > 
> > get/set/make/swap context implementations for MIPS O32
> > incorrectly assume general purpose registers are 32bit
> > but they are defined as 64bit in mcontext_t structure, as per:
> > 
> > https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/mips/sys/ucontext.h;h=f766dacd205f49a479f17b19df476a8103f55a2e;hb=refs/heads/master#l35
> > and
> > https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/mips/sys/ucontext.h;h=f766dacd205f49a479f17b19df476a8103f55a2e;hb=refs/heads/master#l68
> > 
> > This leads to incorrect general purpose register values.
> 
> Is this a user-visible bug (for which a testcase could be written, in 
> which case one should be added and a bug filed in Bugzilla)?  Or is it not 
> user-visible because the functions are consistent with each other (hence 
> all the existing tests passing), just not with the intended structure 
> layout, and calling setcontext with a context from a signal handler has 
> unspecified results anyway?

 Not according to SUSv2 [1] AFAICT:

"The setcontext() function restores the user context pointed to by ucp.  
A successful call to setcontext() does not return; program execution 
resumes at the point specified by the ucp argument passed to setcontext().  
The ucp argument should be created either by a prior call to getcontext() 
or makecontext(), or by being passed as an argument to a signal handler."

-- notice the last statement.

 Even though I meant to I've never got to implementing a test case for 
these calls that would actually exercise this feature.  I envisaged 
writing a testsuite case in the form of a simple scheduler using SIGALRM 
and `swapcontext' to switch among a set of threads of execution -- the 
original purpose of the user context feature set, then obsoleted by POSIX 
threads.

 Simple use was supposed to be covered by stdlib/tst-sigcontext.c, but 
that test case was executively dropped without a review, with commit 
a87cd2894cbd ("Remove tst-sigcontext test."), which is likely why this 
slot size mismatch bug has gone unnoticed so long.

References:

[1] "The Single UNIX Specification, Version 2", The Open Group, 1997
    <http://pubs.opengroup.org/onlinepubs/007908799/xsh/getcontext.html>

 HTH,

  Maciej