Use SHLIB_COMPAT for libc symbols only if building libc.so
Commit Message
On Tue, Mar 18, 2014 at 03:48:14PM -0700, Roland McGrath wrote:
> This seems worse to me. I've been vaguely noodling about some centralized
> macro regime to make the NOT_IN_*/IS_IN_* cases #if-friendly. Let's figure
> out what we want to do about those in general before considering
> SHLIB_COMPAT. I tend to think that it's a more maintainable interface to
> have '#if SHLIB_COMPAT (libfoo, ...)' be a standalone test that encompasses
> the IS_IN_libfoo test (just as it encompasses the SHARED test).
The NOT_IN_libc definition (non-definition actually) is only used to
define IS_IN_libc in cases where there is no other IS_IN_*. This can
be trivially used to set IS_IN_libc to 0 when NOT_IN_libc is set,
giving us the #if-friendly definition we want.
There is only one place (other than SHLIB_COMPAT) where IS_IN_libc is
used directly and I've fixed that up to use #if instead of #ifdef.
A further enhancement here would be to replace all uses of #ifdef
NOT_IN_libc with #if !IS_IN_libc so that NOT_IN_libc has just this one
specific usage.
I have avoided a warning on 'SHARED' by using ifdef on purpose for now
since it can be changed later if needed.
The generated code is identical before and after this change on
x86_64.
Siddhesh
* include/shlib-compat.h: Define IS_IN_libc early.
[NOT_IN_libc]: Define IS_IN_libc to 0.
* nss/nss_files/files-parse.c: Include shlib-compat.h.
[!ENTDATA]: Check for value of IS_IN_libc and not just if it
is defined.
Comments
Ping!
On Mon, Jun 30, 2014 at 04:56:44PM +0530, Siddhesh Poyarekar wrote:
> On Tue, Mar 18, 2014 at 03:48:14PM -0700, Roland McGrath wrote:
> > This seems worse to me. I've been vaguely noodling about some centralized
> > macro regime to make the NOT_IN_*/IS_IN_* cases #if-friendly. Let's figure
> > out what we want to do about those in general before considering
> > SHLIB_COMPAT. I tend to think that it's a more maintainable interface to
> > have '#if SHLIB_COMPAT (libfoo, ...)' be a standalone test that encompasses
> > the IS_IN_libfoo test (just as it encompasses the SHARED test).
>
> The NOT_IN_libc definition (non-definition actually) is only used to
> define IS_IN_libc in cases where there is no other IS_IN_*. This can
> be trivially used to set IS_IN_libc to 0 when NOT_IN_libc is set,
> giving us the #if-friendly definition we want.
>
> There is only one place (other than SHLIB_COMPAT) where IS_IN_libc is
> used directly and I've fixed that up to use #if instead of #ifdef.
>
> A further enhancement here would be to replace all uses of #ifdef
> NOT_IN_libc with #if !IS_IN_libc so that NOT_IN_libc has just this one
> specific usage.
>
> I have avoided a warning on 'SHARED' by using ifdef on purpose for now
> since it can be changed later if needed.
>
> The generated code is identical before and after this change on
> x86_64.
>
> Siddhesh
>
> * include/shlib-compat.h: Define IS_IN_libc early.
> [NOT_IN_libc]: Define IS_IN_libc to 0.
> * nss/nss_files/files-parse.c: Include shlib-compat.h.
> [!ENTDATA]: Check for value of IS_IN_libc and not just if it
> is defined.
>
> diff --git a/include/shlib-compat.h b/include/shlib-compat.h
> index fac0814..3b6a227 100644
> --- a/include/shlib-compat.h
> +++ b/include/shlib-compat.h
> @@ -23,6 +23,12 @@
>
> # include <abi-versions.h>
>
> +# ifndef NOT_IN_libc
> +# define IS_IN_libc 1
> +# else
> +# define IS_IN_libc 0
> +# endif
> +
> /* The file abi-versions.h (generated by scripts/abi-versions.awk) defines
> symbols like `ABI_libm_GLIBC_2_0' for each version set in the source
> code for each library. For a version set that is subsumed by a later
> @@ -45,10 +51,6 @@
> && (!(ABI_##lib##_##obsoleted - 0) \
> || ((ABI_##lib##_##introduced - 0) < (ABI_##lib##_##obsoleted - 0))))
>
> -# ifndef NOT_IN_libc
> -# define IS_IN_libc 1
> -# endif
> -
> /* That header also defines symbols like `VERSION_libm_GLIBC_2_1' to
> the version set name to use for e.g. symbols first introduced into
> libm in the GLIBC_2.1 version. Definitions of symbols with explicit
> diff --git a/nss/nss_files/files-parse.c b/nss/nss_files/files-parse.c
> index 1da1a6f..35a232c 100644
> --- a/nss/nss_files/files-parse.c
> +++ b/nss/nss_files/files-parse.c
> @@ -21,6 +21,7 @@
> #include <string.h>
> #include <stdlib.h>
> #include <stdint.h>
> +#include <shlib-compat.h>
>
> /* These symbols are defined by the including source file:
>
> @@ -73,12 +74,12 @@ struct parser_data
> /* Export the line parser function so it can be used in nss_db. */
> # define parser_stclass /* Global */
> # define parse_line CONCAT(_nss_files_parse_,ENTNAME)
> -# ifdef IS_IN_libc
> +# if defined SHARED && !IS_IN_libc
> +# define nss_files_parse_hidden_def(name) libnss_files_hidden_def (name)
> +# else
> /* We are defining one of the functions that actually lives in libc
> because it is used to implement fget*ent and suchlike. */
> -# define nss_files_parse_hidden_def(name) libc_hidden_def (name)
> -# else
> -# define nss_files_parse_hidden_def(name) libnss_files_hidden_def (name)
> +# define nss_files_parse_hidden_def(name) libc_hidden_def (name)
> # endif
> #endif
>
Ping!
On Tue, Jul 08, 2014 at 03:11:06PM +0530, Siddhesh Poyarekar wrote:
> Ping!
>
> On Mon, Jun 30, 2014 at 04:56:44PM +0530, Siddhesh Poyarekar wrote:
> > On Tue, Mar 18, 2014 at 03:48:14PM -0700, Roland McGrath wrote:
> > > This seems worse to me. I've been vaguely noodling about some centralized
> > > macro regime to make the NOT_IN_*/IS_IN_* cases #if-friendly. Let's figure
> > > out what we want to do about those in general before considering
> > > SHLIB_COMPAT. I tend to think that it's a more maintainable interface to
> > > have '#if SHLIB_COMPAT (libfoo, ...)' be a standalone test that encompasses
> > > the IS_IN_libfoo test (just as it encompasses the SHARED test).
> >
> > The NOT_IN_libc definition (non-definition actually) is only used to
> > define IS_IN_libc in cases where there is no other IS_IN_*. This can
> > be trivially used to set IS_IN_libc to 0 when NOT_IN_libc is set,
> > giving us the #if-friendly definition we want.
> >
> > There is only one place (other than SHLIB_COMPAT) where IS_IN_libc is
> > used directly and I've fixed that up to use #if instead of #ifdef.
> >
> > A further enhancement here would be to replace all uses of #ifdef
> > NOT_IN_libc with #if !IS_IN_libc so that NOT_IN_libc has just this one
> > specific usage.
> >
> > I have avoided a warning on 'SHARED' by using ifdef on purpose for now
> > since it can be changed later if needed.
> >
> > The generated code is identical before and after this change on
> > x86_64.
> >
> > Siddhesh
> >
> > * include/shlib-compat.h: Define IS_IN_libc early.
> > [NOT_IN_libc]: Define IS_IN_libc to 0.
> > * nss/nss_files/files-parse.c: Include shlib-compat.h.
> > [!ENTDATA]: Check for value of IS_IN_libc and not just if it
> > is defined.
> >
> > diff --git a/include/shlib-compat.h b/include/shlib-compat.h
> > index fac0814..3b6a227 100644
> > --- a/include/shlib-compat.h
> > +++ b/include/shlib-compat.h
> > @@ -23,6 +23,12 @@
> >
> > # include <abi-versions.h>
> >
> > +# ifndef NOT_IN_libc
> > +# define IS_IN_libc 1
> > +# else
> > +# define IS_IN_libc 0
> > +# endif
> > +
> > /* The file abi-versions.h (generated by scripts/abi-versions.awk) defines
> > symbols like `ABI_libm_GLIBC_2_0' for each version set in the source
> > code for each library. For a version set that is subsumed by a later
> > @@ -45,10 +51,6 @@
> > && (!(ABI_##lib##_##obsoleted - 0) \
> > || ((ABI_##lib##_##introduced - 0) < (ABI_##lib##_##obsoleted - 0))))
> >
> > -# ifndef NOT_IN_libc
> > -# define IS_IN_libc 1
> > -# endif
> > -
> > /* That header also defines symbols like `VERSION_libm_GLIBC_2_1' to
> > the version set name to use for e.g. symbols first introduced into
> > libm in the GLIBC_2.1 version. Definitions of symbols with explicit
> > diff --git a/nss/nss_files/files-parse.c b/nss/nss_files/files-parse.c
> > index 1da1a6f..35a232c 100644
> > --- a/nss/nss_files/files-parse.c
> > +++ b/nss/nss_files/files-parse.c
> > @@ -21,6 +21,7 @@
> > #include <string.h>
> > #include <stdlib.h>
> > #include <stdint.h>
> > +#include <shlib-compat.h>
> >
> > /* These symbols are defined by the including source file:
> >
> > @@ -73,12 +74,12 @@ struct parser_data
> > /* Export the line parser function so it can be used in nss_db. */
> > # define parser_stclass /* Global */
> > # define parse_line CONCAT(_nss_files_parse_,ENTNAME)
> > -# ifdef IS_IN_libc
> > +# if defined SHARED && !IS_IN_libc
> > +# define nss_files_parse_hidden_def(name) libnss_files_hidden_def (name)
> > +# else
> > /* We are defining one of the functions that actually lives in libc
> > because it is used to implement fget*ent and suchlike. */
> > -# define nss_files_parse_hidden_def(name) libc_hidden_def (name)
> > -# else
> > -# define nss_files_parse_hidden_def(name) libnss_files_hidden_def (name)
> > +# define nss_files_parse_hidden_def(name) libc_hidden_def (name)
> > # endif
> > #endif
> >
>
>
@@ -23,6 +23,12 @@
# include <abi-versions.h>
+# ifndef NOT_IN_libc
+# define IS_IN_libc 1
+# else
+# define IS_IN_libc 0
+# endif
+
/* The file abi-versions.h (generated by scripts/abi-versions.awk) defines
symbols like `ABI_libm_GLIBC_2_0' for each version set in the source
code for each library. For a version set that is subsumed by a later
@@ -45,10 +51,6 @@
&& (!(ABI_##lib##_##obsoleted - 0) \
|| ((ABI_##lib##_##introduced - 0) < (ABI_##lib##_##obsoleted - 0))))
-# ifndef NOT_IN_libc
-# define IS_IN_libc 1
-# endif
-
/* That header also defines symbols like `VERSION_libm_GLIBC_2_1' to
the version set name to use for e.g. symbols first introduced into
libm in the GLIBC_2.1 version. Definitions of symbols with explicit
@@ -21,6 +21,7 @@
#include <string.h>
#include <stdlib.h>
#include <stdint.h>
+#include <shlib-compat.h>
/* These symbols are defined by the including source file:
@@ -73,12 +74,12 @@ struct parser_data
/* Export the line parser function so it can be used in nss_db. */
# define parser_stclass /* Global */
# define parse_line CONCAT(_nss_files_parse_,ENTNAME)
-# ifdef IS_IN_libc
+# if defined SHARED && !IS_IN_libc
+# define nss_files_parse_hidden_def(name) libnss_files_hidden_def (name)
+# else
/* We are defining one of the functions that actually lives in libc
because it is used to implement fget*ent and suchlike. */
-# define nss_files_parse_hidden_def(name) libc_hidden_def (name)
-# else
-# define nss_files_parse_hidden_def(name) libnss_files_hidden_def (name)
+# define nss_files_parse_hidden_def(name) libc_hidden_def (name)
# endif
#endif