[2/2] jmpbuf: Add paddings for target specific usage

  On Tue, Nov 21, 2017 at 2:22 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Nov 15, 2017 at 5:20 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Wed, Nov 15, 2017 at 3:10 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>> On 11/14/2017 02:10 PM, H.J. Lu wrote:
>>>>
>>>> +static int
>>>> +do_test (void)
>>>> +{
>>>> +  sigjmp_buf sj;
>>>> +  struct support_next_to_fault sigset_t_buf
>>>> +    = support_next_to_fault_allocate (SIZEOF_SIGSET_T);
>>>> +  sigset_t *m_p = (sigset_t *) sigset_t_buf.buffer;
>>>> +  sigset_t m;
>>>> +
>>>> +  sigemptyset (&m);
>>>> +  memcpy (m_p, &m, SIZEOF_SIGSET_T);
>>>> +  sigprocmask (SIG_SETMASK, m_p, NULL);
>>>> +  memcpy (&m, m_p, SIZEOF_SIGSET_T);
>>>> +  if (sigsetjmp (sj, 0) == 0)
>>>> +    {
>>>> +      sigaddset (&m, SIGUSR1);
>>>> +      memcpy (m_p, &m, SIZEOF_SIGSET_T);
>>>> +      sigprocmask (SIG_SETMASK, m_p, NULL);
>>>> +      memcpy (&m, m_p, SIZEOF_SIGSET_T);
>>>> +      siglongjmp (sj, 1);
>>>> +      return EXIT_FAILURE;
>>>> +    }
>>>> +  sigprocmask (SIG_SETMASK, NULL, m_p);
>>>> +  memcpy (&m, m_p, SIZEOF_SIGSET_T);
>>>> +  return sigismember (&m, SIGUSR1) ? EXIT_SUCCESS : EXIT_FAILURE;
>>>> +}
>>>
>>>
>>> Sorry, I don't understand anymore what this test is supposed to test and
>>> how.
>>
>> This tests the reduced __jmp_buf_sigset_t used by __saved_mask is
>> bigger than sigset expected by kernel.
>>
>>> To be honest, I don't like how you inject the internal definition of
>>> jmp_buf.  Is this the way we do it for the nptl types?
>>
>> I only need to make some room in
>>
>> /* Calling environment, plus possibly a saved signal mask.  */
>> struct __jmp_buf_tag
>>   {
>>     /* NOTE: The machine-dependent definitions of `__sigsetjmp'
>>        assume that a `jmp_buf' begins with a `__jmp_buf' and that
>>        `__mask_was_saved' follows it.  Do not move these members
>>        or add others before it.  */
>>     __jmp_buf __jmpbuf; /* Calling environment.  */
>>     int __mask_was_saved; /* Saved the signal mask?  */
>>     __sigset_t __saved_mask; /* Saved signal mask.  */
>>   };
>>
>> for target specific purpose.   I changed it to
>>
>> struct __jmp_buf_tag
>>   {
>>     /* NOTE: The machine-dependent definitions of `__sigsetjmp'
>>        assume that a `jmp_buf' begins with a `__jmp_buf' and that
>>        `__mask_was_saved' follows it.  Do not move these members
>>        or add others before it.  */
>>     __jmp_buf __jmpbuf; /* Calling environment.  */
>>     int __mask_was_saved; /* Saved the signal mask?  */
>>     union
>>     {
>>     __sigset_t __saved_mask_compat;
>>     struct
>>       {
>> __jmp_buf_sigset_t __saved_mask;
>> /* Paddings for architecture specific usage.  */
>> unsigned long int __padding[12];
>>       } __saved;
>>     }  __saved_mask;
>>   };
>>
>> #define __saved_mask __saved_mask.__saved.__saved_mask
>>
>> I did only to  __sigset_t  in __jmp_buf_tag and this test verifies that the size
>> of __jmp_buf_sigset_t works with sigprocmask.
>>
>> Are you suggesting we make some room in __sigset_t directly?  This will
>> require very extensive changes. If not, what do you exactly suggest?
>>
>>> I think you should check _JUMP_BUF_SIGSET_NSIG against a kernel constant
>>> (_NSIGS?) somewhere.
>>
>> There is a _Static_assert in include/setjmp.h.  But it isn't triggered.  This
>> updated patch moves it to sysdeps/unix/sysv/linux/__saved_mask.h.   Now
>> I got
>>
>> ../sysdeps/unix/sysv/linux/__saved_mask.h:32:1: error: static
>> assertion failed: "size of ___saved_mask < size of
>> __sigprocmask_sigset_t"
>>  _Static_assert (sizeof (___saved_mask) >= sizeof (__sigprocmask_sigset_t),
>>
>> if __jmp_buf_sigset_t is too small.
>>
>> --
>> H.J.
>
> Any comments, suggestions or objections?  The patch is at
>
> https://sourceware.org/ml/libc-alpha/2017-11/msg00510.html
>

This is the updated patch.  It limited the changes to Linux/x86.
Any comments?

[2/2] jmpbuf: Add paddings for target specific usage

Commit Message

Patch