[v2,2/3] nptl: Change tst-typesizes to _Static_assert
Commit Message
Changes from previous version:
- Move _Static_asserts tests from nptl/tst-typesizes.c to library
build time and remove redundant types sizes asserts.
---
Instead of rely on runtime check to assure correct pthread types
size a better strategy would use _Static_assert to trigger an error
on build time (and thus allowing to check to potentially ABI breakage
on cross-compiling make check).
This patch moves nptl/tst-typesizes.c to libpthread build time on
each specific initialization routine and also remove some runtime
redundant asserts for the same type sizes.
Checked on x86_64-linux-gnu and with a build check for all affected
ABIs (aarch64-linux-gnu, alpha-linux-gnu, arm-linux-gnueabihf,
hppa-linux-gnu, i686-linux-gnu, ia64-linux-gnu, m68k-linux-gnu,
microblaze-linux-gnu, mips64-linux-gnu, mips64-n32-linux-gnu,
mips-linux-gnu, powerpc64le-linux-gnu, powerpc-linux-gnu,
s390-linux-gnu, s390x-linux-gnu, sh4-linux-gnu, sparc64-linux-gnu,
sparcv9-linux-gnu, tilegx-linux-gnu, tilegx-linux-gnu-x32,
tilepro-linux-gnu, x86_64-linux-gnu, and x86_64-linux-x32).
Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
* nptl/pthreadP.h (ASSERT_TYPE_SIZE, ASSERT_PTHREAD_INTERNAL_SIZE):
New macros.
* nptl/pthread_attr_init.c (__pthread_mutex_init): Add build time
checks for expected input type size.
* nptl/pthread_barrier_init.c (__pthread_barrier_init): Likewise.
* nptl/pthread_barrierattr_init.c (pthread_barrierattr_init):
Likewise.
* nptl/pthread_cond_init.c (__pthread_cond_init): Likewise.
* nptl/pthread_condattr_init.c (__pthread_condattr_init): Likewise.
* nptl/pthread_mutex_init.c (__pthread_mutex_init): Likewise.
* nptl/pthread_mutexattr_init.c (__pthread_mutexattr_init): Likewise.
* nptl/pthread_rwlock_init.c (__pthread_rwlock_init): Likewise.
* nptl/pthread_rwlockattr_init.c (pthread_rwlockattr_init): Likewise.
* nptl/sem_init.c (__new_sem_init, __old_sem_init): Likewise
* nptl/pthread_attr_destroy.c (__pthread_attr_destroy): Remove
superflous runtime assert check.
* nptl/pthread_attr_getaffinity.c (__pthread_attr_getaffinity_new):
Likewise.
* nptl/pthread_attr_getdetachstate.c (__pthread_attr_getdetachstate):
Likewise.
* nptl/pthread_attr_getguardsize.c (pthread_attr_getguardsize):
Likewise.
* nptl/pthread_attr_getinheritsched.c (__pthread_attr_getinheritsched):
Likewise.
* nptl/pthread_attr_getschedparam.c (__pthread_attr_getschedparam):
Likewise.
* nptl/pthread_attr_getschedpolicy.c (__pthread_attr_getschedpolicy):
Likewise.
* nptl/pthread_attr_getscope.c (__pthread_attr_getscope): Likewise.
* nptl/pthread_attr_getstack.c (__pthread_attr_getstack): Likewise.
* nptl/pthread_attr_getstackaddr.c (__pthread_attr_getstackaddr):
Likewise.
* nptl/pthread_attr_getstacksize.c (__pthread_attr_getstacksize):
Likewise.
* nptl/pthread_attr_setaffinity.c (__pthread_attr_setaffinity_new):
Likewise.
* nptl/pthread_attr_setdetachstate.c (__pthread_attr_setdetachstate):
Likewise.
* nptl/pthread_attr_setguardsize.c (pthread_attr_setguardsize):
Likewise.
* nptl/pthread_attr_setinheritsched.c
(__pthread_attr_setinheritsched): Likewise.
* nptl/pthread_attr_setschedparam.c (__pthread_attr_setschedparam):
Likewise.
* nptl/pthread_attr_setschedpolicy.c (__pthread_attr_setschedpolicy):
Likewise.
* nptl/pthread_attr_setscope.c (__pthread_attr_setscope): Likewise.
* nptl/pthread_attr_setstack.c (__pthread_attr_setstack,
__old_pthread_attr_setstack): Likewise.
* nptl/pthread_attr_setstackaddr.c (__pthread_attr_setstackaddr):
Likewise.
* nptl/pthread_attr_setstacksize.c (__pthread_attr_setstacksize):
Likewise.
* nptl/pthread_getattr_default_np.c (pthread_getattr_default_np):
Likewise.
* nptl/pthread_mutex_lock.c (__pthread_mutex_lock): Likewise.
* nptl/pthread_setattr_default_np.c (pthread_setattr_default_np):
Likewise.
* nptl/tst-typesizes.c: Remove file.
---
ChangeLog | 60 +++++++++++++++++++++++
nptl/pthreadP.h | 8 ++++
nptl/pthread_attr_destroy.c | 2 -
nptl/pthread_attr_getaffinity.c | 2 -
nptl/pthread_attr_getdetachstate.c | 2 -
nptl/pthread_attr_getguardsize.c | 2 -
nptl/pthread_attr_getinheritsched.c | 2 -
nptl/pthread_attr_getschedparam.c | 2 -
nptl/pthread_attr_getschedpolicy.c | 2 -
nptl/pthread_attr_getscope.c | 2 -
nptl/pthread_attr_getstack.c | 2 -
nptl/pthread_attr_getstackaddr.c | 2 -
nptl/pthread_attr_getstacksize.c | 2 -
nptl/pthread_attr_init.c | 5 +-
nptl/pthread_attr_setaffinity.c | 2 -
nptl/pthread_attr_setdetachstate.c | 2 -
nptl/pthread_attr_setguardsize.c | 2 -
nptl/pthread_attr_setinheritsched.c | 2 -
nptl/pthread_attr_setschedparam.c | 2 -
nptl/pthread_attr_setschedpolicy.c | 2 -
nptl/pthread_attr_setscope.c | 2 -
nptl/pthread_attr_setstack.c | 3 --
nptl/pthread_attr_setstackaddr.c | 2 -
nptl/pthread_attr_setstacksize.c | 3 --
nptl/pthread_barrier_init.c | 4 ++
nptl/pthread_barrierattr_init.c | 4 ++
nptl/pthread_cond_init.c | 2 +
nptl/pthread_condattr_init.c | 4 ++
nptl/pthread_getattr_default_np.c | 2 -
nptl/pthread_mutex_init.c | 2 +
nptl/pthread_mutex_lock.c | 2 -
nptl/pthread_mutexattr_init.c | 4 ++
nptl/pthread_rwlock_init.c | 2 +
nptl/pthread_rwlockattr_init.c | 4 ++
nptl/pthread_setattr_default_np.c | 2 -
nptl/sem_init.c | 4 ++
nptl/tst-typesizes.c | 95 -------------------------------------
37 files changed, 101 insertions(+), 147 deletions(-)
delete mode 100644 nptl/tst-typesizes.c
Comments
Ping.
On 26/10/2017 15:14, Adhemerval Zanella wrote:
> Changes from previous version:
>
> - Move _Static_asserts tests from nptl/tst-typesizes.c to library
> build time and remove redundant types sizes asserts.
>
> ---
>
> Instead of rely on runtime check to assure correct pthread types
> size a better strategy would use _Static_assert to trigger an error
> on build time (and thus allowing to check to potentially ABI breakage
> on cross-compiling make check).
>
> This patch moves nptl/tst-typesizes.c to libpthread build time on
> each specific initialization routine and also remove some runtime
> redundant asserts for the same type sizes.
>
> Checked on x86_64-linux-gnu and with a build check for all affected
> ABIs (aarch64-linux-gnu, alpha-linux-gnu, arm-linux-gnueabihf,
> hppa-linux-gnu, i686-linux-gnu, ia64-linux-gnu, m68k-linux-gnu,
> microblaze-linux-gnu, mips64-linux-gnu, mips64-n32-linux-gnu,
> mips-linux-gnu, powerpc64le-linux-gnu, powerpc-linux-gnu,
> s390-linux-gnu, s390x-linux-gnu, sh4-linux-gnu, sparc64-linux-gnu,
> sparcv9-linux-gnu, tilegx-linux-gnu, tilegx-linux-gnu-x32,
> tilepro-linux-gnu, x86_64-linux-gnu, and x86_64-linux-x32).
>
> Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> * nptl/pthreadP.h (ASSERT_TYPE_SIZE, ASSERT_PTHREAD_INTERNAL_SIZE):
> New macros.
> * nptl/pthread_attr_init.c (__pthread_mutex_init): Add build time
> checks for expected input type size.
> * nptl/pthread_barrier_init.c (__pthread_barrier_init): Likewise.
> * nptl/pthread_barrierattr_init.c (pthread_barrierattr_init):
> Likewise.
> * nptl/pthread_cond_init.c (__pthread_cond_init): Likewise.
> * nptl/pthread_condattr_init.c (__pthread_condattr_init): Likewise.
> * nptl/pthread_mutex_init.c (__pthread_mutex_init): Likewise.
> * nptl/pthread_mutexattr_init.c (__pthread_mutexattr_init): Likewise.
> * nptl/pthread_rwlock_init.c (__pthread_rwlock_init): Likewise.
> * nptl/pthread_rwlockattr_init.c (pthread_rwlockattr_init): Likewise.
> * nptl/sem_init.c (__new_sem_init, __old_sem_init): Likewise
> * nptl/pthread_attr_destroy.c (__pthread_attr_destroy): Remove
> superflous runtime assert check.
> * nptl/pthread_attr_getaffinity.c (__pthread_attr_getaffinity_new):
> Likewise.
> * nptl/pthread_attr_getdetachstate.c (__pthread_attr_getdetachstate):
> Likewise.
> * nptl/pthread_attr_getguardsize.c (pthread_attr_getguardsize):
> Likewise.
> * nptl/pthread_attr_getinheritsched.c (__pthread_attr_getinheritsched):
> Likewise.
> * nptl/pthread_attr_getschedparam.c (__pthread_attr_getschedparam):
> Likewise.
> * nptl/pthread_attr_getschedpolicy.c (__pthread_attr_getschedpolicy):
> Likewise.
> * nptl/pthread_attr_getscope.c (__pthread_attr_getscope): Likewise.
> * nptl/pthread_attr_getstack.c (__pthread_attr_getstack): Likewise.
> * nptl/pthread_attr_getstackaddr.c (__pthread_attr_getstackaddr):
> Likewise.
> * nptl/pthread_attr_getstacksize.c (__pthread_attr_getstacksize):
> Likewise.
> * nptl/pthread_attr_setaffinity.c (__pthread_attr_setaffinity_new):
> Likewise.
> * nptl/pthread_attr_setdetachstate.c (__pthread_attr_setdetachstate):
> Likewise.
> * nptl/pthread_attr_setguardsize.c (pthread_attr_setguardsize):
> Likewise.
> * nptl/pthread_attr_setinheritsched.c
> (__pthread_attr_setinheritsched): Likewise.
> * nptl/pthread_attr_setschedparam.c (__pthread_attr_setschedparam):
> Likewise.
> * nptl/pthread_attr_setschedpolicy.c (__pthread_attr_setschedpolicy):
> Likewise.
> * nptl/pthread_attr_setscope.c (__pthread_attr_setscope): Likewise.
> * nptl/pthread_attr_setstack.c (__pthread_attr_setstack,
> __old_pthread_attr_setstack): Likewise.
> * nptl/pthread_attr_setstackaddr.c (__pthread_attr_setstackaddr):
> Likewise.
> * nptl/pthread_attr_setstacksize.c (__pthread_attr_setstacksize):
> Likewise.
> * nptl/pthread_getattr_default_np.c (pthread_getattr_default_np):
> Likewise.
> * nptl/pthread_mutex_lock.c (__pthread_mutex_lock): Likewise.
> * nptl/pthread_setattr_default_np.c (pthread_setattr_default_np):
> Likewise.
> * nptl/tst-typesizes.c: Remove file.
> ---
> ChangeLog | 60 +++++++++++++++++++++++
> nptl/pthreadP.h | 8 ++++
> nptl/pthread_attr_destroy.c | 2 -
> nptl/pthread_attr_getaffinity.c | 2 -
> nptl/pthread_attr_getdetachstate.c | 2 -
> nptl/pthread_attr_getguardsize.c | 2 -
> nptl/pthread_attr_getinheritsched.c | 2 -
> nptl/pthread_attr_getschedparam.c | 2 -
> nptl/pthread_attr_getschedpolicy.c | 2 -
> nptl/pthread_attr_getscope.c | 2 -
> nptl/pthread_attr_getstack.c | 2 -
> nptl/pthread_attr_getstackaddr.c | 2 -
> nptl/pthread_attr_getstacksize.c | 2 -
> nptl/pthread_attr_init.c | 5 +-
> nptl/pthread_attr_setaffinity.c | 2 -
> nptl/pthread_attr_setdetachstate.c | 2 -
> nptl/pthread_attr_setguardsize.c | 2 -
> nptl/pthread_attr_setinheritsched.c | 2 -
> nptl/pthread_attr_setschedparam.c | 2 -
> nptl/pthread_attr_setschedpolicy.c | 2 -
> nptl/pthread_attr_setscope.c | 2 -
> nptl/pthread_attr_setstack.c | 3 --
> nptl/pthread_attr_setstackaddr.c | 2 -
> nptl/pthread_attr_setstacksize.c | 3 --
> nptl/pthread_barrier_init.c | 4 ++
> nptl/pthread_barrierattr_init.c | 4 ++
> nptl/pthread_cond_init.c | 2 +
> nptl/pthread_condattr_init.c | 4 ++
> nptl/pthread_getattr_default_np.c | 2 -
> nptl/pthread_mutex_init.c | 2 +
> nptl/pthread_mutex_lock.c | 2 -
> nptl/pthread_mutexattr_init.c | 4 ++
> nptl/pthread_rwlock_init.c | 2 +
> nptl/pthread_rwlockattr_init.c | 4 ++
> nptl/pthread_setattr_default_np.c | 2 -
> nptl/sem_init.c | 4 ++
> nptl/tst-typesizes.c | 95 -------------------------------------
> 37 files changed, 101 insertions(+), 147 deletions(-)
> delete mode 100644 nptl/tst-typesizes.c
>
> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> index 444bbd9..bbc55fb 100644
> --- a/nptl/pthreadP.h
> +++ b/nptl/pthreadP.h
> @@ -639,6 +639,14 @@ check_stacksize_attr (size_t st)
> return EINVAL;
> }
>
> +#define ASSERT_TYPE_SIZE(__type, __size) \
> + _Static_assert (sizeof (__type) == __size, \
> + "sizeof (" #__type ") != " #__size)
> +
> +#define ASSERT_PTHREAD_INTERNAL_SIZE(__type, __internal) \
> + _Static_assert (sizeof ((__type *) 0)->__size >= sizeof (__internal), \
> + "sizeof (" #__type ".__size) > sizeof (" #__internal ")")
> +
> #define ASSERT_PTHREAD_STRING(x) __STRING (x)
> #define ASSERT_PTHREAD_INTERNAL_OFFSET(__type, __member, __offset) \
> _Static_assert (offsetof (__type, __member) == __offset, \
> diff --git a/nptl/pthread_attr_destroy.c b/nptl/pthread_attr_destroy.c
> index a9ce51e..cdcdb4a 100644
> --- a/nptl/pthread_attr_destroy.c
> +++ b/nptl/pthread_attr_destroy.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include <errno.h>
> #include <string.h>
> #include <unistd.h>
> @@ -28,7 +27,6 @@ __pthread_attr_destroy (pthread_attr_t *attr)
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> #if SHLIB_COMPAT(libpthread, GLIBC_2_0, GLIBC_2_1)
> diff --git a/nptl/pthread_attr_getaffinity.c b/nptl/pthread_attr_getaffinity.c
> index b9d041a..57ab56d 100644
> --- a/nptl/pthread_attr_getaffinity.c
> +++ b/nptl/pthread_attr_getaffinity.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include <errno.h>
> #include <pthreadP.h>
> #include <string.h>
> @@ -32,7 +31,6 @@ __pthread_attr_getaffinity_new (const pthread_attr_t *attr, size_t cpusetsize,
> {
> const struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (const struct pthread_attr *) attr;
>
> if (iattr->cpuset != NULL)
> diff --git a/nptl/pthread_attr_getdetachstate.c b/nptl/pthread_attr_getdetachstate.c
> index 803a553..0246ca8 100644
> --- a/nptl/pthread_attr_getdetachstate.c
> +++ b/nptl/pthread_attr_getdetachstate.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include "pthreadP.h"
>
>
> @@ -25,7 +24,6 @@ __pthread_attr_getdetachstate (const pthread_attr_t *attr, int *detachstate)
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> *detachstate = (iattr->flags & ATTR_FLAG_DETACHSTATE
> diff --git a/nptl/pthread_attr_getguardsize.c b/nptl/pthread_attr_getguardsize.c
> index b71be6c..d6e01c7 100644
> --- a/nptl/pthread_attr_getguardsize.c
> +++ b/nptl/pthread_attr_getguardsize.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include "pthreadP.h"
>
>
> @@ -25,7 +24,6 @@ pthread_attr_getguardsize (const pthread_attr_t *attr, size_t *guardsize)
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> *guardsize = iattr->guardsize;
> diff --git a/nptl/pthread_attr_getinheritsched.c b/nptl/pthread_attr_getinheritsched.c
> index 2dec230..1dc00ce 100644
> --- a/nptl/pthread_attr_getinheritsched.c
> +++ b/nptl/pthread_attr_getinheritsched.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include "pthreadP.h"
>
>
> @@ -25,7 +24,6 @@ __pthread_attr_getinheritsched (const pthread_attr_t *attr, int *inherit)
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> /* Store the current values. */
> diff --git a/nptl/pthread_attr_getschedparam.c b/nptl/pthread_attr_getschedparam.c
> index 34f9408..86d0709 100644
> --- a/nptl/pthread_attr_getschedparam.c
> +++ b/nptl/pthread_attr_getschedparam.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include <string.h>
> #include "pthreadP.h"
>
> @@ -27,7 +26,6 @@ __pthread_attr_getschedparam (const pthread_attr_t *attr,
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> /* Copy the current values. */
> diff --git a/nptl/pthread_attr_getschedpolicy.c b/nptl/pthread_attr_getschedpolicy.c
> index 65ed417..f9f69be 100644
> --- a/nptl/pthread_attr_getschedpolicy.c
> +++ b/nptl/pthread_attr_getschedpolicy.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include "pthreadP.h"
>
>
> @@ -25,7 +24,6 @@ __pthread_attr_getschedpolicy (const pthread_attr_t *attr, int *policy)
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> /* Store the current values. */
> diff --git a/nptl/pthread_attr_getscope.c b/nptl/pthread_attr_getscope.c
> index 7b36d6a..ba5b5e3 100644
> --- a/nptl/pthread_attr_getscope.c
> +++ b/nptl/pthread_attr_getscope.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include "pthreadP.h"
>
>
> @@ -25,7 +24,6 @@ __pthread_attr_getscope (const pthread_attr_t *attr, int *scope)
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> /* Store the current values. */
> diff --git a/nptl/pthread_attr_getstack.c b/nptl/pthread_attr_getstack.c
> index 79c78c6..9a80bfe 100644
> --- a/nptl/pthread_attr_getstack.c
> +++ b/nptl/pthread_attr_getstack.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include "pthreadP.h"
>
>
> @@ -26,7 +25,6 @@ __pthread_attr_getstack (const pthread_attr_t *attr, void **stackaddr,
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> /* Store the result. */
> diff --git a/nptl/pthread_attr_getstackaddr.c b/nptl/pthread_attr_getstackaddr.c
> index 69f0a7d..ef1a633 100644
> --- a/nptl/pthread_attr_getstackaddr.c
> +++ b/nptl/pthread_attr_getstackaddr.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include <errno.h>
> #include "pthreadP.h"
>
> @@ -26,7 +25,6 @@ __pthread_attr_getstackaddr (const pthread_attr_t *attr, void **stackaddr)
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> /* Some code assumes this function to work even if no stack address
> diff --git a/nptl/pthread_attr_getstacksize.c b/nptl/pthread_attr_getstacksize.c
> index 9f24d4d..d9893cd 100644
> --- a/nptl/pthread_attr_getstacksize.c
> +++ b/nptl/pthread_attr_getstacksize.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include "pthreadP.h"
>
>
> @@ -25,7 +24,6 @@ __pthread_attr_getstacksize (const pthread_attr_t *attr, size_t *stacksize)
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> size_t size = iattr->stacksize;
> diff --git a/nptl/pthread_attr_init.c b/nptl/pthread_attr_init.c
> index 77998ea..eceaf85 100644
> --- a/nptl/pthread_attr_init.c
> +++ b/nptl/pthread_attr_init.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include <errno.h>
> #include <string.h>
> #include <unistd.h>
> @@ -34,12 +33,14 @@ __pthread_attr_init_2_1 (pthread_attr_t *attr)
> {
> struct pthread_attr *iattr;
>
> + ASSERT_TYPE_SIZE (pthread_attr_t, __SIZEOF_PTHREAD_ATTR_T);
> + ASSERT_PTHREAD_INTERNAL_SIZE (pthread_attr_t, struct pthread_attr);
> +
> /* Many elements are initialized to zero so let us do it all at
> once. This also takes care of clearing the bytes which are not
> internally used. */
> memset (attr, '\0', __SIZEOF_PTHREAD_ATTR_T);
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> /* Default guard size specified by the standard. */
> diff --git a/nptl/pthread_attr_setaffinity.c b/nptl/pthread_attr_setaffinity.c
> index 497512e..f684496 100644
> --- a/nptl/pthread_attr_setaffinity.c
> +++ b/nptl/pthread_attr_setaffinity.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include <errno.h>
> #include <limits.h>
> #include <stdlib.h>
> @@ -31,7 +30,6 @@ __pthread_attr_setaffinity_new (pthread_attr_t *attr, size_t cpusetsize,
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> if (cpuset == NULL || cpusetsize == 0)
> diff --git a/nptl/pthread_attr_setdetachstate.c b/nptl/pthread_attr_setdetachstate.c
> index c6fb1bf..2659ce7 100644
> --- a/nptl/pthread_attr_setdetachstate.c
> +++ b/nptl/pthread_attr_setdetachstate.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include <errno.h>
> #include "pthreadP.h"
>
> @@ -26,7 +25,6 @@ __pthread_attr_setdetachstate (pthread_attr_t *attr, int detachstate)
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> /* Catch invalid values. */
> diff --git a/nptl/pthread_attr_setguardsize.c b/nptl/pthread_attr_setguardsize.c
> index 3927793..108b4a5 100644
> --- a/nptl/pthread_attr_setguardsize.c
> +++ b/nptl/pthread_attr_setguardsize.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include "pthreadP.h"
>
>
> @@ -25,7 +24,6 @@ pthread_attr_setguardsize (pthread_attr_t *attr, size_t guardsize)
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> /* Note that we don't round the value here. The standard requires
> diff --git a/nptl/pthread_attr_setinheritsched.c b/nptl/pthread_attr_setinheritsched.c
> index 69a6335..97aeb8d 100644
> --- a/nptl/pthread_attr_setinheritsched.c
> +++ b/nptl/pthread_attr_setinheritsched.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include <errno.h>
> #include "pthreadP.h"
>
> @@ -26,7 +25,6 @@ __pthread_attr_setinheritsched (pthread_attr_t *attr, int inherit)
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> /* Catch invalid values. */
> diff --git a/nptl/pthread_attr_setschedparam.c b/nptl/pthread_attr_setschedparam.c
> index e38d1d8..3eef9ad 100644
> --- a/nptl/pthread_attr_setschedparam.c
> +++ b/nptl/pthread_attr_setschedparam.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include <errno.h>
> #include <string.h>
> #include "pthreadP.h"
> @@ -26,7 +25,6 @@ int
> __pthread_attr_setschedparam (pthread_attr_t *attr,
> const struct sched_param *param)
> {
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> struct pthread_attr *iattr = (struct pthread_attr *) attr;
>
> int ret = check_sched_priority_attr (param->sched_priority,
> diff --git a/nptl/pthread_attr_setschedpolicy.c b/nptl/pthread_attr_setschedpolicy.c
> index 868696d..4f7c9b0 100644
> --- a/nptl/pthread_attr_setschedpolicy.c
> +++ b/nptl/pthread_attr_setschedpolicy.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include <errno.h>
> #include "pthreadP.h"
>
> @@ -26,7 +25,6 @@ __pthread_attr_setschedpolicy (pthread_attr_t *attr, int policy)
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> /* Catch invalid values. */
> diff --git a/nptl/pthread_attr_setscope.c b/nptl/pthread_attr_setscope.c
> index e0fd1dd..bca0ffb 100644
> --- a/nptl/pthread_attr_setscope.c
> +++ b/nptl/pthread_attr_setscope.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include <errno.h>
> #include "pthreadP.h"
>
> @@ -26,7 +25,6 @@ __pthread_attr_setscope (pthread_attr_t *attr, int scope)
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> /* Catch invalid values. */
> diff --git a/nptl/pthread_attr_setstack.c b/nptl/pthread_attr_setstack.c
> index e4f8b29..cb558f5 100644
> --- a/nptl/pthread_attr_setstack.c
> +++ b/nptl/pthread_attr_setstack.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include <errno.h>
> #include <limits.h>
> #include "pthreadP.h"
> @@ -33,7 +32,6 @@ __pthread_attr_setstack (pthread_attr_t *attr, void *stackaddr,
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> /* Catch invalid sizes. */
> @@ -71,7 +69,6 @@ __old_pthread_attr_setstack (pthread_attr_t *attr, void *stackaddr,
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> /* Catch invalid sizes. */
> diff --git a/nptl/pthread_attr_setstackaddr.c b/nptl/pthread_attr_setstackaddr.c
> index ac16f94..78370d8 100644
> --- a/nptl/pthread_attr_setstackaddr.c
> +++ b/nptl/pthread_attr_setstackaddr.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include <errno.h>
> #include "pthreadP.h"
>
> @@ -30,7 +29,6 @@ __pthread_attr_setstackaddr (pthread_attr_t *attr, void *stackaddr)
> EXTRA_PARAM_CHECKS;
> #endif
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> iattr->stackaddr = stackaddr;
> diff --git a/nptl/pthread_attr_setstacksize.c b/nptl/pthread_attr_setstacksize.c
> index 4c0abac..ed8999f 100644
> --- a/nptl/pthread_attr_setstacksize.c
> +++ b/nptl/pthread_attr_setstacksize.c
> @@ -16,7 +16,6 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#include <assert.h>
> #include <errno.h>
> #include <limits.h>
> #include "pthreadP.h"
> @@ -31,7 +30,6 @@ __pthread_attr_setstacksize (pthread_attr_t *attr, size_t stacksize)
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> /* Catch invalid sizes. */
> @@ -58,7 +56,6 @@ __old_pthread_attr_setstacksize (pthread_attr_t *attr, size_t stacksize)
> {
> struct pthread_attr *iattr;
>
> - assert (sizeof (*attr) >= sizeof (struct pthread_attr));
> iattr = (struct pthread_attr *) attr;
>
> /* Catch invalid sizes. */
> diff --git a/nptl/pthread_barrier_init.c b/nptl/pthread_barrier_init.c
> index 9c851fb..bdbd10c 100644
> --- a/nptl/pthread_barrier_init.c
> +++ b/nptl/pthread_barrier_init.c
> @@ -32,6 +32,10 @@ int
> __pthread_barrier_init (pthread_barrier_t *barrier,
> const pthread_barrierattr_t *attr, unsigned int count)
> {
> + ASSERT_TYPE_SIZE (pthread_barrier_t, __SIZEOF_PTHREAD_BARRIER_T);
> + ASSERT_PTHREAD_INTERNAL_SIZE (pthread_barrier_t,
> + struct pthread_barrier);
> +
> struct pthread_barrier *ibarrier;
>
> /* XXX EINVAL is not specified by POSIX as a possible error code for COUNT
> diff --git a/nptl/pthread_barrierattr_init.c b/nptl/pthread_barrierattr_init.c
> index 796652e..81a6f5c 100644
> --- a/nptl/pthread_barrierattr_init.c
> +++ b/nptl/pthread_barrierattr_init.c
> @@ -22,6 +22,10 @@
> int
> pthread_barrierattr_init (pthread_barrierattr_t *attr)
> {
> + ASSERT_TYPE_SIZE (pthread_barrierattr_t, __SIZEOF_PTHREAD_BARRIERATTR_T);
> + ASSERT_PTHREAD_INTERNAL_SIZE (pthread_barrierattr_t,
> + struct pthread_barrierattr);
> +
> ((struct pthread_barrierattr *) attr)->pshared = PTHREAD_PROCESS_PRIVATE;
>
> return 0;
> diff --git a/nptl/pthread_cond_init.c b/nptl/pthread_cond_init.c
> index 0387f00..d23cdb1 100644
> --- a/nptl/pthread_cond_init.c
> +++ b/nptl/pthread_cond_init.c
> @@ -26,6 +26,8 @@
> int
> __pthread_cond_init (pthread_cond_t *cond, const pthread_condattr_t *cond_attr)
> {
> + ASSERT_TYPE_SIZE (pthread_cond_t, __SIZEOF_PTHREAD_COND_T);
> +
> struct pthread_condattr *icond_attr = (struct pthread_condattr *) cond_attr;
>
> memset (cond, 0, sizeof (pthread_cond_t));
> diff --git a/nptl/pthread_condattr_init.c b/nptl/pthread_condattr_init.c
> index 47bcc8b..0896abe 100644
> --- a/nptl/pthread_condattr_init.c
> +++ b/nptl/pthread_condattr_init.c
> @@ -23,6 +23,10 @@
> int
> __pthread_condattr_init (pthread_condattr_t *attr)
> {
> + ASSERT_TYPE_SIZE (pthread_condattr_t, __SIZEOF_PTHREAD_CONDATTR_T);
> + ASSERT_PTHREAD_INTERNAL_SIZE (pthread_condattr_t,
> + struct pthread_condattr);
> +
> struct pthread_condattr *iattr = (struct pthread_condattr *) attr;
> /* Default is not pshared and CLOCK_REALTIME. */
> iattr-> value = CLOCK_REALTIME << 1;
> diff --git a/nptl/pthread_getattr_default_np.c b/nptl/pthread_getattr_default_np.c
> index 771c06b..3357b73 100644
> --- a/nptl/pthread_getattr_default_np.c
> +++ b/nptl/pthread_getattr_default_np.c
> @@ -19,14 +19,12 @@
> #include <errno.h>
> #include <stdlib.h>
> #include <pthreadP.h>
> -#include <assert.h>
>
> int
> pthread_getattr_default_np (pthread_attr_t *out)
> {
> struct pthread_attr *real_out;
>
> - assert (sizeof (*out) >= sizeof (struct pthread_attr));
> real_out = (struct pthread_attr *) out;
>
> lll_lock (__default_pthread_attr_lock, LLL_PRIVATE);
> diff --git a/nptl/pthread_mutex_init.c b/nptl/pthread_mutex_init.c
> index e1f911b..8ab7b1d 100644
> --- a/nptl/pthread_mutex_init.c
> +++ b/nptl/pthread_mutex_init.c
> @@ -58,6 +58,8 @@ __pthread_mutex_init (pthread_mutex_t *mutex,
> {
> const struct pthread_mutexattr *imutexattr;
>
> + ASSERT_TYPE_SIZE (pthread_mutex_t, __SIZEOF_PTHREAD_MUTEX_T);
> +
> assert (sizeof (pthread_mutex_t) <= __SIZEOF_PTHREAD_MUTEX_T);
> ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__nusers,
> __PTHREAD_MUTEX_NUSERS_OFFSET);
> diff --git a/nptl/pthread_mutex_lock.c b/nptl/pthread_mutex_lock.c
> index 7f8254b..1acbe43 100644
> --- a/nptl/pthread_mutex_lock.c
> +++ b/nptl/pthread_mutex_lock.c
> @@ -62,8 +62,6 @@ static int __pthread_mutex_lock_full (pthread_mutex_t *mutex)
> int
> __pthread_mutex_lock (pthread_mutex_t *mutex)
> {
> - assert (sizeof (mutex->__size) >= sizeof (mutex->__data));
> -
> unsigned int type = PTHREAD_MUTEX_TYPE_ELISION (mutex);
>
> LIBC_PROBE (mutex_entry, 1, mutex);
> diff --git a/nptl/pthread_mutexattr_init.c b/nptl/pthread_mutexattr_init.c
> index dcad522..63ade02 100644
> --- a/nptl/pthread_mutexattr_init.c
> +++ b/nptl/pthread_mutexattr_init.c
> @@ -23,6 +23,10 @@
> int
> __pthread_mutexattr_init (pthread_mutexattr_t *attr)
> {
> + ASSERT_TYPE_SIZE (pthread_mutexattr_t, __SIZEOF_PTHREAD_MUTEXATTR_T);
> + ASSERT_PTHREAD_INTERNAL_SIZE (pthread_mutexattr_t,
> + struct pthread_mutexattr);
> +
> if (sizeof (struct pthread_mutexattr) != sizeof (pthread_mutexattr_t))
> memset (attr, '\0', sizeof (*attr));
>
> diff --git a/nptl/pthread_rwlock_init.c b/nptl/pthread_rwlock_init.c
> index 764ba11..c64e49f 100644
> --- a/nptl/pthread_rwlock_init.c
> +++ b/nptl/pthread_rwlock_init.c
> @@ -32,6 +32,8 @@ int
> __pthread_rwlock_init (pthread_rwlock_t *rwlock,
> const pthread_rwlockattr_t *attr)
> {
> + ASSERT_TYPE_SIZE (pthread_rwlock_t, __SIZEOF_PTHREAD_RWLOCK_T);
> +
> const struct pthread_rwlockattr *iattr;
>
> iattr = ((const struct pthread_rwlockattr *) attr) ?: &default_rwlockattr;
> diff --git a/nptl/pthread_rwlockattr_init.c b/nptl/pthread_rwlockattr_init.c
> index 8d90647..32a33cd 100644
> --- a/nptl/pthread_rwlockattr_init.c
> +++ b/nptl/pthread_rwlockattr_init.c
> @@ -22,6 +22,10 @@
> int
> pthread_rwlockattr_init (pthread_rwlockattr_t *attr)
> {
> + ASSERT_TYPE_SIZE (pthread_rwlockattr_t, __SIZEOF_PTHREAD_RWLOCKATTR_T);
> + ASSERT_PTHREAD_INTERNAL_SIZE (pthread_rwlockattr_t,
> + struct pthread_rwlockattr);
> +
> struct pthread_rwlockattr *iattr;
>
> iattr = (struct pthread_rwlockattr *) attr;
> diff --git a/nptl/pthread_setattr_default_np.c b/nptl/pthread_setattr_default_np.c
> index dd1b6fc..2fb787d 100644
> --- a/nptl/pthread_setattr_default_np.c
> +++ b/nptl/pthread_setattr_default_np.c
> @@ -19,7 +19,6 @@
> #include <errno.h>
> #include <stdlib.h>
> #include <pthreadP.h>
> -#include <assert.h>
> #include <string.h>
>
>
> @@ -30,7 +29,6 @@ pthread_setattr_default_np (const pthread_attr_t *in)
> struct pthread_attr attrs;
> int ret;
>
> - assert (sizeof (*in) >= sizeof (struct pthread_attr));
> real_in = (struct pthread_attr *) in;
>
> /* Catch invalid values. */
> diff --git a/nptl/sem_init.c b/nptl/sem_init.c
> index b9b839c..87cadeb 100644
> --- a/nptl/sem_init.c
> +++ b/nptl/sem_init.c
> @@ -27,6 +27,8 @@
> int
> __new_sem_init (sem_t *sem, int pshared, unsigned int value)
> {
> + ASSERT_PTHREAD_INTERNAL_SIZE (sem_t, struct new_sem);
> +
> /* Parameter sanity check. */
> if (__glibc_unlikely (value > SEM_VALUE_MAX))
> {
> @@ -68,6 +70,8 @@ int
> attribute_compat_text_section
> __old_sem_init (sem_t *sem, int pshared, unsigned int value)
> {
> + ASSERT_PTHREAD_INTERNAL_SIZE (sem_t, struct new_sem);
> +
> /* Parameter sanity check. */
> if (__glibc_unlikely (value > SEM_VALUE_MAX))
> {
> diff --git a/nptl/tst-typesizes.c b/nptl/tst-typesizes.c
> deleted file mode 100644
> index 78ed773..0000000
> --- a/nptl/tst-typesizes.c
> +++ /dev/null
> @@ -1,95 +0,0 @@
> -/* Copyright (C) 2005-2017 Free Software Foundation, Inc.
> - This file is part of the GNU C Library.
> - Contributed by Ulrich Drepper <drepper@redhat.com>, 2005.
> -
> - The GNU C Library is free software; you can redistribute it and/or
> - modify it under the terms of the GNU Lesser General Public
> - License as published by the Free Software Foundation; either
> - version 2.1 of the License, or (at your option) any later version.
> -
> - The GNU C Library is distributed in the hope that it will be useful,
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - Lesser General Public License for more details.
> -
> - You should have received a copy of the GNU Lesser General Public
> - License along with the GNU C Library; if not, see
> - <http://www.gnu.org/licenses/>. */
> -
> -#include <stdio.h>
> -#include <pthreadP.h>
> -#include <semaphore.h>
> -
> -static const struct
> -{
> - const char *name;
> - size_t expected;
> - size_t is;
> -} types[] =
> - {
> -#define T(t, c) \
> - { #t, c, sizeof (t) }
> - T (pthread_attr_t, __SIZEOF_PTHREAD_ATTR_T),
> - T (pthread_mutex_t, __SIZEOF_PTHREAD_MUTEX_T),
> - T (pthread_mutexattr_t, __SIZEOF_PTHREAD_MUTEXATTR_T),
> - T (pthread_cond_t, __SIZEOF_PTHREAD_COND_T),
> - T (pthread_condattr_t, __SIZEOF_PTHREAD_CONDATTR_T),
> - T (pthread_rwlock_t, __SIZEOF_PTHREAD_RWLOCK_T),
> - T (pthread_rwlockattr_t, __SIZEOF_PTHREAD_RWLOCKATTR_T),
> - T (pthread_barrier_t, __SIZEOF_PTHREAD_BARRIER_T),
> - T (pthread_barrierattr_t, __SIZEOF_PTHREAD_BARRIERATTR_T)
> - };
> -
> -static int
> -do_test (void)
> -{
> - int result = 0;
> -
> -#define TEST_TYPE(name) \
> - printf ("%s: ", #name); \
> - if (sizeof (name) != sizeof (((name *) 0)->__size)) \
> - { \
> - printf ("expected %zu, is %zu\n", \
> - sizeof (((name *) 0)->__size), sizeof (name)); \
> - result = 1; \
> - } \
> - else \
> - puts ("OK")
> -
> - TEST_TYPE (pthread_mutex_t);
> - TEST_TYPE (pthread_cond_t);
> - TEST_TYPE (pthread_rwlock_t);
> -
> -#define TEST_TYPE2(name, internal) \
> - printf ("%s: ", #name); \
> - if (sizeof (((name *) 0)->__size) < sizeof (internal)) \
> - { \
> - printf ("expected %zu, is %zu\n", \
> - sizeof (((name *) 0)->__size), sizeof (internal)); \
> - result = 1; \
> - } \
> - else \
> - puts ("OK")
> -
> - TEST_TYPE2 (pthread_attr_t, struct pthread_attr);
> - TEST_TYPE2 (pthread_mutexattr_t, struct pthread_mutexattr);
> - TEST_TYPE2 (pthread_condattr_t, struct pthread_condattr);
> - TEST_TYPE2 (pthread_rwlockattr_t, struct pthread_rwlockattr);
> - TEST_TYPE2 (pthread_barrier_t, struct pthread_barrier);
> - TEST_TYPE2 (pthread_barrierattr_t, struct pthread_barrierattr);
> - TEST_TYPE2 (sem_t, struct new_sem);
> - TEST_TYPE2 (sem_t, struct old_sem);
> -
> - for (size_t i = 0; i < sizeof (types) / sizeof (types[0]); ++i)
> - if (types[i].expected != types[i].is)
> - {
> - printf ("%s: expected %zu, is %zu\n",
> - types[i].name, types[i].expected, types[i].is);
> - result = 1;
> - }
> -
> - return result;
> -}
> -
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
>
On 10/26/2017 07:14 PM, Adhemerval Zanella wrote:
> +#define ASSERT_TYPE_SIZE(__type, __size) \
> + _Static_assert (sizeof (__type) == __size, \
> + "sizeof (" #__type ") != " #__size)
> +
> +#define ASSERT_PTHREAD_INTERNAL_SIZE(__type, __internal) \
> + _Static_assert (sizeof ((__type *) 0)->__size >= sizeof (__internal), \
> + "sizeof (" #__type ".__size) > sizeof (" #__internal ")")
No __ prefixes are need for macro arguments because there cannot be a
name clash. For the second macro, there is an operator discrepancy >= vs >.
I think ((__type) { 0 }).__size is vaguely more portable than the null
pointer dereference.
Regarding the structure of this patch, I wonder if it would be better to
have all the checks in a central place, so that it is easier to see if
any are missing. But if you prefer the current approach, this is fine
as well.
I still have to double-check if the current coverage is adequate.
Thanks,
Florian
On 02/11/2017 10:44, Florian Weimer wrote:
> On 10/26/2017 07:14 PM, Adhemerval Zanella wrote:
>> +#define ASSERT_TYPE_SIZE(__type, __size) \
>> + _Static_assert (sizeof (__type) == __size, \
>> + "sizeof (" #__type ") != " #__size)
>> +
>> +#define ASSERT_PTHREAD_INTERNAL_SIZE(__type, __internal) \
>> + _Static_assert (sizeof ((__type *) 0)->__size >= sizeof (__internal), \
>> + "sizeof (" #__type ".__size) > sizeof (" #__internal ")")
>
> No __ prefixes are need for macro arguments because there cannot be a name clash. For the second macro, there is an operator discrepancy >= vs >.
Ack.
>
> I think ((__type) { 0 }).__size is vaguely more portable than the null pointer dereference.
Ack.
>
> Regarding the structure of this patch, I wonder if it would be better to have all the checks in a central place, so that it is easier to see if any are missing. But if you prefer the current approach, this is fine as well.
I think it is more logical to add each possible tests on the implementation
file for the referred type (I used a similar strategy for C11 threads).
>
> I still have to double-check if the current coverage is adequate.
Right.
On 11/02/2017 02:31 PM, Adhemerval Zanella wrote:
>> I still have to double-check if the current coverage is adequate.
> Right.
Coverage is way better than what we have before, so this patch looks
good to me (with the agreed changes).
Thanks,
Florian
@@ -639,6 +639,14 @@ check_stacksize_attr (size_t st)
return EINVAL;
}
+#define ASSERT_TYPE_SIZE(__type, __size) \
+ _Static_assert (sizeof (__type) == __size, \
+ "sizeof (" #__type ") != " #__size)
+
+#define ASSERT_PTHREAD_INTERNAL_SIZE(__type, __internal) \
+ _Static_assert (sizeof ((__type *) 0)->__size >= sizeof (__internal), \
+ "sizeof (" #__type ".__size) > sizeof (" #__internal ")")
+
#define ASSERT_PTHREAD_STRING(x) __STRING (x)
#define ASSERT_PTHREAD_INTERNAL_OFFSET(__type, __member, __offset) \
_Static_assert (offsetof (__type, __member) == __offset, \
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include <errno.h>
#include <string.h>
#include <unistd.h>
@@ -28,7 +27,6 @@ __pthread_attr_destroy (pthread_attr_t *attr)
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
#if SHLIB_COMPAT(libpthread, GLIBC_2_0, GLIBC_2_1)
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include <errno.h>
#include <pthreadP.h>
#include <string.h>
@@ -32,7 +31,6 @@ __pthread_attr_getaffinity_new (const pthread_attr_t *attr, size_t cpusetsize,
{
const struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (const struct pthread_attr *) attr;
if (iattr->cpuset != NULL)
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include "pthreadP.h"
@@ -25,7 +24,6 @@ __pthread_attr_getdetachstate (const pthread_attr_t *attr, int *detachstate)
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
*detachstate = (iattr->flags & ATTR_FLAG_DETACHSTATE
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include "pthreadP.h"
@@ -25,7 +24,6 @@ pthread_attr_getguardsize (const pthread_attr_t *attr, size_t *guardsize)
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
*guardsize = iattr->guardsize;
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include "pthreadP.h"
@@ -25,7 +24,6 @@ __pthread_attr_getinheritsched (const pthread_attr_t *attr, int *inherit)
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
/* Store the current values. */
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include <string.h>
#include "pthreadP.h"
@@ -27,7 +26,6 @@ __pthread_attr_getschedparam (const pthread_attr_t *attr,
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
/* Copy the current values. */
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include "pthreadP.h"
@@ -25,7 +24,6 @@ __pthread_attr_getschedpolicy (const pthread_attr_t *attr, int *policy)
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
/* Store the current values. */
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include "pthreadP.h"
@@ -25,7 +24,6 @@ __pthread_attr_getscope (const pthread_attr_t *attr, int *scope)
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
/* Store the current values. */
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include "pthreadP.h"
@@ -26,7 +25,6 @@ __pthread_attr_getstack (const pthread_attr_t *attr, void **stackaddr,
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
/* Store the result. */
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include <errno.h>
#include "pthreadP.h"
@@ -26,7 +25,6 @@ __pthread_attr_getstackaddr (const pthread_attr_t *attr, void **stackaddr)
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
/* Some code assumes this function to work even if no stack address
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include "pthreadP.h"
@@ -25,7 +24,6 @@ __pthread_attr_getstacksize (const pthread_attr_t *attr, size_t *stacksize)
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
size_t size = iattr->stacksize;
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include <errno.h>
#include <string.h>
#include <unistd.h>
@@ -34,12 +33,14 @@ __pthread_attr_init_2_1 (pthread_attr_t *attr)
{
struct pthread_attr *iattr;
+ ASSERT_TYPE_SIZE (pthread_attr_t, __SIZEOF_PTHREAD_ATTR_T);
+ ASSERT_PTHREAD_INTERNAL_SIZE (pthread_attr_t, struct pthread_attr);
+
/* Many elements are initialized to zero so let us do it all at
once. This also takes care of clearing the bytes which are not
internally used. */
memset (attr, '\0', __SIZEOF_PTHREAD_ATTR_T);
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
/* Default guard size specified by the standard. */
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include <errno.h>
#include <limits.h>
#include <stdlib.h>
@@ -31,7 +30,6 @@ __pthread_attr_setaffinity_new (pthread_attr_t *attr, size_t cpusetsize,
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
if (cpuset == NULL || cpusetsize == 0)
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include <errno.h>
#include "pthreadP.h"
@@ -26,7 +25,6 @@ __pthread_attr_setdetachstate (pthread_attr_t *attr, int detachstate)
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
/* Catch invalid values. */
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include "pthreadP.h"
@@ -25,7 +24,6 @@ pthread_attr_setguardsize (pthread_attr_t *attr, size_t guardsize)
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
/* Note that we don't round the value here. The standard requires
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include <errno.h>
#include "pthreadP.h"
@@ -26,7 +25,6 @@ __pthread_attr_setinheritsched (pthread_attr_t *attr, int inherit)
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
/* Catch invalid values. */
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include <errno.h>
#include <string.h>
#include "pthreadP.h"
@@ -26,7 +25,6 @@ int
__pthread_attr_setschedparam (pthread_attr_t *attr,
const struct sched_param *param)
{
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
struct pthread_attr *iattr = (struct pthread_attr *) attr;
int ret = check_sched_priority_attr (param->sched_priority,
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include <errno.h>
#include "pthreadP.h"
@@ -26,7 +25,6 @@ __pthread_attr_setschedpolicy (pthread_attr_t *attr, int policy)
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
/* Catch invalid values. */
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include <errno.h>
#include "pthreadP.h"
@@ -26,7 +25,6 @@ __pthread_attr_setscope (pthread_attr_t *attr, int scope)
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
/* Catch invalid values. */
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include <errno.h>
#include <limits.h>
#include "pthreadP.h"
@@ -33,7 +32,6 @@ __pthread_attr_setstack (pthread_attr_t *attr, void *stackaddr,
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
/* Catch invalid sizes. */
@@ -71,7 +69,6 @@ __old_pthread_attr_setstack (pthread_attr_t *attr, void *stackaddr,
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
/* Catch invalid sizes. */
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include <errno.h>
#include "pthreadP.h"
@@ -30,7 +29,6 @@ __pthread_attr_setstackaddr (pthread_attr_t *attr, void *stackaddr)
EXTRA_PARAM_CHECKS;
#endif
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
iattr->stackaddr = stackaddr;
@@ -16,7 +16,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <assert.h>
#include <errno.h>
#include <limits.h>
#include "pthreadP.h"
@@ -31,7 +30,6 @@ __pthread_attr_setstacksize (pthread_attr_t *attr, size_t stacksize)
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
/* Catch invalid sizes. */
@@ -58,7 +56,6 @@ __old_pthread_attr_setstacksize (pthread_attr_t *attr, size_t stacksize)
{
struct pthread_attr *iattr;
- assert (sizeof (*attr) >= sizeof (struct pthread_attr));
iattr = (struct pthread_attr *) attr;
/* Catch invalid sizes. */
@@ -32,6 +32,10 @@ int
__pthread_barrier_init (pthread_barrier_t *barrier,
const pthread_barrierattr_t *attr, unsigned int count)
{
+ ASSERT_TYPE_SIZE (pthread_barrier_t, __SIZEOF_PTHREAD_BARRIER_T);
+ ASSERT_PTHREAD_INTERNAL_SIZE (pthread_barrier_t,
+ struct pthread_barrier);
+
struct pthread_barrier *ibarrier;
/* XXX EINVAL is not specified by POSIX as a possible error code for COUNT
@@ -22,6 +22,10 @@
int
pthread_barrierattr_init (pthread_barrierattr_t *attr)
{
+ ASSERT_TYPE_SIZE (pthread_barrierattr_t, __SIZEOF_PTHREAD_BARRIERATTR_T);
+ ASSERT_PTHREAD_INTERNAL_SIZE (pthread_barrierattr_t,
+ struct pthread_barrierattr);
+
((struct pthread_barrierattr *) attr)->pshared = PTHREAD_PROCESS_PRIVATE;
return 0;
@@ -26,6 +26,8 @@
int
__pthread_cond_init (pthread_cond_t *cond, const pthread_condattr_t *cond_attr)
{
+ ASSERT_TYPE_SIZE (pthread_cond_t, __SIZEOF_PTHREAD_COND_T);
+
struct pthread_condattr *icond_attr = (struct pthread_condattr *) cond_attr;
memset (cond, 0, sizeof (pthread_cond_t));
@@ -23,6 +23,10 @@
int
__pthread_condattr_init (pthread_condattr_t *attr)
{
+ ASSERT_TYPE_SIZE (pthread_condattr_t, __SIZEOF_PTHREAD_CONDATTR_T);
+ ASSERT_PTHREAD_INTERNAL_SIZE (pthread_condattr_t,
+ struct pthread_condattr);
+
struct pthread_condattr *iattr = (struct pthread_condattr *) attr;
/* Default is not pshared and CLOCK_REALTIME. */
iattr-> value = CLOCK_REALTIME << 1;
@@ -19,14 +19,12 @@
#include <errno.h>
#include <stdlib.h>
#include <pthreadP.h>
-#include <assert.h>
int
pthread_getattr_default_np (pthread_attr_t *out)
{
struct pthread_attr *real_out;
- assert (sizeof (*out) >= sizeof (struct pthread_attr));
real_out = (struct pthread_attr *) out;
lll_lock (__default_pthread_attr_lock, LLL_PRIVATE);
@@ -58,6 +58,8 @@ __pthread_mutex_init (pthread_mutex_t *mutex,
{
const struct pthread_mutexattr *imutexattr;
+ ASSERT_TYPE_SIZE (pthread_mutex_t, __SIZEOF_PTHREAD_MUTEX_T);
+
assert (sizeof (pthread_mutex_t) <= __SIZEOF_PTHREAD_MUTEX_T);
ASSERT_PTHREAD_INTERNAL_OFFSET (pthread_mutex_t, __data.__nusers,
__PTHREAD_MUTEX_NUSERS_OFFSET);
@@ -62,8 +62,6 @@ static int __pthread_mutex_lock_full (pthread_mutex_t *mutex)
int
__pthread_mutex_lock (pthread_mutex_t *mutex)
{
- assert (sizeof (mutex->__size) >= sizeof (mutex->__data));
-
unsigned int type = PTHREAD_MUTEX_TYPE_ELISION (mutex);
LIBC_PROBE (mutex_entry, 1, mutex);
@@ -23,6 +23,10 @@
int
__pthread_mutexattr_init (pthread_mutexattr_t *attr)
{
+ ASSERT_TYPE_SIZE (pthread_mutexattr_t, __SIZEOF_PTHREAD_MUTEXATTR_T);
+ ASSERT_PTHREAD_INTERNAL_SIZE (pthread_mutexattr_t,
+ struct pthread_mutexattr);
+
if (sizeof (struct pthread_mutexattr) != sizeof (pthread_mutexattr_t))
memset (attr, '\0', sizeof (*attr));
@@ -32,6 +32,8 @@ int
__pthread_rwlock_init (pthread_rwlock_t *rwlock,
const pthread_rwlockattr_t *attr)
{
+ ASSERT_TYPE_SIZE (pthread_rwlock_t, __SIZEOF_PTHREAD_RWLOCK_T);
+
const struct pthread_rwlockattr *iattr;
iattr = ((const struct pthread_rwlockattr *) attr) ?: &default_rwlockattr;
@@ -22,6 +22,10 @@
int
pthread_rwlockattr_init (pthread_rwlockattr_t *attr)
{
+ ASSERT_TYPE_SIZE (pthread_rwlockattr_t, __SIZEOF_PTHREAD_RWLOCKATTR_T);
+ ASSERT_PTHREAD_INTERNAL_SIZE (pthread_rwlockattr_t,
+ struct pthread_rwlockattr);
+
struct pthread_rwlockattr *iattr;
iattr = (struct pthread_rwlockattr *) attr;
@@ -19,7 +19,6 @@
#include <errno.h>
#include <stdlib.h>
#include <pthreadP.h>
-#include <assert.h>
#include <string.h>
@@ -30,7 +29,6 @@ pthread_setattr_default_np (const pthread_attr_t *in)
struct pthread_attr attrs;
int ret;
- assert (sizeof (*in) >= sizeof (struct pthread_attr));
real_in = (struct pthread_attr *) in;
/* Catch invalid values. */
@@ -27,6 +27,8 @@
int
__new_sem_init (sem_t *sem, int pshared, unsigned int value)
{
+ ASSERT_PTHREAD_INTERNAL_SIZE (sem_t, struct new_sem);
+
/* Parameter sanity check. */
if (__glibc_unlikely (value > SEM_VALUE_MAX))
{
@@ -68,6 +70,8 @@ int
attribute_compat_text_section
__old_sem_init (sem_t *sem, int pshared, unsigned int value)
{
+ ASSERT_PTHREAD_INTERNAL_SIZE (sem_t, struct new_sem);
+
/* Parameter sanity check. */
if (__glibc_unlikely (value > SEM_VALUE_MAX))
{
deleted file mode 100644
@@ -1,95 +0,0 @@
-/* Copyright (C) 2005-2017 Free Software Foundation, Inc.
- This file is part of the GNU C Library.
- Contributed by Ulrich Drepper <drepper@redhat.com>, 2005.
-
- The GNU C Library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
-
- The GNU C Library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public
- License along with the GNU C Library; if not, see
- <http://www.gnu.org/licenses/>. */
-
-#include <stdio.h>
-#include <pthreadP.h>
-#include <semaphore.h>
-
-static const struct
-{
- const char *name;
- size_t expected;
- size_t is;
-} types[] =
- {
-#define T(t, c) \
- { #t, c, sizeof (t) }
- T (pthread_attr_t, __SIZEOF_PTHREAD_ATTR_T),
- T (pthread_mutex_t, __SIZEOF_PTHREAD_MUTEX_T),
- T (pthread_mutexattr_t, __SIZEOF_PTHREAD_MUTEXATTR_T),
- T (pthread_cond_t, __SIZEOF_PTHREAD_COND_T),
- T (pthread_condattr_t, __SIZEOF_PTHREAD_CONDATTR_T),
- T (pthread_rwlock_t, __SIZEOF_PTHREAD_RWLOCK_T),
- T (pthread_rwlockattr_t, __SIZEOF_PTHREAD_RWLOCKATTR_T),
- T (pthread_barrier_t, __SIZEOF_PTHREAD_BARRIER_T),
- T (pthread_barrierattr_t, __SIZEOF_PTHREAD_BARRIERATTR_T)
- };
-
-static int
-do_test (void)
-{
- int result = 0;
-
-#define TEST_TYPE(name) \
- printf ("%s: ", #name); \
- if (sizeof (name) != sizeof (((name *) 0)->__size)) \
- { \
- printf ("expected %zu, is %zu\n", \
- sizeof (((name *) 0)->__size), sizeof (name)); \
- result = 1; \
- } \
- else \
- puts ("OK")
-
- TEST_TYPE (pthread_mutex_t);
- TEST_TYPE (pthread_cond_t);
- TEST_TYPE (pthread_rwlock_t);
-
-#define TEST_TYPE2(name, internal) \
- printf ("%s: ", #name); \
- if (sizeof (((name *) 0)->__size) < sizeof (internal)) \
- { \
- printf ("expected %zu, is %zu\n", \
- sizeof (((name *) 0)->__size), sizeof (internal)); \
- result = 1; \
- } \
- else \
- puts ("OK")
-
- TEST_TYPE2 (pthread_attr_t, struct pthread_attr);
- TEST_TYPE2 (pthread_mutexattr_t, struct pthread_mutexattr);
- TEST_TYPE2 (pthread_condattr_t, struct pthread_condattr);
- TEST_TYPE2 (pthread_rwlockattr_t, struct pthread_rwlockattr);
- TEST_TYPE2 (pthread_barrier_t, struct pthread_barrier);
- TEST_TYPE2 (pthread_barrierattr_t, struct pthread_barrierattr);
- TEST_TYPE2 (sem_t, struct new_sem);
- TEST_TYPE2 (sem_t, struct old_sem);
-
- for (size_t i = 0; i < sizeof (types) / sizeof (types[0]); ++i)
- if (types[i].expected != types[i].is)
- {
- printf ("%s: expected %zu, is %zu\n",
- types[i].name, types[i].expected, types[i].is);
- result = 1;
- }
-
- return result;
-}
-
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"