Re: [Fortran, Patch, PR120711, v1] 1/(3) Fix out of bounds access in cleanup of array constructor
Hello Andre, I get a regression on this testcase with a patch that is otherwise regression-free. I think the testcase is invalid. It does: type(container), allocatable :: list(:) list = [list, new_elem(5)] so it's using the variable 'list' unallocated. The original testcase in the PR had an extra zero-sized allocate statement before the assignment. I think it's the only missing bit.
[PATCH] libgfortran: add fallback for trigonometric pi-based functions
Hi all, This patch introduces a fallback implementation for trigonometric pi-based functions. This implementation supports float, double, and long double data types. I've revised the test cases for r4 and r8 types, and all tests passed successfully on the aarch64-linux platform. If this looks good, I will address f128/r16 in a subsequent patch. (A related patch concerning libquadmath is currently under review.) Thanks, YuaoFrom f8f2031e5e4bcd03e7382342907334cd33dd2ec6 Mon Sep 17 00:00:00 2001 From: Yuao Ma Date: Sat, 5 Jul 2025 17:06:18 +0800 Subject: [PATCH] libgfortran: add fallback for trigonometric pi-based functions This patch introduces a fallback implementation for pi-based trigonometric functions, ensuring broader compatibility and robustness. The new implementation supports float, double, and long double data types. Accordingly, the test cases for r4 and r8 have been revised to reflect these changes. libgfortran/ChangeLog: * Makefile.am: Add c23_functions.c to Makefile. * Makefile.in: Regenerate. * config.h.in: Regenerate. * configure: Regenerate. * configure.ac: Check if trig-pi functions exist. * gfortran.map: Add a section for c23 functions. * libgfortran.h: Include c23 proto file. * c23_protos.h: Add c23 proto file and trig-pi funcs. * intrinsics/c23_functions.c: Add trig-pi fallback impls. gcc/testsuite/ChangeLog: * gfortran.dg/dec_math_5.f90: Revise test to use fallback. Signed-off-by: Yuao Ma Co-authored-by: Steven G. Kargl --- gcc/testsuite/gfortran.dg/dec_math_5.f90 | 36 +- libgfortran/Makefile.am |4 + libgfortran/Makefile.in |8 + libgfortran/c23_protos.h | 133 +++ libgfortran/config.h.in | 63 ++ libgfortran/configure| 1020 +- libgfortran/configure.ac | 23 + libgfortran/gfortran.map | 25 + libgfortran/intrinsics/c23_functions.c | 308 +++ libgfortran/libgfortran.h|1 + 10 files changed, 1598 insertions(+), 23 deletions(-) create mode 100644 libgfortran/c23_protos.h create mode 100644 libgfortran/intrinsics/c23_functions.c diff --git a/gcc/testsuite/gfortran.dg/dec_math_5.f90 b/gcc/testsuite/gfortran.dg/dec_math_5.f90 index a7ff3275236..7bcf07fce67 100644 --- a/gcc/testsuite/gfortran.dg/dec_math_5.f90 +++ b/gcc/testsuite/gfortran.dg/dec_math_5.f90 @@ -102,26 +102,26 @@ program p if (abs(c1 - 0.5) > e3) stop 39 if (abs(d1 - 0.5) > e4) stop 40 - a1 = acospi(0.5) - b1 = acospi(-0.5) + a1 = 0.5; a1 = acospi(a1) + b1 = -0.5; b1 = acospi(b1) c1 = acospi(0.5) d1 = acospi(-0.5) - if (abs(a1 - 1.0 / 3) > e1) stop 41 - if (abs(b1 - 2.0 / 3) > e2) stop 42 + if (abs(a1 - 1._4 / 3) > e1) stop 41 + if (abs(b1 - 2._8 / 3) > e2) stop 42 if (abs(c1 - 1.0 / 3) > e3) stop 43 if (abs(d1 - 2.0 / 3) > e4) stop 44 - a1 = asinpi(0.5) - b1 = asinpi(-0.5) + a1 = 0.5; a1 = asinpi(a1) + b1 = -0.5; b1 = asinpi(b1) c1 = asinpi(0.5) d1 = asinpi(-0.5) - if (abs(a1 - 1.0 / 6) > e1) stop 45 - if (abs(b1 + 1.0 / 6) > e2) stop 46 + if (abs(a1 - 1._4 / 6) > e1) stop 45 + if (abs(b1 + 1._8 / 6) > e2) stop 46 if (abs(c1 - 1.0 / 6) > e3) stop 47 if (abs(d1 + 1.0 / 6) > e4) stop 48 - a1 = atanpi(1.0) - b1 = atanpi(-1.0) + a1 = 1.0; a1 = atanpi(a1) + b1 = -1.0; b1 = atanpi(b1) c1 = atanpi(1.0) d1 = atanpi(-1.0) if (abs(a1 - 0.25) > e1) stop 49 @@ -129,8 +129,8 @@ program p if (abs(c1 - 0.25) > e3) stop 51 if (abs(d1 + 0.25) > e4) stop 52 - a1 = atan2pi(1.0, 1.0) - b1 = atan2pi(1.0, 1.0) + a1 = 1.0; a1 = atan2pi(a1, a1) + b1 = 1.0; b1 = atan2pi(b1, b1) c1 = atan2pi(1.0, 1.0) d1 = atan2pi(1.0, 1.0) if (abs(a1 - 0.25) > e1) stop 53 @@ -138,8 +138,8 @@ program p if (abs(c1 - 0.25) > e3) stop 55 if (abs(d1 - 0.25) > e4) stop 56 - a1 = cospi(1._4 / 3) - b1 = cospi(-1._8 / 3) + a1 = 1._4 / 3; a1 = cospi(a1) + b1 = -1._8 / 3; b1 = cospi(b1) c1 = cospi(4._ep / 3) d1 = cospi(-4._16 / 3) if (abs(a1 - 0.5) > e1) stop 57 @@ -147,8 +147,8 @@ program p if (abs(c1 + 0.5) > e3) stop 59 if (abs(d1 + 0.5) > e4) stop 60 - a1 = sinpi(1._4 / 6) - b1 = sinpi(-1._8 / 6) + a1 = 1._4 / 6; a1 = sinpi(a1) + b1 = -1._8 / 6; b1 = sinpi(b1) c1 = sinpi(5._ep / 6) d1 = sinpi(-7._16 / 6) if (abs(a1 - 0.5) > e1) stop 61 @@ -156,8 +156,8 @@ program p if (abs(c1 - 0.5) > e3) stop 63 if (abs(d1 - 0.5) > e4) stop 64 - a1 = tanpi(0.25) - b1 = tanpi(-0.25) + a1 = 0.25; a1 = tanpi(a1) + b1 = -0.25; b1 = tanpi(b1) c1 = tanpi(1.25) d1 = tanpi(-1.25) if (abs(a1 - 1.0) > e1) stop 65 diff --git a/libgfortran/Makefile.am b/libgfortran/Makefile.am index 4f3b3033224..82cf51b6bcc 100644 --- a/libgfortran/Makefile.am +++ b/libgfortran/Makefile.am @@ -165,6 +165,7 @@ if !LIBGFOR_MINIMAL gfor_helper_src+= \ intrinsics/access.c \ +intrinsics/c
Re: [Fortran, Patch, PR120843, v3] Fix reject valid, because of inconformable coranks
On Wed, Jul 2, 2025 at 09:32 Jerry D wrote: > On 7/2/25 9:02 AM, Steve Kargl wrote: > > On Wed, Jul 02, 2025 at 04:36:38AM -0700, Damian Rouson wrote: > >> git branch > >> gir checkout > >> git add > >> git commit > >> git rebase > >> git push > >> > > The above IS the process everyone uses on gcc. The email is only a "pull > request" for reviewing the patches. It works quite well. Once it is > reviewed/approved, the requester does the last step which is the push. > > For this particular patch, 'save as' the file, "patch -p1 > This is great as long as one does it often enough to remember the command syntax, knows which directory to set as one’s present working directory, and the patch applies cleanly. Too often, these conditions aren’t true for me so I don’t bother trying anymore. However, if I knew that I could just pull a branch, I’d be a more frequent tester, which would be especially nice in the cases when the patch fixes a bug report that I submitted. Instead, because of the current process, I only test sometime after the patch has been approved and pushed a branch, which is pretty late in the game — especially if getting past one bug simply gets me far enough to expose another bug. These two options aren’t mutually exclusive: if the person emailing a patch can also push their local branch to any public remote — presumably one under their GitHub account — then I’d pull the branch while others apply the patch. D > > There are others on the gcc team evaluating a platform, not GitHub, which > will > likely be integrated to generate an email review/approve request. It is > unlikely > the email will go away. I would expect multiple means of doing a "pull > request". > > > I don't use git other than 'git clone', 'git reset --hard', > > and 'git diff'. If gfortran development goes this route, > > I am done. > > > > -- > > steve > > Cheers, > > Jerry >
Re: [PATCH] libgfortran: add fallback for trigonometric pi-based functions
On Sat, Jul 05, 2025 at 05:20:02PM +0800, Yuao Ma wrote: > > diff --git a/libgfortran/configure b/libgfortran/configure > index 9898a94a372..971f1e9df5e 100755 > --- a/libgfortran/configure > +++ b/libgfortran/configure > @@ -16413,7 +16413,7 @@ else > We can't simply define LARGE_OFF_T to be 9223372036854775807, > since some C++ compilers masquerading as C compilers > incorrectly reject 9223372036854775807. */ > -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62)) > +#define LARGE_OFF_T off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << > 31)) What is the purpose of this change? -- Steve
Re: [Patch, fortran] PR106135 - Implement F2018 IMPORT statements
Paul,
Either resolve.cc has sufficiently evolved since you
submitted your patch or the patch is somehow mangled.
When I apply it to my tree for resolve.cc, I see
Hunk #1 succeeded at 3919.
Hunk #2 succeeded at 4223.
Hunk #3 succeeded at 7940 (offset -28 lines).
Hunk #4 succeeded at 8068 (offset -28 lines).
Hunk #5 succeeded at 10752 (offset 17 lines).
Hunk #6 succeeded at 11299 with fuzz 2 (offset 276 lines).
Hmm... The next patch looks like a unified diff to me...
Hunk #5 puts
+ gfc_code *old_code = code;
into resolve_select_type().
while Hunk #6 puts this piece of code in resolve_select_rank()
+ /* Check the symbol itself. */
+
+ if (gfc_current_ns->import_state != IMPORT_NOT_SET
+ && (c->ts.type == BT_DERIVED || c->ts.type == BT_CLASS))
+ {
+ st = gfc_find_symtree (gfc_current_ns->sym_root,
+ c->ts.u.derived->name);
+ if (!check_sym_import_status (c->ts.u.derived, st, NULL, old_code,
+ gfc_current_ns))
+ error++;
+ }
Based on the error++ line, I've moved the code up into
resolve_select_type() where I believe it belongs.
--
steve
On Mon, Jun 23, 2025 at 05:43:31PM +0100, Paul Richard Thomas wrote:
> Hello All,
>
> I was mulling over the F2018 status of gfortran, when I came across the
> additions to the IMPORT statement. This seemed like such a useful addition
> to fortran that I set about an implementation; thinking that this would be
> low hanging fruit. Parsing and checking the constraints C897-8100 turned
> out to be straightforward. C8101 was already implemented for F2008 IMPORT.
> C8102 required a lot more work! (Please see the patch for the constraints.)
>
> Steve K got in touch, when he found out that we had been working in
> parallel on the new IMPORT features. Thus encouraged by our exchanges, I
> ground on until the patch reached its present state. I think that the
> ChangeLog is clear enough, even if the patch came out a bit long winded.
>
> Of the existing IMPORT tests, only import3.f90 needed modification by
> setting -std=f2008 because of the change in the wording of the error
> messages. The new test, import12.f90, is complete IMHO but I am open to
> suggestions for additions. I cannot return to working on this until the
> second week of July so you have plenty of time to test and comment.
>
> Regtests fine with x86_64 on FC42. OK for mainline?
>
> Paul
Re: [PATCH] libgfortran: add fallback for trigonometric pi-based functions
On 7/6/2025 12:49 PM, Steve Kargl wrote: On Sun, Jul 06, 2025 at 08:43:06AM +0800, Yuao Ma wrote: Hi Steve, On 7/6/2025 12:25 AM, Steve Kargl wrote: On Sat, Jul 05, 2025 at 05:20:02PM +0800, Yuao Ma wrote: diff --git a/libgfortran/configure b/libgfortran/configure index 9898a94a372..971f1e9df5e 100755 --- a/libgfortran/configure +++ b/libgfortran/configure @@ -16413,7 +16413,7 @@ else We can't simply define LARGE_OFF_T to be 9223372036854775807, since some C++ compilers masquerading as C compilers incorrectly reject 9223372036854775807. */ -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62)) +#define LARGE_OFF_T off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31)) What is the purpose of this change? Since I don't have root/sudo permissions on my devbox, I manually downloaded and compiled the autoconf 2.69 tarball. This means there might be some minor discrepancies compared to the version shipped with OS distributions. I suspect the issue could be related to platforms where `off_t` is 32-bit, causing a left shift of 62 to result in undefined behavior. The commit at https://cgit.git.savannah.gnu.org/cgit/autoconf.git/commit/?id=a1d8293f3bfa2516f9a0424e3a6e63c2f8e93c6e seems to support my theory. This patch is not okay to commit with this change. Changing LARGE_OFF_T has nothing to do with implementing the half-cycle trig functions. Would it be possible to regenerate the configure file in a separate patch first, before we address the trig-pi patch? I believe this regeneration is a bug fix originating from autoconf 2.69, and it would be beneficial for GCC to incorporate this modification. Beyond libgfortran, libcpp and libiberty are also affected by this issue. This is indeed the direct output from my autoconf 2.69, and manually reverting parts of the generated file seems odd. Additionally, besides the LARGE_OFF_T issue, are there any other issues this patch needs to address? Yuao
Re: [PATCH] libgfortran: add fallback for trigonometric pi-based functions
Hi Steve, On 7/6/2025 12:25 AM, Steve Kargl wrote: On Sat, Jul 05, 2025 at 05:20:02PM +0800, Yuao Ma wrote: diff --git a/libgfortran/configure b/libgfortran/configure index 9898a94a372..971f1e9df5e 100755 --- a/libgfortran/configure +++ b/libgfortran/configure @@ -16413,7 +16413,7 @@ else We can't simply define LARGE_OFF_T to be 9223372036854775807, since some C++ compilers masquerading as C compilers incorrectly reject 9223372036854775807. */ -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62)) +#define LARGE_OFF_T off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31)) What is the purpose of this change? Since I don't have root/sudo permissions on my devbox, I manually downloaded and compiled the autoconf 2.69 tarball. This means there might be some minor discrepancies compared to the version shipped with OS distributions. I suspect the issue could be related to platforms where `off_t` is 32-bit, causing a left shift of 62 to result in undefined behavior. The commit at https://cgit.git.savannah.gnu.org/cgit/autoconf.git/commit/?id=a1d8293f3bfa2516f9a0424e3a6e63c2f8e93c6e seems to support my theory. Thanks, Yuao
Re: [PATCH] libgfortran: add fallback for trigonometric pi-based functions
On Sun, Jul 06, 2025 at 08:43:06AM +0800, Yuao Ma wrote: > Hi Steve, > > On 7/6/2025 12:25 AM, Steve Kargl wrote: > > On Sat, Jul 05, 2025 at 05:20:02PM +0800, Yuao Ma wrote: > > > diff --git a/libgfortran/configure b/libgfortran/configure > > > index 9898a94a372..971f1e9df5e 100755 > > > --- a/libgfortran/configure > > > +++ b/libgfortran/configure > > > @@ -16413,7 +16413,7 @@ else > > > We can't simply define LARGE_OFF_T to be 9223372036854775807, > > > since some C++ compilers masquerading as C compilers > > > incorrectly reject 9223372036854775807. */ > > > -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62)) > > > +#define LARGE_OFF_T off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) > > > << 31)) > > > > What is the purpose of this change? > > > > Since I don't have root/sudo permissions on my devbox, I manually downloaded > and compiled the autoconf 2.69 tarball. This means there might be some minor > discrepancies compared to the version shipped with OS distributions. > > I suspect the issue could be related to platforms where `off_t` is 32-bit, > causing a left shift of 62 to result in undefined behavior. The commit at > https://cgit.git.savannah.gnu.org/cgit/autoconf.git/commit/?id=a1d8293f3bfa2516f9a0424e3a6e63c2f8e93c6e > seems to support my theory. > This patch is not okay to commit with this change. Changing LARGE_OFF_T has nothing to do with implementing the half-cycle trig functions. -- Steve
