Re: [PATCH] libgfortran: Disable gthreads weak symbols for glibc 2.34

Re: [PATCH] libgfortran: Disable gthreads weak symbols for glibc 2.34

[Andrew Pinski]

[-- Attachment #1: Type: text/plain, Size: 5985 bytes --]

On Tue, Apr 9, 2024, 10:07 H.J. Lu <hjl.tools@gmail.com> wrote:

> Since Glibc 2.34 all pthreads symbols are defined directly in libc not
> libpthread, and since Glibc 2.32 we have used __libc_single_threaded to
> avoid unnecessary locking in single-threaded programs. This means there
> is no reason to avoid linking to libpthread now, and so no reason to use
> weak symbols defined in gthr-posix.h for all the pthread_xxx functions.
>


First you forgot to cc fortran@. Second the issue is in gthrd-posix.h which
should be fixed instead of libgfortran since the issue will also be seen
with libobjc, and the other users of gthrd.

Note the fix for libstdc++ was also done in the wrong location too and
should have done once and for all in gthrd-posix.h.


Thanks,
Andrew


> Also add prune_warnings to libgomp.exp to prune glibc static link warning:
>
> .*: warning: Using 'dlopen' in statically linked applications requires at
> runtime the shared libraries from the glibc version us ed for linking
>
> libgfortran/
>
>         PR libgfortran/114646
>         * acinclude.m4: Define GTHREAD_USE_WEAK 0 for glibc 2.34 or
>         above on Linux.
>         * configure: Regenerated.
>
> libgomp/
>
>         PR libgfortran/114646
>         * testsuite/lib/libgomp.exp (prune_warnings): New.
>         * testsuite/libgomp.fortran/pr114646-1.f90: New test.
>         * testsuite/libgomp.fortran/pr114646-2.f90: Likewise.
> ---
>  libgfortran/acinclude.m4                      | 14 +++++++++
>  libgfortran/configure                         | 29 +++++++++++++++++++
>  libgomp/testsuite/lib/libgomp.exp             | 14 +++++++++
>  .../testsuite/libgomp.fortran/pr114646-1.f90  | 11 +++++++
>  .../testsuite/libgomp.fortran/pr114646-2.f90  | 22 ++++++++++++++
>  5 files changed, 90 insertions(+)
>  create mode 100644 libgomp/testsuite/libgomp.fortran/pr114646-1.f90
>  create mode 100644 libgomp/testsuite/libgomp.fortran/pr114646-2.f90
>
> diff --git a/libgfortran/acinclude.m4 b/libgfortran/acinclude.m4
> index a73207e5465..f4642494c4f 100644
> --- a/libgfortran/acinclude.m4
> +++ b/libgfortran/acinclude.m4
> @@ -92,6 +92,20 @@ void foo (void);
>        AC_DEFINE(GTHREAD_USE_WEAK, 0,
>                 [Define to 0 if the target shouldn't use #pragma weak])
>        ;;
> +    *-*-linux*)
> +      AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
> +#include <features.h>
> +#if !__GLIBC_PREREQ(2, 34)
> +#error glibc version is too old
> +#endif
> +]], [[]])],
> +                       libgfor_cv_use_pragma_weak=no,
> +                       libgfor_cv_use_pragma_weak=yes)
> +      if test $libgfor_cv_use_pragma_weak = no; then
> +        AC_DEFINE(GTHREAD_USE_WEAK, 0,
> +                 [Define to 0 if the target shouldn't use #pragma weak])
> +      fi
> +      ;;
>    esac])
>
>  dnl Check whether target effectively supports weakref
> diff --git a/libgfortran/configure b/libgfortran/configure
> index 774dd52fc95..1f477256b75 100755
> --- a/libgfortran/configure
> +++ b/libgfortran/configure
> @@ -31057,6 +31057,35 @@ $as_echo "#define SUPPORTS_WEAK 1" >>confdefs.h
>
>  $as_echo "#define GTHREAD_USE_WEAK 0" >>confdefs.h
>
> +      ;;
> +    *-*-linux*)
> +      cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> +/* end confdefs.h.  */
> +
> +#include <features.h>
> +#if !__GLIBC_PREREQ(2, 34)
> +#error glibc version is too old
> +#endif
> +
> +int
> +main ()
> +{
> +
> +  ;
> +  return 0;
> +}
> +_ACEOF
> +if ac_fn_c_try_compile "$LINENO"; then :
> +  libgfor_cv_use_pragma_weak=no
> +else
> +  libgfor_cv_use_pragma_weak=yes
> +fi
> +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
> +      if test $libgfor_cv_use_pragma_weak = no; then
> +
> +$as_echo "#define GTHREAD_USE_WEAK 0" >>confdefs.h
> +
> +      fi
>        ;;
>    esac
>
> diff --git a/libgomp/testsuite/lib/libgomp.exp
> b/libgomp/testsuite/lib/libgomp.exp
> index cab926a798b..9cfa6d7b31d 100644
> --- a/libgomp/testsuite/lib/libgomp.exp
> +++ b/libgomp/testsuite/lib/libgomp.exp
> @@ -54,6 +54,20 @@ set dg-do-what-default run
>
>  set libgomp_compile_options ""
>
> +# Prune messages that aren't useful.
> +
> +proc prune_warnings { text } {
> +
> +    verbose "prune_warnings: entry: $text" 2
> +
> +    # Ignore warning from -static: warning: Using 'dlopen' in statically
> linked applications requires at runtime the shared libraries from the glibc
> version used for linking
> +    regsub -all "(^|\n)\[^\n\]*: warning: Using 'dlopen' in statically
> linked\[^\n\]*" $text "" text
> +
> +    verbose "prune_warnings: exit: $text" 2
> +
> +    return $text
> +}
> +
>  #
>  # libgomp_init
>  #
> diff --git a/libgomp/testsuite/libgomp.fortran/pr114646-1.f90
> b/libgomp/testsuite/libgomp.fortran/pr114646-1.f90
> new file mode 100644
> index 00000000000..a48e6103343
> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.fortran/pr114646-1.f90
> @@ -0,0 +1,11 @@
> +! PR libgfortran/114646
> +! { dg-do run }
> +! { dg-additional-options "-static" }
> +
> +!$OMP PARALLEL
> +!$OMP CRITICAL
> + write(6,*) "Hello world"
> +!$OMP END CRITICAL
> +!$OMP END PARALLEL
> + write(6,*) "Done!"
> +END
> diff --git a/libgomp/testsuite/libgomp.fortran/pr114646-2.f90
> b/libgomp/testsuite/libgomp.fortran/pr114646-2.f90
> new file mode 100644
> index 00000000000..8c0d7526f95
> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.fortran/pr114646-2.f90
> @@ -0,0 +1,22 @@
> +! PR libgfortran/114646
> +! { dg-do run }
> +! { dg-additional-options "-static" }
> +
> +      use omp_lib
> +      implicit none
> +      integer, parameter :: NT = 4
> +      integer :: nThreads(NT)
> +
> +      print *, 'Call omp_set_dynamic'
> +!$    call omp_set_dynamic(.false.)
> +      print *, 'Call omp_set_num_threads'
> +!$    call omp_set_num_threads(NT)
> +      print *, 'Now enter the parallel region'
> +
> +!$omp parallel default(none) shared(nThreads)
> +      nThreads(omp_get_thread_num()+1) = omp_get_num_threads()
> +!$omp end parallel
> +
> +      print*, nThreads
> +
> +      END
> --
> 2.44.0
>
>

Re: [PATCH] libgfortran: Disable gthreads weak symbols for glibc 2.34

[H.J. Lu]

On Tue, Apr 9, 2024 at 10:25 AM Andrew Pinski <pinskia@gmail.com> wrote:
>
>
>
> On Tue, Apr 9, 2024, 10:07 H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> Since Glibc 2.34 all pthreads symbols are defined directly in libc not
>> libpthread, and since Glibc 2.32 we have used __libc_single_threaded to
>> avoid unnecessary locking in single-threaded programs. This means there
>> is no reason to avoid linking to libpthread now, and so no reason to use
>> weak symbols defined in gthr-posix.h for all the pthread_xxx functions.
>
>
>
> First you forgot to cc fortran@. Second the issue is in gthrd-posix.h which should be fixed instead of libgfortran since the issue will also be seen with libobjc, and the other users of gthrd.

Weak symbol reference to pthread doesn't fail for all static executables.
Fixing it on a per-library basis is one approach.

> Note the fix for libstdc++ was also done in the wrong location too and should have done once and for all in gthrd-posix.h.
>
>
> Thanks,
> Andrew
>
>>
>> Also add prune_warnings to libgomp.exp to prune glibc static link warning:
>>
>> .*: warning: Using 'dlopen' in statically linked applications requires at runtime the shared libraries from the glibc version us ed for linking
>>
>> libgfortran/
>>
>>         PR libgfortran/114646
>>         * acinclude.m4: Define GTHREAD_USE_WEAK 0 for glibc 2.34 or
>>         above on Linux.
>>         * configure: Regenerated.
>>
>> libgomp/
>>
>>         PR libgfortran/114646
>>         * testsuite/lib/libgomp.exp (prune_warnings): New.
>>         * testsuite/libgomp.fortran/pr114646-1.f90: New test.
>>         * testsuite/libgomp.fortran/pr114646-2.f90: Likewise.
>> ---
>>  libgfortran/acinclude.m4                      | 14 +++++++++
>>  libgfortran/configure                         | 29 +++++++++++++++++++
>>  libgomp/testsuite/lib/libgomp.exp             | 14 +++++++++
>>  .../testsuite/libgomp.fortran/pr114646-1.f90  | 11 +++++++
>>  .../testsuite/libgomp.fortran/pr114646-2.f90  | 22 ++++++++++++++
>>  5 files changed, 90 insertions(+)
>>  create mode 100644 libgomp/testsuite/libgomp.fortran/pr114646-1.f90
>>  create mode 100644 libgomp/testsuite/libgomp.fortran/pr114646-2.f90
>>
>> diff --git a/libgfortran/acinclude.m4 b/libgfortran/acinclude.m4
>> index a73207e5465..f4642494c4f 100644
>> --- a/libgfortran/acinclude.m4
>> +++ b/libgfortran/acinclude.m4
>> @@ -92,6 +92,20 @@ void foo (void);
>>        AC_DEFINE(GTHREAD_USE_WEAK, 0,
>>                 [Define to 0 if the target shouldn't use #pragma weak])
>>        ;;
>> +    *-*-linux*)
>> +      AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
>> +#include <features.h>
>> +#if !__GLIBC_PREREQ(2, 34)
>> +#error glibc version is too old
>> +#endif
>> +]], [[]])],
>> +                       libgfor_cv_use_pragma_weak=no,
>> +                       libgfor_cv_use_pragma_weak=yes)
>> +      if test $libgfor_cv_use_pragma_weak = no; then
>> +        AC_DEFINE(GTHREAD_USE_WEAK, 0,
>> +                 [Define to 0 if the target shouldn't use #pragma weak])
>> +      fi
>> +      ;;
>>    esac])
>>
>>  dnl Check whether target effectively supports weakref
>> diff --git a/libgfortran/configure b/libgfortran/configure
>> index 774dd52fc95..1f477256b75 100755
>> --- a/libgfortran/configure
>> +++ b/libgfortran/configure
>> @@ -31057,6 +31057,35 @@ $as_echo "#define SUPPORTS_WEAK 1" >>confdefs.h
>>
>>  $as_echo "#define GTHREAD_USE_WEAK 0" >>confdefs.h
>>
>> +      ;;
>> +    *-*-linux*)
>> +      cat confdefs.h - <<_ACEOF >conftest.$ac_ext
>> +/* end confdefs.h.  */
>> +
>> +#include <features.h>
>> +#if !__GLIBC_PREREQ(2, 34)
>> +#error glibc version is too old
>> +#endif
>> +
>> +int
>> +main ()
>> +{
>> +
>> +  ;
>> +  return 0;
>> +}
>> +_ACEOF
>> +if ac_fn_c_try_compile "$LINENO"; then :
>> +  libgfor_cv_use_pragma_weak=no
>> +else
>> +  libgfor_cv_use_pragma_weak=yes
>> +fi
>> +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>> +      if test $libgfor_cv_use_pragma_weak = no; then
>> +
>> +$as_echo "#define GTHREAD_USE_WEAK 0" >>confdefs.h
>> +
>> +      fi
>>        ;;
>>    esac
>>
>> diff --git a/libgomp/testsuite/lib/libgomp.exp b/libgomp/testsuite/lib/libgomp.exp
>> index cab926a798b..9cfa6d7b31d 100644
>> --- a/libgomp/testsuite/lib/libgomp.exp
>> +++ b/libgomp/testsuite/lib/libgomp.exp
>> @@ -54,6 +54,20 @@ set dg-do-what-default run
>>
>>  set libgomp_compile_options ""
>>
>> +# Prune messages that aren't useful.
>> +
>> +proc prune_warnings { text } {
>> +
>> +    verbose "prune_warnings: entry: $text" 2
>> +
>> +    # Ignore warning from -static: warning: Using 'dlopen' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
>> +    regsub -all "(^|\n)\[^\n\]*: warning: Using 'dlopen' in statically linked\[^\n\]*" $text "" text
>> +
>> +    verbose "prune_warnings: exit: $text" 2
>> +
>> +    return $text
>> +}
>> +
>>  #
>>  # libgomp_init
>>  #
>> diff --git a/libgomp/testsuite/libgomp.fortran/pr114646-1.f90 b/libgomp/testsuite/libgomp.fortran/pr114646-1.f90
>> new file mode 100644
>> index 00000000000..a48e6103343
>> --- /dev/null
>> +++ b/libgomp/testsuite/libgomp.fortran/pr114646-1.f90
>> @@ -0,0 +1,11 @@
>> +! PR libgfortran/114646
>> +! { dg-do run }
>> +! { dg-additional-options "-static" }
>> +
>> +!$OMP PARALLEL
>> +!$OMP CRITICAL
>> + write(6,*) "Hello world"
>> +!$OMP END CRITICAL
>> +!$OMP END PARALLEL
>> + write(6,*) "Done!"
>> +END
>> diff --git a/libgomp/testsuite/libgomp.fortran/pr114646-2.f90 b/libgomp/testsuite/libgomp.fortran/pr114646-2.f90
>> new file mode 100644
>> index 00000000000..8c0d7526f95
>> --- /dev/null
>> +++ b/libgomp/testsuite/libgomp.fortran/pr114646-2.f90
>> @@ -0,0 +1,22 @@
>> +! PR libgfortran/114646
>> +! { dg-do run }
>> +! { dg-additional-options "-static" }
>> +
>> +      use omp_lib
>> +      implicit none
>> +      integer, parameter :: NT = 4
>> +      integer :: nThreads(NT)
>> +
>> +      print *, 'Call omp_set_dynamic'
>> +!$    call omp_set_dynamic(.false.)
>> +      print *, 'Call omp_set_num_threads'
>> +!$    call omp_set_num_threads(NT)
>> +      print *, 'Now enter the parallel region'
>> +
>> +!$omp parallel default(none) shared(nThreads)
>> +      nThreads(omp_get_thread_num()+1) = omp_get_num_threads()
>> +!$omp end parallel
>> +
>> +      print*, nThreads
>> +
>> +      END
>> --
>> 2.44.0
>>


-- 
H.J.