RE: libcilkrts: Use AC_USE_SYSTEM_EXTENSIONS (was: Cilk Library)

["Tannenbaum, Barry M" <barry.m.tannenbaum@intel.com>]

The problem arose in our internal build which is a (relatively) simple Makefile. Now that I know what AC_USE_SYSTEM_EXTENSIONS is doing I can adjust it accordingly.

Thanks.

  - Barry

-----Original Message-----
From: Thomas Schwinge [mailto:thomas@codesourcery.com] 
Sent: Wednesday, October 01, 2014 10:02 AM
To: Tannenbaum, Barry M
Cc: gcc-patches@gcc.gnu.org; Iyer, Balaji V; Zamyatin, Igor
Subject: Re: libcilkrts: Use AC_USE_SYSTEM_EXTENSIONS (was: Cilk Library)

Hi!

On Mon, 29 Sep 2014 20:13:38 +0200, I wrote:
> On Wed, 9 Oct 2013 18:32:11 +0000, "Iyer, Balaji V" <balaji.v.iyer@intel.com> wrote:
> > [libcilkrts]
> 
> Here is a patch to have libcilkrts use AC_USE_SYSTEM_EXTENSIONS (as 
> other libraries are doing) instead of manually fiddling with the 
> _GNU_SOURCE definition.  This increases portability, as most of those 
> definitions are currently hard-coded for __linux__ only.  Tested on 
> x86 GNU/Hurd, and
> x86_64 GNU/Linux is in progress.  OK for trunk once testing completed?

Barry had a question about this change, so let me clarify this:

>     	libcilkrts/
>     	* configure.ac (AC_USE_SYSTEM_EXTENSIONS): Instantiate.
>     	<HAVE_PTHREAD_AFFINITY_NP>: Don't define _GNU_SOURCE.
>     	* configure: Regenerate.
>     	* runtime/os-unix.c [__linux__] (_GNU_SOURCE): Don't define.
>     	* runtime/sysdep-unix.c [__linux__] (_GNU_SOURCE): Likewise.

> --- libcilkrts/configure.ac
> +++ libcilkrts/configure.ac
> @@ -39,6 +39,9 @@ AC_PREREQ([2.64])
>  AC_CANONICAL_SYSTEM
>  target_alias=${target_alias-$host_alias}
>  AC_SUBST(target_alias)
> +
> +AC_USE_SYSTEM_EXTENSIONS

> @@ -188,8 +191,7 @@ AC_LINK_IFELSE(
>  # Check for pthread_{,attr_}[sg]etaffinity_np.
>  AC_LINK_IFELSE(
>   [AC_LANG_PROGRAM(
> -  [#define _GNU_SOURCE
> -   #include <pthread.h>],
> +  [#include <pthread.h>],
>    [cpu_set_t cpuset;
>     pthread_attr_t attr;
>     pthread_getaffinity_np (pthread_self (), sizeof (cpu_set_t), 
> &cpuset);

> --- libcilkrts/runtime/os-unix.c
> +++ libcilkrts/runtime/os-unix.c
> @@ -36,13 +36,6 @@
>   *  POSSIBILITY OF SUCH DAMAGE.
>   
> **********************************************************************
> ****/
>  
> -#ifdef __linux__
> -    // define _GNU_SOURCE before *any* #include.
> -    // Even <stdint.h> will break later #includes if this macro is not
> -    // already defined when it is #included.
> -#   define _GNU_SOURCE
> -#endif

> --- libcilkrts/runtime/sysdep-unix.c
> +++ libcilkrts/runtime/sysdep-unix.c
> @@ -39,13 +39,6 @@
>   **************************************************************************
>   */
>  
> -#ifdef __linux__
> -    // define _GNU_SOURCE before *any* #include.
> -    // Even <stdint.h> will break later #includes if this macro is not
> -    // already defined when it is #included.
> -#   define _GNU_SOURCE
> -#endif

On Wed, 1 Oct 2014 13:18:51 +0000, "Tannenbaum, Barry M" <barry.m.tannenbaum@intel.com> wrote:
> I've been looking at your 3rd edit to remove the use of _GNU_SOURCE for portability. We didn't define this arbitrarily. We want to call dladdr() to get the shared object associated with an address. The docs state that in order to call this function, we should be defining _GNU_SOURCE.

You're correct that -D_GNU_SOURCE is required for this.  What I proposed is to remove the *manual* _GNU_SOURCE definitions, that are currently conditioned to __linux__, given that I had put AC_USE_SYSTEM_EXTENSIONS into configure.ac, which globally adds -D_GNU_SOURCE as well as other similar defines to the compiler flags, see <https://www.gnu.org/software/autoconf/manual/autoconf-2.64/html_node/Posix-Variants.html>.
Of course, this is a bit of a "sledgehammer approach" to the problem, but is what is being done in other libraries that ship with GCC.

Now, I don't know all the contexts in which the libcilkrts code is being built.  I thought all of them -- or, at least those where the _GNU_SOURCE definition does have any effect: GNU systems using glibc -- are using the configure script, but maybe that's not true.  In this case, in these other build systems, if any, also (maybe just unconditionally) add -D_GNU_SOURCE to the compiler flags.  If that's not an option, leave in the manual _GNU_SOURCE definitions in the *.c files, conditioned to __linux__ -- which is ugly ;-) but does fulfil its purpose.

> How should I be determining if this functionality is available?

You can then assume that dladdr is available.


Grüße,
 Thomas