[libfortran] Fix PR 123446, broken bootstrap
Commit Message
Hello world,
the attached patch hopefully fixes the bootstrap problem on
systems without threading support. I have tested it as far
as I could, but I could not find any way to test it. It looks
OK to me, but...
I also tested it on a system where there is no pthreads in
libc, that should hopefully also be fixed.
Anybody up for testing on one of the systems that I do not have
access to? OK for trunk?
Best regards
Thomas
PR libfortran/123446
PR libfortran/119136
libgfortran/ChangeLog:
* io/async.h: DEBUG_ASYNC needs gtreads support.
(LOCK_UNIT): Only lock when there is pthreads support and it is active.
Otherwise, just set unit->self to 1.
(UNLOCK_UNIT): Only unlock when there is pthreads support and it is active.
Otherwise, just set unit->self to 0.
(TRYLOCK_UNIT): Only try locking when thee is pthreads support and it is
active. Otherwise, return unit->self.
(OWN_THREAD_ID): New macro.
* io/io.h: gfc_unit's self is an int when there is no gthreads support.
* io/unit.c (check_for_recursive): Check for equality of unit which
locked to OWN_THREAD_ID.
Comments
Hello world,
I just realized that it is possible to run a check by #undef - ing
__GTHREADS_CXX0X in async.h. Doing so promptly found another
syntax error, which this version of the patch fixes.
As this should definitely restore bootstrap, I plan to commit
this tonight unless there are objections. Further fallout can be
handled later.
ChangeLog identical to the one below.
Best regards
Thomas
> the attached patch hopefully fixes the bootstrap problem on
> systems without threading support. I have tested it as far
> as I could, but I could not find any way to test it. It looks
> OK to me, but...
>
> I also tested it on a system where there is no pthreads in
> libc, that should hopefully also be fixed.
>
> Anybody up for testing on one of the systems that I do not have
> access to? OK for trunk?
>
> Best regards
>
> Thomas
>
> PR libfortran/123446
> PR libfortran/119136
>
> libgfortran/ChangeLog:
>
> * io/async.h: DEBUG_ASYNC needs gtreads support.
> (LOCK_UNIT): Only lock when there is pthreads support and it is
> active.
> Otherwise, just set unit->self to 1.
> (UNLOCK_UNIT): Only unlock when there is pthreads support and it is
> active.
> Otherwise, just set unit->self to 0.
> (TRYLOCK_UNIT): Only try locking when thee is pthreads support and
> it is
> active. Otherwise, return unit->self.
> (OWN_THREAD_ID): New macro.
> * io/io.h: gfc_unit's self is an int when there is no gthreads
> support.
> * io/unit.c (check_for_recursive): Check for equality of unit which
> locked to OWN_THREAD_ID.
Edit: I just saw your updated patch. Thanks, will try it.
The following still has merit, though:
> Date: Wed, 7 Jan 2026 23:16:20 +0100
> From: Thomas Koenig <tkoenig@netcologne.de>
> Hello world,
>
> the attached patch hopefully fixes the bootstrap problem on
> systems without threading support. I have tested it as far
> as I could, but I could not find any way to test it. It looks
> OK to me, but...
>
> I also tested it on a system where there is no pthreads in
> libc, that should hopefully also be fixed.
>
> Anybody up for testing on one of the systems that I do not have
> access to?
You always have access to a GNU simulator toolchain. The
page "https://gcc.gnu.org/simtest-howto.html" is...a bit
dated... (yours truly takes the most blame), but you can
easily derive the information you need from it. (Most
importantly, I'd suggest to build and install binutils+sim
*separately*, not in a "combined tree". Also,
cris-elf+cris-sim isn't mentioned there, probably because
the dejagnu support wasn't in the recommended version - I
think it is now.)
Having said that...
> OK for trunk?
>
> Best regards
>
> Thomas
>
> PR libfortran/123446
> PR libfortran/119136
>
> libgfortran/ChangeLog:
>
> * io/async.h: DEBUG_ASYNC needs gtreads support.
> (LOCK_UNIT): Only lock when there is pthreads support and it is active.
> Otherwise, just set unit->self to 1.
> (UNLOCK_UNIT): Only unlock when there is pthreads support and it is active.
> Otherwise, just set unit->self to 0.
> (TRYLOCK_UNIT): Only try locking when thee is pthreads support and it is
> active. Otherwise, return unit->self.
> (OWN_THREAD_ID): New macro.
> * io/io.h: gfc_unit's self is an int when there is no gthreads support.
> * io/unit.c (check_for_recursive): Check for equality of unit which
> locked to OWN_THREAD_ID.
...I tested your patch, but unfortunately the status at
r16-6568-gbba999a7f330 is still broken for cris-elf with
that patch. Maybe the cause is a subsequent patch as I
don't see aio_do mentioned in your patch. Anyway, the
breakage at r16-6568-gbba999a7f330:
----->
libtool: compile: /obj/./gcc/xgcc -B/obj/./gcc/ -nostdinc -B/obj/cris-elf/newlib/ -isystem /obj/cris-elf/newlib/targ-include -isystem /src/newlib/libc/include -B/obj/cris-elf/libgloss/cris -L/obj/cris-elf/libgloss/libnosys -L/src/libgloss/cris -B/prefix/cris-elf/bin/ -B/prefix/cris-elf/lib/ -isystem /prefix/cris-elf/include -isystem /prefix/cris-elf/sys-include -DHAVE_CONFIG_H -I. -I/src/libgfortran -iquote/src/libgfortran/io -I/src/libgfortran/../gcc -I/src/libgfortran/../gcc/config -I../../.././gcc -I/src/libgfortran/../libgcc -I../../libgcc -I/src/libgfortran/../libbacktrace -I../../libbacktrace -I../libbacktrace -std=gnu11 -Wall -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -Wextra -Wwrite-strings -Werror=implicit-function-declaration -Werror=vla -fcx-fortran-rules -ffunction-sections -fdata-sections -g -O2 -march=v8 -mbest-lib-options -MT generated/any_l1.lo -MD -MP -MF generated/.deps/any_l1.Tpo -c /src/libgfortran/generated/any_l1.c -o generated/any_l1.o
In file included from /src/libgfortran/runtime/error.c:28:
/src/libgfortran/io/async.h:443:5: error: expected identifier or '(' before '}' token
443 | })
| ^
/src/libgfortran/io/async.h:443:6: error: expected identifier or '(' before ')' token
443 | })
| ^
/src/libgfortran/io/async.h:644:67: warning: 'enum aio_do' declared inside parameter list will not be visible outside of this definition or declaration
644 | void enqueue_transfer (async_unit * au, transfer_args * arg, enum aio_do);
| ^~~~~~
/src/libgfortran/io/async.h:647:39: warning: 'enum aio_do' declared inside parameter list will not be visible outside of this definition or declaration
647 | void enqueue_done (async_unit *, enum aio_do type);
| ^~~~~~
/src/libgfortran/io/async.h:650:41: warning: 'enum aio_do' declared inside parameter list will not be visible outside of this definition or declaration
650 | int enqueue_done_id (async_unit *, enum aio_do type);
| ^~~~~~
make[6]: *** [Makefile:4537: runtime/error.lo] Error 1
-----<
I'll test at r16-6515-ge32c3fb4311bca too.
brgds, H-P
> Date: Thu, 8 Jan 2026 08:06:55 +0100
> From: Thomas Koenig <tkoenig@netcologne.de>
>
>
> I just realized that it is possible to run a check by #undef - ing
> __GTHREADS_CXX0X in async.h. Doing so promptly found another
> syntax error, which this version of the patch fixes.
But to what version should that patch be applied?
It has near the top:
+++ b/gcc/fortran/libgfortran.h
@@ -143,6 +143,7 @@ typedef enum
LIBERROR_INQUIRE_INTERNAL_UNIT, /* Must be different from STAT_STOPPED_IMAGE. */
LIBERROR_BAD_WAIT_ID,
LIBERROR_NO_MEMORY,
+ LIBERROR_RECURSIVE_IO,
LIBERROR_LAST /* Not a real error, the last error # + 1. */
}
libgfortran_error_codes;
But that's already in r16-6515-ge32c3fb4311bca, so what do I
do with your newer patch? Apply to r16-6514?
brgds, H-P
> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Thu, 8 Jan 2026 08:19:15 +0100
> But that's already in r16-6515-ge32c3fb4311bca, so what do I
> do with your newer patch? Apply to r16-6514?
> From: Thomas Koenig <tkoenig@netcologne.de>
[the "p3a.diff" patch]
That appears to do it! With your "p3a.diff" patch applied
to r16-6514 (i.e. in place of the faulty r16-6515), build is
restored for cris-elf. Test-results are not yet ready.
brgds, H-P
> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Thu, 8 Jan 2026 09:01:56 +0100
> That appears to do it! With your "p3a.diff" patch applied
> to r16-6514 (i.e. in place of the faulty r16-6515), build is
> restored for cris-elf. Test-results are not yet ready.
Test results are clean at r16-6514 + p3a.diff (i.e. no
regressions compared to r16-6514). Thanks again for the
fix, I hope it gets committed soon.
brgds, H-P
Am 08.01.26 um 16:56 schrieb Hans-Peter Nilsson:
>> From: Hans-Peter Nilsson <hp@axis.com>
>> Date: Thu, 8 Jan 2026 09:01:56 +0100
>
>> That appears to do it! With your "p3a.diff" patch applied
>> to r16-6514 (i.e. in place of the faulty r16-6515), build is
>> restored for cris-elf. Test-results are not yet ready.
>
> Test results are clean at r16-6514 + p3a.diff (i.e. no
> regressions compared to r16-6514). Thanks again for the
> fix, I hope it gets committed soon.
Committed as r16-6586-gd5964a270de33349ff137b1835046e65ec81fa1e
(this may be pushing obvious and simple for a bit, but I did want
to get this out of the way as fast as possible).
Sorry for breaking bootstrap, and thanks a lot for the help!
Best regards
Thomas
On Thu, Jan 08, 2026 at 09:42:53PM +0100, Thomas Koenig wrote:
> Am 08.01.26 um 16:56 schrieb Hans-Peter Nilsson:
> > > From: Hans-Peter Nilsson <hp@axis.com>
> > > Date: Thu, 8 Jan 2026 09:01:56 +0100
> >
> > > That appears to do it! With your "p3a.diff" patch applied
> > > to r16-6514 (i.e. in place of the faulty r16-6515), build is
> > > restored for cris-elf. Test-results are not yet ready.
> >
> > Test results are clean at r16-6514 + p3a.diff (i.e. no
> > regressions compared to r16-6514). Thanks again for the
> > fix, I hope it gets committed soon.
>
> Committed as r16-6586-gd5964a270de33349ff137b1835046e65ec81fa1e
> (this may be pushing obvious and simple for a bit, but I did want
> to get this out of the way as fast as possible).
>
> Sorry for breaking bootstrap, and thanks a lot for the help!
>
Unfortunately, it is still broken. (unit)->self has a type
of __gthread_t, which appears to be mapped to pthread_t
on FreeBSD, which is a struct pthread *.
In file included from ../../../gcc/libgfortran/io/unit.c:30:
../../../gcc/libgfortran/io/unit.c: In function 'insert_unit':
../../../gcc/libgfortran/io/async.h:396:18: error: assignment to '__gthread_t' {aka 'struct pthread *'} from 'int' makes pointer from integer without a cast [-Wint-conversion]
396 | (unit)->self = 1; \
| ^
../../../gcc/libgfortran/io/unit.c:250:3: note: in expansion of macro 'LOCK_UNI'
250 | LOCK_UNIT (u);
| ^~~~~~~~~
../../../gcc/libgfortran/io/unit.c: In function 'check_for_recursive':
../../../gcc/libgfortran/io/async.h:436:13: error: assignment to 'int' from '__gthread_t' {aka 'struct pthread *'} makes integer from pointer without a cast [-Wint-conversion]
436 | res = (unit)->self; \
| ^
../../../gcc/libgfortran/io/unit.c:366:19: note: in expansion of macro 'TRYLOCK_UNIT'
366 | if (TRYLOCK_UNIT(p))
| ^~~~~~~~~~~~
../../../gcc/libgfortran/io/async.h:437:22: error: assignment to '__gthread_t' {aka 'struct pthread *'} from 'int' makes pointer from integer without a cast [-Wint-conversion]
437 | (unit)->self = 1; \
| ^
../../../gcc/libgfortran/io/unit.c:366:19: note: in expansion of macro 'TRYLOCK_ UNIT'
366 | if (TRYLOCK_UNIT(p))
| ^~~~~~~~~~~~
../../../gcc/libgfortran/io/async.h:558:66: error: pointer/integer type mismatch in conditional expression [-Wint-conversion]
558 | #define OWN_THREAD_ID (__gthread_active_p () ? __gthread_self () : 1)
| ^
../../../gcc/libgfortran/io/unit.c:373:71: note: in expansion of macro 'OWN_THRE AD_ID'
373 | if (__atomic_load_n (&p->self, __ATOMIC_RELAXED) == OW N_THREAD_ID)
| ^~ ~~~~~~~~~~~
../../../gcc/libgfortran/io/unit.c: In function 'get_gfc_unit':
../../../gcc/libgfortran/io/async.h:436:13: error: assignment to 'int' from '__g thread_t' {aka 'struct pthread *'} makes integer from pointer without a cast [-W int-conversion]
436 | res = (unit)->self; \
| ^
../../../gcc/libgfortran/io/unit.c:450:13: note: in expansion of macro 'TRYLOCK_ UNIT'
450 | if (! TRYLOCK_UNIT (p))
| ^~~~~~~~~~~~
../../../gcc/libgfortran/io/async.h:437:22: error: assignment to '__gthread_t' {aka 'struct pthread *'} from 'int' makes pointer from integer without a cast [-Wint-conversion]
437 | (unit)->self = 1; \
| ^
../../../gcc/libgfortran/io/unit.c:450:13: note: in expansion of macro 'TRYLOCK_UNIT'
450 | if (! TRYLOCK_UNIT (p))
| ^~~~~~~~~~~~
../../../gcc/libgfortran/io/async.h:396:18: error: assignment to '__gthread_t' {aka 'struct pthread *'} from 'int' makes pointer from integer without a cast [-Wint-conversion]
396 | (unit)->self = 1; \
| ^
../../../gcc/libgfortran/io/unit.c:465:7: note: in expansion of macro 'LOCK_UNI'
465 | LOCK_UNIT (p);
| ^~~~~~~~~
On Thu, Jan 08, 2026 at 06:54:03PM -0800, Steve Kargl wrote:
> On Thu, Jan 08, 2026 at 09:42:53PM +0100, Thomas Koenig wrote:
> > Am 08.01.26 um 16:56 schrieb Hans-Peter Nilsson:
> > > > From: Hans-Peter Nilsson <hp@axis.com>
> > > > Date: Thu, 8 Jan 2026 09:01:56 +0100
> > >
> > > > That appears to do it! With your "p3a.diff" patch applied
> > > > to r16-6514 (i.e. in place of the faulty r16-6515), build is
> > > > restored for cris-elf. Test-results are not yet ready.
> > >
> > > Test results are clean at r16-6514 + p3a.diff (i.e. no
> > > regressions compared to r16-6514). Thanks again for the
> > > fix, I hope it gets committed soon.
> >
> > Committed as r16-6586-gd5964a270de33349ff137b1835046e65ec81fa1e
> > (this may be pushing obvious and simple for a bit, but I did want
> > to get this out of the way as fast as possible).
> >
> > Sorry for breaking bootstrap, and thanks a lot for the help!
> >
>
> Unfortunately, it is still broken. (unit)->self has a type
> of __gthread_t, which appears to be mapped to pthread_t
> on FreeBSD, which is a struct pthread *.
>
> In file included from ../../../gcc/libgfortran/io/unit.c:30:
> ../../../gcc/libgfortran/io/unit.c: In function 'insert_unit':
> ../../../gcc/libgfortran/io/async.h:396:18: error: assignment to '__gthread_t' {aka 'struct pthread *'} from 'int' makes pointer from integer without a cast [-Wint-conversion]
> 396 | (unit)->self = 1; \
If I add a cast here and elsewhere, I can get bootstrap to complete.
(unit)->self = (__gthread_t)1;
@@ -42,6 +42,9 @@
#undef DEBUG_ASYNC
#ifdef DEBUG_ASYNC
+#ifndef __GTHREADS_CXX0X
+#error "gthreads support required for DEBUG_ASYNC"
+#endif
/* Define this if you want to use ANSI color escape sequences in your
debugging output. */
@@ -369,25 +372,56 @@
#define DEBUG_LINE(...) __VA_ARGS__
-#else
+#else /* DEBUG_ASYNC */
+
#define DEBUG_PRINTF(...) {}
#define CHECK_LOCK(au, mutex, status) {}
#define NOTE(str, ...) {}
#define DEBUG_LINE(...)
#define T_ERROR(func, ...) func(__VA_ARGS__)
#define LOCK(mutex) INTERN_LOCK (mutex)
-#define LOCK_UNIT(unit) do { \
- if (__gthread_active_p ()) { \
- LOCK (&(unit)->lock); (unit)->self = __gthread_self (); \
- } \
+
+#ifdef __GTHREADS_CXX0X
+
+/* When pthreads are not active, we do not touch the lock for locking /
+ unlocking; the only use for this is checking for recursion. */
+
+#define LOCK_UNIT(unit) do { \
+ if (__gthread_active_p ()) { \
+ LOCK (&(unit)->lock); (unit)->self = __gthread_self (); \
+ } else { \
+ (unit)->self = 1; \
+ } \
+ } while(0)
+#else
+
+#define LOCK_UNIT(unit) do { \
+ (unit)->self = 1; \
} while(0)
+
+#endif
+
#define UNLOCK(mutex) INTERN_UNLOCK (mutex)
+
+#ifdef __GTHREADS_CXX0X
+
#define UNLOCK_UNIT(unit) do { \
if (__gthread_active_p ()) { \
(unit)->self = 0 ; UNLOCK(&(unit)->lock); \
+ } else { \
+ (unit)->self = 0; \
} \
+} while(0)
+#else
+#define UNLOCK_UNIT(unit) do { \
+ (unit)->self = 0; \
} while(0)
+
+#endif
+
#define TRYLOCK(mutex) (__gthread_mutex_trylock (mutex))
+
+#ifdef __GTHREADS_CXX0X
#define TRYLOCK_UNIT(unit) ({ \
int res; \
if (__gthread_active_p ()) { \
@@ -395,10 +429,20 @@
if (!res) \
(unit)->self = __gthread_self (); \
} \
- else \
- res = 0; \
+ else { \
+ res = (unit)->self; \
+ (unit)->self = 1; \
+ } \
res; \
})
+#else
+#define TRYLOCK_UNIT(unit) ({ \
+ int res = (unit)->self; \
+ (unit)->self = 1; \
+ res;
+ })
+#endif
+
#ifdef __GTHREAD_RWLOCK_INIT
#define RDLOCK(rwlock) INTERN_RDLOCK (rwlock)
#define WRLOCK(rwlock) INTERN_WRLOCK (rwlock)
@@ -504,6 +548,15 @@ DEBUG_LINE (extern __gthread_rwlock_t debug_queue_rwlock;)
extern __thread gfc_unit *thread_unit;
#endif
+/* When threading is not active, or there is no thread system, we fake the ID
+ to be 1. */
+
+#ifdef __GTHREADS_CXX0X
+#define OWN_THREAD_ID (__gthread_active_p () ? __gthread_self () : 1)
+#else
+#define OWN_THREAD_ID 1
+#endif
+
enum aio_do {
AIO_INVALID = 0,
AIO_DATA_TRANSFER_INIT,
@@ -583,7 +636,7 @@ bool async_wait_id (st_parameter_common *, async_unit *, int);
internal_proto (async_wait_id);
bool collect_async_errors (st_parameter_common *, async_unit *);
-internal_proto (collect_async_errors);
+internal_proto (collect_async_errors);
void async_close (async_unit *);
internal_proto (async_close);
@@ -730,6 +730,8 @@ typedef struct gfc_unit
GFC_IO_INT size_used;
#ifdef __GTHREADS_CXX0X
__gthread_t self;
+#else
+ int self;
#endif
}
gfc_unit;
@@ -348,6 +348,7 @@ get_gfc_unit_from_unit_root (int n)
/* Recursive I/O is not allowed. Check to see if the UNIT exists and if
so, check if the UNIT is locked already. This check does not apply
to DTIO. */
+
void
check_for_recursive (st_parameter_dt *dtp)
{
@@ -367,9 +368,9 @@ check_for_recursive (st_parameter_dt *dtp)
/* The lock failed. This unit is locked either our own
thread, which is illegal recursive I/O, or somebody by
else, in which case we are doing OpenMP or similar; this
- is harmless and permitted. */
- __gthread_t locker = __atomic_load_n (&p->self, __ATOMIC_RELAXED);
- if (locker == __gthread_self ())
+ is harmless and permitted. When threading is not active, or
+ there is no thread system, we fake the ID to be 1. */
+ if (__atomic_load_n (&p->self, __ATOMIC_RELAXED) == OWN_THREAD_ID)
generate_error (&dtp->common, LIBERROR_RECURSIVE_IO, NULL);
return;
}