Fix -Os getc_unlocked linknamespace, localplt issues (bug 15105, bug 19463)

Message ID alpine.DEB.2.20.1802161809440.7696@digraph.polyomino.org.uk
State New, archived
Headers

Commit Message

Joseph Myers Feb. 16, 2018, 6:10 p.m. UTC
  Continuing the fixes for linknamespace and localplt test failures with
-Os that arise from functions not being inlined in that case, this
patch fixes such failures for getc_unlocked.  This is relative to a
tree with <https://sourceware.org/ml/libc-alpha/2018-02/msg00442.html>
(pending review) applied, but has only trivial textual dependence on
that patch.

__getc_unlocked already exists; this patch makes it explicitly hidden,
calls it where needed for namespace reasons, adds an inline function
for it when inline functions are used and adds libc_hidden_proto /
libc_hidden_weak for getc_unlocked.

Tested for x86_64 (both without -Os to make sure that case continues
to work, and with -Os to make sure all the relevant linknamespace and
localplt test failures are resolved).  Because of other such failures
that remain after this patch, neither of the bugs can yet be closed.

2018-02-16  Joseph Myers  <joseph@codesourcery.com>

	[BZ #15105]
	[BZ #19463]
	* libio/getc_u.c (getc_unlocked): Use libc_hidden_weak.
	* include/stdio.h [!_ISOMAC] (__getc_unlocked): Use
	attribute_hidden, and define inline if [__USE_EXTERN_INLINES].
	[!_ISOMAC] (getc_unlocked): Use libc_hidden_proto.
	* misc/getttyent.c (__getttyent): Call __getc_unlocked instead of
	getc_unlocked.
	* time/tzfile.c (__tzfile_read): Likewise.
  

Comments

Joseph Myers Feb. 21, 2018, 3:54 p.m. UTC | #1
Ping.  This patch 
<https://sourceware.org/ml/libc-alpha/2018-02/msg00475.html> is pending 
review.
  
Andreas Schwab Feb. 21, 2018, 4:48 p.m. UTC | #2
On Feb 16 2018, Joseph Myers <joseph@codesourcery.com> wrote:

> 	[BZ #15105]
> 	[BZ #19463]
> 	* libio/getc_u.c (getc_unlocked): Use libc_hidden_weak.
> 	* include/stdio.h [!_ISOMAC] (__getc_unlocked): Use
> 	attribute_hidden, and define inline if [__USE_EXTERN_INLINES].
> 	[!_ISOMAC] (getc_unlocked): Use libc_hidden_proto.
> 	* misc/getttyent.c (__getttyent): Call __getc_unlocked instead of
> 	getc_unlocked.
> 	* time/tzfile.c (__tzfile_read): Likewise.

Ok.

Andreas.
  
H.J. Lu Feb. 21, 2018, 5:02 p.m. UTC | #3
On Fri, Feb 16, 2018 at 10:10 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> Continuing the fixes for linknamespace and localplt test failures with
> -Os that arise from functions not being inlined in that case, this
> patch fixes such failures for getc_unlocked.  This is relative to a
> tree with <https://sourceware.org/ml/libc-alpha/2018-02/msg00442.html>
> (pending review) applied, but has only trivial textual dependence on
> that patch.
>
> __getc_unlocked already exists; this patch makes it explicitly hidden,
> calls it where needed for namespace reasons, adds an inline function
> for it when inline functions are used and adds libc_hidden_proto /
> libc_hidden_weak for getc_unlocked.
>
> Tested for x86_64 (both without -Os to make sure that case continues
> to work, and with -Os to make sure all the relevant linknamespace and
> localplt test failures are resolved).  Because of other such failures
> that remain after this patch, neither of the bugs can yet be closed.
>
> 2018-02-16  Joseph Myers  <joseph@codesourcery.com>
>
>         [BZ #15105]
>         [BZ #19463]
>         * libio/getc_u.c (getc_unlocked): Use libc_hidden_weak.
>         * include/stdio.h [!_ISOMAC] (__getc_unlocked): Use
>         attribute_hidden, and define inline if [__USE_EXTERN_INLINES].
>         [!_ISOMAC] (getc_unlocked): Use libc_hidden_proto.
>         * misc/getttyent.c (__getttyent): Call __getc_unlocked instead of
>         getc_unlocked.
>         * time/tzfile.c (__tzfile_read): Likewise.

LGTM.

Thanks.
  

Patch

diff --git a/include/stdio.h b/include/stdio.h
index 1bf49a9..937235f 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -119,7 +119,7 @@  extern void __funlockfile (FILE *__stream) attribute_hidden;
    possible.  */
 extern int __ftrylockfile (FILE *__stream);
 
-extern int __getc_unlocked (FILE *__fp);
+extern int __getc_unlocked (FILE *__fp) attribute_hidden;
 extern wint_t __getwc_unlocked (FILE *__fp);
 
 extern int __fxprintf (FILE *__fp, const char *__fmt, ...)
@@ -187,6 +187,7 @@  libc_hidden_proto (feof_unlocked)
 extern __typeof (feof_unlocked) __feof_unlocked attribute_hidden;
 libc_hidden_proto (ferror_unlocked)
 extern __typeof (ferror_unlocked) __ferror_unlocked attribute_hidden;
+libc_hidden_proto (getc_unlocked)
 libc_hidden_proto (fmemopen)
 /* The prototype needs repeating instead of using __typeof to use
    __THROW in C++ tests.  */
@@ -219,6 +220,12 @@  __NTH (__ferror_unlocked (FILE *__stream))
 {
   return __ferror_unlocked_body (__stream);
 }
+
+__extern_inline int
+__getc_unlocked (FILE *__fp)
+{
+  return __getc_unlocked_body (__fp);
+}
 #  endif
 
 # endif /* not _ISOMAC */
diff --git a/libio/getc_u.c b/libio/getc_u.c
index 7d50332..ce2bb88 100644
--- a/libio/getc_u.c
+++ b/libio/getc_u.c
@@ -37,4 +37,5 @@  __getc_unlocked (FILE *fp)
 }
 
 weak_alias (__getc_unlocked, getc_unlocked)
+libc_hidden_weak (getc_unlocked)
 weak_alias (__getc_unlocked, fgetc_unlocked)
diff --git a/misc/getttyent.c b/misc/getttyent.c
index 0fa4ddb..92d92b0 100644
--- a/misc/getttyent.c
+++ b/misc/getttyent.c
@@ -79,7 +79,7 @@  __getttyent (void)
 		}
 		/* skip lines that are too big */
 		if (!strchr (p, '\n')) {
-			while ((c = getc_unlocked(tf)) != '\n' && c != EOF)
+			while ((c = __getc_unlocked(tf)) != '\n' && c != EOF)
 				;
 			continue;
 		}
diff --git a/time/tzfile.c b/time/tzfile.c
index 1e59fd5..3e39723 100644
--- a/time/tzfile.c
+++ b/time/tzfile.c
@@ -343,11 +343,11 @@  __tzfile_read (const char *file, size_t extra, char **extrap)
 					      sizeof (x), f) != sizeof (x),
 			    0))
 	goto lose;
-      c = getc_unlocked (f);
+      c = __getc_unlocked (f);
       if (__glibc_unlikely ((unsigned int) c > 1u))
 	goto lose;
       types[i].isdst = c;
-      c = getc_unlocked (f);
+      c = __getc_unlocked (f);
       if (__glibc_unlikely ((size_t) c > chars))
 	/* Bogus index in data file.  */
 	goto lose;
@@ -376,7 +376,7 @@  __tzfile_read (const char *file, size_t extra, char **extrap)
 
   for (i = 0; i < num_isstd; ++i)
     {
-      int c = getc_unlocked (f);
+      int c = __getc_unlocked (f);
       if (__glibc_unlikely (c == EOF))
 	goto lose;
       types[i].isstd = c != 0;
@@ -386,7 +386,7 @@  __tzfile_read (const char *file, size_t extra, char **extrap)
 
   for (i = 0; i < num_isgmt; ++i)
     {
-      int c = getc_unlocked (f);
+      int c = __getc_unlocked (f);
       if (__glibc_unlikely (c == EOF))
 	goto lose;
       types[i].isgmt = c != 0;
@@ -398,7 +398,7 @@  __tzfile_read (const char *file, size_t extra, char **extrap)
   if (sizeof (time_t) == 8 && tzspec != NULL)
     {
       /* Skip over the newline first.  */
-      if (getc_unlocked (f) != '\n'
+      if (__getc_unlocked (f) != '\n'
 	  || (__fread_unlocked (tzspec, 1, tzspec_len - 1, f)
 	      != tzspec_len - 1))
 	tzspec = NULL;
@@ -440,7 +440,7 @@  __tzfile_read (const char *file, size_t extra, char **extrap)
       char *tzstr = malloc (tzspec_len);
       if (tzstr == NULL)
 	goto lose;
-      if (getc_unlocked (f) != '\n'
+      if (__getc_unlocked (f) != '\n'
 	  || (__fread_unlocked (tzstr, 1, tzspec_len - 1, f)
 	      != tzspec_len - 1))
 	{