diff mbox

Mark internal functions with attribute_hidden

Message ID CAMe9rOr39bsXStfuXoP9F8aaPyq5gSdu3WmJUQ+2gjA_AEKYpQ@mail.gmail.com
State New, archived
Headers show

Commit Message

H.J. Lu Aug. 17, 2017, 6:57 p.m. UTC
On Thu, Aug 17, 2017 at 7:01 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Thu, 17 Aug 2017, H.J. Lu wrote:
>
>> Yes, this is target independent.   It should benefit static PIE build for all
>> targets.  On x86, it bypasses GOT/PLT.
>
> Specifically, on x86 I'd consider this (making functions hidden if they
> are not exported from the shared library containing their definitions) to
> be part of the fix for bug 18822.
>

You are right.  Here is the same patch with libc.so size comparison on
i686 and x86-64.

OK for trunk?

Thanks.

Comments

H.J. Lu Aug. 22, 2017, 5:58 p.m. UTC | #1
On Thu, Aug 17, 2017 at 11:57 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Aug 17, 2017 at 7:01 AM, Joseph Myers <joseph@codesourcery.com> wrote:
>> On Thu, 17 Aug 2017, H.J. Lu wrote:
>>
>>> Yes, this is target independent.   It should benefit static PIE build for all
>>> targets.  On x86, it bypasses GOT/PLT.
>>
>> Specifically, on x86 I'd consider this (making functions hidden if they
>> are not exported from the shared library containing their definitions) to
>> be part of the fix for bug 18822.
>>
>
> You are right.  Here is the same patch with libc.so size comparison on
> i686 and x86-64.
>
> OK for trunk?
>
>

Any comments or objections?
diff mbox

Patch

From 695abbdf691d7742b17ab4801c4ffa849cc31978 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sun, 13 Aug 2017 07:00:22 -0700
Subject: [PATCH] Mark internal functions with attribute_hidden [BZ #18822]

Mark internal functions with attribute_hidden to allow direct access to
internal functions within libc.so and libc.a without using GOT nor PLT.

Size comparison of libc.so:

On x86-64:
        text	   data	    bss	    dec	    hex
Before: 1728577	  20584	  17088	1766249	 1af369
After : 1728593	  20584	  17088	1766265	 1af379

The only change is __gconv_release_shlib in iconv/gconv_dl.c is inlined
since it is hidden, which increases the code size of gconv_dl.os by 18
bytes.

On i686:
        text	   data	    bss	    dec	    hex
Before: 1869039	  11444	  11112	1891595	 1cdd0b
After : 1868635	  11444	  11112	1891191	 1cdb77

The code size is decreased by avoiding GOT/PLT for hidden functions.

	[BZ #18822]
	* iconv/gconv_int.h (__gconv_open): Add attribute_hidden.
	(__gconv_close): Likewise.
	(__gconv): Likewise.
	(__gconv_find_transform): Likewise.
	(__gconv_lookup_cache): Likewise.
	(__gconv_compare_alias_cache): Likewise.
	(__gconv_load_cache): Likewise.
	(__gconv_get_path): Likewise.
	(__gconv_close_transform): Likewise.
	(__gconv_release_cache): Likewise.
	(__gconv_find_shlib): Likewise.
	(__gconv_release_shlib): Likewise.
	(__gconv_get_builtin_trans): Likewise.
	(__gconv_compare_alias): Likewise.
	* include/dlfcn.h (_dlerror_run): Likewise.
	* include/stdio.h (__fortify_fail_abort): Likewise.
	* include/time.h (__tz_compute): Likewise.
	(__strptime_internal): Likewise.
	* intl/gettextP.h (_nl_find_domain): Likewise.
	(_nl_load_domain): Likewise.
	(_nl_find_msg): Likewise.
	* intl/plural-exp.h (FREE_EXPRESSION): Likewise.
	(EXTRACT_PLURAL_EXPRESSION): Likewise.
	* locale/coll-lookup.h (__collidx_table_lookup): Likewise.
	* resolv/gai_misc.h (__gai_enqueue_request): Likewise.
	(__gai_find_request): Likewise.
	(__gai_remove_request): Likewise.
	(__gai_notify): Likewise.
	(__gai_notify_only): Likewise.
	* sysdeps/generic/aio_misc.h (__aio_sigqueue): Likewise.
	* sysdeps/generic/ldsodefs.h (_dl_symbol_value): Likewise.
	(_dl_fini): Likewise.
	(_dl_non_dynamic_init): Likewise.
	(_dl_aux_init): Likewise.
	* sysdeps/i386/machine-gmon.h (mcount_internal): Likewise.
	* sysdeps/unix/sysv/linux/i386/olddirent.h (__old_getdents64):
	Likewise.
	* wcsmbs/wcsmbsload.h (__wcsmbs_load_conv): Likewise.
	(__wcsmbs_clone_conv): Likewise.
	(__wcsmbs_named_conv): Likewise.
---
 iconv/gconv_int.h                        | 31 ++++++++++++++++---------------
 include/dlfcn.h                          |  2 +-
 include/stdio.h                          |  2 +-
 include/time.h                           |  4 ++--
 intl/gettextP.h                          |  8 ++++----
 intl/plural-exp.h                        |  4 ++--
 locale/coll-lookup.h                     |  4 ++--
 resolv/gai_misc.h                        | 10 +++++-----
 sysdeps/generic/aio_misc.h               |  2 +-
 sysdeps/generic/ldsodefs.h               | 10 ++++++----
 sysdeps/i386/machine-gmon.h              |  2 +-
 sysdeps/unix/sysv/linux/i386/olddirent.h |  2 +-
 wcsmbs/wcsmbsload.h                      |  6 +++---
 13 files changed, 45 insertions(+), 42 deletions(-)

diff --git a/iconv/gconv_int.h b/iconv/gconv_int.h
index 85a67ad31b..52b8aad020 100644
--- a/iconv/gconv_int.h
+++ b/iconv/gconv_int.h
@@ -157,11 +157,11 @@  __libc_lock_define (extern, __gconv_lock attribute_hidden)
 /* Return in *HANDLE decriptor for transformation from FROMSET to TOSET.  */
 extern int __gconv_open (const char *toset, const char *fromset,
 			 __gconv_t *handle, int flags)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Free resources associated with transformation descriptor CD.  */
 extern int __gconv_close (__gconv_t cd)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Transform at most *INBYTESLEFT bytes from buffer starting at *INBUF
    according to rules described by CD and place up to *OUTBYTESLEFT
@@ -170,36 +170,37 @@  extern int __gconv_close (__gconv_t cd)
 extern int __gconv (__gconv_t cd, const unsigned char **inbuf,
 		    const unsigned char *inbufend, unsigned char **outbuf,
 		    unsigned char *outbufend, size_t *irreversible)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Return in *HANDLE a pointer to an array with *NSTEPS elements describing
    the single steps necessary for transformation from FROMSET to TOSET.  */
 extern int __gconv_find_transform (const char *toset, const char *fromset,
 				   struct __gconv_step **handle,
 				   size_t *nsteps, int flags)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Search for transformation in cache data.  */
 extern int __gconv_lookup_cache (const char *toset, const char *fromset,
 				 struct __gconv_step **handle, size_t *nsteps,
 				 int flags)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Compare the two name for whether they are after alias expansion the
    same.  This function uses the cache and fails if none is
    loaded.  */
 extern int __gconv_compare_alias_cache (const char *name1, const char *name2,
-					int *result) internal_function;
+					int *result)
+     internal_function attribute_hidden;
 
 /* Free data associated with a step's structure.  */
 extern void __gconv_release_step (struct __gconv_step *step)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Read all the configuration data and cache it.  */
 extern void __gconv_read_conf (void) attribute_hidden;
 
 /* Try to read module cache file.  */
-extern int __gconv_load_cache (void) internal_function;
+extern int __gconv_load_cache (void) internal_function attribute_hidden;
 
 /* Retrieve pointer to internal cache.  */
 extern void *__gconv_get_cache (void);
@@ -211,7 +212,7 @@  extern struct gconv_module *__gconv_get_modules_db (void);
 extern void *__gconv_get_alias_db (void);
 
 /* Determine the directories we are looking in.  */
-extern void __gconv_get_path (void) internal_function;
+extern void __gconv_get_path (void) internal_function attribute_hidden;
 
 /* Comparison function to search alias.  */
 extern int __gconv_alias_compare (const void *p1, const void *p2)
@@ -221,33 +222,33 @@  extern int __gconv_alias_compare (const void *p1, const void *p2)
    cause the code to be unloaded.  */
 extern int __gconv_close_transform (struct __gconv_step *steps,
 				    size_t nsteps)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Free all resources allocated for the transformation record when
    using the cache.  */
 extern void __gconv_release_cache (struct __gconv_step *steps, size_t nsteps)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Load shared object named by NAME.  If already loaded increment reference
    count.  */
 extern struct __gconv_loaded_object *__gconv_find_shlib (const char *name)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Release shared object.  If no further reference is available unload
    the object.  */
 extern void __gconv_release_shlib (struct __gconv_loaded_object *handle)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Fill STEP with information about builtin module with NAME.  */
 extern void __gconv_get_builtin_trans (const char *name,
 				       struct __gconv_step *step)
-     internal_function;
+     internal_function attribute_hidden;
 
 libc_hidden_proto (__gconv_transliterate)
 
 /* If NAME is an codeset alias expand it.  */
 extern int __gconv_compare_alias (const char *name1, const char *name2)
-     internal_function;
+     internal_function attribute_hidden;
 
 
 /* Builtin transformations.  */
diff --git a/include/dlfcn.h b/include/dlfcn.h
index 51cc1dfde8..6aad074e8e 100644
--- a/include/dlfcn.h
+++ b/include/dlfcn.h
@@ -75,7 +75,7 @@  extern void *_dl_vsym (void *handle, const char *name, const char *version,
    arranges for `dlerror' to return the error details.
    ARGS is passed as argument to OPERATE.  */
 extern int _dlerror_run (void (*operate) (void *), void *args)
-     internal_function;
+    internal_function attribute_hidden;
 
 #ifdef SHARED
 # define DL_CALLER_DECL /* Nothing */
diff --git a/include/stdio.h b/include/stdio.h
index 509447c528..5c0ef8ed24 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -100,7 +100,7 @@  extern void __libc_message (enum __libc_message_action action,
 			    const char *__fnt, ...);
 extern void __fortify_fail (const char *msg) __attribute__ ((__noreturn__));
 extern void __fortify_fail_abort (_Bool, const char *msg)
-  __attribute__ ((__noreturn__));
+  __attribute__ ((__noreturn__)) attribute_hidden;
 libc_hidden_proto (__fortify_fail)
 libc_hidden_proto (__fortify_fail_abort)
 
diff --git a/include/time.h b/include/time.h
index 9956b82eb8..958cb8600d 100644
--- a/include/time.h
+++ b/include/time.h
@@ -46,7 +46,7 @@  extern void __tzfile_default (const char *std, const char *dst,
 			      long int stdoff, long int dstoff);
 extern void __tzset_parse_tz (const char *tz);
 extern void __tz_compute (time_t timer, struct tm *tm, int use_localtime)
-     __THROW internal_function;
+     __THROW internal_function attribute_hidden;
 
 /* Subroutine of `mktime'.  Return the `time_t' representation of TP and
    normalize TP, given that a `struct tm *' maps to a `time_t' as performed
@@ -93,7 +93,7 @@  extern int __getclktck (void);
 extern char * __strptime_internal (const char *rp, const char *fmt,
 				   struct tm *tm, void *statep,
 				   locale_t locparam)
-     internal_function;
+     internal_function attribute_hidden;
 
 extern double __difftime (time_t time1, time_t time0);
 
diff --git a/intl/gettextP.h b/intl/gettextP.h
index eeb8970852..f88cd782ab 100644
--- a/intl/gettextP.h
+++ b/intl/gettextP.h
@@ -256,22 +256,22 @@  extern const char *_nl_locale_name_default (void);
 struct loaded_l10nfile *_nl_find_domain (const char *__dirname, char *__locale,
 					 const char *__domainname,
 					 struct binding *__domainbinding)
-     internal_function;
+     internal_function attribute_hidden;
 void _nl_load_domain (struct loaded_l10nfile *__domain,
 		      struct binding *__domainbinding)
-     internal_function;
+     internal_function attribute_hidden;
 
 #ifdef IN_LIBGLOCALE
 char *_nl_find_msg (struct loaded_l10nfile *domain_file,
 		    struct binding *domainbinding, const char *encoding,
 		    const char *msgid,
 		    size_t *lengthp)
-     internal_function;
+     internal_function attribute_hidden;
 #else
 char *_nl_find_msg (struct loaded_l10nfile *domain_file,
 		    struct binding *domainbinding, const char *msgid,
 		    int convert, size_t *lengthp)
-     internal_function;
+     internal_function attribute_hidden;
 #endif
 
 /* The internal variables in the standalone libintl.a must have different
diff --git a/intl/plural-exp.h b/intl/plural-exp.h
index 144aa1ed4b..af6c9b9c49 100644
--- a/intl/plural-exp.h
+++ b/intl/plural-exp.h
@@ -106,13 +106,13 @@  struct parse_args
 #endif
 
 extern void FREE_EXPRESSION (struct expression *exp)
-     internal_function;
+     internal_function attribute_hidden;
 extern int PLURAL_PARSE (struct parse_args *arg);
 extern const struct expression GERMANIC_PLURAL attribute_hidden;
 extern void EXTRACT_PLURAL_EXPRESSION (const char *nullentry,
 				       const struct expression **pluralp,
 				       unsigned long int *npluralsp)
-     internal_function;
+     internal_function attribute_hidden;
 
 #if !defined (_LIBC) && !defined (IN_LIBINTL) && !defined (IN_LIBGLOCALE)
 extern unsigned long int plural_eval (const struct expression *pexp,
diff --git a/locale/coll-lookup.h b/locale/coll-lookup.h
index d95408ad68..55a0b59236 100644
--- a/locale/coll-lookup.h
+++ b/locale/coll-lookup.h
@@ -20,8 +20,8 @@ 
 
 /* Lookup in a table of int32_t, with default value 0.  */
 extern int32_t __collidx_table_lookup (const char *table, uint32_t wc)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Lookup in a table of uint32_t, with default value 0xffffffff.  */
 extern uint32_t __collseq_table_lookup (const char *table, uint32_t wc)
-     internal_function;
+     internal_function attribute_hidden;
diff --git a/resolv/gai_misc.h b/resolv/gai_misc.h
index 6679d2b7d5..01b2f89738 100644
--- a/resolv/gai_misc.h
+++ b/resolv/gai_misc.h
@@ -76,23 +76,23 @@  extern pthread_mutex_t __gai_requests_mutex;
 
 /* Enqueue request.  */
 extern struct requestlist *__gai_enqueue_request (struct gaicb *gaicbp)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Find request on wait list.  */
 extern struct requestlist *__gai_find_request (const struct gaicb *gaicbp)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Remove request from waitlist.  */
 extern int __gai_remove_request (struct gaicb *gaicbp)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Notify initiator of request and tell this everybody listening.  */
 extern void __gai_notify (struct requestlist *req)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Notify initiator of request.  */
 extern int __gai_notify_only (struct sigevent *sigev, pid_t caller_pid)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Send the signal.  */
 extern int __gai_sigqueue (int sig, const union sigval val, pid_t caller_pid)
diff --git a/sysdeps/generic/aio_misc.h b/sysdeps/generic/aio_misc.h
index a941a68aaa..e2264ba2d4 100644
--- a/sysdeps/generic/aio_misc.h
+++ b/sysdeps/generic/aio_misc.h
@@ -41,7 +41,7 @@  typedef union
 
 /* Send the signal.  */
 extern int __aio_sigqueue (int sig, const union sigval val, pid_t caller_pid)
-     internal_function;
+     internal_function attribute_hidden;
 
 
 #endif /* aio_misc.h */
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index 49e673dd24..e4004eee78 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -912,7 +912,7 @@  extern lookup_t _dl_lookup_symbol_x (const char *undef,
 
 /* Look up symbol NAME in MAP's scope and return its run-time address.  */
 extern ElfW(Addr) _dl_symbol_value (struct link_map *map, const char *name)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Add the new link_map NEW to the end of the namespace list.  */
 extern void _dl_add_to_namespace_list (struct link_map *new, Lmid_t nsid)
@@ -968,7 +968,7 @@  extern void _dl_init (struct link_map *main_map, int argc, char **argv,
 
 /* Call the finalizer functions of all shared objects whose
    initializer functions have completed.  */
-extern void _dl_fini (void);
+extern void _dl_fini (void) attribute_hidden;
 
 /* Sort array MAPS according to dependencies of the contained objects.  */
 extern void _dl_sort_fini (struct link_map **maps, size_t nmaps, char *used,
@@ -1147,10 +1147,12 @@  extern struct link_map *_dl_find_dso_for_object (const ElfW(Addr) addr);
 rtld_hidden_proto (_dl_find_dso_for_object)
 
 /* Initialization which is normally done by the dynamic linker.  */
-extern void _dl_non_dynamic_init (void) internal_function;
+extern void _dl_non_dynamic_init (void)
+     internal_function attribute_hidden;
 
 /* Used by static binaries to check the auxiliary vector.  */
-extern void _dl_aux_init (ElfW(auxv_t) *av) internal_function;
+extern void _dl_aux_init (ElfW(auxv_t) *av)
+     internal_function attribute_hidden;
 
 
 __END_DECLS
diff --git a/sysdeps/i386/machine-gmon.h b/sysdeps/i386/machine-gmon.h
index 3e90b8c0c7..81e168e4e6 100644
--- a/sysdeps/i386/machine-gmon.h
+++ b/sysdeps/i386/machine-gmon.h
@@ -30,7 +30,7 @@ 
 #define mcount_internal __mcount_internal
 
 extern void mcount_internal (u_long frompc, u_long selfpc)
-  __attribute__ ((regparm (2)));
+  __attribute__ ((regparm (2))) attribute_hidden;
 
 #define _MCOUNT_DECL(frompc, selfpc)                \
   __attribute__ ((regparm (2)))			    \
diff --git a/sysdeps/unix/sysv/linux/i386/olddirent.h b/sysdeps/unix/sysv/linux/i386/olddirent.h
index 413f78d108..4875a1f2fa 100644
--- a/sysdeps/unix/sysv/linux/i386/olddirent.h
+++ b/sysdeps/unix/sysv/linux/i386/olddirent.h
@@ -34,7 +34,7 @@  extern struct __old_dirent64 *__old_readdir64 (DIR *__dirp);
 extern int __old_readdir64_r (DIR *__dirp, struct __old_dirent64 *__entry,
 			  struct __old_dirent64 **__result);
 extern __ssize_t __old_getdents64 (int __fd, char *__buf, size_t __nbytes)
-	internal_function;
+	internal_function attribute_hidden;
 int __old_scandir64 (const char * __dir,
 		     struct __old_dirent64 *** __namelist,
 		     int (*__selector) (const struct __old_dirent64 *),
diff --git a/wcsmbs/wcsmbsload.h b/wcsmbs/wcsmbsload.h
index 3e254a877e..eb2a2bea33 100644
--- a/wcsmbs/wcsmbsload.h
+++ b/wcsmbs/wcsmbsload.h
@@ -38,15 +38,15 @@  extern const struct gconv_fcts __wcsmbs_gconv_fcts_c attribute_hidden;
 
 /* Load conversion functions for the currently selected locale.  */
 extern void __wcsmbs_load_conv (struct __locale_data *new_category)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Clone the current `__wcsmbs_load_conv' value.  */
 extern void __wcsmbs_clone_conv (struct gconv_fcts *copy)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Find the conversion functions for converting to and from NAME.  */
 extern int __wcsmbs_named_conv (struct gconv_fcts *copy, const char *name)
-     internal_function;
+     internal_function attribute_hidden;
 
 /* Function used for the `private.cleanup' hook.  */
 extern void _nl_cleanup_ctype (struct __locale_data *)
-- 
2.13.5