[PATCH, v2] Fortran: fix argument checking of intrinsics C_SIZEOF, C_F_POINTER [PR106500]
Hi FX!
On 4/9/24 09:32, FX Coudert wrote:
Hi Harald,
Thanks for the patch.
+ if (attr.function)
+{
+ gfc_error ("FPTR at %L to C_F_POINTER is a function returning a pointer",
+ >where);
+ return false;
+}
+
if (fptr->rank > 0 && !is_c_interoperable (fptr, , false, true))
return gfc_notify_std (GFC_STD_F2018, "Noninteroperable array FPTR "
"at %L to C_F_POINTER: %s", >where, msg);
In both of these gfc_error(), could we change our error message to say "FPTR
argument” instead of “FPTR”? “FPTR to C_F_POINTER” does not really make sense to me.
This would be more in line with what the generally do:
Error: 'x' argument of 'sqrt' intrinsic at (1) must be REAL or COMPLEX
So maybe “FPTR argument to C_F_POINTER at %L” ? That’s much more readable to me.
Good point! I did indeed feel a little uncomfortable with the text
and adjusted both messages accordingly to your suggestion.
I also forgot to add one update of a pattern, and found a cornercase
where the tightening of checks for C_F_POINTER was too strong.
Corrected and now covered in an extension of the corresponding testcase.
Otherwise, OK.
FX
Thanks for the review!
If there are no further comments, I will commit tomorrow.
Thanks,
Harald
From 5983a07f11c88d920241141732fa742735cdb8ea Mon Sep 17 00:00:00 2001
From: Harald Anlauf
Date: Tue, 9 Apr 2024 23:07:59 +0200
Subject: [PATCH] Fortran: fix argument checking of intrinsics C_SIZEOF,
C_F_POINTER [PR106500]
The interpretation of the F2018 standard regarding valid arguments to the
intrinsic C_SIZEOF(X) was clarified in an edit to 18-007r1:
https://j3-fortran.org/doc/year/22/22-101r1.txt
loosening restrictions and giving examples. The F2023 text has:
! F2023:18.2.3.8 C_SIZEOF (X)
!
! X shall be a data entity with interoperable type and type parameters,
! and shall not be an assumed-size array, an assumed-rank array that
! is associated with an assumed-size array, an unallocated allocatable
! variable, or a pointer that is not associated.
where
! 3.41 data entity
! data object, result of the evaluation of an expression, or the
! result of the execution of a function reference
Update the checking code for interoperable arguments accordingly, and extend
to reject functions returning pointer as FPTR argument to C_F_POINTER.
gcc/fortran/ChangeLog:
PR fortran/106500
* check.cc (is_c_interoperable): Fix checks for C_SIZEOF.
(gfc_check_c_f_pointer): Reject function returning a pointer as FPTR,
and improve an error message.
gcc/testsuite/ChangeLog:
PR fortran/106500
* gfortran.dg/c_sizeof_6.f90: Remove wrong dg-error.
* gfortran.dg/sizeof_2.f90: Adjust pattern.
* gfortran.dg/c_f_pointer_tests_9.f90: New test.
* gfortran.dg/c_sizeof_7.f90: New test.
---
gcc/fortran/check.cc | 26 +++-
.../gfortran.dg/c_f_pointer_tests_9.f90 | 37
gcc/testsuite/gfortran.dg/c_sizeof_6.f90 | 2 +-
gcc/testsuite/gfortran.dg/c_sizeof_7.f90 | 42 +++
gcc/testsuite/gfortran.dg/sizeof_2.f90| 2 +-
5 files changed, 96 insertions(+), 13 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/c_f_pointer_tests_9.f90
create mode 100644 gcc/testsuite/gfortran.dg/c_sizeof_7.f90
diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc
index db74dcf3f40..2f50d84b876 100644
--- a/gcc/fortran/check.cc
+++ b/gcc/fortran/check.cc
@@ -5299,18 +5299,14 @@ is_c_interoperable (gfc_expr *expr, const char **msg, bool c_loc, bool c_f_ptr)
return false;
}
- if (!c_loc && expr->rank > 0 && expr->expr_type != EXPR_ARRAY)
+ /* Checks for C_SIZEOF need to take into account edits to 18-007r1, see
+ https://j3-fortran.org/doc/year/22/22-101r1.txt . */
+ if (!c_loc && !c_f_ptr && expr->rank > 0 && expr->expr_type == EXPR_VARIABLE)
{
gfc_array_ref *ar = gfc_find_array_ref (expr);
- if (ar->type != AR_FULL)
+ if (ar->type == AR_FULL && ar->as->type == AS_ASSUMED_SIZE)
{
- *msg = "Only whole-arrays are interoperable";
- return false;
- }
- if (!c_f_ptr && ar->as->type != AS_EXPLICIT
- && ar->as->type != AS_ASSUMED_SIZE)
- {
- *msg = "Only explicit-size and assumed-size arrays are interoperable";
+ *msg = "Assumed-size arrays are not interoperable";
return false;
}
}
@@ -5475,9 +5471,17 @@ gfc_check_c_f_pointer (gfc_expr *cptr, gfc_expr *fptr, gfc_expr *shape)
return false;
}
+ if (fptr->ts.type == BT_PROCEDURE && attr.function)
+{
+ gfc_error ("FPTR argument to C_F_POINTER at %L is a function "
+ "returning a pointer", >where);
+ return false;
+}
+
if (fptr->rank > 0 && !is_c_interoperable (fptr, , false, true))
-return gfc_notify_std (GFC_STD_F2018, "Noninteroperable array FPTR "
- "at %L to C_F_POINTER: %s", >where, msg);
+return gfc_notify_std (GFC_STD_F2018,
+ "Noninteroperable array FPTR argument to "
+ "C_F_POINTER at %L: %s", >where, msg);
return
Re: [PATCH] libgfortran: Disable gthreads weak symbols for glibc 2.34
On Tue, Apr 9, 2024 at 10:25 AM Andrew Pinski wrote:
>
>
>
> On Tue, Apr 9, 2024, 10:07 H.J. Lu 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
>> +#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
>> +#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
>>
Re: [PATCH] libgfortran: Disable gthreads weak symbols for glibc 2.34
On Tue, Apr 9, 2024, 10:07 H.J. Lu 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
> +#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
> +#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 000..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
>
[Patch, fortran] PR113956 - [13/14 Regression] ice in gfc_trans_pointer_assignment, at fortran/trans-expr.cc:10524
Patch pushed after pre-approval by Harald on Bugzilla. Fortran: Fix ICE in gfc_trans_pointer_assignment [PR113956] 2024-04-09 Paul Thomas gcc/fortran PR fortran/113956 * trans-expr.cc (gfc_trans_pointer_assignment): Remove assert causing the ICE since it was unnecesary. gcc/testsuite/ PR fortran/113956 * gfortran.dg/pr113956.f90: New test. Paul
Re: [PATCH] Fortran: fix argument checking of intrinsics C_SIZEOF, C_F_POINTER [PR106500]
Hi Harald,
Thanks for the patch.
> + if (attr.function)
> +{
> + gfc_error ("FPTR at %L to C_F_POINTER is a function returning a
> pointer",
> + >where);
> + return false;
> +}
> +
>if (fptr->rank > 0 && !is_c_interoperable (fptr, , false, true))
> return gfc_notify_std (GFC_STD_F2018, "Noninteroperable array FPTR "
> "at %L to C_F_POINTER: %s", >where, msg);
In both of these gfc_error(), could we change our error message to say "FPTR
argument” instead of “FPTR”? “FPTR to C_F_POINTER” does not really make sense
to me.
This would be more in line with what the generally do:
> Error: 'x' argument of 'sqrt' intrinsic at (1) must be REAL or COMPLEX
So maybe “FPTR argument to C_F_POINTER at %L” ? That’s much more readable to me.
Otherwise, OK.
FX
