From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id BB6A73858D28 for ; Fri, 17 Dec 2021 17:44:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BB6A73858D28 Received: from pps.filterd (m0098416.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 1BHG3IxS007692 for ; Fri, 17 Dec 2021 17:44:52 GMT Received: from ppma02dal.us.ibm.com (a.bd.3ea9.ip4.static.sl-reverse.com [169.62.189.10]) by mx0b-001b2d01.pphosted.com with ESMTP id 3d08snvkvt-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 17 Dec 2021 17:44:52 +0000 Received: from pps.filterd (ppma02dal.us.ibm.com [127.0.0.1]) by ppma02dal.us.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 1BHHSga2027042 for ; Fri, 17 Dec 2021 17:44:51 GMT Received: from b01cxnp22033.gho.pok.ibm.com (b01cxnp22033.gho.pok.ibm.com [9.57.198.23]) by ppma02dal.us.ibm.com with ESMTP id 3cy7fxdx9a-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 17 Dec 2021 17:44:51 +0000 Received: from b01ledav003.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com [9.57.199.108]) by b01cxnp22033.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 1BHHioQB35193326 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 17 Dec 2021 17:44:50 GMT Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 4FCE6B2072; Fri, 17 Dec 2021 17:44:50 +0000 (GMT) Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 0920BB2064; Fri, 17 Dec 2021 17:44:49 +0000 (GMT) Received: from [9.65.241.23] (unknown [9.65.241.23]) by b01ledav003.gho.pok.ibm.com (Postfix) with ESMTP; Fri, 17 Dec 2021 17:44:49 +0000 (GMT) Message-ID: Date: Fri, 17 Dec 2021 11:44:49 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0 Subject: Re: [PATCH] Apply asm redirections in stdio.h before first use [BZ #27087] Content-Language: en-US To: Tulio Magno Quites Machado Filho , libc-alpha@sourceware.org References: <20211215192352.628453-1-tuliom@linux.ibm.com> From: Paul E Murphy In-Reply-To: <20211215192352.628453-1-tuliom@linux.ibm.com> Content-Type: text/plain; charset=UTF-8; format=flowed X-TM-AS-GCONF: 00 X-Proofpoint-ORIG-GUID: oW1iWdBbK3qx38GWV3xYPoRotN6hqdME X-Proofpoint-GUID: oW1iWdBbK3qx38GWV3xYPoRotN6hqdME Content-Transfer-Encoding: 7bit X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.790,Hydra:6.0.425,FMLib:17.11.62.513 definitions=2021-12-17_07,2021-12-16_01,2021-12-02_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 suspectscore=0 phishscore=0 mlxlogscore=999 spamscore=0 priorityscore=1501 impostorscore=0 mlxscore=0 malwarescore=0 adultscore=0 lowpriorityscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000 definitions=main-2112170100 X-Spam-Status: No, score=-13.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Dec 2021 17:44:54 -0000 On 12/15/21 1:23 PM, Tulio Magno Quites Machado Filho via Libc-alpha wrote: > Compilers may not be able to apply asm redirections to functions after > these functions are used for the first time, e.g. clang 13. > Fix [BZ #27087] by applying all long double-related asm redirections > before using functions in bits/stdio.h. > However, as these asm redirections depend on the declarations provided > by libio/bits/stdio2.h, this header was split in 2: > > - libio/bits/stdio2-dec.h contains all function declarations; > - libio/bits/stdio2.h remains with the remaining contents, including > redirections. > --- > include/bits/stdio2-dec.h | 1 + > libio/Makefile | 2 +- > libio/bits/stdio2-dec.h | 110 ++++++++++++++++++++++++++++++++++++++ > libio/bits/stdio2.h | 62 --------------------- > libio/stdio.h | 18 +++++-- > 5 files changed, 125 insertions(+), 68 deletions(-) > create mode 100644 include/bits/stdio2-dec.h > create mode 100644 libio/bits/stdio2-dec.h > > diff --git a/include/bits/stdio2-dec.h b/include/bits/stdio2-dec.h > new file mode 100644 > index 0000000000..4d74e4bf82 > --- /dev/null > +++ b/include/bits/stdio2-dec.h > @@ -0,0 +1 @@ > +#include > diff --git a/libio/Makefile b/libio/Makefile > index 5336b7d595..19e2143d11 100644 > --- a/libio/Makefile > +++ b/libio/Makefile > @@ -23,7 +23,7 @@ subdir := libio > include ../Makeconfig > > headers := stdio.h \ > - bits/stdio.h bits/stdio2.h bits/stdio-ldbl.h \ > + bits/stdio.h bits/stdio2.h bits/stdio2-dec.h bits/stdio-ldbl.h \ > bits/types/FILE.h bits/types/__FILE.h bits/types/struct_FILE.h \ > bits/types/__fpos_t.h bits/types/__fpos64_t.h \ > bits/types/cookie_io_functions_t.h > diff --git a/libio/bits/stdio2-dec.h b/libio/bits/stdio2-dec.h > new file mode 100644 > index 0000000000..80ab10bccf > --- /dev/null > +++ b/libio/bits/stdio2-dec.h > @@ -0,0 +1,110 @@ > +/* Checking macros for stdio functions. Declarations only. > + Copyright (C) 2004-2021 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 > + . */ > + > +#ifndef _BITS_STDIO2_DEC_H > +#define _BITS_STDIO2_DEC_H 1 > + > +#ifndef _STDIO_H > +# error "Never include directly; use instead." > +#endif > + > +extern int __sprintf_chk (char *__restrict __s, int __flag, size_t __slen, > + const char *__restrict __format, ...) __THROW > + __attr_access ((__write_only__, 1, 3)); > +extern int __vsprintf_chk (char *__restrict __s, int __flag, size_t __slen, > + const char *__restrict __format, > + __gnuc_va_list __ap) __THROW > + __attr_access ((__write_only__, 1, 3));OK. > + > +#if defined __USE_ISOC99 || defined __USE_UNIX98 > + > +extern int __snprintf_chk (char *__restrict __s, size_t __n, int __flag, > + size_t __slen, const char *__restrict __format, > + ...) __THROW > + __attr_access ((__write_only__, 1, 2)); > +extern int __vsnprintf_chk (char *__restrict __s, size_t __n, int __flag, > + size_t __slen, const char *__restrict __format, > + __gnuc_va_list __ap) __THROW; > + > +#endif > + OK. > +#if __USE_FORTIFY_LEVEL > 1 > + > +extern int __fprintf_chk (FILE *__restrict __stream, int __flag, > + const char *__restrict __format, ...); > +extern int __printf_chk (int __flag, const char *__restrict __format, ...); > +extern int __vfprintf_chk (FILE *__restrict __stream, int __flag, > + const char *__restrict __format, __gnuc_va_list __ap); > +extern int __vprintf_chk (int __flag, const char *__restrict __format, > + __gnuc_va_list __ap); > + > +# ifdef __USE_XOPEN2K8 > +extern int __dprintf_chk (int __fd, int __flag, const char *__restrict __fmt, > + ...) __attribute__ ((__format__ (__printf__, 3, 4))); > +extern int __vdprintf_chk (int __fd, int __flag, > + const char *__restrict __fmt, __gnuc_va_list __arg) > + __attribute__ ((__format__ (__printf__, 3, 0))); > +# endif > + > +# ifdef __USE_GNU > + > +extern int __asprintf_chk (char **__restrict __ptr, int __flag, > + const char *__restrict __fmt, ...) > + __THROW __attribute__ ((__format__ (__printf__, 3, 4))) __wur; > +extern int __vasprintf_chk (char **__restrict __ptr, int __flag, > + const char *__restrict __fmt, __gnuc_va_list __arg) > + __THROW __attribute__ ((__format__ (__printf__, 3, 0))) __wur; > +extern int __obstack_printf_chk (struct obstack *__restrict __obstack, > + int __flag, const char *__restrict __format, > + ...) > + __THROW __attribute__ ((__format__ (__printf__, 3, 4))); > +extern int __obstack_vprintf_chk (struct obstack *__restrict __obstack, > + int __flag, > + const char *__restrict __format, > + __gnuc_va_list __args) > + __THROW __attribute__ ((__format__ (__printf__, 3, 0))); > + > +# endif > +#endif OK. > + > +#if __GLIBC_USE (DEPRECATED_GETS) > +extern char *__gets_chk (char *__str, size_t) __wur; > +#endif OK. > + > +extern char *__fgets_chk (char *__restrict __s, size_t __size, int __n, > + FILE *__restrict __stream) > + __wur __attr_access ((__write_only__, 1, 3)); > + OK. > +extern size_t __fread_chk (void *__restrict __ptr, size_t __ptrlen, > + size_t __size, size_t __n, > + FILE *__restrict __stream) __wur; > + OK. > +#ifdef __USE_GNU > +extern char *__fgets_unlocked_chk (char *__restrict __s, size_t __size, > + int __n, FILE *__restrict __stream) > + __wur __attr_access ((__write_only__, 1, 3)); > +#endif OK. > + > +#ifdef __USE_MISC > +# undef fread_unlocked > +extern size_t __fread_unlocked_chk (void *__restrict __ptr, size_t __ptrlen, > + size_t __size, size_t __n, > + FILE *__restrict __stream) __wur; > +#endif > + > +#endif /* bits/stdio2-dec.h. */ OK. > diff --git a/libio/bits/stdio2.h b/libio/bits/stdio2.h > index 40ff16b01b..4570f86a44 100644 > --- a/libio/bits/stdio2.h > +++ b/libio/bits/stdio2.h > @@ -23,14 +23,6 @@ > # error "Never include directly; use instead." > #endif > > -extern int __sprintf_chk (char *__restrict __s, int __flag, size_t __slen, > - const char *__restrict __format, ...) __THROW > - __attr_access ((__write_only__, 1, 3)); > -extern int __vsprintf_chk (char *__restrict __s, int __flag, size_t __slen, > - const char *__restrict __format, > - __gnuc_va_list __ap) __THROW > - __attr_access ((__write_only__, 1, 3)); > - OK. > #ifdef __va_arg_pack > __fortify_function int > __NTH (sprintf (char *__restrict __s, const char *__restrict __fmt, ...)) > @@ -54,15 +46,6 @@ __NTH (vsprintf (char *__restrict __s, const char *__restrict __fmt, > } > > #if defined __USE_ISOC99 || defined __USE_UNIX98 > - > -extern int __snprintf_chk (char *__restrict __s, size_t __n, int __flag, > - size_t __slen, const char *__restrict __format, > - ...) __THROW > - __attr_access ((__write_only__, 1, 2)); > -extern int __vsnprintf_chk (char *__restrict __s, size_t __n, int __flag, > - size_t __slen, const char *__restrict __format, > - __gnuc_va_list __ap) __THROW; > - OK. > # ifdef __va_arg_pack > __fortify_function int > __NTH (snprintf (char *__restrict __s, size_t __n, > @@ -89,15 +72,6 @@ __NTH (vsnprintf (char *__restrict __s, size_t __n, > #endif > > #if __USE_FORTIFY_LEVEL > 1 > - > -extern int __fprintf_chk (FILE *__restrict __stream, int __flag, > - const char *__restrict __format, ...); > -extern int __printf_chk (int __flag, const char *__restrict __format, ...); > -extern int __vfprintf_chk (FILE *__restrict __stream, int __flag, > - const char *__restrict __format, __gnuc_va_list __ap); > -extern int __vprintf_chk (int __flag, const char *__restrict __format, > - __gnuc_va_list __ap); > -OK. > # ifdef __va_arg_pack > __fortify_function int > fprintf (FILE *__restrict __stream, const char *__restrict __fmt, ...) > @@ -136,12 +110,6 @@ vfprintf (FILE *__restrict __stream, > } > > # ifdef __USE_XOPEN2K8 > -extern int __dprintf_chk (int __fd, int __flag, const char *__restrict __fmt, > - ...) __attribute__ ((__format__ (__printf__, 3, 4))); > -extern int __vdprintf_chk (int __fd, int __flag, > - const char *__restrict __fmt, __gnuc_va_list __arg) > - __attribute__ ((__format__ (__printf__, 3, 0))); > - OK. > # ifdef __va_arg_pack > __fortify_function int > dprintf (int __fd, const char *__restrict __fmt, ...) > @@ -162,23 +130,6 @@ vdprintf (int __fd, const char *__restrict __fmt, __gnuc_va_list __ap) > # endif > > # ifdef __USE_GNU > - > -extern int __asprintf_chk (char **__restrict __ptr, int __flag, > - const char *__restrict __fmt, ...) > - __THROW __attribute__ ((__format__ (__printf__, 3, 4))) __wur; > -extern int __vasprintf_chk (char **__restrict __ptr, int __flag, > - const char *__restrict __fmt, __gnuc_va_list __arg) > - __THROW __attribute__ ((__format__ (__printf__, 3, 0))) __wur; > -extern int __obstack_printf_chk (struct obstack *__restrict __obstack, > - int __flag, const char *__restrict __format, > - ...) > - __THROW __attribute__ ((__format__ (__printf__, 3, 4))); > -extern int __obstack_vprintf_chk (struct obstack *__restrict __obstack, > - int __flag, > - const char *__restrict __format, > - __gnuc_va_list __args) > - __THROW __attribute__ ((__format__ (__printf__, 3, 0))); > - OK. > # ifdef __va_arg_pack > __fortify_function int > __NTH (asprintf (char **__restrict __ptr, const char *__restrict __fmt, ...)) > @@ -231,7 +182,6 @@ __NTH (obstack_vprintf (struct obstack *__restrict __obstack, > #endif > > #if __GLIBC_USE (DEPRECATED_GETS) > -extern char *__gets_chk (char *__str, size_t) __wur; OK. > extern char *__REDIRECT (__gets_warn, (char *__str), gets) > __wur __warnattr ("please use fgets or getline instead, gets can't " > "specify buffer size"); > @@ -245,9 +195,6 @@ gets (char *__str) > } > #endif > > -extern char *__fgets_chk (char *__restrict __s, size_t __size, int __n, > - FILE *__restrict __stream) > - __wur __attr_access ((__write_only__, 1, 3)); OK. > extern char *__REDIRECT (__fgets_alias, > (char *__restrict __s, int __n, > FILE *__restrict __stream), fgets) > @@ -269,9 +216,6 @@ fgets (char *__restrict __s, int __n, FILE *__restrict __stream) > return __fgets_chk (__s, sz, __n, __stream); > } > > -extern size_t __fread_chk (void *__restrict __ptr, size_t __ptrlen, > - size_t __size, size_t __n, > - FILE *__restrict __stream) __wur; OK. > extern size_t __REDIRECT (__fread_alias, > (void *__restrict __ptr, size_t __size, > size_t __n, FILE *__restrict __stream), > @@ -297,9 +241,6 @@ fread (void *__restrict __ptr, size_t __size, size_t __n, > } > > #ifdef __USE_GNU > -extern char *__fgets_unlocked_chk (char *__restrict __s, size_t __size, > - int __n, FILE *__restrict __stream) > - __wur __attr_access ((__write_only__, 1, 3)); OK. > extern char *__REDIRECT (__fgets_unlocked_alias, > (char *__restrict __s, int __n, > FILE *__restrict __stream), fgets_unlocked) > @@ -324,9 +265,6 @@ fgets_unlocked (char *__restrict __s, int __n, FILE *__restrict __stream) > > #ifdef __USE_MISC > # undef fread_unlocked > -extern size_t __fread_unlocked_chk (void *__restrict __ptr, size_t __ptrlen, > - size_t __size, size_t __n, > - FILE *__restrict __stream) __wur; OK. > extern size_t __REDIRECT (__fread_unlocked_alias, > (void *__restrict __ptr, size_t __size, > size_t __n, FILE *__restrict __stream), > diff --git a/libio/stdio.h b/libio/stdio.h > index f208f5ef79..da763c9f1b 100644 > --- a/libio/stdio.h > +++ b/libio/stdio.h > @@ -885,20 +885,28 @@ extern void funlockfile (FILE *__stream) __THROW; > extern int __uflow (FILE *); > extern int __overflow (FILE *, int); > > +#if __USE_FORTIFY_LEVEL > 0 && defined __fortify_function > +/* Declare all functions from bits/stdio2-dec.h first. */ > +# include > +#endif > + > +/* The following headers provide asm redirections. These redirections must > + appear before the first usage of these functions, e.g. in bits/stdio.h. */ > +#include OK. I think you are just moving the existing code, but is this include redundant? It seems to happen a bit earlier around line 430. > +#if defined __LDBL_COMPAT || __LDOUBLE_REDIRECTS_TO_FLOAT128_ABI == 1 > +# include > +#endif > + > /* If we are compiling with optimizing read this file. It contains > several optimizing inline functions and macros. */ > #ifdef __USE_EXTERN_INLINES > # include > #endif > #if __USE_FORTIFY_LEVEL > 0 && defined __fortify_function > +/* Now include the function definitions and redirects too. */ > # include > #endif > > -#include > -#if defined __LDBL_COMPAT || __LDOUBLE_REDIRECTS_TO_FLOAT128_ABI == 1 > -# include > -#endif > - OK. > __END_DECLS > > #endif /* included. */ > One minor question about the new file name, is "dec" meant to read as "declaration"? This LGTM provided it passes build-many-glibc.py.