bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] Avoid having GNULIB_NAMESPACE::func always inject references to


From: Pedro Alves
Subject: [PATCH] Avoid having GNULIB_NAMESPACE::func always inject references to rpl_func
Date: Sat, 12 Nov 2016 16:22:54 +0000

Currently, with C++ namespace support enabled, by defining
GNULIB_NAMESPACE, the replacement symbols put in the GNULIB_NAMESPACE
namespace are function pointers that point to the rpl_func functions.
This means that the linker must pull in the rpl_func functions, even
when "GNULIB_NAMESPACE::func(....)" is not called anywhere in the
program.

Fix that by making the GNULIB_NAMESPACE::func symbols be C++ (empty)
objects with inline conversion operators instead of function pointers
that point to the replacement.

This ends up addressing the exiting comment about optimization as
well:

~~~~~~
  /* If we were to write
       rettype (*const func) parameters = ::func;
     like above in _GL_CXXALIAS_RPL_1, the compiler could optimize calls
     better (remove an indirection through a 'static' pointer variable),
~~~~~~

Original motivation:

Now that GDB is a C++ program, we stumble on the fact that gnulib
defines its replacements as '#define func rpl_func' macros, which get
in the way of making use of "func" as name of unrelated symbols, in
classes, namespaces, etc.

Gnulib's C++ namespace support was designed to solve that problem, and
I tried enabling it in GDB.  After tweaking all replacement function
calls to use the appropriate namespace [e.g., printf(....); ->
gnulib::printf(....);], I stumbled on another problem: gdbserver's
in-process agent (IPA) shared library fails to link at -O0, with:

 tracepoint-ipa.o:(.data.rel.ro+0x0): undefined reference to `rpl_mbrtowc'
 format-ipa.o:(.data.rel.ro+0x0): undefined reference to `rpl_mbrtowc'
 utils-ipa.o:(.data.rel.ro+0x0): undefined reference to `rpl_mbrtowc'
 regcache-ipa.o:(.data.rel.ro+0x0): undefined reference to `rpl_mbrtowc'
 remote-utils-ipa.o:(.data.rel.ro+0x0): more undefined references to 
`rpl_mbrtowc' follow
 collect2: error: ld returned 1 exit status
 Makefile:327: recipe for target 'libinproctrace.so' failed
 make: *** [libinproctrace.so] Error 1

This is even though gdbserver's IPA DSO doesn't call mbrtowc anywhere.

Now, the IPA does not link with gnulib purposedly -- it's designed to
be injected in the inferior and be a bridge for running async-signal
safe code that GDB JITs and injects in the inferior's address space.
I.e., it doesn't require much out of the inferior's runtime.  Still,
it's very convenient that it shares a lot of code with gdb and
gdbserver, and that in turn means using gnulib headers and some gnulib
header-only features.

Making the IPA link with gnulib would fix this, of course, but that'd
require building gnulib as -fPIC, which we don't do currently, all for
satisfying a link dependency on a function that we don't actually want
to use.

In any case, ending up with strong references to replacement functions
even if they're not used means that binaries end up containing gnulib
code they don't really use.  So this seems like a good change even not
considering the GDB's IPA.

ChangeLog:
2016-11-12  Pedro Alves  <address@hidden>

        Avoid having GNULIB_NAMESPACE::func always inject references to
        rpl_func.
        * build-aux/snippet/c++defs.h [__cplusplus && GNULIB_NAMESPACE]
        (_GL_CXXALIAS_RPL_1, _GL_CXXALIAS_RPL_CAST_1, _GL_CXXALIAS_SYS)
        (_GL_CXXALIAS_SYS_CAST, _GL_CXXALIAS_SYS_CAST2): Define a wrapper
        struct instead of a function pointer.
---
 build-aux/snippet/c++defs.h | 66 +++++++++++++++++++++++++++++++--------------
 1 file changed, 46 insertions(+), 20 deletions(-)

diff --git a/build-aux/snippet/c++defs.h b/build-aux/snippet/c++defs.h
index 5f19630..8fc95e3 100644
--- a/build-aux/snippet/c++defs.h
+++ b/build-aux/snippet/c++defs.h
@@ -111,14 +111,22 @@
    that redirects to rpl_func, if GNULIB_NAMESPACE is defined.
    Example:
      _GL_CXXALIAS_RPL (open, int, (const char *filename, int flags, ...));
- */
+
+   Wrapping rpl_func in an object with an inline conversion operator
+   avoids a reference to rpl_func unless GNULIB_NAMESPACE::func is
+   actually used in the program.  */
 #define _GL_CXXALIAS_RPL(func,rettype,parameters) \
   _GL_CXXALIAS_RPL_1 (func, rpl_##func, rettype, parameters)
 #if defined __cplusplus && defined GNULIB_NAMESPACE
 # define _GL_CXXALIAS_RPL_1(func,rpl_func,rettype,parameters) \
     namespace GNULIB_NAMESPACE                                \
     {                                                         \
-      rettype (*const func) parameters = ::rpl_func;          \
+      static const struct _gl_ ## func ## _wrapper            \
+      {                                                       \
+        typedef rettype (*type) parameters;                   \
+        inline type rpl () const { return ::rpl_func; }       \
+        inline operator type () const  { return rpl (); }     \
+      } func = {};                                            \
     }                                                         \
     _GL_EXTERN_C int _gl_cxxalias_dummy
 #else
@@ -135,8 +143,13 @@
 # define _GL_CXXALIAS_RPL_CAST_1(func,rpl_func,rettype,parameters) \
     namespace GNULIB_NAMESPACE                                     \
     {                                                              \
-      rettype (*const func) parameters =                           \
-        reinterpret_cast<rettype(*)parameters>(::rpl_func);        \
+      static const struct _gl_ ## func ## _wrapper                 \
+      {                                                            \
+        typedef rettype (*type) parameters;                        \
+        inline type rpl () const                                   \
+        { return reinterpret_cast<type>(::rpl_func); }             \
+        inline operator type () const { return rpl (); }           \
+      } func = {};                                                 \
     }                                                              \
     _GL_EXTERN_C int _gl_cxxalias_dummy
 #else
@@ -150,18 +163,20 @@
    is defined.
    Example:
      _GL_CXXALIAS_SYS (open, int, (const char *filename, int flags, ...));
- */
+
+   Wrapping func in an object with an inline conversion operator
+   avoids a reference to func unless GNULIB_NAMESPACE::func is
+   actually used in the program.  */
 #if defined __cplusplus && defined GNULIB_NAMESPACE
-  /* If we were to write
-       rettype (*const func) parameters = ::func;
-     like above in _GL_CXXALIAS_RPL_1, the compiler could optimize calls
-     better (remove an indirection through a 'static' pointer variable),
-     but then the _GL_CXXALIASWARN macro below would cause a warning not only
-     for uses of ::func but also for uses of GNULIB_NAMESPACE::func.  */
-# define _GL_CXXALIAS_SYS(func,rettype,parameters) \
-    namespace GNULIB_NAMESPACE                     \
-    {                                              \
-      static rettype (*func) parameters = ::func;  \
+# define _GL_CXXALIAS_SYS(func,rettype,parameters)            \
+    namespace GNULIB_NAMESPACE                                \
+    {                                                         \
+      static const struct _gl_ ## func ## _wrapper            \
+      {                                                       \
+        typedef rettype (*type) parameters;                   \
+        inline type rpl () const { return ::func; }           \
+        inline operator type () const { return rpl (); }      \
+      } func = {};                                            \
     }                                              \
     _GL_EXTERN_C int _gl_cxxalias_dummy
 #else
@@ -178,8 +193,13 @@
 # define _GL_CXXALIAS_SYS_CAST(func,rettype,parameters) \
     namespace GNULIB_NAMESPACE                          \
     {                                                   \
-      static rettype (*func) parameters =               \
-        reinterpret_cast<rettype(*)parameters>(::func); \
+      static const struct _gl_ ## func ## _wrapper      \
+      {                                                 \
+        typedef rettype (*type) parameters;             \
+        inline type rpl () const                        \
+        { return reinterpret_cast<type>(::func); }      \
+        inline operator type () const { return rpl (); }\
+      } func = {};                                      \
     }                                                   \
     _GL_EXTERN_C int _gl_cxxalias_dummy
 #else
@@ -202,9 +222,15 @@
 # define _GL_CXXALIAS_SYS_CAST2(func,rettype,parameters,rettype2,parameters2) \
     namespace GNULIB_NAMESPACE                                                \
     {                                                                         \
-      static rettype (*func) parameters =                                     \
-        reinterpret_cast<rettype(*)parameters>(                               \
-          (rettype2(*)parameters2)(::func));                                  \
+      static const struct _gl_ ## func ## _wrapper                            \
+      {                                                                       \
+        typedef rettype (*type) parameters;                                   \
+                                                                              \
+        inline type rpl () const                                              \
+        { return reinterpret_cast<type>((rettype2 (*) parameters2)(::func)); }\
+                                                                              \
+        inline operator type () const { return rpl (); }                      \
+      } func = {};                                                            \
     }                                                                         \
     _GL_EXTERN_C int _gl_cxxalias_dummy
 #else
-- 
2.5.5




reply via email to

[Prev in Thread] Current Thread [Next in Thread]