From: "H.J. Lu" <hjl.tools@gmail.com>
To: Florian Weimer <fweimer@redhat.com>
Cc: GNU C Library <libc-alpha@sourceware.org>
Subject: Re: [PATCH 2/2] jmpbuf: Add paddings for target specific usage
Date: Tue, 21 Nov 2017 22:22:00 -0000 [thread overview]
Message-ID: <CAMe9rOrnF0D9jcALGUuGdZg+DczqowSNeQqq7YuAPmwS8tikAQ@mail.gmail.com> (raw)
In-Reply-To: <CAMe9rOoypSA9VJoSW-mTEwp_Usvm2=VhJVSu34WnRVmN81eTYg@mail.gmail.com>
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.
next prev parent reply other threads:[~2017-11-21 22:22 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-07 22:39 H.J. Lu
2017-11-08 6:06 ` Florian Weimer
2017-11-08 18:27 ` H.J. Lu
2017-11-13 13:09 ` Florian Weimer
2017-11-13 14:05 ` H.J. Lu
2017-11-13 16:34 ` Florian Weimer
2017-11-13 16:44 ` H.J. Lu
2017-11-13 19:40 ` H.J. Lu
2017-11-13 23:22 ` H.J. Lu
2017-11-14 12:26 ` Florian Weimer
2017-11-14 13:11 ` H.J. Lu
2017-11-15 11:10 ` Florian Weimer
2017-11-15 13:20 ` H.J. Lu
2017-11-21 22:22 ` H.J. Lu [this message]
2017-11-25 16:34 ` H.J. Lu
2017-11-29 21:16 ` H.J. Lu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAMe9rOrnF0D9jcALGUuGdZg+DczqowSNeQqq7YuAPmwS8tikAQ@mail.gmail.com \
--to=hjl.tools@gmail.com \
--cc=fweimer@redhat.com \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).