Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 26 Jul 2017 18:09:00 +0300
From:      Michael Zhilin <mizhka@freebsd.org>
To:        Eric van Gyzen <vangyzen@freebsd.org>, kib@freebsd.org
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org,  svn-src-head@freebsd.org, Michael Zhilin <mizhka@gmail.com>
Subject:   Re: svn commit: r318539 - head/lib/libthr/thread
Message-ID:  <CAF19XB%2B9%2BxFWuF5YLrzyczT83uMajDOhTftVEr4YJ7K4C_PcrA@mail.gmail.com>
In-Reply-To: <201705191304.v4JD45Sn021851@repo.freebsd.org>
References:  <201705191304.v4JD45Sn021851@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Hi Eric, Konstantin,

This commit breaks libthread_db a bit, particular change is:
-struct pthread_key _thread_keytable[PTHREAD_KEYS_MAX];
+static struct pthread_key _thread_keytable[PTHREAD_KEYS_MAX];

In libthread_db there is check if _thread_keytable is found in symbols of
debugged process:
https://svnweb.freebsd.org/base/head/lib/libthread_db/libthr_db.c?revision=241720&view=markup#l148

LOOKUP_SYM(ph, "_thread_keytable", &ta->thread_keytable_addr);

If symbol is not found, pt_ta_new returns "TD_NOLIBTHREAD", even if process
uses libthr.
It impacts sysutils/pstack port, it doesn't work for multithreaded
processes anymore.

Eric, Konstantin,
Could you please advise what is best way to fix it:

   - rollback change of "_thread_keytable" (it looks easy)
   - or remove "_thread_keytable" from libthread_db

?

Thanks!

On Fri, May 19, 2017 at 4:04 PM, Eric van Gyzen <vangyzen@freebsd.org>
wrote:

> Author: vangyzen
> Date: Fri May 19 13:04:05 2017
> New Revision: 318539
> URL: https://svnweb.freebsd.org/changeset/base/318539
>
> Log:
>   libthr: fix warnings at WARNS=6
>
>   Fix warnings about the following when WARNS=6 (which I will commit soon):
>
>   - casting away const
>   - no previous 'extern' declaration for non-static variable
>   - others as explained by #pragmas and comments
>   - unused parameters
>
>   The last is the only functional change.
>
>   Reviewed by:  kib
>   MFC after:    3 days
>   Sponsored by: Dell EMC
>   Differential Revision:        https://reviews.freebsd.org/D10808
>
> Modified:
>   head/lib/libthr/thread/thr_attr.c
>   head/lib/libthr/thread/thr_exit.c
>   head/lib/libthr/thread/thr_sig.c
>   head/lib/libthr/thread/thr_spec.c
>   head/lib/libthr/thread/thr_stack.c
>   head/lib/libthr/thread/thr_symbols.c
>   head/lib/libthr/thread/thr_umtx.c
>   head/lib/libthr/thread/thr_umtx.h
>
> Modified: head/lib/libthr/thread/thr_attr.c
> ============================================================
> ==================
> --- head/lib/libthr/thread/thr_attr.c   Fri May 19 13:02:19 2017
> (r318538)
> +++ head/lib/libthr/thread/thr_attr.c   Fri May 19 13:04:05 2017
> (r318539)
> @@ -607,7 +607,7 @@ _pthread_attr_setaffinity_np(pthread_att
>                         /* Kernel checks invalid bits, we check it here
> too. */
>                         size_t i;
>                         for (i = kern_size; i < cpusetsize; ++i) {
> -                               if (((char *)cpusetp)[i])
> +                               if (((const char *)cpusetp)[i])
>                                         return (EINVAL);
>                         }
>                 }
>
> Modified: head/lib/libthr/thread/thr_exit.c
> ============================================================
> ==================
> --- head/lib/libthr/thread/thr_exit.c   Fri May 19 13:02:19 2017
> (r318538)
> +++ head/lib/libthr/thread/thr_exit.c   Fri May 19 13:04:05 2017
> (r318539)
> @@ -119,7 +119,8 @@ _Unwind_GetCFA(struct _Unwind_Context *c
>  #endif /* PIC */
>
>  static void
> -thread_unwind_cleanup(_Unwind_Reason_Code code, struct _Unwind_Exception
> *e)
> +thread_unwind_cleanup(_Unwind_Reason_Code code __unused,
> +    struct _Unwind_Exception *e __unused)
>  {
>         /*
>          * Specification said that _Unwind_Resume should not be used here,
> @@ -130,10 +131,10 @@ thread_unwind_cleanup(_Unwind_Reason_Cod
>  }
>
>  static _Unwind_Reason_Code
> -thread_unwind_stop(int version, _Unwind_Action actions,
> -       int64_t exc_class,
> -       struct _Unwind_Exception *exc_obj,
> -       struct _Unwind_Context *context, void *stop_parameter)
> +thread_unwind_stop(int version __unused, _Unwind_Action actions,
> +       int64_t exc_class __unused,
> +       struct _Unwind_Exception *exc_obj __unused,
> +       struct _Unwind_Context *context, void *stop_parameter __unused)
>  {
>         struct pthread *curthread = _get_curthread();
>         struct pthread_cleanup *cur;
>
> Modified: head/lib/libthr/thread/thr_sig.c
> ============================================================
> ==================
> --- head/lib/libthr/thread/thr_sig.c    Fri May 19 13:02:19 2017
> (r318538)
> +++ head/lib/libthr/thread/thr_sig.c    Fri May 19 13:04:05 2017
> (r318539)
> @@ -441,7 +441,7 @@ _thr_signal_init(int dlopened)
>  }
>
>  void
> -_thr_sigact_unload(struct dl_phdr_info *phdr_info)
> +_thr_sigact_unload(struct dl_phdr_info *phdr_info __unused)
>  {
>  #if 0
>         struct pthread *curthread = _get_curthread();
>
> Modified: head/lib/libthr/thread/thr_spec.c
> ============================================================
> ==================
> --- head/lib/libthr/thread/thr_spec.c   Fri May 19 13:02:19 2017
> (r318538)
> +++ head/lib/libthr/thread/thr_spec.c   Fri May 19 13:04:05 2017
> (r318539)
> @@ -42,7 +42,7 @@ __FBSDID("$FreeBSD$");
>
>  #include "thr_private.h"
>
> -struct pthread_key _thread_keytable[PTHREAD_KEYS_MAX];
> +static struct pthread_key _thread_keytable[PTHREAD_KEYS_MAX];
>
>  __weak_reference(_pthread_key_create, pthread_key_create);
>  __weak_reference(_pthread_key_delete, pthread_key_delete);
>
> Modified: head/lib/libthr/thread/thr_stack.c
> ============================================================
> ==================
> --- head/lib/libthr/thread/thr_stack.c  Fri May 19 13:02:19 2017
> (r318538)
> +++ head/lib/libthr/thread/thr_stack.c  Fri May 19 13:04:05 2017
> (r318539)
> @@ -290,6 +290,19 @@ _thr_stack_alloc(struct pthread_attr *at
>                 return (-1);
>  }
>
> +/*
> + * Disable this warning from clang:
> + *
> + * cast from 'char *' to
> + *    'struct stack *' increases required alignment from 1 to 8
> + *    [-Werror,-Wcast-align]
> + *                 spare_stack = (struct stack *)
> + */
> +#ifdef __clang__
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wcast-align"
> +#endif
> +
>  /* This function must be called with _thread_list_lock held. */
>  void
>  _thr_stack_free(struct pthread_attr *attr)
> @@ -316,3 +329,7 @@ _thr_stack_free(struct pthread_attr *att
>                 attr->stackaddr_attr = NULL;
>         }
>  }
> +
> +#ifdef __clang__
> +#pragma GCC diagnostic pop
> +#endif
>
> Modified: head/lib/libthr/thread/thr_symbols.c
> ============================================================
> ==================
> --- head/lib/libthr/thread/thr_symbols.c        Fri May 19 13:02:19 2017
>       (r318538)
> +++ head/lib/libthr/thread/thr_symbols.c        Fri May 19 13:04:05 2017
>       (r318539)
> @@ -37,6 +37,10 @@ __FBSDID("$FreeBSD$");
>
>  #include "thr_private.h"
>
> +#ifdef __clang__
> +#pragma GCC diagnostic ignored "-Wmissing-variable-declarations"
> +#endif
> +
>  /* A collection of symbols needed by debugger */
>
>  /* int _libthr_debug */
>
> Modified: head/lib/libthr/thread/thr_umtx.c
> ============================================================
> ==================
> --- head/lib/libthr/thread/thr_umtx.c   Fri May 19 13:02:19 2017
> (r318538)
> +++ head/lib/libthr/thread/thr_umtx.c   Fri May 19 13:04:05 2017
> (r318539)
> @@ -168,7 +168,7 @@ __thr_umutex_timedlock(struct umutex *mt
>  }
>
>  int
> -__thr_umutex_unlock(struct umutex *mtx, uint32_t id)
> +__thr_umutex_unlock(struct umutex *mtx)
>  {
>
>         return (_umtx_op_err(mtx, UMTX_OP_MUTEX_UNLOCK, 0, 0, 0));
>
> Modified: head/lib/libthr/thread/thr_umtx.h
> ============================================================
> ==================
> --- head/lib/libthr/thread/thr_umtx.h   Fri May 19 13:02:19 2017
> (r318538)
> +++ head/lib/libthr/thread/thr_umtx.h   Fri May 19 13:04:05 2017
> (r318539)
> @@ -44,7 +44,7 @@ int __thr_umutex_lock(struct umutex *mtx
>  int __thr_umutex_lock_spin(struct umutex *mtx, uint32_t id) __hidden;
>  int __thr_umutex_timedlock(struct umutex *mtx, uint32_t id,
>         const struct timespec *timeout) __hidden;
> -int __thr_umutex_unlock(struct umutex *mtx, uint32_t id) __hidden;
> +int __thr_umutex_unlock(struct umutex *mtx) __hidden;
>  int __thr_umutex_trylock(struct umutex *mtx) __hidden;
>  int __thr_umutex_set_ceiling(struct umutex *mtx, uint32_t ceiling,
>         uint32_t *oldceiling) __hidden;
> @@ -155,7 +155,7 @@ _thr_umutex_unlock2(struct umutex *mtx,
>                 if (atomic_cmpset_rel_32(&mtx->m_owner, id, noncst ?
>                     UMUTEX_RB_NOTRECOV : UMUTEX_UNOWNED))
>                         return (0);
> -               return (__thr_umutex_unlock(mtx, id));
> +               return (__thr_umutex_unlock(mtx));
>         }
>
>         do {
>
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAF19XB%2B9%2BxFWuF5YLrzyczT83uMajDOhTftVEr4YJ7K4C_PcrA>