* [PATCH v2 0/4] Move libio lock single-thread optimization to generic libc-lock
@ 2022-04-26 19:15 Adhemerval Zanella
2022-04-26 19:15 ` [PATCH v2 1/4] libio: Assume _IO_MTSAFE_IO Adhemerval Zanella
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: Adhemerval Zanella @ 2022-04-26 19:15 UTC (permalink / raw)
To: libc-alpha, Florian Weimer
This patchset removes some old unused code (_IO_lock_inexpensive
and _IO_MTSAFE_IO), consolidate the stdio-lock.h and move the
single-thread lock optimization to generic code instead of
per-function usage.
Adhemerval Zanella (4):
libio: Assume _IO_MTSAFE_IO
Consolidate stdio-lock.h
Move libio lock single-thread optimization to generic libc-lock (BZ
#27842)
Assume _LIBC and libc module for libc-lock.h
Makeconfig | 2 -
debug/Makefile | 43 +++++--------
grp/Makefile | 4 +-
gshadow/Makefile | 4 +-
htl/Makefile | 2 -
hurd/vpprintf.c | 4 --
include/stdio.h | 2 +-
libio/Makefile | 2 -
libio/Versions | 3 -
libio/clearerr.c | 4 --
libio/feof.c | 7 ---
libio/ferror.c | 6 --
libio/fileno.c | 2 +-
libio/fputc.c | 7 ---
libio/genops.c | 66 -------------------
libio/getc.c | 8 ---
libio/getchar.c | 7 ---
libio/iofdopen.c | 4 --
libio/iofflush.c | 7 ---
libio/iofgets.c | 7 ---
libio/iofopen.c | 4 --
libio/iofopncook.c | 6 --
libio/iofputs.c | 7 ---
libio/iofread.c | 6 --
libio/iofwrite.c | 6 +-
libio/iopopen.c | 14 -----
libio/ioungetc.c | 2 -
libio/iovdprintf.c | 2 -
libio/iovsprintf.c | 2 -
libio/libio.h | 30 +++------
libio/libioP.h | 30 ++-------
libio/memstream.c | 7 ---
libio/obprintf.c | 2 -
libio/oldiofdopen.c | 4 --
libio/oldiofopen.c | 4 --
libio/oldiopopen.c | 14 -----
libio/oldstdfiles.c | 6 --
libio/putc.c | 7 ---
libio/putchar.c | 5 --
libio/stdfiles.c | 11 +---
libio/vasprintf.c | 2 -
libio/vsnprintf.c | 2 -
libio/vswprintf.c | 2 -
libio/wgenops.c | 2 -
libio/wmemstream.c | 7 ---
nptl/Makefile | 3 -
nptl/pthread_create.c | 3 -
pwd/Makefile | 1 -
shadow/Makefile | 4 +-
stdio-common/Makefile | 2 -
stdio-common/flockfile.c | 1 -
stdio-common/vfprintf-internal.c | 4 --
stdlib/Makefile | 9 ---
stdlib/strfmon_l.c | 2 -
stdlib/strfrom-skeleton.c | 2 -
sysdeps/generic/stdio-lock.h | 25 +++-----
sysdeps/htl/stdio-lock.h | 57 -----------------
sysdeps/ieee754/float128/Makefile | 4 --
sysdeps/mach/libc-lock.h | 18 ++----
sysdeps/nptl/libc-lock.h | 74 ++++------------------
sysdeps/nptl/stdio-lock.h | 101 ------------------------------
sysdeps/unix/sysv/linux/Makefile | 1 -
wcsmbs/Makefile | 2 -
63 files changed, 65 insertions(+), 623 deletions(-)
delete mode 100644 sysdeps/htl/stdio-lock.h
delete mode 100644 sysdeps/nptl/stdio-lock.h
--
2.34.1
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v2 1/4] libio: Assume _IO_MTSAFE_IO 2022-04-26 19:15 [PATCH v2 0/4] Move libio lock single-thread optimization to generic libc-lock Adhemerval Zanella @ 2022-04-26 19:15 ` Adhemerval Zanella 2022-04-27 12:34 ` Florian Weimer 2022-04-26 19:15 ` [PATCH v2 2/4] Consolidate stdio-lock.h Adhemerval Zanella ` (2 subsequent siblings) 3 siblings, 1 reply; 19+ messages in thread From: Adhemerval Zanella @ 2022-04-26 19:15 UTC (permalink / raw) To: libc-alpha, Florian Weimer It is already set by default on all supported architectures and it is an expectation that stdio works on multi-threaded environments. --- Makeconfig | 2 -- debug/Makefile | 43 ++++++++++++------------------- grp/Makefile | 4 +-- gshadow/Makefile | 4 +-- htl/Makefile | 2 -- hurd/vpprintf.c | 4 --- include/stdio.h | 2 +- libio/Makefile | 2 -- libio/clearerr.c | 4 --- libio/feof.c | 5 ---- libio/ferror.c | 4 --- libio/fileno.c | 2 +- libio/fputc.c | 5 ---- libio/genops.c | 38 --------------------------- libio/getc.c | 6 ----- libio/getchar.c | 7 +---- libio/iofdopen.c | 4 --- libio/iofflush.c | 7 ----- libio/iofgets.c | 7 ----- libio/iofopen.c | 4 --- libio/iofopncook.c | 4 --- libio/iofputs.c | 7 ----- libio/iofread.c | 6 ----- libio/iofwrite.c | 6 +---- libio/iopopen.c | 14 ---------- libio/iovdprintf.c | 2 -- libio/iovsprintf.c | 2 -- libio/libio.h | 26 ++++++------------- libio/libioP.h | 30 +++------------------ libio/memstream.c | 4 --- libio/obprintf.c | 2 -- libio/oldiofdopen.c | 4 --- libio/oldiofopen.c | 4 --- libio/oldiopopen.c | 14 ---------- libio/oldstdfiles.c | 6 ----- libio/putc.c | 5 ---- libio/putchar.c | 5 ---- libio/stdfiles.c | 11 +------- libio/vasprintf.c | 2 -- libio/vsnprintf.c | 2 -- libio/vswprintf.c | 2 -- libio/wgenops.c | 2 -- libio/wmemstream.c | 4 --- nptl/Makefile | 3 --- pwd/Makefile | 1 - shadow/Makefile | 4 +-- stdio-common/Makefile | 2 -- stdio-common/vfprintf-internal.c | 4 --- stdlib/Makefile | 9 ------- stdlib/strfmon_l.c | 2 -- stdlib/strfrom-skeleton.c | 2 -- sysdeps/generic/stdio-lock.h | 4 +-- sysdeps/ieee754/float128/Makefile | 4 --- sysdeps/nptl/libc-lock.h | 2 +- sysdeps/unix/sysv/linux/Makefile | 1 - wcsmbs/Makefile | 2 -- 56 files changed, 43 insertions(+), 317 deletions(-) diff --git a/Makeconfig b/Makeconfig index 0aa5fb0099..14fd013d57 100644 --- a/Makeconfig +++ b/Makeconfig @@ -1377,8 +1377,6 @@ endif # A sysdeps Makeconfig fragment may set libc-reentrant to yes. ifeq (yes,$(libc-reentrant)) defines += -D_LIBC_REENTRANT - -libio-mtsafe = -D_IO_MTSAFE_IO endif # The name to give to a test in test results summaries. diff --git a/debug/Makefile b/debug/Makefile index 96029f32ee..6b7c5a1fcb 100644 --- a/debug/Makefile +++ b/debug/Makefile @@ -68,32 +68,23 @@ elide-routines.o := stack_chk_fail_local CFLAGS-stack_chk_fail.c += $(no-stack-protector) CFLAGS-backtrace.c += -fno-omit-frame-pointer -funwind-tables -CFLAGS-sprintf_chk.c += $(libio-mtsafe) -CFLAGS-snprintf_chk.c += $(libio-mtsafe) -CFLAGS-vsprintf_chk.c += $(libio-mtsafe) -CFLAGS-vsnprintf_chk.c += $(libio-mtsafe) -CFLAGS-asprintf_chk.c += $(libio-mtsafe) -CFLAGS-vasprintf_chk.c += $(libio-mtsafe) -CFLAGS-obprintf_chk.c += $(libio-mtsafe) -CFLAGS-dprintf_chk.c += $(libio-mtsafe) -fexceptions -CFLAGS-vdprintf_chk.c += $(libio-mtsafe) -fexceptions -CFLAGS-printf_chk.c += $(libio-mtsafe) -fexceptions -CFLAGS-fprintf_chk.c += $(libio-mtsafe) -fexceptions -CFLAGS-vprintf_chk.c += $(libio-mtsafe) -fexceptions -CFLAGS-vfprintf_chk.c += $(libio-mtsafe) -fexceptions -CFLAGS-gets_chk.c += $(libio-mtsafe) -fexceptions -CFLAGS-fgets_chk.c += $(libio-mtsafe) -fexceptions -CFLAGS-fgets_u_chk.c += $(libio-mtsafe) -fexceptions -CFLAGS-fread_chk.c += $(libio-mtsafe) -fexceptions -CFLAGS-fread_u_chk.c += $(libio-mtsafe) -fexceptions -CFLAGS-swprintf_chk.c += $(libio-mtsafe) -CFLAGS-vswprintf_chk.c += $(libio-mtsafe) -CFLAGS-wprintf_chk.c += $(libio-mtsafe) -fexceptions -CFLAGS-fwprintf_chk.c += $(libio-mtsafe) -fexceptions -CFLAGS-vwprintf_chk.c += $(libio-mtsafe) -fexceptions -CFLAGS-vfwprintf_chk.c += $(libio-mtsafe) -fexceptions -CFLAGS-fgetws_chk.c += $(libio-mtsafe) -fexceptions -CFLAGS-fgetws_u_chk.c += $(libio-mtsafe) -fexceptions +CFLAGS-dprintf_chk.c += -fexceptions +CFLAGS-vdprintf_chk.c += -fexceptions +CFLAGS-printf_chk.c += -fexceptions +CFLAGS-fprintf_chk.c += -fexceptions +CFLAGS-vprintf_chk.c += -fexceptions +CFLAGS-vfprintf_chk.c += -fexceptions +CFLAGS-gets_chk.c += -fexceptions +CFLAGS-fgets_chk.c += -fexceptions +CFLAGS-fgets_u_chk.c += -fexceptions +CFLAGS-fread_chk.c += -fexceptions +CFLAGS-fread_u_chk.c += -fexceptions +CFLAGS-wprintf_chk.c += -fexceptions +CFLAGS-fwprintf_chk.c += -fexceptions +CFLAGS-vwprintf_chk.c += -fexceptions +CFLAGS-vfwprintf_chk.c += -fexceptions +CFLAGS-fgetws_chk.c += -fexceptions +CFLAGS-fgetws_u_chk.c += -fexceptions CFLAGS-read_chk.c += -fexceptions -fasynchronous-unwind-tables CFLAGS-pread_chk.c += -fexceptions -fasynchronous-unwind-tables CFLAGS-pread64_chk.c += -fexceptions -fasynchronous-unwind-tables diff --git a/grp/Makefile b/grp/Makefile index 48f3914e29..a3dc650cde 100644 --- a/grp/Makefile +++ b/grp/Makefile @@ -52,8 +52,8 @@ CFLAGS-getgrnam_r.c += -fexceptions CFLAGS-getgrent_r.c += -fexceptions CFLAGS-getgrent.c += -fexceptions CFLAGS-fgetgrent.c += -fexceptions -CFLAGS-fgetgrent_r.c += -fexceptions $(libio-mtsafe) -CFLAGS-putgrent.c += -fexceptions $(libio-mtsafe) +CFLAGS-fgetgrent_r.c += -fexceptions +CFLAGS-putgrent.c += -fexceptions CFLAGS-initgroups.c += -fexceptions CFLAGS-getgrgid.c += -fexceptions diff --git a/gshadow/Makefile b/gshadow/Makefile index eff303f538..3f9274ba99 100644 --- a/gshadow/Makefile +++ b/gshadow/Makefile @@ -31,8 +31,8 @@ tests = tst-gshadow tst-putsgent tst-fgetsgent_r CFLAGS-getsgent_r.c += -fexceptions CFLAGS-getsgent.c += -fexceptions CFLAGS-fgetsgent.c += -fexceptions -CFLAGS-fgetsgent_r.c += -fexceptions $(libio-mtsafe) -CFLAGS-putsgent.c += -fexceptions $(libio-mtsafe) +CFLAGS-fgetsgent_r.c += -fexceptions +CFLAGS-putsgent.c += -fexceptions CFLAGS-getsgnam.c += -fexceptions CFLAGS-getsgnam_r.c += -fexceptions diff --git a/htl/Makefile b/htl/Makefile index 0b403e2fca..cdc20141bf 100644 --- a/htl/Makefile +++ b/htl/Makefile @@ -174,8 +174,6 @@ install-lib-ldscripts := libpthread_syms.a include ../Makeconfig -CFLAGS-lockfile.c = -D_IO_MTSAFE_IO - all: # Make this the default target; it will be defined in Rules. subdir_install: $(inst_libdir)/libpthread2.a $(inst_libdir)/libpthread_syms.a diff --git a/hurd/vpprintf.c b/hurd/vpprintf.c index 67450399f5..7dbd4fca85 100644 --- a/hurd/vpprintf.c +++ b/hurd/vpprintf.c @@ -42,13 +42,9 @@ vpprintf (io_t port, const char *format, va_list arg) struct locked_FILE { struct _IO_cookie_file cfile; -#ifdef _IO_MTSAFE_IO _IO_lock_t lock; -#endif } temp_f; -#ifdef _IO_MTSAFE_IO temp_f.cfile.__fp.file._lock = &temp_f.lock; -#endif _IO_cookie_init (&temp_f.cfile, _IO_NO_READS, (void *) port, (cookie_io_functions_t) { write: do_write }); diff --git a/include/stdio.h b/include/stdio.h index 23b7fd288c..e9a7c77c24 100644 --- a/include/stdio.h +++ b/include/stdio.h @@ -1,5 +1,5 @@ #ifndef _STDIO_H -# if !defined _ISOMAC && defined _IO_MTSAFE_IO +# if !defined _ISOMAC # include <stdio-lock.h> # endif diff --git a/libio/Makefile b/libio/Makefile index e97387743f..88b4c20da6 100644 --- a/libio/Makefile +++ b/libio/Makefile @@ -92,8 +92,6 @@ routines += clearerr_u feof_u ferror_u fputc_u getc_u getchar_u \ iofputs_u endif -CPPFLAGS += $(libio-mtsafe) - # Support for exception handling. CFLAGS-fileops.c += -fexceptions CFLAGS-fputc.c += -fexceptions diff --git a/libio/clearerr.c b/libio/clearerr.c index 47d9ceae1a..5aecee096b 100644 --- a/libio/clearerr.c +++ b/libio/clearerr.c @@ -26,7 +26,3 @@ clearerr (FILE *fp) _IO_clearerr (fp); _IO_funlockfile (fp); } - -#ifndef _IO_MTSAFE_IO -weak_alias (clearerr, clearerr_unlocked) -#endif diff --git a/libio/feof.c b/libio/feof.c index d6b95f40e8..7f79b2795e 100644 --- a/libio/feof.c +++ b/libio/feof.c @@ -41,8 +41,3 @@ _IO_feof (FILE *fp) } weak_alias (_IO_feof, feof) - -#ifndef _IO_MTSAFE_IO -#undef feof_unlocked -weak_alias (_IO_feof, feof_unlocked) -#endif diff --git a/libio/ferror.c b/libio/ferror.c index bee2ef8f07..d2489c54d3 100644 --- a/libio/ferror.c +++ b/libio/ferror.c @@ -42,7 +42,3 @@ _IO_ferror (FILE *fp) weak_alias (_IO_ferror, ferror) -#ifndef _IO_MTSAFE_IO -#undef ferror_unlocked -weak_alias (_IO_ferror, ferror_unlocked) -#endif diff --git a/libio/fileno.c b/libio/fileno.c index ec479bedcf..3e5138fa0b 100644 --- a/libio/fileno.c +++ b/libio/fileno.c @@ -46,6 +46,6 @@ libc_hidden_weak (fileno) /* The fileno implementation for libio does not require locking because it only accesses once a single variable and this is already atomic - (at least at thread level). Therefore we don't test _IO_MTSAFE_IO here. */ + (at least at thread level). */ weak_alias (__fileno, fileno_unlocked) diff --git a/libio/fputc.c b/libio/fputc.c index 8143f2c096..d29a9d0aee 100644 --- a/libio/fputc.c +++ b/libio/fputc.c @@ -39,8 +39,3 @@ fputc (int c, FILE *fp) _IO_release_lock (fp); return result; } - -#ifndef _IO_MTSAFE_IO -#undef fputc_unlocked -weak_alias (fputc, fputc_unlocked) -#endif diff --git a/libio/genops.c b/libio/genops.c index 1b629eb695..ccfbcd149d 100644 --- a/libio/genops.c +++ b/libio/genops.c @@ -32,13 +32,10 @@ #include <stdbool.h> #include <sched.h> -#ifdef _IO_MTSAFE_IO static _IO_lock_t list_all_lock = _IO_lock_initializer; -#endif static FILE *run_fp; -#ifdef _IO_MTSAFE_IO static void flush_cleanup (void *not_used) { @@ -46,7 +43,6 @@ flush_cleanup (void *not_used) _IO_funlockfile (run_fp); _IO_lock_unlock (list_all_lock); } -#endif void _IO_un_link (struct _IO_FILE_plus *fp) @@ -54,12 +50,10 @@ _IO_un_link (struct _IO_FILE_plus *fp) if (fp->file._flags & _IO_LINKED) { FILE **f; -#ifdef _IO_MTSAFE_IO _IO_cleanup_region_start_noarg (flush_cleanup); _IO_lock_lock (list_all_lock); run_fp = (FILE *) fp; _IO_flockfile ((FILE *) fp); -#endif if (_IO_list_all == NULL) ; else if (fp == _IO_list_all) @@ -72,12 +66,10 @@ _IO_un_link (struct _IO_FILE_plus *fp) break; } fp->file._flags &= ~_IO_LINKED; -#ifdef _IO_MTSAFE_IO _IO_funlockfile ((FILE *) fp); run_fp = NULL; _IO_lock_unlock (list_all_lock); _IO_cleanup_region_end (0); -#endif } } libc_hidden_def (_IO_un_link) @@ -88,20 +80,16 @@ _IO_link_in (struct _IO_FILE_plus *fp) if ((fp->file._flags & _IO_LINKED) == 0) { fp->file._flags |= _IO_LINKED; -#ifdef _IO_MTSAFE_IO _IO_cleanup_region_start_noarg (flush_cleanup); _IO_lock_lock (list_all_lock); run_fp = (FILE *) fp; _IO_flockfile ((FILE *) fp); -#endif fp->file._chain = (FILE *) _IO_list_all; _IO_list_all = fp; -#ifdef _IO_MTSAFE_IO _IO_funlockfile ((FILE *) fp); run_fp = NULL; _IO_lock_unlock (list_all_lock); _IO_cleanup_region_end (0); -#endif } } libc_hidden_def (_IO_link_in) @@ -551,10 +539,8 @@ _IO_old_init (FILE *fp, int flags) #if _IO_JUMPS_OFFSET fp->_vtable_offset = 0; #endif -#ifdef _IO_MTSAFE_IO if (fp->_lock != NULL) _IO_lock_init (*fp->_lock); -#endif } void @@ -617,10 +603,8 @@ _IO_default_finish (FILE *fp, int dummy) _IO_un_link ((struct _IO_FILE_plus *) fp); -#ifdef _IO_MTSAFE_IO if (fp->_lock != NULL) _IO_lock_fini (*fp->_lock); -#endif } libc_hidden_def (_IO_default_finish) @@ -687,10 +671,8 @@ _IO_flush_all_lockp (int do_lock) int result = 0; FILE *fp; -#ifdef _IO_MTSAFE_IO _IO_cleanup_region_start_noarg (flush_cleanup); _IO_lock_lock (list_all_lock); -#endif for (fp = (FILE *) _IO_list_all; fp != NULL; fp = fp->_chain) { @@ -711,10 +693,8 @@ _IO_flush_all_lockp (int do_lock) run_fp = NULL; } -#ifdef _IO_MTSAFE_IO _IO_lock_unlock (list_all_lock); _IO_cleanup_region_end (0); -#endif return result; } @@ -733,10 +713,8 @@ _IO_flush_all_linebuffered (void) { FILE *fp; -#ifdef _IO_MTSAFE_IO _IO_cleanup_region_start_noarg (flush_cleanup); _IO_lock_lock (list_all_lock); -#endif for (fp = (FILE *) _IO_list_all; fp != NULL; fp = fp->_chain) { @@ -750,10 +728,8 @@ _IO_flush_all_linebuffered (void) run_fp = NULL; } -#ifdef _IO_MTSAFE_IO _IO_lock_unlock (list_all_lock); _IO_cleanup_region_end (0); -#endif } libc_hidden_def (_IO_flush_all_linebuffered) weak_alias (_IO_flush_all_linebuffered, _flushlbf) @@ -782,10 +758,8 @@ _IO_unbuffer_all (void) { FILE *fp; -#ifdef _IO_MTSAFE_IO _IO_cleanup_region_start_noarg (flush_cleanup); _IO_lock_lock (list_all_lock); -#endif for (fp = (FILE *) _IO_list_all; fp; fp = fp->_chain) { @@ -800,7 +774,6 @@ _IO_unbuffer_all (void) /* Iff stream is un-orientated, it wasn't used. */ && (legacy || fp->_mode != 0)) { -#ifdef _IO_MTSAFE_IO int cnt; #define MAXTRIES 2 for (cnt = 0; cnt < MAXTRIES; ++cnt) @@ -810,7 +783,6 @@ _IO_unbuffer_all (void) /* Give the other thread time to finish up its use of the stream. */ __sched_yield (); -#endif if (! legacy && ! dealloc_buffers && !(fp->_flags & _IO_USER_BUF)) { @@ -826,10 +798,8 @@ _IO_unbuffer_all (void) if (! legacy && fp->_mode > 0) _IO_wsetb (fp, NULL, NULL, 0); -#ifdef _IO_MTSAFE_IO if (cnt < MAXTRIES && fp->_lock != NULL) _IO_lock_unlock (*fp->_lock); -#endif } /* Make sure that never again the wide char functions can be @@ -838,10 +808,8 @@ _IO_unbuffer_all (void) fp->_mode = -1; } -#ifdef _IO_MTSAFE_IO _IO_lock_unlock (list_all_lock); _IO_cleanup_region_end (0); -#endif } @@ -1092,27 +1060,21 @@ libc_hidden_def (_IO_iter_file) void _IO_list_lock (void) { -#ifdef _IO_MTSAFE_IO _IO_lock_lock (list_all_lock); -#endif } libc_hidden_def (_IO_list_lock) void _IO_list_unlock (void) { -#ifdef _IO_MTSAFE_IO _IO_lock_unlock (list_all_lock); -#endif } libc_hidden_def (_IO_list_unlock) void _IO_list_resetlock (void) { -#ifdef _IO_MTSAFE_IO _IO_lock_init (list_all_lock); -#endif } libc_hidden_def (_IO_list_resetlock) diff --git a/libio/getc.c b/libio/getc.c index 55a6cc479a..8c79cf702f 100644 --- a/libio/getc.c +++ b/libio/getc.c @@ -46,9 +46,3 @@ _IO_getc (FILE *fp) weak_alias (_IO_getc, getc) weak_alias (_IO_getc, fgetc) - -#ifndef _IO_MTSAFE_IO -#undef getc_unlocked -weak_alias (_IO_getc, getc_unlocked) -weak_alias (_IO_getc, fgetc_unlocked) -#endif diff --git a/libio/getchar.c b/libio/getchar.c index 6ceff06a84..e0f4f471b3 100644 --- a/libio/getchar.c +++ b/libio/getchar.c @@ -39,9 +39,4 @@ getchar (void) result = _IO_getc_unlocked (stdin); _IO_release_lock (stdin); return result; -} - -#ifndef _IO_MTSAFE_IO -#undef getchar_unlocked -weak_alias (getchar, getchar_unlocked) -#endif +} \ No newline at end of file diff --git a/libio/iofdopen.c b/libio/iofdopen.c index 47b602d3c2..b8d082ebc5 100644 --- a/libio/iofdopen.c +++ b/libio/iofdopen.c @@ -37,9 +37,7 @@ _IO_new_fdopen (int fd, const char *mode) struct locked_FILE { struct _IO_FILE_plus fp; -#ifdef _IO_MTSAFE_IO _IO_lock_t lock; -#endif struct _IO_wide_data wd; } *new_f; int i; @@ -122,9 +120,7 @@ _IO_new_fdopen (int fd, const char *mode) new_f = (struct locked_FILE *) malloc (sizeof (struct locked_FILE)); if (new_f == NULL) return NULL; -#ifdef _IO_MTSAFE_IO new_f->fp.file._lock = &new_f->lock; -#endif _IO_no_init (&new_f->fp.file, 0, 0, &new_f->wd, #if _G_HAVE_MMAP (use_mmap && (read_write & _IO_NO_WRITES)) diff --git a/libio/iofflush.c b/libio/iofflush.c index 5b542664a8..6ea046cf48 100644 --- a/libio/iofflush.c +++ b/libio/iofflush.c @@ -46,10 +46,3 @@ libc_hidden_def (_IO_fflush) weak_alias (_IO_fflush, fflush) libc_hidden_weak (fflush) - -#ifndef _IO_MTSAFE_IO -strong_alias (_IO_fflush, __fflush_unlocked) -libc_hidden_def (__fflush_unlocked) -weak_alias (_IO_fflush, fflush_unlocked) -libc_hidden_weak (fflush_unlocked) -#endif diff --git a/libio/iofgets.c b/libio/iofgets.c index 58d30fca96..f19526de46 100644 --- a/libio/iofgets.c +++ b/libio/iofgets.c @@ -66,10 +66,3 @@ _IO_fgets (char *buf, int n, FILE *fp) } weak_alias (_IO_fgets, fgets) - -# ifndef _IO_MTSAFE_IO -strong_alias (_IO_fgets, __fgets_unlocked) -libc_hidden_def (__fgets_unlocked) -weak_alias (_IO_fgets, fgets_unlocked) -libc_hidden_weak (fgets_unlocked) -# endif diff --git a/libio/iofopen.c b/libio/iofopen.c index e7471fa3ae..777f8c9e1c 100644 --- a/libio/iofopen.c +++ b/libio/iofopen.c @@ -58,17 +58,13 @@ __fopen_internal (const char *filename, const char *mode, int is32) struct locked_FILE { struct _IO_FILE_plus fp; -#ifdef _IO_MTSAFE_IO _IO_lock_t lock; -#endif struct _IO_wide_data wd; } *new_f = (struct locked_FILE *) malloc (sizeof (struct locked_FILE)); if (new_f == NULL) return NULL; -#ifdef _IO_MTSAFE_IO new_f->fp.file._lock = &new_f->lock; -#endif _IO_no_init (&new_f->fp.file, 0, 0, &new_f->wd, &_IO_wfile_jumps); _IO_JUMPS (&new_f->fp) = &_IO_file_jumps; _IO_new_file_init_internal (&new_f->fp); diff --git a/libio/iofopncook.c b/libio/iofopncook.c index e108ad2199..16bcc7f4a3 100644 --- a/libio/iofopncook.c +++ b/libio/iofopncook.c @@ -179,9 +179,7 @@ _IO_fopencookie (void *cookie, const char *mode, struct locked_FILE { struct _IO_cookie_file cfile; -#ifdef _IO_MTSAFE_IO _IO_lock_t lock; -#endif } *new_f; switch (*mode++) @@ -205,9 +203,7 @@ _IO_fopencookie (void *cookie, const char *mode, new_f = (struct locked_FILE *) malloc (sizeof (struct locked_FILE)); if (new_f == NULL) return NULL; -#ifdef _IO_MTSAFE_IO new_f->cfile.__fp.file._lock = &new_f->lock; -#endif _IO_cookie_init (&new_f->cfile, read_write, cookie, io_functions); diff --git a/libio/iofputs.c b/libio/iofputs.c index 999e069877..f8e0ee7200 100644 --- a/libio/iofputs.c +++ b/libio/iofputs.c @@ -44,10 +44,3 @@ libc_hidden_def (_IO_fputs) weak_alias (_IO_fputs, fputs) libc_hidden_weak (fputs) - -# ifndef _IO_MTSAFE_IO -strong_alias (_IO_fputs, __fputs_unlocked) -libc_hidden_def (__fputs_unlocked) -weak_alias (_IO_fputs, fputs_unlocked) -libc_hidden_ver (_IO_fputs, fputs_unlocked) -# endif diff --git a/libio/iofread.c b/libio/iofread.c index d358136953..66f69ac93f 100644 --- a/libio/iofread.c +++ b/libio/iofread.c @@ -42,9 +42,3 @@ _IO_fread (void *buf, size_t size, size_t count, FILE *fp) libc_hidden_def (_IO_fread) weak_alias (_IO_fread, fread) - -# ifndef _IO_MTSAFE_IO -strong_alias (_IO_fread, __fread_unlocked) -libc_hidden_def (__fread_unlocked) -weak_alias (_IO_fread, fread_unlocked) -# endif diff --git a/libio/iofwrite.c b/libio/iofwrite.c index f86b5458da..ff741fec8e 100644 --- a/libio/iofwrite.c +++ b/libio/iofwrite.c @@ -24,6 +24,7 @@ This exception applies to code released by its copyright holders in files containing the exception. */ +#include <stdio.h> #include "libioP.h" size_t @@ -49,10 +50,5 @@ _IO_fwrite (const void *buf, size_t size, size_t count, FILE *fp) } libc_hidden_def (_IO_fwrite) -# include <stdio.h> weak_alias (_IO_fwrite, fwrite) libc_hidden_weak (fwrite) -# ifndef _IO_MTSAFE_IO -weak_alias (_IO_fwrite, fwrite_unlocked) -libc_hidden_weak (fwrite_unlocked) -# endif diff --git a/libio/iopopen.c b/libio/iopopen.c index 06778cf110..676c739aba 100644 --- a/libio/iopopen.c +++ b/libio/iopopen.c @@ -49,7 +49,6 @@ static const struct _IO_jump_t _IO_proc_jumps; static struct _IO_proc_file *proc_file_chain; -#ifdef _IO_MTSAFE_IO static _IO_lock_t proc_file_chain_lock = _IO_lock_initializer; static void @@ -57,7 +56,6 @@ unlock (void *not_used) { _IO_lock_unlock (proc_file_chain_lock); } -#endif /* POSIX states popen shall ensure that any streams from previous popen() calls that remain open in the parent process should be closed in the new @@ -189,16 +187,12 @@ _IO_new_proc_open (FILE *fp, const char *command, const char *mode) child_pipe_fd) != 0) goto spawn_failure; -#ifdef _IO_MTSAFE_IO _IO_cleanup_region_start_noarg (unlock); _IO_lock_lock (proc_file_chain_lock); -#endif spawn_ok = spawn_process (&fa, fp, command, do_cloexec, pipe_fds, parent_end, child_end, child_pipe_fd); -#ifdef _IO_MTSAFE_IO _IO_lock_unlock (proc_file_chain_lock); _IO_cleanup_region_end (0); -#endif __posix_spawn_file_actions_destroy (&fa); @@ -221,18 +215,14 @@ _IO_new_popen (const char *command, const char *mode) struct locked_FILE { struct _IO_proc_file fpx; -#ifdef _IO_MTSAFE_IO _IO_lock_t lock; -#endif } *new_f; FILE *fp; new_f = (struct locked_FILE *) malloc (sizeof (struct locked_FILE)); if (new_f == NULL) return NULL; -#ifdef _IO_MTSAFE_IO new_f->fpx.file.file._lock = &new_f->lock; -#endif fp = &new_f->fpx.file.file; _IO_init_internal (fp, 0); _IO_JUMPS (&new_f->fpx.file) = &_IO_proc_jumps; @@ -254,10 +244,8 @@ _IO_new_proc_close (FILE *fp) int status = -1; /* Unlink from proc_file_chain. */ -#ifdef _IO_MTSAFE_IO _IO_cleanup_region_start_noarg (unlock); _IO_lock_lock (proc_file_chain_lock); -#endif for ( ; *ptr != NULL; ptr = &(*ptr)->next) { if (*ptr == (_IO_proc_file *) fp) @@ -267,10 +255,8 @@ _IO_new_proc_close (FILE *fp) break; } } -#ifdef _IO_MTSAFE_IO _IO_lock_unlock (proc_file_chain_lock); _IO_cleanup_region_end (0); -#endif if (status < 0 || __close_nocancel (_IO_fileno(fp)) < 0) return -1; diff --git a/libio/iovdprintf.c b/libio/iovdprintf.c index 1e483868c9..4c5a90e58d 100644 --- a/libio/iovdprintf.c +++ b/libio/iovdprintf.c @@ -35,9 +35,7 @@ __vdprintf_internal (int d, const char *format, va_list arg, struct _IO_wide_data wd; int done; -#ifdef _IO_MTSAFE_IO tmpfil.file._lock = NULL; -#endif _IO_no_init (&tmpfil.file, _IO_USER_LOCK, 0, &wd, &_IO_wfile_jumps); _IO_JUMPS (&tmpfil) = &_IO_file_jumps; _IO_new_file_init_internal (&tmpfil); diff --git a/libio/iovsprintf.c b/libio/iovsprintf.c index 72c67bf27b..cbe59f6fec 100644 --- a/libio/iovsprintf.c +++ b/libio/iovsprintf.c @@ -73,9 +73,7 @@ __vsprintf_internal (char *string, size_t maxlen, _IO_strfile sf; int ret; -#ifdef _IO_MTSAFE_IO sf._sbf._f._lock = NULL; -#endif _IO_no_init (&sf._sbf._f, _IO_USER_LOCK, -1, NULL, NULL); /* When called from fortified sprintf/vsprintf, erase the destination buffer and try to detect overflows. When called from regular diff --git a/libio/libio.h b/libio/libio.h index d0184df878..42ff6143af 100644 --- a/libio/libio.h +++ b/libio/libio.h @@ -36,7 +36,7 @@ #include <stdio.h> -#if defined _IO_MTSAFE_IO && !defined _IO_lock_t_defined +#if !defined _IO_lock_t_defined # error "Someone forgot to include stdio-lock.h" #endif @@ -198,10 +198,13 @@ extern void _IO_flockfile (FILE *) __THROW; extern void _IO_funlockfile (FILE *) __THROW; extern int _IO_ftrylockfile (FILE *) __THROW; -#define _IO_peekc(_fp) _IO_peekc_unlocked (_fp) -#define _IO_flockfile(_fp) /**/ -#define _IO_funlockfile(_fp) ((void) 0) -#define _IO_ftrylockfile(_fp) /**/ +#define _IO_peekc(_fp) _IO_peekc_locked (_fp) +#define _IO_flockfile(_fp) \ + if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_lock_lock (*(_fp)->_lock) +#define _IO_funlockfile(_fp) \ + if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_lock_unlock (*(_fp)->_lock) +#define _IO_ftrylockfile(__fp) \ + if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_lock_trylock (*(_fp)->_lock) #ifndef _IO_cleanup_region_start #define _IO_cleanup_region_start(_fct, _fp) /**/ #endif @@ -272,17 +275,4 @@ libc_hidden_proto (_IO_padn) libc_hidden_proto (_IO_putc) libc_hidden_proto (_IO_sgetn) -#ifdef _IO_MTSAFE_IO -# undef _IO_peekc -# undef _IO_flockfile -# undef _IO_funlockfile -# undef _IO_ftrylockfile - -# define _IO_peekc(_fp) _IO_peekc_locked (_fp) -# define _IO_flockfile(_fp) \ - if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_lock_lock (*(_fp)->_lock) -# define _IO_funlockfile(_fp) \ - if (((_fp)->_flags & _IO_USER_LOCK) == 0) _IO_lock_unlock (*(_fp)->_lock) -#endif /* _IO_MTSAFE_IO */ - #endif /* _LIBIO_H */ diff --git a/libio/libioP.h b/libio/libioP.h index ba4fdbd200..114084b83a 100644 --- a/libio/libioP.h +++ b/libio/libioP.h @@ -803,33 +803,18 @@ extern int __vfwscanf_internal (FILE *fp, const wchar_t *format, va_list argp, extern int _IO_vscanf (const char *, va_list) __THROW; -#ifdef _IO_MTSAFE_IO /* check following! */ -# ifdef _IO_USE_OLD_IO_FILE -# define FILEBUF_LITERAL(CHAIN, FLAGS, FD, WDP) \ +#ifdef _IO_USE_OLD_IO_FILE +# define FILEBUF_LITERAL(CHAIN, FLAGS, FD, WDP) \ { _IO_MAGIC+_IO_LINKED+_IO_IS_FILEBUF+FLAGS, \ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, (FILE *) CHAIN, FD, \ 0, _IO_pos_BAD, 0, 0, { 0 }, &_IO_stdfile_##FD##_lock } -# else -# define FILEBUF_LITERAL(CHAIN, FLAGS, FD, WDP) \ - { _IO_MAGIC+_IO_LINKED+_IO_IS_FILEBUF+FLAGS, \ - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, (FILE *) CHAIN, FD, \ - 0, _IO_pos_BAD, 0, 0, { 0 }, &_IO_stdfile_##FD##_lock, _IO_pos_BAD,\ - NULL, WDP, 0 } -# endif #else -# ifdef _IO_USE_OLD_IO_FILE -# define FILEBUF_LITERAL(CHAIN, FLAGS, FD, WDP) \ +# define FILEBUF_LITERAL(CHAIN, FLAGS, FD, WDP) \ { _IO_MAGIC+_IO_LINKED+_IO_IS_FILEBUF+FLAGS, \ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, (FILE *) CHAIN, FD, \ - 0, _IO_pos_BAD } -# else -# define FILEBUF_LITERAL(CHAIN, FLAGS, FD, WDP) \ - { _IO_MAGIC+_IO_LINKED+_IO_IS_FILEBUF+FLAGS, \ - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, (FILE *) CHAIN, FD, \ - 0, _IO_pos_BAD, 0, 0, { 0 }, 0, _IO_pos_BAD, \ + 0, _IO_pos_BAD, 0, 0, { 0 }, &_IO_stdfile_##FD##_lock, _IO_pos_BAD,\ NULL, WDP, 0 } -# endif #endif #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1) @@ -884,13 +869,6 @@ _IO_acquire_lock_fct (FILE **p) _IO_funlockfile (fp); } -#if !defined _IO_MTSAFE_IO && IS_IN (libc) -# define _IO_acquire_lock(_fp) \ - do { -# define _IO_release_lock(_fp) \ - } while (0) -#endif - /* Collect all vtables in a special section for vtable verification. These symbols cover the extent of this section. */ symbol_set_declare (__libc_IO_vtables) diff --git a/libio/memstream.c b/libio/memstream.c index 1ae8cd9544..7ab61876ba 100644 --- a/libio/memstream.c +++ b/libio/memstream.c @@ -66,9 +66,7 @@ __open_memstream (char **bufloc, size_t *sizeloc) struct locked_FILE { struct _IO_FILE_memstream fp; -#ifdef _IO_MTSAFE_IO _IO_lock_t lock; -#endif struct _IO_wide_data wd; } *new_f; char *buf; @@ -76,9 +74,7 @@ __open_memstream (char **bufloc, size_t *sizeloc) new_f = (struct locked_FILE *) malloc (sizeof (struct locked_FILE)); if (new_f == NULL) return NULL; -#ifdef _IO_MTSAFE_IO new_f->fp._sf._sbf._f._lock = &new_f->lock; -#endif buf = calloc (1, BUFSIZ); if (buf == NULL) diff --git a/libio/obprintf.c b/libio/obprintf.c index c220be9adc..ed0f39d85a 100644 --- a/libio/obprintf.c +++ b/libio/obprintf.c @@ -127,9 +127,7 @@ __obstack_vprintf_internal (struct obstack *obstack, const char *format, int size; int room; -#ifdef _IO_MTSAFE_IO new_f.ofile.file.file._lock = NULL; -#endif _IO_no_init (&new_f.ofile.file.file, _IO_USER_LOCK, -1, NULL, NULL); _IO_JUMPS (&new_f.ofile.file) = &_IO_obstack_jumps; diff --git a/libio/oldiofdopen.c b/libio/oldiofdopen.c index 1ce050f1f4..0b0097e9e2 100644 --- a/libio/oldiofdopen.c +++ b/libio/oldiofdopen.c @@ -41,9 +41,7 @@ _IO_old_fdopen (int fd, const char *mode) struct locked_FILE { struct _IO_FILE_complete_plus fp; -#ifdef _IO_MTSAFE_IO _IO_lock_t lock; -#endif } *new_f; int fd_flags; @@ -96,9 +94,7 @@ _IO_old_fdopen (int fd, const char *mode) new_f = (struct locked_FILE *) malloc (sizeof (struct locked_FILE)); if (new_f == NULL) return NULL; -#ifdef _IO_MTSAFE_IO new_f->fp.file._file._lock = &new_f->lock; -#endif _IO_old_init (&new_f->fp.file._file, 0); _IO_JUMPS_FILE_plus (&new_f->fp) = &_IO_old_file_jumps; _IO_old_file_init_internal ((struct _IO_FILE_plus *) &new_f->fp); diff --git a/libio/oldiofopen.c b/libio/oldiofopen.c index c73911fdd8..f3abd9ee07 100644 --- a/libio/oldiofopen.c +++ b/libio/oldiofopen.c @@ -39,16 +39,12 @@ _IO_old_fopen (const char *filename, const char *mode) struct locked_FILE { struct _IO_FILE_complete_plus fp; -#ifdef _IO_MTSAFE_IO _IO_lock_t lock; -#endif } *new_f = (struct locked_FILE *) malloc (sizeof (struct locked_FILE)); if (new_f == NULL) return NULL; -#ifdef _IO_MTSAFE_IO new_f->fp.file._file._lock = &new_f->lock; -#endif _IO_old_init (&new_f->fp.file._file, 0); _IO_JUMPS_FILE_plus (&new_f->fp) = &_IO_old_file_jumps; _IO_old_file_init_internal ((struct _IO_FILE_plus *) &new_f->fp); diff --git a/libio/oldiopopen.c b/libio/oldiopopen.c index f9149413e8..e24acf752e 100644 --- a/libio/oldiopopen.c +++ b/libio/oldiopopen.c @@ -47,7 +47,6 @@ typedef struct _IO_proc_file _IO_proc_file; static struct _IO_proc_file *old_proc_file_chain; -#ifdef _IO_MTSAFE_IO static _IO_lock_t proc_file_chain_lock = _IO_lock_initializer; static void @@ -55,7 +54,6 @@ unlock (void *not_used) { _IO_lock_unlock (proc_file_chain_lock); } -#endif FILE * attribute_compat_text_section @@ -118,16 +116,12 @@ _IO_old_proc_open (FILE *fp, const char *command, const char *mode) _IO_fileno (fp) = parent_end; /* Link into old_proc_file_chain. */ -#ifdef _IO_MTSAFE_IO _IO_cleanup_region_start_noarg (unlock); _IO_lock_lock (proc_file_chain_lock); -#endif ((_IO_proc_file *) fp)->next = old_proc_file_chain; old_proc_file_chain = (_IO_proc_file *) fp; -#ifdef _IO_MTSAFE_IO _IO_lock_unlock (proc_file_chain_lock); _IO_cleanup_region_end (0); -#endif _IO_mask_flags (fp, read_or_write, _IO_NO_READS|_IO_NO_WRITES); return fp; @@ -140,18 +134,14 @@ _IO_old_popen (const char *command, const char *mode) struct locked_FILE { struct _IO_proc_file fpx; -#ifdef _IO_MTSAFE_IO _IO_lock_t lock; -#endif } *new_f; FILE *fp; new_f = (struct locked_FILE *) malloc (sizeof (struct locked_FILE)); if (new_f == NULL) return NULL; -#ifdef _IO_MTSAFE_IO new_f->fpx.file.file._file._lock = &new_f->lock; -#endif fp = &new_f->fpx.file.file._file; _IO_old_init (fp, 0); _IO_JUMPS_FILE_plus (&new_f->fpx.file) = &_IO_old_proc_jumps; @@ -174,10 +164,8 @@ _IO_old_proc_close (FILE *fp) int status = -1; /* Unlink from old_proc_file_chain. */ -#ifdef _IO_MTSAFE_IO _IO_cleanup_region_start_noarg (unlock); _IO_lock_lock (proc_file_chain_lock); -#endif for ( ; *ptr != NULL; ptr = &(*ptr)->next) { if (*ptr == (_IO_proc_file *) fp) @@ -187,10 +175,8 @@ _IO_old_proc_close (FILE *fp) break; } } -#ifdef _IO_MTSAFE_IO _IO_lock_unlock (proc_file_chain_lock); _IO_cleanup_region_end (0); -#endif if (status < 0 || __close (_IO_fileno(fp)) < 0) return -1; diff --git a/libio/oldstdfiles.c b/libio/oldstdfiles.c index 81a7f4f64d..30d8e39ef9 100644 --- a/libio/oldstdfiles.c +++ b/libio/oldstdfiles.c @@ -33,16 +33,10 @@ #define _IO_USE_OLD_IO_FILE #include "libioP.h" -#ifdef _IO_MTSAFE_IO #define DEF_STDFILE(NAME, FD, CHAIN, FLAGS) \ static _IO_lock_t _IO_stdfile_##FD##_lock = _IO_lock_initializer; \ struct _IO_FILE_plus NAME \ = {FILEBUF_LITERAL(CHAIN, FLAGS, FD, NULL), &_IO_old_file_jumps}; -#else -#define DEF_STDFILE(NAME, FD, CHAIN, FLAGS) \ - struct _IO_FILE_plus NAME \ - = {FILEBUF_LITERAL(CHAIN, FLAGS, FD, NULL), &_IO_old_file_jumps}; -#endif DEF_STDFILE(_IO_stdin_, 0, 0, _IO_NO_WRITES); DEF_STDFILE(_IO_stdout_, 1, &_IO_stdin_, _IO_NO_READS); diff --git a/libio/putc.c b/libio/putc.c index 7036c5b6bb..44e30649c9 100644 --- a/libio/putc.c +++ b/libio/putc.c @@ -37,8 +37,3 @@ libc_hidden_def (_IO_putc) #undef putc weak_alias (_IO_putc, putc) - -#ifndef _IO_MTSAFE_IO -#undef putc_unlocked -weak_alias (_IO_putc, putc_unlocked) -#endif diff --git a/libio/putchar.c b/libio/putchar.c index 7eff3d5f94..9aff59c090 100644 --- a/libio/putchar.c +++ b/libio/putchar.c @@ -29,8 +29,3 @@ putchar (int c) _IO_release_lock (stdout); return result; } - -#if defined weak_alias && !defined _IO_MTSAFE_IO -#undef putchar_unlocked -weak_alias (putchar, putchar_unlocked) -#endif diff --git a/libio/stdfiles.c b/libio/stdfiles.c index 31cf72dfd0..b5c1dc4208 100644 --- a/libio/stdfiles.c +++ b/libio/stdfiles.c @@ -32,22 +32,13 @@ #include "libioP.h" -#ifdef _IO_MTSAFE_IO -# define DEF_STDFILE(NAME, FD, CHAIN, FLAGS) \ +#define DEF_STDFILE(NAME, FD, CHAIN, FLAGS) \ static _IO_lock_t _IO_stdfile_##FD##_lock = _IO_lock_initializer; \ static struct _IO_wide_data _IO_wide_data_##FD \ = { ._wide_vtable = &_IO_wfile_jumps }; \ struct _IO_FILE_plus NAME \ = {FILEBUF_LITERAL(CHAIN, FLAGS, FD, &_IO_wide_data_##FD), \ &_IO_file_jumps}; -#else -# define DEF_STDFILE(NAME, FD, CHAIN, FLAGS) \ - static struct _IO_wide_data _IO_wide_data_##FD \ - = { ._wide_vtable = &_IO_wfile_jumps }; \ - struct _IO_FILE_plus NAME \ - = {FILEBUF_LITERAL(CHAIN, FLAGS, FD, &_IO_wide_data_##FD), \ - &_IO_file_jumps}; -#endif DEF_STDFILE(_IO_2_1_stdin_, 0, 0, _IO_NO_WRITES); DEF_STDFILE(_IO_2_1_stdout_, 1, &_IO_2_1_stdin_, _IO_NO_READS); diff --git a/libio/vasprintf.c b/libio/vasprintf.c index 4430a266c6..c90a21cb6f 100644 --- a/libio/vasprintf.c +++ b/libio/vasprintf.c @@ -45,9 +45,7 @@ __vasprintf_internal (char **result_ptr, const char *format, va_list args, string = (char *) malloc (init_string_size); if (string == NULL) return -1; -#ifdef _IO_MTSAFE_IO sf._sbf._f._lock = NULL; -#endif _IO_no_init (&sf._sbf._f, _IO_USER_LOCK, -1, NULL, NULL); _IO_JUMPS (&sf._sbf) = &_IO_str_jumps; _IO_str_init_static_internal (&sf, string, init_string_size, string); diff --git a/libio/vsnprintf.c b/libio/vsnprintf.c index 8dae66761d..5aa01665d8 100644 --- a/libio/vsnprintf.c +++ b/libio/vsnprintf.c @@ -95,9 +95,7 @@ __vsnprintf_internal (char *string, size_t maxlen, const char *format, { _IO_strnfile sf; int ret; -#ifdef _IO_MTSAFE_IO sf.f._sbf._f._lock = NULL; -#endif /* We need to handle the special case where MAXLEN is 0. Use the overflow buffer right from the start. */ diff --git a/libio/vswprintf.c b/libio/vswprintf.c index 5a9ccdff3e..c6f4706573 100644 --- a/libio/vswprintf.c +++ b/libio/vswprintf.c @@ -95,9 +95,7 @@ __vswprintf_internal (wchar_t *string, size_t maxlen, const wchar_t *format, _IO_wstrnfile sf; int ret; struct _IO_wide_data wd; -#ifdef _IO_MTSAFE_IO sf.f._sbf._f._lock = NULL; -#endif if (maxlen == 0) /* Since we have to write at least the terminating L'\0' a buffer diff --git a/libio/wgenops.c b/libio/wgenops.c index e7ea75aed2..96e10f6d1c 100644 --- a/libio/wgenops.c +++ b/libio/wgenops.c @@ -185,10 +185,8 @@ _IO_wdefault_finish (FILE *fp, int dummy) fp->_IO_save_base = NULL; } -#ifdef _IO_MTSAFE_IO if (fp->_lock != NULL) _IO_lock_fini (*fp->_lock); -#endif _IO_un_link ((struct _IO_FILE_plus *) fp); } diff --git a/libio/wmemstream.c b/libio/wmemstream.c index e7f898c7fb..9366ef4aad 100644 --- a/libio/wmemstream.c +++ b/libio/wmemstream.c @@ -67,9 +67,7 @@ open_wmemstream (wchar_t **bufloc, size_t *sizeloc) struct locked_FILE { struct _IO_FILE_wmemstream fp; -#ifdef _IO_MTSAFE_IO _IO_lock_t lock; -#endif struct _IO_wide_data wd; } *new_f; wchar_t *buf; @@ -77,9 +75,7 @@ open_wmemstream (wchar_t **bufloc, size_t *sizeloc) new_f = (struct locked_FILE *) malloc (sizeof (struct locked_FILE)); if (new_f == NULL) return NULL; -#ifdef _IO_MTSAFE_IO new_f->fp._sf._sbf._f._lock = &new_f->lock; -#endif buf = calloc (1, BUFSIZ); if (buf == NULL) diff --git a/nptl/Makefile b/nptl/Makefile index b585663974..a85da416de 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -432,9 +432,6 @@ $(objpfx)multidir.mk: $(common-objpfx)config.make endif -CFLAGS-ftrylockfile.c += $(libio-mtsafe) -CFLAGS-funlockfile.c += $(libio-mtsafe) - link-libc-static := $(common-objpfx)libc.a $(static-gnulib) \ $(common-objpfx)libc.a diff --git a/pwd/Makefile b/pwd/Makefile index 848e7d0e12..da8e84c46c 100644 --- a/pwd/Makefile +++ b/pwd/Makefile @@ -37,6 +37,5 @@ ifeq ($(have-thread-library),yes) CFLAGS-getpwent_r.c += -fexceptions CFLAGS-getpwent.c += -fexceptions CFLAGS-getpw.c += -fexceptions -CFLAGS-fgetpwent_r.c += $(libio-mtsafe) endif diff --git a/shadow/Makefile b/shadow/Makefile index b9faa3aa53..1b4ee5d50f 100644 --- a/shadow/Makefile +++ b/shadow/Makefile @@ -32,8 +32,8 @@ tests = tst-shadow tst-putspent CFLAGS-getspent_r.c += -fexceptions CFLAGS-getspent.c += -fexceptions CFLAGS-fgetspent.c += -fexceptions -CFLAGS-fgetspent_r.c += -fexceptions $(libio-mtsafe) -CFLAGS-putspent.c += -fexceptions $(libio-mtsafe) +CFLAGS-fgetspent_r.c += -fexceptions +CFLAGS-putspent.c += -fexceptions CFLAGS-getspnam.c += -fexceptions CFLAGS-getspnam_r.c += -fexceptions diff --git a/stdio-common/Makefile b/stdio-common/Makefile index a1603e82fe..ddeacb1342 100644 --- a/stdio-common/Makefile +++ b/stdio-common/Makefile @@ -357,8 +357,6 @@ CFLAGS-tst-gets.c += -Wno-deprecated-declarations # the fortified version had the same bug. CFLAGS-tst-bz11319-fortify2.c += -D_FORTIFY_SOURCE=2 -CPPFLAGS += $(libio-mtsafe) - $(objpfx)tst-setvbuf1.out: /dev/null $(objpfx)tst-setvbuf1 $(test-program-cmd) > $@ 2>&1; \ $(evaluate-test) diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c index 59bd76c890..b915803c66 100644 --- a/stdio-common/vfprintf-internal.c +++ b/stdio-common/vfprintf-internal.c @@ -2141,9 +2141,7 @@ struct helper_file struct _IO_wide_data _wide_data; #endif FILE *_put_stream; -#ifdef _IO_MTSAFE_IO _IO_lock_t lock; -#endif }; static int @@ -2251,9 +2249,7 @@ buffered_vfprintf (FILE *s, const CHAR_T *format, va_list args, #if _IO_JUMPS_OFFSET hp->_vtable_offset = 0; #endif -#ifdef _IO_MTSAFE_IO hp->_lock = NULL; -#endif hp->_flags2 = s->_flags2; _IO_JUMPS (&helper._f) = (struct _IO_jump_t *) &_IO_helper_jumps; diff --git a/stdlib/Makefile b/stdlib/Makefile index 60fc59c12c..1a10b18931 100644 --- a/stdlib/Makefile +++ b/stdlib/Makefile @@ -359,15 +359,6 @@ CFLAGS-system.c += -fexceptions CFLAGS-system.os = -fomit-frame-pointer CFLAGS-fmtmsg.c += -fexceptions -CFLAGS-strfmon.c += $(libio-mtsafe) -CFLAGS-strfmon_l.c += $(libio-mtsafe) - -# The strfrom class of functions call __printf_fp in order to convert the -# floating-point value to characters. This requires the value of IO_MTSAFE_IO. -CFLAGS-strfromd.c += $(libio-mtsafe) -CFLAGS-strfromf.c += $(libio-mtsafe) -CFLAGS-strfroml.c += $(libio-mtsafe) - CFLAGS-tst-bsearch.c += $(stack-align-test-flags) CFLAGS-tst-qsort.c += $(stack-align-test-flags) CFLAGS-tst-makecontext.c += -funwind-tables diff --git a/stdlib/strfmon_l.c b/stdlib/strfmon_l.c index d9b22088c7..e6fb378877 100644 --- a/stdlib/strfmon_l.c +++ b/stdlib/strfmon_l.c @@ -523,9 +523,7 @@ __vstrfmon_l_internal (char *s, size_t maxsize, locale_t loc, out_string (sign_string); /* Print the number. */ -#ifdef _IO_MTSAFE_IO f._sbf._f._lock = NULL; -#endif _IO_init_internal (&f._sbf._f, 0); _IO_JUMPS (&f._sbf) = &_IO_str_jumps; _IO_str_init_static_internal (&f, dest, (s + maxsize) - dest, dest); diff --git a/stdlib/strfrom-skeleton.c b/stdlib/strfrom-skeleton.c index 1fba04bf6a..0ccdd756cd 100644 --- a/stdlib/strfrom-skeleton.c +++ b/stdlib/strfrom-skeleton.c @@ -37,9 +37,7 @@ int STRFROM (char *dest, size_t size, const char *format, FLOAT f) { _IO_strnfile sfile; -#ifdef _IO_MTSAFE_IO sfile.f._sbf._f._lock = NULL; -#endif int done; diff --git a/sysdeps/generic/stdio-lock.h b/sysdeps/generic/stdio-lock.h index cfdaf82577..14cf458bdd 100644 --- a/sysdeps/generic/stdio-lock.h +++ b/sysdeps/generic/stdio-lock.h @@ -27,8 +27,8 @@ __libc_lock_define_recursive (typedef, _IO_lock_t) /* We need recursive (counting) mutexes. */ #ifdef _LIBC_LOCK_RECURSIVE_INITIALIZER # define _IO_lock_initializer _LIBC_LOCK_RECURSIVE_INITIALIZER -#elif _IO_MTSAFE_IO - #error libio needs recursive mutexes for _IO_MTSAFE_IO +#else +# error libio needs recursive mutexes #endif #define _IO_lock_init(_name) __libc_lock_init_recursive (_name) diff --git a/sysdeps/ieee754/float128/Makefile b/sysdeps/ieee754/float128/Makefile index 571a841809..166e630290 100644 --- a/sysdeps/ieee754/float128/Makefile +++ b/sysdeps/ieee754/float128/Makefile @@ -1,10 +1,6 @@ ifeq ($(subdir),stdlib) routines += float1282mpn strfromf128 routines += strtof128 strtof128_l strtof128_nan mpn2float128 - -# The strfrom class of functions call __printf_fp in order to convert the -# floating-point value to characters. This requires the value of IO_MTSAFE_IO. -CFLAGS-strfromf128.c += $(libio-mtsafe) endif ifeq ($(subdir),wcsmbs) diff --git a/sysdeps/nptl/libc-lock.h b/sysdeps/nptl/libc-lock.h index 5af476c48b..c186375c31 100644 --- a/sysdeps/nptl/libc-lock.h +++ b/sysdeps/nptl/libc-lock.h @@ -25,7 +25,7 @@ /* Mutex type. */ -#if defined _LIBC || defined _IO_MTSAFE_IO +#if defined _LIBC # if (!IS_IN (libc) && !IS_IN (libpthread)) || !defined _LIBC typedef struct { pthread_mutex_t mutex; } __libc_lock_recursive_t; # else diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile index ca953804d0..b3bf4217a5 100644 --- a/sysdeps/unix/sysv/linux/Makefile +++ b/sysdeps/unix/sysv/linux/Makefile @@ -317,7 +317,6 @@ tests += tst-affinity tst-affinity-pid tests-static := tst-affinity-static tests += $(tests-static) -CFLAGS-fork.c = $(libio-mtsafe) CFLAGS-getpid.o = -fomit-frame-pointer CFLAGS-getpid.os = -fomit-frame-pointer endif diff --git a/wcsmbs/Makefile b/wcsmbs/Makefile index df9a85f4a9..afb0610b55 100644 --- a/wcsmbs/Makefile +++ b/wcsmbs/Makefile @@ -103,8 +103,6 @@ CFLAGS-isoc99_fwscanf.c += -fexceptions CFLAGS-isoc99_vwscanf.c += -fexceptions CFLAGS-isoc99_vfwscanf.c += -fexceptions -CPPFLAGS += $(libio-mtsafe) - # We need to find the default version of strtold_l in stdlib. CPPFLAGS-wcstold_l.c += -I../stdlib -- 2.34.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/4] libio: Assume _IO_MTSAFE_IO 2022-04-26 19:15 ` [PATCH v2 1/4] libio: Assume _IO_MTSAFE_IO Adhemerval Zanella @ 2022-04-27 12:34 ` Florian Weimer 2022-04-27 18:40 ` Adhemerval Zanella 0 siblings, 1 reply; 19+ messages in thread From: Florian Weimer @ 2022-04-27 12:34 UTC (permalink / raw) To: Adhemerval Zanella via Libc-alpha * Adhemerval Zanella via Libc-alpha: > It is already set by default on all supported architectures and it is > an expectation that stdio works on multi-threaded environments. So … this cleanup has got stuck in the past because it's actually more than just a cleanup. We actually build most of glibc without libio locking. Looks like misc/ nss/ posix/ have not been covered before. Maybe we should just file a bug for the missing locking and fix this with this commit? > index 5af476c48b..c186375c31 100644 > --- a/sysdeps/nptl/libc-lock.h > +++ b/sysdeps/nptl/libc-lock.h > @@ -25,7 +25,7 @@ > > > /* Mutex type. */ > -#if defined _LIBC || defined _IO_MTSAFE_IO > +#if defined _LIBC > # if (!IS_IN (libc) && !IS_IN (libpthread)) || !defined _LIBC > typedef struct { pthread_mutex_t mutex; } __libc_lock_recursive_t; > # else This doesn't look quite right. Would we want to compile this unconditionally now? There's also some weirdness I can't explain. I get strange before/after symbol differences: DIFF eu-readelf -s after strip: nptl/pthread_rwlock_unlock.os --- /tmp/Left-ltjeu50v.o 2022-04-27 14:29:39.070599437 +0200 +++ /tmp/Right-jee20kfm.o 2022-04-27 14:29:39.074599395 +0200 @@ -1,5 +1,5 @@ -Symbol table [11] '.symtab' contains 14 entries: +Symbol table [11] '.symtab' contains 13 entries: 4 local symbols String table: [12] '.strtab' Num: Value Size Type Bind Vis Ndx Name 0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UNDEF @@ -12,7 +12,6 @@ 7: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 ___pthread_rwlock_unlo> 8: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF __GI___libc_fatal 9: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 __GI___pthread_rwlock_> - 10: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 __pthread_rwlock_unlock - 11: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 __pthread_rwlock_unloc> - 12: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 pthread_rwlock_unlock@> - 13: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 pthread_rwlock_unlock@> + 10: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 __pthread_rwlock_unloc> + 11: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 pthread_rwlock_unlock@> + 12: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 pthread_rwlock_unlock@> Can you reproduce this? Thanks, Florian ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/4] libio: Assume _IO_MTSAFE_IO 2022-04-27 12:34 ` Florian Weimer @ 2022-04-27 18:40 ` Adhemerval Zanella 2022-04-27 19:35 ` Adhemerval Zanella 0 siblings, 1 reply; 19+ messages in thread From: Adhemerval Zanella @ 2022-04-27 18:40 UTC (permalink / raw) To: Florian Weimer, Adhemerval Zanella via Libc-alpha On 27/04/2022 09:34, Florian Weimer wrote: > * Adhemerval Zanella via Libc-alpha: > >> It is already set by default on all supported architectures and it is >> an expectation that stdio works on multi-threaded environments. > > So … this cleanup has got stuck in the past because it's actually more > than just a cleanup. We actually build most of glibc without libio > locking. Looks like misc/ nss/ posix/ have not been covered before. > > Maybe we should just file a bug for the missing locking and fix this > with this commit? It seems a better approach, I will check which files as missing support and open a bug report. > >> index 5af476c48b..c186375c31 100644 >> --- a/sysdeps/nptl/libc-lock.h >> +++ b/sysdeps/nptl/libc-lock.h >> @@ -25,7 +25,7 @@ >> >> >> /* Mutex type. */ >> -#if defined _LIBC || defined _IO_MTSAFE_IO >> +#if defined _LIBC >> # if (!IS_IN (libc) && !IS_IN (libpthread)) || !defined _LIBC >> typedef struct { pthread_mutex_t mutex; } __libc_lock_recursive_t; >> # else > > This doesn't look quite right. Would we want to compile this > unconditionally now? Afaiu using __libc_lock_recursive without _IO_MTSAFE_IO will resulting in using __libc_lock_recursive_opaque__, which will result in a undefined type. Also, I really think we should not tie stdio code with general lock primitives (as _IO_MTSAFE_IO is doing here). > > There's also some weirdness I can't explain. I get strange before/after > symbol differences: > > DIFF eu-readelf -s after strip: nptl/pthread_rwlock_unlock.os > --- /tmp/Left-ltjeu50v.o 2022-04-27 14:29:39.070599437 +0200 > +++ /tmp/Right-jee20kfm.o 2022-04-27 14:29:39.074599395 +0200 > @@ -1,5 +1,5 @@ > > -Symbol table [11] '.symtab' contains 14 entries: > +Symbol table [11] '.symtab' contains 13 entries: > 4 local symbols String table: [12] '.strtab' > Num: Value Size Type Bind Vis Ndx Name > 0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UNDEF > @@ -12,7 +12,6 @@ > 7: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 ___pthread_rwlock_unlo> > 8: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF __GI___libc_fatal > 9: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 __GI___pthread_rwlock_> > - 10: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 __pthread_rwlock_unlock > - 11: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 __pthread_rwlock_unloc> > - 12: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 pthread_rwlock_unlock@> > - 13: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 pthread_rwlock_unlock@> > + 10: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 __pthread_rwlock_unloc> > + 11: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 pthread_rwlock_unlock@> > + 12: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 pthread_rwlock_unlock@> > > Can you reproduce this? I didn't check the symbol generation, I will take a look. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/4] libio: Assume _IO_MTSAFE_IO 2022-04-27 18:40 ` Adhemerval Zanella @ 2022-04-27 19:35 ` Adhemerval Zanella 0 siblings, 0 replies; 19+ messages in thread From: Adhemerval Zanella @ 2022-04-27 19:35 UTC (permalink / raw) To: Florian Weimer, Adhemerval Zanella via Libc-alpha On 27/04/2022 15:40, Adhemerval Zanella wrote: > > > On 27/04/2022 09:34, Florian Weimer wrote: >> * Adhemerval Zanella via Libc-alpha: >> >>> It is already set by default on all supported architectures and it is >>> an expectation that stdio works on multi-threaded environments. >> >> So … this cleanup has got stuck in the past because it's actually more >> than just a cleanup. We actually build most of glibc without libio >> locking. Looks like misc/ nss/ posix/ have not been covered before. >> >> Maybe we should just file a bug for the missing locking and fix this >> with this commit? > > It seems a better approach, I will check which files as missing support > and open a bug report. > >> >>> index 5af476c48b..c186375c31 100644 >>> --- a/sysdeps/nptl/libc-lock.h >>> +++ b/sysdeps/nptl/libc-lock.h >>> @@ -25,7 +25,7 @@ >>> >>> >>> /* Mutex type. */ >>> -#if defined _LIBC || defined _IO_MTSAFE_IO >>> +#if defined _LIBC >>> # if (!IS_IN (libc) && !IS_IN (libpthread)) || !defined _LIBC >>> typedef struct { pthread_mutex_t mutex; } __libc_lock_recursive_t; >>> # else >> >> This doesn't look quite right. Would we want to compile this >> unconditionally now? > > Afaiu using __libc_lock_recursive without _IO_MTSAFE_IO will resulting > in using __libc_lock_recursive_opaque__, which will result in a undefined > type. > > Also, I really think we should not tie stdio code with general lock > primitives (as _IO_MTSAFE_IO is doing here). > >> >> There's also some weirdness I can't explain. I get strange before/after >> symbol differences: >> >> DIFF eu-readelf -s after strip: nptl/pthread_rwlock_unlock.os >> --- /tmp/Left-ltjeu50v.o 2022-04-27 14:29:39.070599437 +0200 >> +++ /tmp/Right-jee20kfm.o 2022-04-27 14:29:39.074599395 +0200 >> @@ -1,5 +1,5 @@ >> >> -Symbol table [11] '.symtab' contains 14 entries: >> +Symbol table [11] '.symtab' contains 13 entries: >> 4 local symbols String table: [12] '.strtab' >> Num: Value Size Type Bind Vis Ndx Name >> 0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UNDEF >> @@ -12,7 +12,6 @@ >> 7: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 ___pthread_rwlock_unlo> >> 8: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UNDEF __GI___libc_fatal >> 9: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 __GI___pthread_rwlock_> >> - 10: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 __pthread_rwlock_unlock >> - 11: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 __pthread_rwlock_unloc> >> - 12: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 pthread_rwlock_unlock@> >> - 13: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 pthread_rwlock_unlock@> >> + 10: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 __pthread_rwlock_unloc> >> + 11: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 pthread_rwlock_unlock@> >> + 12: 0000000000000000 463 FUNC GLOBAL DEFAULT 1 pthread_rwlock_unlock@> >> >> Can you reproduce this? > > I didn't check the symbol generation, I will take a look. I think it might be from: diff --git a/include/stdio.h b/include/stdio.h index 23b7fd288c..e9a7c77c24 100644 --- a/include/stdio.h +++ b/include/stdio.h @@ -1,5 +1,5 @@ #ifndef _STDIO_H -# if !defined _ISOMAC && defined _IO_MTSAFE_IO +# if !defined _ISOMAC # include <stdio-lock.h> # endif Where tt now pulls libc-lockP.h from from libc-lock.h. And libc-lockP.h contains a lot of duplicated definition already preseted on pthreadP.h, and worse, it it defined some hidden_proto not presented in pthreadP.h. I will send a patch to consolidate this definitions prior the one that removes _IO_MTSAFE_IO. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 2/4] Consolidate stdio-lock.h 2022-04-26 19:15 [PATCH v2 0/4] Move libio lock single-thread optimization to generic libc-lock Adhemerval Zanella 2022-04-26 19:15 ` [PATCH v2 1/4] libio: Assume _IO_MTSAFE_IO Adhemerval Zanella @ 2022-04-26 19:15 ` Adhemerval Zanella 2022-04-27 13:25 ` Florian Weimer 2022-04-26 19:15 ` [PATCH v2 3/4] Move libio lock single-thread optimization to generic libc-lock (BZ #27842) Adhemerval Zanella 2022-04-26 19:15 ` [PATCH v2 4/4] Assume _LIBC and libc module for libc-lock.h Adhemerval Zanella 3 siblings, 1 reply; 19+ messages in thread From: Adhemerval Zanella @ 2022-04-26 19:15 UTC (permalink / raw) To: libc-alpha, Florian Weimer The NPTL replicate the sysdeps/nptl/libc-lock.h recursive lock. Use the Hurd one as generic one, which already uses the __libc_lock functions. The libc_malloc_debug is the only module that uses the __libc_lock_init_recursive directly in the code and it is kept to use the internal lock definitions. Checked on x86_64-linux-gnu and i686-linux-gnu. --- sysdeps/generic/stdio-lock.h | 21 +++----- sysdeps/htl/stdio-lock.h | 57 -------------------- sysdeps/nptl/libc-lock.h | 20 +++---- sysdeps/nptl/stdio-lock.h | 101 ----------------------------------- 4 files changed, 15 insertions(+), 184 deletions(-) delete mode 100644 sysdeps/htl/stdio-lock.h delete mode 100644 sysdeps/nptl/stdio-lock.h diff --git a/sysdeps/generic/stdio-lock.h b/sysdeps/generic/stdio-lock.h index 14cf458bdd..fd61f0b5b7 100644 --- a/sysdeps/generic/stdio-lock.h +++ b/sysdeps/generic/stdio-lock.h @@ -45,20 +45,13 @@ __libc_lock_define_recursive (typedef, _IO_lock_t) #define _IO_cleanup_region_end(_doit) \ __libc_cleanup_region_end (_doit) -#if defined _LIBC && IS_IN (libc) - -# ifdef __EXCEPTIONS -# define _IO_acquire_lock(_fp) \ +#define _IO_acquire_lock(_fp) \ do { \ - FILE *_IO_acquire_lock_file \ - __attribute__((cleanup (_IO_acquire_lock_fct))) \ - = (_fp); \ - _IO_flockfile (_IO_acquire_lock_file); -# else -# define _IO_acquire_lock(_fp) _IO_acquire_lock_needs_exceptions_enabled -# endif -# define _IO_release_lock(_fp) ; } while (0) - -#endif + _IO_cleanup_region_start((void (*) (void *)) &_IO_funlockfile, _fp); \ + _IO_flockfile (_fp); +#define _IO_release_lock(_fp) \ + _IO_funlockfile (_fp); \ + _IO_cleanup_region_end (0); \ + } while (0) #endif /* stdio-lock.h */ diff --git a/sysdeps/htl/stdio-lock.h b/sysdeps/htl/stdio-lock.h deleted file mode 100644 index c0a03c5a13..0000000000 --- a/sysdeps/htl/stdio-lock.h +++ /dev/null @@ -1,57 +0,0 @@ -/* Thread package specific definitions of stream lock type. Hurd version. - Copyright (C) 2000-2022 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - <https://www.gnu.org/licenses/>. */ - -#ifndef _STDIO_LOCK_H -#define _STDIO_LOCK_H 1 - -#include <libc-lock.h> - -__libc_lock_define_recursive (typedef, _IO_lock_t) -#define _IO_lock_t_defined 1 - -/* We need recursive (counting) mutexes. */ -# define _IO_lock_initializer _LIBC_LOCK_RECURSIVE_INITIALIZER - -#define _IO_lock_init(_name) __libc_lock_init_recursive (_name) -#define _IO_lock_fini(_name) __libc_lock_fini_recursive (_name) -#define _IO_lock_lock(_name) __libc_lock_lock_recursive (_name) -#define _IO_lock_trylock(_name) __libc_lock_trylock_recursive (_name) -#define _IO_lock_unlock(_name) __libc_lock_unlock_recursive (_name) - - -#define _IO_cleanup_region_start(_fct, _fp) \ - __libc_cleanup_region_start (((_fp)->_flags & _IO_USER_LOCK) == 0, _fct, _fp) -#define _IO_cleanup_region_start_noarg(_fct) \ - __libc_cleanup_region_start (1, _fct, NULL) -#define _IO_cleanup_region_end(_doit) \ - __libc_cleanup_region_end (_doit) - -#if defined _LIBC && IS_IN (libc) - -# define _IO_acquire_lock(_fp) \ - do { \ - _IO_cleanup_region_start((void (*) (void *)) &_IO_funlockfile, _fp); \ - _IO_flockfile (_fp); -# define _IO_release_lock(_fp) \ - _IO_funlockfile (_fp); \ - _IO_cleanup_region_end (0); \ - } while (0) - -#endif - -#endif /* stdio-lock.h */ diff --git a/sysdeps/nptl/libc-lock.h b/sysdeps/nptl/libc-lock.h index c186375c31..6c2d6acfd1 100644 --- a/sysdeps/nptl/libc-lock.h +++ b/sysdeps/nptl/libc-lock.h @@ -25,14 +25,10 @@ /* Mutex type. */ -#if defined _LIBC -# if (!IS_IN (libc) && !IS_IN (libpthread)) || !defined _LIBC +#if !IS_IN (libc) && !IS_IN (libc_malloc_debug) typedef struct { pthread_mutex_t mutex; } __libc_lock_recursive_t; -# else -typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t; -# endif #else -typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t; +typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t; #endif /* Define a lock variable NAME with storage class CLASS. The lock must be @@ -47,7 +43,7 @@ typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t; /* Define an initialized recursive lock variable NAME with storage class CLASS. */ -#if defined _LIBC && (IS_IN (libc) || IS_IN (libpthread)) +#if IS_IN (libc) || IS_IN (libc_malloc_debug) # define __libc_lock_define_initialized_recursive(CLASS, NAME) \ CLASS __libc_lock_recursive_t NAME = _LIBC_LOCK_RECURSIVE_INITIALIZER; # define _LIBC_LOCK_RECURSIVE_INITIALIZER \ @@ -60,7 +56,7 @@ typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t; #endif /* Initialize a recursive mutex. */ -#if defined _LIBC && (IS_IN (libc) || IS_IN (libpthread)) +#if IS_IN (libc) || IS_IN (libc_malloc_debug) # define __libc_lock_init_recursive(NAME) \ ((void) ((NAME) = (__libc_lock_recursive_t) _LIBC_LOCK_RECURSIVE_INITIALIZER)) #else @@ -78,7 +74,7 @@ typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t; #endif /* Finalize recursive named lock. */ -#if defined _LIBC && (IS_IN (libc) || IS_IN (libpthread)) +#if IS_IN (libc) || IS_IN (libc_malloc_debug) # define __libc_lock_fini_recursive(NAME) ((void) 0) #else # define __libc_lock_fini_recursive(NAME) \ @@ -86,7 +82,7 @@ typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t; #endif /* Lock the recursive named lock variable. */ -#if defined _LIBC && (IS_IN (libc) || IS_IN (libpthread)) +#if IS_IN (libc) || IS_IN (libc_malloc_debug) # define __libc_lock_lock_recursive(NAME) \ do { \ void *self = THREAD_SELF; \ @@ -103,7 +99,7 @@ typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t; #endif /* Try to lock the recursive named lock variable. */ -#if defined _LIBC && (IS_IN (libc) || IS_IN (libpthread)) +#if IS_IN (libc) || IS_IN (libc_malloc_debug) # define __libc_lock_trylock_recursive(NAME) \ ({ \ int result = 0; \ @@ -128,7 +124,7 @@ typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t; #endif /* Unlock the recursive named lock variable. */ -#if defined _LIBC && (IS_IN (libc) || IS_IN (libpthread)) +#if IS_IN (libc) || IS_IN (libc_malloc_debug) /* We do no error checking here. */ # define __libc_lock_unlock_recursive(NAME) \ do { \ diff --git a/sysdeps/nptl/stdio-lock.h b/sysdeps/nptl/stdio-lock.h deleted file mode 100644 index afa0b779c8..0000000000 --- a/sysdeps/nptl/stdio-lock.h +++ /dev/null @@ -1,101 +0,0 @@ -/* Thread package specific definitions of stream lock type. NPTL version. - Copyright (C) 2000-2022 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library; if not, see - <https://www.gnu.org/licenses/>. */ - -#ifndef _STDIO_LOCK_H -#define _STDIO_LOCK_H 1 - -#include <libc-lock.h> -#include <lowlevellock.h> - - -typedef struct { int lock; int cnt; void *owner; } _IO_lock_t; -#define _IO_lock_t_defined 1 - -#define _IO_lock_initializer { LLL_LOCK_INITIALIZER, 0, NULL } - -#define _IO_lock_init(_name) \ - ((void) ((_name) = (_IO_lock_t) _IO_lock_initializer)) - -#define _IO_lock_fini(_name) \ - ((void) 0) - -#define _IO_lock_lock(_name) \ - do { \ - void *__self = THREAD_SELF; \ - if ((_name).owner != __self) \ - { \ - lll_lock ((_name).lock, LLL_PRIVATE); \ - (_name).owner = __self; \ - } \ - ++(_name).cnt; \ - } while (0) - -#define _IO_lock_trylock(_name) \ - ({ \ - int __result = 0; \ - void *__self = THREAD_SELF; \ - if ((_name).owner != __self) \ - { \ - if (lll_trylock ((_name).lock) == 0) \ - { \ - (_name).owner = __self; \ - (_name).cnt = 1; \ - } \ - else \ - __result = EBUSY; \ - } \ - else \ - ++(_name).cnt; \ - __result; \ - }) - -#define _IO_lock_unlock(_name) \ - do { \ - if (--(_name).cnt == 0) \ - { \ - (_name).owner = NULL; \ - lll_unlock ((_name).lock, LLL_PRIVATE); \ - } \ - } while (0) - - - -#define _IO_cleanup_region_start(_fct, _fp) \ - __libc_cleanup_region_start (((_fp)->_flags & _IO_USER_LOCK) == 0, _fct, _fp) -#define _IO_cleanup_region_start_noarg(_fct) \ - __libc_cleanup_region_start (1, _fct, NULL) -#define _IO_cleanup_region_end(_doit) \ - __libc_cleanup_region_end (_doit) - -#if defined _LIBC && IS_IN (libc) - -# ifdef __EXCEPTIONS -# define _IO_acquire_lock(_fp) \ - do { \ - FILE *_IO_acquire_lock_file \ - __attribute__((cleanup (_IO_acquire_lock_fct))) \ - = (_fp); \ - _IO_flockfile (_IO_acquire_lock_file); -# else -# define _IO_acquire_lock(_fp) _IO_acquire_lock_needs_exceptions_enabled -# endif -# define _IO_release_lock(_fp) ; } while (0) - -#endif - -#endif /* stdio-lock.h */ -- 2.34.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/4] Consolidate stdio-lock.h 2022-04-26 19:15 ` [PATCH v2 2/4] Consolidate stdio-lock.h Adhemerval Zanella @ 2022-04-27 13:25 ` Florian Weimer 2022-04-27 16:15 ` Adhemerval Zanella 0 siblings, 1 reply; 19+ messages in thread From: Florian Weimer @ 2022-04-27 13:25 UTC (permalink / raw) To: Adhemerval Zanella via Libc-alpha * Adhemerval Zanella via Libc-alpha: > diff --git a/sysdeps/generic/stdio-lock.h b/sysdeps/generic/stdio-lock.h > index 14cf458bdd..fd61f0b5b7 100644 > --- a/sysdeps/generic/stdio-lock.h > +++ b/sysdeps/generic/stdio-lock.h > @@ -45,20 +45,13 @@ __libc_lock_define_recursive (typedef, _IO_lock_t) > #define _IO_cleanup_region_end(_doit) \ > __libc_cleanup_region_end (_doit) > > -#if defined _LIBC && IS_IN (libc) > - > -# ifdef __EXCEPTIONS > -# define _IO_acquire_lock(_fp) \ > +#define _IO_acquire_lock(_fp) \ > do { \ > - FILE *_IO_acquire_lock_file \ > - __attribute__((cleanup (_IO_acquire_lock_fct))) \ > - = (_fp); \ > - _IO_flockfile (_IO_acquire_lock_file); > -# else > -# define _IO_acquire_lock(_fp) _IO_acquire_lock_needs_exceptions_enabled > -# endif > -# define _IO_release_lock(_fp) ; } while (0) > - > -#endif > + _IO_cleanup_region_start((void (*) (void *)) &_IO_funlockfile, _fp); \ > + _IO_flockfile (_fp); > +#define _IO_release_lock(_fp) \ > + _IO_funlockfile (_fp); \ > + _IO_cleanup_region_end (0); \ > + } while (0) > > #endif /* stdio-lock.h */ I think this change replaces unwind tables for -fexceptions builds. If GCC can't turn the indirect call to the unlock function into a direct call, this will result in a loss of hardening due to the additional indirect function call. This change may also lose C++ unwinding compatibility for some fopencookie use cases, I think. Thanks, Florian ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/4] Consolidate stdio-lock.h 2022-04-27 13:25 ` Florian Weimer @ 2022-04-27 16:15 ` Adhemerval Zanella 2022-04-27 18:00 ` Florian Weimer 0 siblings, 1 reply; 19+ messages in thread From: Adhemerval Zanella @ 2022-04-27 16:15 UTC (permalink / raw) To: Florian Weimer, Adhemerval Zanella via Libc-alpha On 27/04/2022 10:25, Florian Weimer wrote: > * Adhemerval Zanella via Libc-alpha: > >> diff --git a/sysdeps/generic/stdio-lock.h b/sysdeps/generic/stdio-lock.h >> index 14cf458bdd..fd61f0b5b7 100644 >> --- a/sysdeps/generic/stdio-lock.h >> +++ b/sysdeps/generic/stdio-lock.h >> @@ -45,20 +45,13 @@ __libc_lock_define_recursive (typedef, _IO_lock_t) >> #define _IO_cleanup_region_end(_doit) \ >> __libc_cleanup_region_end (_doit) >> >> -#if defined _LIBC && IS_IN (libc) >> - >> -# ifdef __EXCEPTIONS >> -# define _IO_acquire_lock(_fp) \ >> +#define _IO_acquire_lock(_fp) \ >> do { \ >> - FILE *_IO_acquire_lock_file \ >> - __attribute__((cleanup (_IO_acquire_lock_fct))) \ >> - = (_fp); \ >> - _IO_flockfile (_IO_acquire_lock_file); >> -# else >> -# define _IO_acquire_lock(_fp) _IO_acquire_lock_needs_exceptions_enabled >> -# endif >> -# define _IO_release_lock(_fp) ; } while (0) >> - >> -#endif >> + _IO_cleanup_region_start((void (*) (void *)) &_IO_funlockfile, _fp); \ >> + _IO_flockfile (_fp); >> +#define _IO_release_lock(_fp) \ >> + _IO_funlockfile (_fp); \ >> + _IO_cleanup_region_end (0); \ >> + } while (0) >> >> #endif /* stdio-lock.h */ > > I think this change replaces unwind tables for -fexceptions builds. If > GCC can't turn the indirect call to the unlock function into a direct > call, this will result in a loss of hardening due to the additional > indirect function call. > > This change may also lose C++ unwinding compatibility for some > fopencookie use cases, I think. This is an internal header where if __EXCEPTIONS is not defined we will get a compiler error because of an undefined symbol (_IO_acquire_lock_needs_exceptions_enabled). So internally all _IO_acquire_lock usage already requires __EXCEPTIONS, so the fallback is just unused definitions. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/4] Consolidate stdio-lock.h 2022-04-27 16:15 ` Adhemerval Zanella @ 2022-04-27 18:00 ` Florian Weimer 2022-04-27 18:35 ` Adhemerval Zanella 0 siblings, 1 reply; 19+ messages in thread From: Florian Weimer @ 2022-04-27 18:00 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: Adhemerval Zanella via Libc-alpha * Adhemerval Zanella: > On 27/04/2022 10:25, Florian Weimer wrote: >> * Adhemerval Zanella via Libc-alpha: >> >>> diff --git a/sysdeps/generic/stdio-lock.h b/sysdeps/generic/stdio-lock.h >>> index 14cf458bdd..fd61f0b5b7 100644 >>> --- a/sysdeps/generic/stdio-lock.h >>> +++ b/sysdeps/generic/stdio-lock.h >>> @@ -45,20 +45,13 @@ __libc_lock_define_recursive (typedef, _IO_lock_t) >>> #define _IO_cleanup_region_end(_doit) \ >>> __libc_cleanup_region_end (_doit) >>> >>> -#if defined _LIBC && IS_IN (libc) >>> - >>> -# ifdef __EXCEPTIONS >>> -# define _IO_acquire_lock(_fp) \ >>> +#define _IO_acquire_lock(_fp) \ >>> do { \ >>> - FILE *_IO_acquire_lock_file \ >>> - __attribute__((cleanup (_IO_acquire_lock_fct))) \ >>> - = (_fp); \ >>> - _IO_flockfile (_IO_acquire_lock_file); >>> -# else >>> -# define _IO_acquire_lock(_fp) _IO_acquire_lock_needs_exceptions_enabled >>> -# endif >>> -# define _IO_release_lock(_fp) ; } while (0) >>> - >>> -#endif >>> + _IO_cleanup_region_start((void (*) (void *)) &_IO_funlockfile, _fp); \ >>> + _IO_flockfile (_fp); >>> +#define _IO_release_lock(_fp) \ >>> + _IO_funlockfile (_fp); \ >>> + _IO_cleanup_region_end (0); \ >>> + } while (0) >>> >>> #endif /* stdio-lock.h */ >> >> I think this change replaces unwind tables for -fexceptions builds. If >> GCC can't turn the indirect call to the unlock function into a direct >> call, this will result in a loss of hardening due to the additional >> indirect function call. >> >> This change may also lose C++ unwinding compatibility for some >> fopencookie use cases, I think. > > This is an internal header where if __EXCEPTIONS is not defined we will > get a compiler error because of an undefined symbol > (_IO_acquire_lock_needs_exceptions_enabled). So internally all > _IO_acquire_lock usage already requires __EXCEPTIONS, so the fallback > is just unused definitions. I see this code generation change in libio/fputc.os. The new code uses an on-stack pointer saved at the start of the cleanup region: + 90: 48 8b 05 00 00 00 00 mov 0x0(%rip),%rax # 97 <fputc+0x97> + 93: R_X86_64_REX_GOTPCRELX _IO_funlockfile-0x4 + 97: 48 89 e7 mov %rsp,%rdi + 9a: 48 89 04 24 mov %rax,(%rsp) + 9e: e8 00 00 00 00 call a3 <fputc+0xa3> + 9f: R_X86_64_PLT32 __GI___libc_cleanup_push_defer-0x4 + a3: 8b 45 00 mov 0x0(%rbp),%eax + a6: 25 00 80 00 00 and $0x8000,%eax + ab: 0f 85 d1 00 00 00 jne 182 <fputc+0x182> + b1: 64 4c 8b 2c 25 10 00 mov %fs:0x10,%r13 + b8: 00 00 + ba: 48 8b bd 88 00 00 00 mov 0x88(%rbp),%rdi + c1: 4c 39 6f 08 cmp %r13,0x8(%rdi) + c5: 74 1a je e1 <fputc+0xe1> + c7: ba 01 00 00 00 mov $0x1,%edx + cc: f0 0f b1 17 lock cmpxchg %edx,(%rdi) + d0: 0f 85 a2 00 00 00 jne 178 <fputc+0x178> + d6: 48 8b bd 88 00 00 00 mov 0x88(%rbp),%rdi + dd: 4c 89 6f 08 mov %r13,0x8(%rdi) + e1: 83 47 04 01 addl $0x1,0x4(%rdi) + e5: 41 bd 01 00 00 00 mov $0x1,%r13d + eb: e9 3d ff ff ff jmp 2d <fputc+0x2d> + f0: 48 89 e7 mov %rsp,%rdi + f3: e8 00 00 00 00 call f8 <fputc+0xf8> + f4: R_X86_64_PLT32 __GI___libc_cleanup_pop_restore-0x4 This is a from a build with CFLAGS="-O2 -fexceptions -s -DNDEBUG" (for comparison purposes). The old code just inlined the _IO_funlockfile fast path. (We seem to lack libc_hidden_proto/libc_hidden_def for _IO_funlockfile.) Thanks, Florian ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/4] Consolidate stdio-lock.h 2022-04-27 18:00 ` Florian Weimer @ 2022-04-27 18:35 ` Adhemerval Zanella 2022-04-27 18:44 ` Florian Weimer 0 siblings, 1 reply; 19+ messages in thread From: Adhemerval Zanella @ 2022-04-27 18:35 UTC (permalink / raw) To: Florian Weimer; +Cc: Adhemerval Zanella via Libc-alpha On 27/04/2022 15:00, Florian Weimer wrote: > * Adhemerval Zanella: > >> On 27/04/2022 10:25, Florian Weimer wrote: >>> * Adhemerval Zanella via Libc-alpha: >>> >>>> diff --git a/sysdeps/generic/stdio-lock.h b/sysdeps/generic/stdio-lock.h >>>> index 14cf458bdd..fd61f0b5b7 100644 >>>> --- a/sysdeps/generic/stdio-lock.h >>>> +++ b/sysdeps/generic/stdio-lock.h >>>> @@ -45,20 +45,13 @@ __libc_lock_define_recursive (typedef, _IO_lock_t) >>>> #define _IO_cleanup_region_end(_doit) \ >>>> __libc_cleanup_region_end (_doit) >>>> >>>> -#if defined _LIBC && IS_IN (libc) >>>> - >>>> -# ifdef __EXCEPTIONS >>>> -# define _IO_acquire_lock(_fp) \ >>>> +#define _IO_acquire_lock(_fp) \ >>>> do { \ >>>> - FILE *_IO_acquire_lock_file \ >>>> - __attribute__((cleanup (_IO_acquire_lock_fct))) \ >>>> - = (_fp); \ >>>> - _IO_flockfile (_IO_acquire_lock_file); >>>> -# else >>>> -# define _IO_acquire_lock(_fp) _IO_acquire_lock_needs_exceptions_enabled >>>> -# endif >>>> -# define _IO_release_lock(_fp) ; } while (0) >>>> - >>>> -#endif >>>> + _IO_cleanup_region_start((void (*) (void *)) &_IO_funlockfile, _fp); \ >>>> + _IO_flockfile (_fp); >>>> +#define _IO_release_lock(_fp) \ >>>> + _IO_funlockfile (_fp); \ >>>> + _IO_cleanup_region_end (0); \ >>>> + } while (0) >>>> >>>> #endif /* stdio-lock.h */ >>> >>> I think this change replaces unwind tables for -fexceptions builds. If >>> GCC can't turn the indirect call to the unlock function into a direct >>> call, this will result in a loss of hardening due to the additional >>> indirect function call. >>> >>> This change may also lose C++ unwinding compatibility for some >>> fopencookie use cases, I think. >> >> This is an internal header where if __EXCEPTIONS is not defined we will >> get a compiler error because of an undefined symbol >> (_IO_acquire_lock_needs_exceptions_enabled). So internally all >> _IO_acquire_lock usage already requires __EXCEPTIONS, so the fallback >> is just unused definitions. > > I see this code generation change in libio/fputc.os. The new code uses > an on-stack pointer saved at the start of the cleanup region: > > + 90: 48 8b 05 00 00 00 00 mov 0x0(%rip),%rax # 97 <fputc+0x97> > + 93: R_X86_64_REX_GOTPCRELX _IO_funlockfile-0x4 > + 97: 48 89 e7 mov %rsp,%rdi > + 9a: 48 89 04 24 mov %rax,(%rsp) > + 9e: e8 00 00 00 00 call a3 <fputc+0xa3> > + 9f: R_X86_64_PLT32 __GI___libc_cleanup_push_defer-0x4 > + a3: 8b 45 00 mov 0x0(%rbp),%eax > + a6: 25 00 80 00 00 and $0x8000,%eax > + ab: 0f 85 d1 00 00 00 jne 182 <fputc+0x182> > + b1: 64 4c 8b 2c 25 10 00 mov %fs:0x10,%r13 > + b8: 00 00 > + ba: 48 8b bd 88 00 00 00 mov 0x88(%rbp),%rdi > + c1: 4c 39 6f 08 cmp %r13,0x8(%rdi) > + c5: 74 1a je e1 <fputc+0xe1> > + c7: ba 01 00 00 00 mov $0x1,%edx > + cc: f0 0f b1 17 lock cmpxchg %edx,(%rdi) > + d0: 0f 85 a2 00 00 00 jne 178 <fputc+0x178> > + d6: 48 8b bd 88 00 00 00 mov 0x88(%rbp),%rdi > + dd: 4c 89 6f 08 mov %r13,0x8(%rdi) > + e1: 83 47 04 01 addl $0x1,0x4(%rdi) > + e5: 41 bd 01 00 00 00 mov $0x1,%r13d > + eb: e9 3d ff ff ff jmp 2d <fputc+0x2d> > + f0: 48 89 e7 mov %rsp,%rdi > + f3: e8 00 00 00 00 call f8 <fputc+0xf8> > + f4: R_X86_64_PLT32 __GI___libc_cleanup_pop_restore-0x4 > > This is a from a build with CFLAGS="-O2 -fexceptions -s -DNDEBUG" > (for comparison purposes). > > The old code just inlined the _IO_funlockfile fast path. > > (We seem to lack libc_hidden_proto/libc_hidden_def for _IO_funlockfile.) You are right. The change now uses __libc_cleanup_region_start from libc-lock.h instead of the cleanup version. I am not sure about hardening, but afaiu __libc_cleanup_push_defer should handle C++ unwinding for fopencookie. It seems that the Hurd version is indeed to the best option, so I think for generic it would be better to just use: diff --git a/sysdeps/generic/stdio-lock.h b/sysdeps/generic/stdio-lock.h index 14cf458bdd..a42131f5a5 100644 --- a/sysdeps/generic/stdio-lock.h +++ b/sysdeps/generic/stdio-lock.h @@ -45,20 +45,12 @@ __libc_lock_define_recursive (typedef, _IO_lock_t) #define _IO_cleanup_region_end(_doit) \ __libc_cleanup_region_end (_doit) -#if defined _LIBC && IS_IN (libc) - -# ifdef __EXCEPTIONS -# define _IO_acquire_lock(_fp) \ +#define _IO_acquire_lock(_fp) \ do { \ FILE *_IO_acquire_lock_file \ __attribute__((cleanup (_IO_acquire_lock_fct))) \ = (_fp); \ _IO_flockfile (_IO_acquire_lock_file); -# else -# define _IO_acquire_lock(_fp) _IO_acquire_lock_needs_exceptions_enabled -# endif -# define _IO_release_lock(_fp) ; } while (0) - -#endif +#define _IO_release_lock(_fp) ; } while (0) #endif /* stdio-lock.h */ ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/4] Consolidate stdio-lock.h 2022-04-27 18:35 ` Adhemerval Zanella @ 2022-04-27 18:44 ` Florian Weimer 2022-04-27 18:49 ` Adhemerval Zanella 0 siblings, 1 reply; 19+ messages in thread From: Florian Weimer @ 2022-04-27 18:44 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: Adhemerval Zanella via Libc-alpha * Adhemerval Zanella: > You are right. The change now uses __libc_cleanup_region_start from > libc-lock.h instead of the cleanup version. I am not sure about > hardening, but afaiu __libc_cleanup_push_defer should handle C++ > unwinding for fopencookie. No, C++ unwinding will right go through these frames. We only interleave in the other direction (pthread_cancel unwinding running C++ destructors). > It seems that the Hurd version is indeed to the best option, so I think for > generic it would be better to just use: > > diff --git a/sysdeps/generic/stdio-lock.h b/sysdeps/generic/stdio-lock.h > index 14cf458bdd..a42131f5a5 100644 > --- a/sysdeps/generic/stdio-lock.h > +++ b/sysdeps/generic/stdio-lock.h > @@ -45,20 +45,12 @@ __libc_lock_define_recursive (typedef, _IO_lock_t) > #define _IO_cleanup_region_end(_doit) \ > __libc_cleanup_region_end (_doit) > > -#if defined _LIBC && IS_IN (libc) > - > -# ifdef __EXCEPTIONS > -# define _IO_acquire_lock(_fp) \ > +#define _IO_acquire_lock(_fp) \ > do { \ > FILE *_IO_acquire_lock_file \ > __attribute__((cleanup (_IO_acquire_lock_fct))) \ > = (_fp); \ > _IO_flockfile (_IO_acquire_lock_file); > -# else > -# define _IO_acquire_lock(_fp) _IO_acquire_lock_needs_exceptions_enabled > -# endif > -# define _IO_release_lock(_fp) ; } while (0) > - > -#endif > +#define _IO_release_lock(_fp) ; } while (0) > > #endif /* stdio-lock.h */ I suppose so. Thanks, Florian ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/4] Consolidate stdio-lock.h 2022-04-27 18:44 ` Florian Weimer @ 2022-04-27 18:49 ` Adhemerval Zanella 0 siblings, 0 replies; 19+ messages in thread From: Adhemerval Zanella @ 2022-04-27 18:49 UTC (permalink / raw) To: Florian Weimer; +Cc: Adhemerval Zanella via Libc-alpha On 27/04/2022 15:44, Florian Weimer wrote: > * Adhemerval Zanella: > >> You are right. The change now uses __libc_cleanup_region_start from >> libc-lock.h instead of the cleanup version. I am not sure about >> hardening, but afaiu __libc_cleanup_push_defer should handle C++ >> unwinding for fopencookie. > > No, C++ unwinding will right go through these frames. We only > interleave in the other direction (pthread_cancel unwinding running C++ > destructors). Right, so Hurd version is indeed broken. > >> It seems that the Hurd version is indeed to the best option, so I think for >> generic it would be better to just use: >> >> diff --git a/sysdeps/generic/stdio-lock.h b/sysdeps/generic/stdio-lock.h >> index 14cf458bdd..a42131f5a5 100644 >> --- a/sysdeps/generic/stdio-lock.h >> +++ b/sysdeps/generic/stdio-lock.h >> @@ -45,20 +45,12 @@ __libc_lock_define_recursive (typedef, _IO_lock_t) >> #define _IO_cleanup_region_end(_doit) \ >> __libc_cleanup_region_end (_doit) >> >> -#if defined _LIBC && IS_IN (libc) >> - >> -# ifdef __EXCEPTIONS >> -# define _IO_acquire_lock(_fp) \ >> +#define _IO_acquire_lock(_fp) \ >> do { \ >> FILE *_IO_acquire_lock_file \ >> __attribute__((cleanup (_IO_acquire_lock_fct))) \ >> = (_fp); \ >> _IO_flockfile (_IO_acquire_lock_file); >> -# else >> -# define _IO_acquire_lock(_fp) _IO_acquire_lock_needs_exceptions_enabled >> -# endif >> -# define _IO_release_lock(_fp) ; } while (0) >> - >> -#endif >> +#define _IO_release_lock(_fp) ; } while (0) >> >> #endif /* stdio-lock.h */ > > I suppose so. > I will update the patch. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 3/4] Move libio lock single-thread optimization to generic libc-lock (BZ #27842) 2022-04-26 19:15 [PATCH v2 0/4] Move libio lock single-thread optimization to generic libc-lock Adhemerval Zanella 2022-04-26 19:15 ` [PATCH v2 1/4] libio: Assume _IO_MTSAFE_IO Adhemerval Zanella 2022-04-26 19:15 ` [PATCH v2 2/4] Consolidate stdio-lock.h Adhemerval Zanella @ 2022-04-26 19:15 ` Adhemerval Zanella 2022-04-27 13:30 ` Florian Weimer 2022-04-26 19:15 ` [PATCH v2 4/4] Assume _LIBC and libc module for libc-lock.h Adhemerval Zanella 3 siblings, 1 reply; 19+ messages in thread From: Adhemerval Zanella @ 2022-04-26 19:15 UTC (permalink / raw) To: libc-alpha, Florian Weimer This patch moves the single thread stdio optimization (d2e04918833d9) to the libc-lock. With generic support there is no need to add per-function code to handle the single-thread case, and it allows to removes the _IO_enable_locks (since once process goes multithread the locks will be always used). It also handles the memory streams requirement (de895ddcd7fc), since the SINGLE_THREAD_P already contains te information whether the process is multithread (so there is no need to disable the optimization because such stream are listed in _IO_list_all). Finally it also removed the flockfile uses a read-modify-write operation on _flags2 outside a lock region (BZ #27842). Checked on x86_64-linux-gnu, i686-linux-gnu, and aarch64-linux-gnu. --- libio/Versions | 3 --- libio/feof.c | 2 -- libio/ferror.c | 2 -- libio/fputc.c | 2 -- libio/genops.c | 28 ---------------------------- libio/getc.c | 2 -- libio/getchar.c | 4 +--- libio/iofopncook.c | 2 -- libio/ioungetc.c | 2 -- libio/libio.h | 4 ---- libio/memstream.c | 3 --- libio/putc.c | 2 -- libio/wmemstream.c | 3 --- nptl/pthread_create.c | 3 --- stdio-common/flockfile.c | 1 - sysdeps/mach/libc-lock.h | 9 +++++---- sysdeps/nptl/libc-lock.h | 7 ++++--- 17 files changed, 10 insertions(+), 69 deletions(-) diff --git a/libio/Versions b/libio/Versions index b91a7bc914..b528fae670 100644 --- a/libio/Versions +++ b/libio/Versions @@ -159,9 +159,6 @@ libc { # Used by NPTL and librt __libc_fatal; - # Used by NPTL - _IO_enable_locks; - __fseeko64; __ftello64; } diff --git a/libio/feof.c b/libio/feof.c index 7f79b2795e..41e7beeca7 100644 --- a/libio/feof.c +++ b/libio/feof.c @@ -32,8 +32,6 @@ _IO_feof (FILE *fp) { int result; CHECK_FILE (fp, EOF); - if (!_IO_need_lock (fp)) - return _IO_feof_unlocked (fp); _IO_flockfile (fp); result = _IO_feof_unlocked (fp); _IO_funlockfile (fp); diff --git a/libio/ferror.c b/libio/ferror.c index d2489c54d3..a60efc3f0f 100644 --- a/libio/ferror.c +++ b/libio/ferror.c @@ -32,8 +32,6 @@ _IO_ferror (FILE *fp) { int result; CHECK_FILE (fp, EOF); - if (!_IO_need_lock (fp)) - return _IO_ferror_unlocked (fp); _IO_flockfile (fp); result = _IO_ferror_unlocked (fp); _IO_funlockfile (fp); diff --git a/libio/fputc.c b/libio/fputc.c index d29a9d0aee..c3d66500d1 100644 --- a/libio/fputc.c +++ b/libio/fputc.c @@ -32,8 +32,6 @@ fputc (int c, FILE *fp) { int result; CHECK_FILE (fp, EOF); - if (!_IO_need_lock (fp)) - return _IO_putc_unlocked (c, fp); _IO_acquire_lock (fp); result = _IO_putc_unlocked (c, fp); _IO_release_lock (fp); diff --git a/libio/genops.c b/libio/genops.c index ccfbcd149d..eca50bacbc 100644 --- a/libio/genops.c +++ b/libio/genops.c @@ -488,39 +488,11 @@ _IO_init (FILE *fp, int flags) _IO_init_internal (fp, flags); } -static int stdio_needs_locking; - -/* In a single-threaded process most stdio locks can be omitted. After - _IO_enable_locks is called, locks are not optimized away any more. - It must be first called while the process is still single-threaded. - - This lock optimization can be disabled on a per-file basis by setting - _IO_FLAGS2_NEED_LOCK, because a file can have user-defined callbacks - or can be locked with flockfile and then a thread may be created - between a lock and unlock, so omitting the lock is not valid. - - Here we have to make sure that the flag is set on all existing files - and files created later. */ -void -_IO_enable_locks (void) -{ - _IO_ITER i; - - if (stdio_needs_locking) - return; - stdio_needs_locking = 1; - for (i = _IO_iter_begin (); i != _IO_iter_end (); i = _IO_iter_next (i)) - _IO_iter_file (i)->_flags2 |= _IO_FLAGS2_NEED_LOCK; -} -libc_hidden_def (_IO_enable_locks) - void _IO_old_init (FILE *fp, int flags) { fp->_flags = _IO_MAGIC|flags; fp->_flags2 = 0; - if (stdio_needs_locking) - fp->_flags2 |= _IO_FLAGS2_NEED_LOCK; fp->_IO_buf_base = NULL; fp->_IO_buf_end = NULL; fp->_IO_read_base = NULL; diff --git a/libio/getc.c b/libio/getc.c index 8c79cf702f..18647aa3e1 100644 --- a/libio/getc.c +++ b/libio/getc.c @@ -34,8 +34,6 @@ _IO_getc (FILE *fp) { int result; CHECK_FILE (fp, EOF); - if (!_IO_need_lock (fp)) - return _IO_getc_unlocked (fp); _IO_acquire_lock (fp); result = _IO_getc_unlocked (fp); _IO_release_lock (fp); diff --git a/libio/getchar.c b/libio/getchar.c index e0f4f471b3..4ab1e7b065 100644 --- a/libio/getchar.c +++ b/libio/getchar.c @@ -33,10 +33,8 @@ int getchar (void) { int result; - if (!_IO_need_lock (stdin)) - return _IO_getc_unlocked (stdin); _IO_acquire_lock (stdin); result = _IO_getc_unlocked (stdin); _IO_release_lock (stdin); return result; -} \ No newline at end of file +} diff --git a/libio/iofopncook.c b/libio/iofopncook.c index 16bcc7f4a3..5ecca41717 100644 --- a/libio/iofopncook.c +++ b/libio/iofopncook.c @@ -162,8 +162,6 @@ _IO_cookie_init (struct _IO_cookie_file *cfile, int read_write, _IO_mask_flags (&cfile->__fp.file, read_write, _IO_NO_READS+_IO_NO_WRITES+_IO_IS_APPENDING); - cfile->__fp.file._flags2 |= _IO_FLAGS2_NEED_LOCK; - /* We use a negative number different from -1 for _fileno to mark that this special stream is not associated with a real file, but still has to be treated as such. */ diff --git a/libio/ioungetc.c b/libio/ioungetc.c index 92d5572e21..547953dd16 100644 --- a/libio/ioungetc.c +++ b/libio/ioungetc.c @@ -33,8 +33,6 @@ ungetc (int c, FILE *fp) CHECK_FILE (fp, EOF); if (c == EOF) return EOF; - if (!_IO_need_lock (fp)) - return _IO_sputbackc (fp, (unsigned char) c); _IO_acquire_lock (fp); result = _IO_sputbackc (fp, (unsigned char) c); _IO_release_lock (fp); diff --git a/libio/libio.h b/libio/libio.h index 42ff6143af..e1e8ffdc54 100644 --- a/libio/libio.h +++ b/libio/libio.h @@ -88,7 +88,6 @@ typedef struct #define _IO_FLAGS2_USER_WBUF 8 #define _IO_FLAGS2_NOCLOSE 32 #define _IO_FLAGS2_CLOEXEC 64 -#define _IO_FLAGS2_NEED_LOCK 128 /* _IO_pos_BAD is an off64_t value indicating error, unknown, or EOF. */ #define _IO_pos_BAD ((off64_t) -1) @@ -212,9 +211,6 @@ extern int _IO_ftrylockfile (FILE *) __THROW; #define _IO_cleanup_region_end(_Doit) /**/ #endif -#define _IO_need_lock(_fp) \ - (((_fp)->_flags2 & _IO_FLAGS2_NEED_LOCK) != 0) - extern int _IO_vfscanf (FILE * __restrict, const char * __restrict, __gnuc_va_list, int *__restrict); extern __ssize_t _IO_padn (FILE *, int, __ssize_t); diff --git a/libio/memstream.c b/libio/memstream.c index 7ab61876ba..15022b72ee 100644 --- a/libio/memstream.c +++ b/libio/memstream.c @@ -92,9 +92,6 @@ __open_memstream (char **bufloc, size_t *sizeloc) new_f->fp.bufloc = bufloc; new_f->fp.sizeloc = sizeloc; - /* Disable single thread optimization. BZ 21735. */ - new_f->fp._sf._sbf._f._flags2 |= _IO_FLAGS2_NEED_LOCK; - return (FILE *) &new_f->fp._sf._sbf; } libc_hidden_def (__open_memstream) diff --git a/libio/putc.c b/libio/putc.c index 44e30649c9..f4cc2024b6 100644 --- a/libio/putc.c +++ b/libio/putc.c @@ -25,8 +25,6 @@ _IO_putc (int c, FILE *fp) { int result; CHECK_FILE (fp, EOF); - if (!_IO_need_lock (fp)) - return _IO_putc_unlocked (c, fp); _IO_acquire_lock (fp); result = _IO_putc_unlocked (c, fp); _IO_release_lock (fp); diff --git a/libio/wmemstream.c b/libio/wmemstream.c index 9366ef4aad..abaf421069 100644 --- a/libio/wmemstream.c +++ b/libio/wmemstream.c @@ -94,9 +94,6 @@ open_wmemstream (wchar_t **bufloc, size_t *sizeloc) new_f->fp.bufloc = bufloc; new_f->fp.sizeloc = sizeloc; - /* Disable single thread optimization. BZ 21735. */ - new_f->fp._sf._sbf._f._flags2 |= _IO_FLAGS2_NEED_LOCK; - return (FILE *) &new_f->fp._sf._sbf; } diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index e7a099acb7..4f45ea36bc 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -740,9 +740,6 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr, collect_default_sched (pd); } - if (__glibc_unlikely (__nptl_nthreads == 1)) - _IO_enable_locks (); - /* Pass the descriptor to the caller. */ *newthread = (pthread_t) pd; diff --git a/stdio-common/flockfile.c b/stdio-common/flockfile.c index a5decb450f..49f72c69ab 100644 --- a/stdio-common/flockfile.c +++ b/stdio-common/flockfile.c @@ -22,7 +22,6 @@ void __flockfile (FILE *stream) { - stream->_flags2 |= _IO_FLAGS2_NEED_LOCK; _IO_lock_lock (*stream->_lock); } weak_alias (__flockfile, flockfile); diff --git a/sysdeps/mach/libc-lock.h b/sysdeps/mach/libc-lock.h index 225eb67f5a..ee38948d1e 100644 --- a/sysdeps/mach/libc-lock.h +++ b/sysdeps/mach/libc-lock.h @@ -106,9 +106,9 @@ typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t; __libc_lock_recursive_t *const __lock = &(NAME); \ void *__self = __libc_lock_owner_self (); \ int __r = 0; \ - if (__self == __lock->owner) \ + if (!SINGLE_THREAD_P && __self == __lock->owner) \ ++__lock->cnt; \ - else if ((__r = lll_trylock (__lock->lock)) == 0) \ + else if (!SINGLE_THREAD_P && (__r = lll_trylock (__lock->lock)) == 0) \ __lock->owner = __self, __lock->cnt = 1; \ __r; \ }) @@ -117,7 +117,7 @@ typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t; ({ \ __libc_lock_recursive_t *const __lock = &(NAME); \ void *__self = __libc_lock_owner_self (); \ - if (__self != __lock->owner) \ + if (!SINGLE_THREAD_P && __self != __lock->owner) \ { \ lll_lock (__lock->lock, 0); \ __lock->owner = __self; \ @@ -132,7 +132,8 @@ typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t; if (--__lock->cnt == 0) \ { \ __lock->owner = 0; \ - lll_unlock (__lock->lock, 0); \ + if (!SINGLE_THREAD_P) \ + lll_unlock (__lock->lock, 0); \ } \ }) diff --git a/sysdeps/nptl/libc-lock.h b/sysdeps/nptl/libc-lock.h index 6c2d6acfd1..abd84e71b4 100644 --- a/sysdeps/nptl/libc-lock.h +++ b/sysdeps/nptl/libc-lock.h @@ -86,7 +86,7 @@ typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t; # define __libc_lock_lock_recursive(NAME) \ do { \ void *self = THREAD_SELF; \ - if ((NAME).owner != self) \ + if (!SINGLE_THREAD_P && (NAME).owner != self) \ { \ lll_lock ((NAME).lock, LLL_PRIVATE); \ (NAME).owner = self; \ @@ -104,7 +104,7 @@ typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t; ({ \ int result = 0; \ void *self = THREAD_SELF; \ - if ((NAME).owner != self) \ + if (!SINGLE_THREAD_P && (NAME).owner != self) \ { \ if (lll_trylock ((NAME).lock) == 0) \ { \ @@ -131,7 +131,8 @@ typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t; if (--(NAME).cnt == 0) \ { \ (NAME).owner = NULL; \ - lll_unlock ((NAME).lock, LLL_PRIVATE); \ + if (!SINGLE_THREAD_P) \ + lll_unlock ((NAME).lock, LLL_PRIVATE); \ } \ } while (0) #else -- 2.34.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/4] Move libio lock single-thread optimization to generic libc-lock (BZ #27842) 2022-04-26 19:15 ` [PATCH v2 3/4] Move libio lock single-thread optimization to generic libc-lock (BZ #27842) Adhemerval Zanella @ 2022-04-27 13:30 ` Florian Weimer 2022-04-27 16:32 ` Adhemerval Zanella 0 siblings, 1 reply; 19+ messages in thread From: Florian Weimer @ 2022-04-27 13:30 UTC (permalink / raw) To: Adhemerval Zanella via Libc-alpha * Adhemerval Zanella via Libc-alpha: > diff --git a/sysdeps/nptl/libc-lock.h b/sysdeps/nptl/libc-lock.h > index 6c2d6acfd1..abd84e71b4 100644 > --- a/sysdeps/nptl/libc-lock.h > +++ b/sysdeps/nptl/libc-lock.h > @@ -86,7 +86,7 @@ typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t; > # define __libc_lock_lock_recursive(NAME) \ > do { \ > void *self = THREAD_SELF; \ > - if ((NAME).owner != self) \ > + if (!SINGLE_THREAD_P && (NAME).owner != self) \ > { \ > lll_lock ((NAME).lock, LLL_PRIVATE); \ > (NAME).owner = self; \ > @@ -104,7 +104,7 @@ typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t; > ({ \ > int result = 0; \ > void *self = THREAD_SELF; \ > - if ((NAME).owner != self) \ > + if (!SINGLE_THREAD_P && (NAME).owner != self) \ > { \ > if (lll_trylock ((NAME).lock) == 0) \ > { \ > @@ -131,7 +131,8 @@ typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t; > if (--(NAME).cnt == 0) \ > { \ > (NAME).owner = NULL; \ > - lll_unlock ((NAME).lock, LLL_PRIVATE); \ > + if (!SINGLE_THREAD_P) \ > + lll_unlock ((NAME).lock, LLL_PRIVATE); \ > } \ > } while (0) > #else I don't think this is correct if threads are created in the lock region. Thanks, Florian ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/4] Move libio lock single-thread optimization to generic libc-lock (BZ #27842) 2022-04-27 13:30 ` Florian Weimer @ 2022-04-27 16:32 ` Adhemerval Zanella 2022-04-28 16:39 ` Adhemerval Zanella 0 siblings, 1 reply; 19+ messages in thread From: Adhemerval Zanella @ 2022-04-27 16:32 UTC (permalink / raw) To: Florian Weimer, Adhemerval Zanella via Libc-alpha On 27/04/2022 10:30, Florian Weimer wrote: > * Adhemerval Zanella via Libc-alpha: > >> diff --git a/sysdeps/nptl/libc-lock.h b/sysdeps/nptl/libc-lock.h >> index 6c2d6acfd1..abd84e71b4 100644 >> --- a/sysdeps/nptl/libc-lock.h >> +++ b/sysdeps/nptl/libc-lock.h >> @@ -86,7 +86,7 @@ typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t; >> # define __libc_lock_lock_recursive(NAME) \ >> do { \ >> void *self = THREAD_SELF; \ >> - if ((NAME).owner != self) \ >> + if (!SINGLE_THREAD_P && (NAME).owner != self) \ >> { \ >> lll_lock ((NAME).lock, LLL_PRIVATE); \ >> (NAME).owner = self; \ >> @@ -104,7 +104,7 @@ typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t; >> ({ \ >> int result = 0; \ >> void *self = THREAD_SELF; \ >> - if ((NAME).owner != self) \ >> + if (!SINGLE_THREAD_P && (NAME).owner != self) \ >> { \ >> if (lll_trylock ((NAME).lock) == 0) \ >> { \ >> @@ -131,7 +131,8 @@ typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t; >> if (--(NAME).cnt == 0) \ >> { \ >> (NAME).owner = NULL; \ >> - lll_unlock ((NAME).lock, LLL_PRIVATE); \ >> + if (!SINGLE_THREAD_P) \ >> + lll_unlock ((NAME).lock, LLL_PRIVATE); \ >> } \ >> } while (0) >> #else > > I don't think this is correct if threads are created in the lock region. I was not sure about this one and I think we the main issue in fact there is we can't use the single-thread optimization on unlock. Maybe a better option would to use a different scheme as proposed by https://hal.inria.fr/hal-01236734/document, where we can embedded lock and cnt in only one variable (as the lll_lock already does). ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/4] Move libio lock single-thread optimization to generic libc-lock (BZ #27842) 2022-04-27 16:32 ` Adhemerval Zanella @ 2022-04-28 16:39 ` Adhemerval Zanella 2022-04-28 16:56 ` Florian Weimer 0 siblings, 1 reply; 19+ messages in thread From: Adhemerval Zanella @ 2022-04-28 16:39 UTC (permalink / raw) To: Florian Weimer, Adhemerval Zanella via Libc-alpha On 27/04/2022 13:32, Adhemerval Zanella wrote: > > > On 27/04/2022 10:30, Florian Weimer wrote: >> * Adhemerval Zanella via Libc-alpha: >> >>> diff --git a/sysdeps/nptl/libc-lock.h b/sysdeps/nptl/libc-lock.h >>> index 6c2d6acfd1..abd84e71b4 100644 >>> --- a/sysdeps/nptl/libc-lock.h >>> +++ b/sysdeps/nptl/libc-lock.h >>> @@ -86,7 +86,7 @@ typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t; >>> # define __libc_lock_lock_recursive(NAME) \ >>> do { \ >>> void *self = THREAD_SELF; \ >>> - if ((NAME).owner != self) \ >>> + if (!SINGLE_THREAD_P && (NAME).owner != self) \ >>> { \ >>> lll_lock ((NAME).lock, LLL_PRIVATE); \ >>> (NAME).owner = self; \ >>> @@ -104,7 +104,7 @@ typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t; >>> ({ \ >>> int result = 0; \ >>> void *self = THREAD_SELF; \ >>> - if ((NAME).owner != self) \ >>> + if (!SINGLE_THREAD_P && (NAME).owner != self) \ >>> { \ >>> if (lll_trylock ((NAME).lock) == 0) \ >>> { \ >>> @@ -131,7 +131,8 @@ typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t; >>> if (--(NAME).cnt == 0) \ >>> { \ >>> (NAME).owner = NULL; \ >>> - lll_unlock ((NAME).lock, LLL_PRIVATE); \ >>> + if (!SINGLE_THREAD_P) \ >>> + lll_unlock ((NAME).lock, LLL_PRIVATE); \ >>> } \ >>> } while (0) >>> #else >> >> I don't think this is correct if threads are created in the lock region. > > I was not sure about this one and I think we the main issue in fact there is > we can't use the single-thread optimization on unlock. Maybe a better option > would to use a different scheme as proposed by > https://hal.inria.fr/hal-01236734/document, where we can embedded lock and > cnt in only one variable (as the lll_lock already does). Don't 99f841c441feeaa9a3d97fd91bb3d6ec8073c982 have the issue for pthread_mutex_lock ? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/4] Move libio lock single-thread optimization to generic libc-lock (BZ #27842) 2022-04-28 16:39 ` Adhemerval Zanella @ 2022-04-28 16:56 ` Florian Weimer 2022-04-28 17:14 ` Adhemerval Zanella 0 siblings, 1 reply; 19+ messages in thread From: Florian Weimer @ 2022-04-28 16:56 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: Adhemerval Zanella via Libc-alpha * Adhemerval Zanella: >>>> (NAME).owner = NULL; \ >>>> - lll_unlock ((NAME).lock, LLL_PRIVATE); \ >>>> + if (!SINGLE_THREAD_P) \ >>>> + lll_unlock ((NAME).lock, LLL_PRIVATE); \ >>>> } \ >>>> } while (0) >>>> #else >>> >>> I don't think this is correct if threads are created in the lock region. >> >> I was not sure about this one and I think we the main issue in fact there is >> we can't use the single-thread optimization on unlock. Maybe a better option >> would to use a different scheme as proposed by >> https://hal.inria.fr/hal-01236734/document, where we can embedded lock and >> cnt in only one variable (as the lll_lock already does). > > Don't 99f841c441feeaa9a3d97fd91bb3d6ec8073c982 have the issue for pthread_mutex_lock ? No, that optimization follows our documented guidance, namely: | Most applications should perform the same actions whether or not | @code{__libc_single_threaded} is true, except with less | synchronization. If this rule is followed, a process that | subsequently becomes multi-threaded is already in a consistent state. Thanks, Florian ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 3/4] Move libio lock single-thread optimization to generic libc-lock (BZ #27842) 2022-04-28 16:56 ` Florian Weimer @ 2022-04-28 17:14 ` Adhemerval Zanella 0 siblings, 0 replies; 19+ messages in thread From: Adhemerval Zanella @ 2022-04-28 17:14 UTC (permalink / raw) To: Florian Weimer; +Cc: Adhemerval Zanella via Libc-alpha On 28/04/2022 13:56, Florian Weimer wrote: > * Adhemerval Zanella: > >>>>> (NAME).owner = NULL; \ >>>>> - lll_unlock ((NAME).lock, LLL_PRIVATE); \ >>>>> + if (!SINGLE_THREAD_P) \ >>>>> + lll_unlock ((NAME).lock, LLL_PRIVATE); \ >>>>> } \ >>>>> } while (0) >>>>> #else >>>> >>>> I don't think this is correct if threads are created in the lock region. >>> >>> I was not sure about this one and I think we the main issue in fact there is >>> we can't use the single-thread optimization on unlock. Maybe a better option >>> would to use a different scheme as proposed by >>> https://hal.inria.fr/hal-01236734/document, where we can embedded lock and >>> cnt in only one variable (as the lll_lock already does). >> >> Don't 99f841c441feeaa9a3d97fd91bb3d6ec8073c982 have the issue for pthread_mutex_lock ? > > No, that optimization follows our documented guidance, namely: > > | Most applications should perform the same actions whether or not > | @code{__libc_single_threaded} is true, except with less > | synchronization. If this rule is followed, a process that > | subsequently becomes multi-threaded is already in a consistent state. > So I wonder if /* Lock the recursive named lock variable. */ #define __libc_lock_lock_recursive(NAME) \ do { \ void *self = THREAD_SELF; \ if ((NAME).owner != self) \ { \ if (SINGLE_THREAD_P) \ (NAME).lock = 1; \ else \ lll_lock ((NAME).lock, LLL_PRIVATE); \ (NAME).owner = self; \ } \ ++(NAME).cnt; \ } while (0) /* Unlock the recursive named lock variable. */ /* We do no error checking here. */ #define __libc_lock_unlock_recursive(NAME) \ do { \ if (--(NAME).cnt == 0) \ { \ (NAME).owner = NULL; \ if (SINGLE_THREAD_P) \ (NAME).lock = 0; \ else \ lll_unlock ((NAME).lock, LLL_PRIVATE); \ } \ } while (0) Or if we are bounded to keep the current practice to check for single-thread and skip locks internally. It would be good to consolidate all the internal lock usage and have the single-thread lock optimizations on all locks, not only on pthread_mutex_lock. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 4/4] Assume _LIBC and libc module for libc-lock.h 2022-04-26 19:15 [PATCH v2 0/4] Move libio lock single-thread optimization to generic libc-lock Adhemerval Zanella ` (2 preceding siblings ...) 2022-04-26 19:15 ` [PATCH v2 3/4] Move libio lock single-thread optimization to generic libc-lock (BZ #27842) Adhemerval Zanella @ 2022-04-26 19:15 ` Adhemerval Zanella 3 siblings, 0 replies; 19+ messages in thread From: Adhemerval Zanella @ 2022-04-26 19:15 UTC (permalink / raw) To: libc-alpha, Florian Weimer The libc-lock.h is not used outside glibc nor with modules different than libc or libc_malloc_debug. Checked on x86_64-linux-gnu. --- sysdeps/mach/libc-lock.h | 9 ------ sysdeps/nptl/libc-lock.h | 67 ++++++---------------------------------- 2 files changed, 10 insertions(+), 66 deletions(-) diff --git a/sysdeps/mach/libc-lock.h b/sysdeps/mach/libc-lock.h index ee38948d1e..bb1a492fa9 100644 --- a/sysdeps/mach/libc-lock.h +++ b/sysdeps/mach/libc-lock.h @@ -19,8 +19,6 @@ #ifndef _LIBC_LOCK_H #define _LIBC_LOCK_H 1 -#ifdef _LIBC - #include <tls.h> #include <lowlevellock.h> @@ -38,11 +36,6 @@ extern char __libc_lock_self0[0]; #define __libc_lock_owner_self() \ (__LIBC_NO_TLS () ? (void *)&__libc_lock_self0 : THREAD_SELF) -#else -typedef struct __libc_lock_opaque__ __libc_lock_t; -typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t; -#endif - /* Define a lock variable NAME with storage class CLASS. The lock must be initialized with __libc_lock_init before it can be used (or define it with __libc_lock_define_initialized, below). Use `extern' for CLASS to @@ -215,7 +208,6 @@ struct __libc_once /* Get once control variable. */ #define __libc_once_get(ONCE_CONTROL) ((ONCE_CONTROL).done != 0) -#ifdef _LIBC /* We need portable names for some functions. E.g., when they are used as argument to __libc_cleanup_region_start. */ #define __libc_mutex_unlock __libc_lock_unlock @@ -223,6 +215,5 @@ struct __libc_once /* Hide the definitions which are only supposed to be used inside libc in a separate file. This file is not present in the installation! */ # include <libc-lockP.h> -#endif #endif /* libc-lock.h */ diff --git a/sysdeps/nptl/libc-lock.h b/sysdeps/nptl/libc-lock.h index abd84e71b4..5c85a250d7 100644 --- a/sysdeps/nptl/libc-lock.h +++ b/sysdeps/nptl/libc-lock.h @@ -25,11 +25,7 @@ /* Mutex type. */ -#if !IS_IN (libc) && !IS_IN (libc_malloc_debug) -typedef struct { pthread_mutex_t mutex; } __libc_lock_recursive_t; -#else typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t; -#endif /* Define a lock variable NAME with storage class CLASS. The lock must be initialized with __libc_lock_init before it can be used (or define it @@ -43,68 +39,36 @@ typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t; /* Define an initialized recursive lock variable NAME with storage class CLASS. */ -#if IS_IN (libc) || IS_IN (libc_malloc_debug) -# define __libc_lock_define_initialized_recursive(CLASS, NAME) \ +#define __libc_lock_define_initialized_recursive(CLASS, NAME) \ CLASS __libc_lock_recursive_t NAME = _LIBC_LOCK_RECURSIVE_INITIALIZER; -# define _LIBC_LOCK_RECURSIVE_INITIALIZER \ +#define _LIBC_LOCK_RECURSIVE_INITIALIZER \ { LLL_LOCK_INITIALIZER, 0, NULL } -#else -# define __libc_lock_define_initialized_recursive(CLASS,NAME) \ - CLASS __libc_lock_recursive_t NAME = _LIBC_LOCK_RECURSIVE_INITIALIZER; -# define _LIBC_LOCK_RECURSIVE_INITIALIZER \ - {PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP} -#endif /* Initialize a recursive mutex. */ -#if IS_IN (libc) || IS_IN (libc_malloc_debug) -# define __libc_lock_init_recursive(NAME) \ +#define __libc_lock_init_recursive(NAME) \ ((void) ((NAME) = (__libc_lock_recursive_t) _LIBC_LOCK_RECURSIVE_INITIALIZER)) -#else -# define __libc_lock_init_recursive(NAME) \ - do { \ - if (__pthread_mutex_init != NULL) \ - { \ - pthread_mutexattr_t __attr; \ - __pthread_mutexattr_init (&__attr); \ - __pthread_mutexattr_settype (&__attr, PTHREAD_MUTEX_RECURSIVE_NP); \ - __pthread_mutex_init (&(NAME).mutex, &__attr); \ - __pthread_mutexattr_destroy (&__attr); \ - } \ - } while (0) -#endif /* Finalize recursive named lock. */ -#if IS_IN (libc) || IS_IN (libc_malloc_debug) -# define __libc_lock_fini_recursive(NAME) ((void) 0) -#else -# define __libc_lock_fini_recursive(NAME) \ - __libc_maybe_call (__pthread_mutex_destroy, (&(NAME).mutex), 0) -#endif +#define __libc_lock_fini_recursive(NAME) ((void) 0) /* Lock the recursive named lock variable. */ -#if IS_IN (libc) || IS_IN (libc_malloc_debug) -# define __libc_lock_lock_recursive(NAME) \ +#define __libc_lock_lock_recursive(NAME) \ do { \ void *self = THREAD_SELF; \ - if (!SINGLE_THREAD_P && (NAME).owner != self) \ + if (!SINGLE_THREAD_P && (NAME).owner != self) \ { \ lll_lock ((NAME).lock, LLL_PRIVATE); \ (NAME).owner = self; \ } \ ++(NAME).cnt; \ } while (0) -#else -# define __libc_lock_lock_recursive(NAME) \ - __libc_maybe_call (__pthread_mutex_lock, (&(NAME).mutex), 0) -#endif /* Try to lock the recursive named lock variable. */ -#if IS_IN (libc) || IS_IN (libc_malloc_debug) -# define __libc_lock_trylock_recursive(NAME) \ +#define __libc_lock_trylock_recursive(NAME) \ ({ \ int result = 0; \ void *self = THREAD_SELF; \ - if (!SINGLE_THREAD_P && (NAME).owner != self) \ + if (!SINGLE_THREAD_P && (NAME).owner != self) \ { \ if (lll_trylock ((NAME).lock) == 0) \ { \ @@ -118,15 +82,10 @@ typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t; ++(NAME).cnt; \ result; \ }) -#else -# define __libc_lock_trylock_recursive(NAME) \ - __libc_maybe_call (__pthread_mutex_trylock, (&(NAME).mutex), 0) -#endif /* Unlock the recursive named lock variable. */ -#if IS_IN (libc) || IS_IN (libc_malloc_debug) /* We do no error checking here. */ -# define __libc_lock_unlock_recursive(NAME) \ +#define __libc_lock_unlock_recursive(NAME) \ do { \ if (--(NAME).cnt == 0) \ { \ @@ -135,10 +94,6 @@ typedef struct { int lock; int cnt; void *owner; } __libc_lock_recursive_t; lll_unlock ((NAME).lock, LLL_PRIVATE); \ } \ } while (0) -#else -# define __libc_lock_unlock_recursive(NAME) \ - __libc_maybe_call (__pthread_mutex_unlock, (&(NAME).mutex), 0) -#endif /* Put the unwind buffer BUFFER on the per-thread callback stack. The caller must fill BUFFER->__routine and BUFFER->__arg before calling @@ -178,8 +133,6 @@ libc_hidden_proto (__libc_cleanup_pop_restore) /* Hide the definitions which are only supposed to be used inside libc in a separate file. This file is not present in the installation! */ -#ifdef _LIBC -# include "libc-lockP.h" -#endif +#include "libc-lockP.h" #endif /* libc-lock.h */ -- 2.34.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2022-04-28 17:15 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-26 19:15 [PATCH v2 0/4] Move libio lock single-thread optimization to generic libc-lock Adhemerval Zanella 2022-04-26 19:15 ` [PATCH v2 1/4] libio: Assume _IO_MTSAFE_IO Adhemerval Zanella 2022-04-27 12:34 ` Florian Weimer 2022-04-27 18:40 ` Adhemerval Zanella 2022-04-27 19:35 ` Adhemerval Zanella 2022-04-26 19:15 ` [PATCH v2 2/4] Consolidate stdio-lock.h Adhemerval Zanella 2022-04-27 13:25 ` Florian Weimer 2022-04-27 16:15 ` Adhemerval Zanella 2022-04-27 18:00 ` Florian Weimer 2022-04-27 18:35 ` Adhemerval Zanella 2022-04-27 18:44 ` Florian Weimer 2022-04-27 18:49 ` Adhemerval Zanella 2022-04-26 19:15 ` [PATCH v2 3/4] Move libio lock single-thread optimization to generic libc-lock (BZ #27842) Adhemerval Zanella 2022-04-27 13:30 ` Florian Weimer 2022-04-27 16:32 ` Adhemerval Zanella 2022-04-28 16:39 ` Adhemerval Zanella 2022-04-28 16:56 ` Florian Weimer 2022-04-28 17:14 ` Adhemerval Zanella 2022-04-26 19:15 ` [PATCH v2 4/4] Assume _LIBC and libc module for libc-lock.h Adhemerval Zanella
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).