This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 2/2] jmpbuf: Add paddings for target specific usage
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
--
H.J.