Browse Source

[3113_test] Fix issues raised by static linking

This fix:
* Ensures the user library accesses the correct ServerHooks
  singleton object.
* Provides a way to initialize the BIND 10 logging framework in
  the context of the user-written hooks library.
Stephen Morris 11 years ago
parent
commit
91eca98edc

+ 1 - 0
configure.ac

@@ -181,6 +181,7 @@ AC_HELP_STRING([--enable-static-link],
   [build programs with static link [[default=no]]]),
   [enable_static_link=yes], [enable_static_link=no])
 AM_CONDITIONAL(USE_STATIC_LINK, test $enable_static_link = yes)
+AM_COND_IF([USE_STATIC_LINK], [AC_DEFINE([USE_STATIC_LINK], [1], [BIND 10 was statically linked?])])
 
 # Check validity about some libtool options
 if test $enable_static_link = yes -a $enable_static = no; then

+ 1 - 1
src/lib/hooks/Makefile.am

@@ -29,7 +29,7 @@ lib_LTLIBRARIES = libb10-hooks.la
 libb10_hooks_la_SOURCES  =
 libb10_hooks_la_SOURCES += callout_handle.cc callout_handle.h
 libb10_hooks_la_SOURCES += callout_manager.cc callout_manager.h
-libb10_hooks_la_SOURCES += hooks.h
+libb10_hooks_la_SOURCES += hooks.h hooks.cc
 libb10_hooks_la_SOURCES += hooks_log.cc hooks_log.h
 libb10_hooks_la_SOURCES += hooks_manager.cc hooks_manager.h
 libb10_hooks_la_SOURCES += library_handle.cc library_handle.h

+ 3 - 2
src/lib/hooks/callout_handle.cc

@@ -30,7 +30,8 @@ namespace hooks {
 CalloutHandle::CalloutHandle(const boost::shared_ptr<CalloutManager>& manager,
                     const boost::shared_ptr<LibraryManagerCollection>& lmcoll)
     : lm_collection_(lmcoll), arguments_(), context_collection_(),
-      manager_(manager), skip_(false) {
+      manager_(manager), server_hooks_(ServerHooks::getServerHooks()),
+      skip_(false) {
 
     // Call the "context_create" hook.  We should be OK doing this - although
     // the constructor has not finished running, all the member variables
@@ -148,7 +149,7 @@ CalloutHandle::getHookName() const {
     // ... and look up the hook.
     string hook = "";
     try {
-        hook = ServerHooks::getServerHooks().getName(index);
+        hook = server_hooks_.getName(index);
     } catch (const NoSuchHook&) {
         // Hook index is invalid, so this methods probably called from outside
         // a callout being executed via a call to CalloutManager::callCallouts.

+ 7 - 0
src/lib/hooks/callout_handle.h

@@ -28,6 +28,8 @@
 namespace isc {
 namespace hooks {
 
+class ServerHooks;
+
 /// @brief No such argument
 ///
 /// Thrown if an attempt is made access an argument that does not exist.
@@ -369,6 +371,11 @@ private:
     /// Callout manager.
     boost::shared_ptr<CalloutManager> manager_;
 
+    /// Reference to the singleton ServerHooks object.  See the
+    /// @ref hooksmgMaintenanceGuide for information as to why the class holds
+    /// a reference instead of accessing the singleton within the code.
+    ServerHooks& server_hooks_;
+
     /// "Skip" flag, indicating if the caller should bypass remaining callouts.
     bool skip_;
 };

+ 8 - 9
src/lib/hooks/callout_manager.cc

@@ -31,7 +31,8 @@ namespace hooks {
 
 // Constructor
 CalloutManager::CalloutManager(int num_libraries)
-    : current_hook_(-1), current_library_(-1),
+    : server_hooks_(ServerHooks::getServerHooks()),
+      current_hook_(-1), current_library_(-1),
       hook_vector_(ServerHooks::getServerHooks().getCount()),
       library_handle_(this), pre_library_handle_(this, 0),
       post_library_handle_(this, INT_MAX), num_libraries_(num_libraries)
@@ -72,7 +73,7 @@ CalloutManager::registerCallout(const std::string& name, CalloutPtr callout) {
 
     // Get the index associated with this hook (validating the name in the
     // process).
-    int hook_index = ServerHooks::getServerHooks().getIndex(name);
+    int hook_index = server_hooks_.getIndex(name);
 
     // Iterate through the callout vector for the hook from start to end,
     // looking for the first entry where the library index is greater than
@@ -147,21 +148,19 @@ CalloutManager::callCallouts(int hook_index, CalloutHandle& callout_handle) {
                 if (status == 0) {
                     LOG_DEBUG(hooks_logger, HOOKS_DBG_EXTENDED_CALLS,
                               HOOKS_CALLOUT_CALLED).arg(current_library_)
-                        .arg(ServerHooks::getServerHooks()
-                            .getName(current_hook_))
+                        .arg(server_hooks_.getName(current_hook_))
                         .arg(PointerConverter(i->second).dlsymPtr());
                 } else {
                     LOG_ERROR(hooks_logger, HOOKS_CALLOUT_ERROR)
                         .arg(current_library_)
-                        .arg(ServerHooks::getServerHooks()
-                            .getName(current_hook_))
+                        .arg(server_hooks_.getName(current_hook_))
                         .arg(PointerConverter(i->second).dlsymPtr());
                 }
             } catch (const std::exception& e) {
                 // Any exception, not just ones based on isc::Exception
                 LOG_ERROR(hooks_logger, HOOKS_CALLOUT_EXCEPTION)
                     .arg(current_library_)
-                    .arg(ServerHooks::getServerHooks().getName(current_hook_))
+                    .arg(server_hooks_.getName(current_hook_))
                     .arg(PointerConverter(i->second).dlsymPtr())
                     .arg(e.what());
             }
@@ -184,7 +183,7 @@ CalloutManager::deregisterCallout(const std::string& name, CalloutPtr callout) {
 
     // Get the index associated with this hook (validating the name in the
     // process).
-    int hook_index = ServerHooks::getServerHooks().getIndex(name);
+    int hook_index = server_hooks_.getIndex(name);
 
     /// Construct a CalloutEntry matching the current library and the callout
     /// we want to remove.
@@ -227,7 +226,7 @@ CalloutManager::deregisterAllCallouts(const std::string& name) {
 
     // Get the index associated with this hook (validating the name in the
     // process).
-    int hook_index = ServerHooks::getServerHooks().getIndex(name);
+    int hook_index = server_hooks_.getIndex(name);
 
     /// Construct a CalloutEntry matching the current library (the callout
     /// pointer is NULL as we are not checking that).

+ 7 - 0
src/lib/hooks/callout_manager.h

@@ -338,6 +338,13 @@ private:
         }
     };
 
+    // Member variables
+
+    /// Reference to the singleton ServerHooks object.  See the
+    /// @ref hooksmgMaintenanceGuide for information as to why the class holds
+    /// a reference instead of accessing the singleton within the code.
+    ServerHooks& server_hooks_;
+
     /// Current hook.  When a call is made to callCallouts, this holds the
     /// index of the current hook.  It is set to an invalid value (-1)
     /// otherwise.

+ 31 - 0
src/lib/hooks/hooks.cc

@@ -0,0 +1,31 @@
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <hooks/hooks.h>
+#include <log/message_dictionary.h>
+#include <log/message_initializer.h>
+
+
+namespace isc {
+namespace hooks {
+
+// Load the logging message dictionary if not already loaded
+
+void
+hooks_static_link_init() {
+    isc::log::MessageInitializer::loadDictionary();
+}
+
+} // namespace hooks
+} // namespace isc

+ 39 - 0
src/lib/hooks/hooks.h

@@ -15,6 +15,7 @@
 #ifndef HOOKS_H
 #define HOOKS_H
 
+#include <config.h>
 #include <hooks/callout_handle.h>
 #include <hooks/library_handle.h>
 
@@ -35,4 +36,42 @@ typedef int (*unload_function_ptr)();
 
 } // Anonymous namespace
 
+namespace isc {
+namespace hooks {
+
+/// @brief User-Library Initialization for Statically-Linked BIND 10
+///
+/// If BIND 10 is statically-linked, a user-created hooks library will not be
+/// able to access symbols in it.  In particular, it will not be able to access
+/// singleton objects.
+///
+/// The hooks framework handles some of this.  For example, although there is
+/// a singleton ServerHooks object, hooks framework objects store a reference
+/// to it when they are created.  When the user library needs to register a
+/// callout (which requires access to the ServerHooks information), it accesses
+/// the ServerHooks object through a pointer passed from the BIND 10 image.
+///
+/// The logging framework is more problematical. Here the code is partly
+/// statically linked (the BIND 10 logging library) and partly shared (the
+/// log4cplus).  The state of the former is not accessible to the user library,
+/// but the state of the latter is.  So within the user library, we need to
+/// initialize the BIND 10 logging library but not initialize the log4cplus
+/// code.  Some of the initialization is done when the library is loaded, but
+/// other parts are done at run-time.
+///
+/// This function - to be called by the user library code in its load() function
+/// when running against a statically linked BIND 10 - initializes the BIND 10
+/// logging library.  In particular, it loads the message dictionary with the
+/// text of the BIND 10 messages.
+///
+/// @note This means that the virtual address space is loaded with two copies
+/// of the message dictionary.  Depending on how the user libraries are linked,
+/// loading multiple user libraries may involve loading one message dictionary
+/// per library.
+
+void hooks_static_link_init();
+
+} // namespace hooks
+} // namespace isc
+
 #endif  // HOOKS_H

+ 26 - 17
src/lib/hooks/tests/Makefile.am

@@ -9,6 +9,12 @@ AM_CPPFLAGS += $(BOOST_INCLUDES) $(MULTITHREADING_FLAG)
 # But older GCC compilers don't have the flag.     
 AM_CXXFLAGS  = $(WARNING_NO_MISSING_FIELD_INITIALIZERS_CFLAG)
 
+# Common libraries used in user libraries
+AM_LIBADD  = $(top_builddir)/src/lib/hooks/libb10-hooks.la
+AM_LIBADD += $(top_builddir)/src/lib/log/libb10-log.la
+AM_LIBADD += $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
+AM_LIBADD += $(top_builddir)/src/lib/util/libb10-util.la
+
 if USE_CLANGPP
 # see ../Makefile.am
 AM_CXXFLAGS += -Wno-unused-parameter
@@ -42,53 +48,54 @@ lib_LTLIBRARIES = libnvl.la libivl.la libfxl.la libbcl.la liblcl.la liblecl.la \
 libnvl_la_SOURCES  = no_version_library.cc
 libnvl_la_CXXFLAGS = $(AM_CXXFLAGS)
 libnvl_la_CPPFLAGS = $(AM_CPPFLAGS) $(LOG4CPLUS_INCLUDES)
-libnvl_la_LDFLAGS = -avoid-version -export-dynamic -module
+libnvl_la_LDFLAGS  = -avoid-version -export-dynamic -module
 
 # Incorrect version function
 libivl_la_SOURCES  = incorrect_version_library.cc
 libivl_la_CXXFLAGS = $(AM_CXXFLAGS)
 libivl_la_CPPFLAGS = $(AM_CPPFLAGS) $(LOG4CPLUS_INCLUDES)
-libivl_la_LDFLAGS = -avoid-version -export-dynamic -module
+libivl_la_LDFLAGS  = -avoid-version -export-dynamic -module
 
 # All framework functions throw an exception
-libfxl_la_SOURCES = framework_exception_library.cc
+libfxl_la_SOURCES  = framework_exception_library.cc
 libfxl_la_CXXFLAGS = $(AM_CXXFLAGS)
 libfxl_la_CPPFLAGS = $(AM_CPPFLAGS) $(LOG4CPLUS_INCLUDES)
-libfxl_la_LDFLAGS = -avoid-version -export-dynamic -module
+libfxl_la_LDFLAGS  = -avoid-version -export-dynamic -module
 
 # The basic callout library - contains standard callouts
 libbcl_la_SOURCES  = basic_callout_library.cc
 libbcl_la_CXXFLAGS = $(AM_CXXFLAGS)
 libbcl_la_CPPFLAGS = $(AM_CPPFLAGS) $(LOG4CPLUS_INCLUDES)
-libbcl_la_LDFLAGS = -avoid-version -export-dynamic -module
+libbcl_la_LDFLAGS  = -avoid-version -export-dynamic -module
+libbcl_la_LIBADD    = $(AM_LIBADD)
 
 # The load callout library - contains a load function
 liblcl_la_SOURCES  = load_callout_library.cc
 liblcl_la_CXXFLAGS = $(AM_CXXFLAGS)
 liblcl_la_CPPFLAGS = $(AM_CPPFLAGS) $(LOG4CPLUS_INCLUDES)
-liblcl_la_LDFLAGS = -avoid-version -export-dynamic -module
+liblcl_la_LDFLAGS  = -avoid-version -export-dynamic -module
+liblcl_la_LIBADD    = $(AM_LIBADD)
 
 # The load error callout library - contains a load function that returns
 # an error.
 liblecl_la_SOURCES  = load_error_callout_library.cc
 liblecl_la_CXXFLAGS = $(AM_CXXFLAGS)
 liblecl_la_CPPFLAGS = $(AM_CPPFLAGS) $(LOG4CPLUS_INCLUDES)
-liblecl_la_LDFLAGS = -avoid-version -export-dynamic -module
+liblecl_la_LDFLAGS  = -avoid-version -export-dynamic -module
 
 # The unload callout library - contains an unload function that
 # creates a marker file.
 libucl_la_SOURCES  = unload_callout_library.cc
 libucl_la_CXXFLAGS = $(AM_CXXFLAGS)
 libucl_la_CPPFLAGS = $(AM_CPPFLAGS) $(LOG4CPLUS_INCLUDES)
-libucl_la_LDFLAGS = -avoid-version -export-dynamic -module
+libucl_la_LDFLAGS  = -avoid-version -export-dynamic -module
 
 # The full callout library - contains all three framework functions.
 libfcl_la_SOURCES  = full_callout_library.cc
 libfcl_la_CXXFLAGS = $(AM_CXXFLAGS)
 libfcl_la_CPPFLAGS = $(AM_CPPFLAGS) $(LOG4CPLUS_INCLUDES)
-libfcl_la_LDFLAGS = -avoid-version -export-dynamic -module -Bstatic $(top_builddir)/src/lib/exceptions/libb10-exceptions.la -Bstatic $(top_builddir)/src/lib/hooks/libb10-hooks.la
-libfcl_la_LIBADD  = $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
-libfcl_la_LIBADD += $(top_builddir)/src/lib/hooks/libb10-hooks.la
+libfcl_la_LDFLAGS  = -avoid-version -export-dynamic -module
+libfcl_la_LIBADD    = $(AM_LIBADD)
 
 TESTS += run_unittests
 run_unittests_SOURCES  = run_unittests.cc
@@ -107,14 +114,16 @@ nodist_run_unittests_SOURCES += test_libraries.h
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 
 run_unittests_LDFLAGS  = $(AM_LDFLAGS)  $(GTEST_LDFLAGS)
+if USE_STATIC_LINK
+run_unittests_LDFLAGS += -static
+endif
 
 run_unittests_LDADD    = $(AM_LDADD)    $(GTEST_LDADD)
-
-run_unittests_LDADD += $(top_builddir)/src/lib/hooks/libb10-hooks.la
-run_unittests_LDADD += $(top_builddir)/src/lib/log/libb10-log.la
-run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
-run_unittests_LDADD += $(top_builddir)/src/lib/util/libb10-util.la
-run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
+run_unittests_LDADD   += $(top_builddir)/src/lib/hooks/libb10-hooks.la
+run_unittests_LDADD   += $(top_builddir)/src/lib/log/libb10-log.la
+run_unittests_LDADD   += $(top_builddir)/src/lib/exceptions/libb10-exceptions.la
+run_unittests_LDADD   += $(top_builddir)/src/lib/util/libb10-util.la
+run_unittests_LDADD   += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
 endif
 
 noinst_PROGRAMS = $(TESTS)

+ 17 - 8
src/lib/hooks/tests/basic_callout_library.cc

@@ -24,16 +24,16 @@
 /// - A context_create callout is supplied.
 ///
 /// - Three "standard" callouts are supplied corresponding to the hooks
-///   "hookpt_one", "hookpt_two", "hookpt_three".  All do some trivial calculations
-///   on the arguments supplied to it and the context variables, returning
-///   intermediate results through the "result" argument. The result of
-///   executing all four callouts in order is:
+///   "hookpt_one", "hookpt_two", "hookpt_three".  All do some trivial
+///   calculations on the arguments supplied to it and the context variables,
+///   returning intermediate results through the "result" argument. The result
+///   of executing all four callouts in order is:
 ///
 ///   @f[ (10 + data_1) * data_2 - data_3 @f]
 ///
 ///   ...where data_1, data_2 and data_3 are the values passed in arguments of
-///   the same name to the three callouts (data_1 passed to hookpt_one, data_2 to
-///   hookpt_two etc.) and the result is returned in the argument "result".
+///   the same name to the three callouts (data_1 passed to hookpt_one, data_2
+///   to hookpt_two etc.) and the result is returned in the argument "result".
 
 #include <hooks/hooks.h>
 #include <fstream>
@@ -104,12 +104,21 @@ hookpt_three(CalloutHandle& handle) {
     return (0);
 }
 
-// Framework functions.  Only version() is supplied here.
+// Framework functions.
 
 int
 version() {
     return (BIND10_HOOKS_VERSION);
 }
 
-};
+// load() initializes the user library if the main image was statically linked.
+int
+load(isc::hooks::LibraryHandle&) {
+#ifdef USE_STATIC_LINK
+    hooks_static_link_init();
+#endif
+    return (0);
+}
+
+}
 

+ 6 - 2
src/lib/hooks/tests/full_callout_library.cc

@@ -34,8 +34,8 @@
 ///   @f[ ((7 * data_1) - data_2) * data_3 @f]
 ///
 ///   ...where data_1, data_2 and data_3 are the values passed in arguments of
-///   the same name to the three callouts (data_1 passed to hookpt_one, data_2 to
-///   hookpt_two etc.) and the result is returned in the argument "result".
+///   the same name to the three callouts (data_1 passed to hookpt_one, data_2
+///   to hookpt_two etc.) and the result is returned in the argument "result".
 
 #include <hooks/hooks.h>
 #include <hooks/tests/marker_file.h>
@@ -116,6 +116,10 @@ version() {
 
 int
 load(LibraryHandle& handle) {
+    // Initialize if the main image was statically linked
+#ifdef USE_STATIC_LINK
+    isc::hooks::hooks_static_link_init();
+#endif
     // Register the non-standard functions
     handle.registerCallout("hookpt_two", hook_nonstandard_two);
     handle.registerCallout("hookpt_three", hook_nonstandard_three);

+ 6 - 2
src/lib/hooks/tests/load_callout_library.cc

@@ -30,8 +30,8 @@
 ///   @f[ ((5 * data_1) + data_2) * data_3 @f]
 ///
 ///   ...where data_1, data_2 and data_3 are the values passed in arguments of
-///   the same name to the three callouts (data_1 passed to hookpt_one, data_2 to
-///   hookpt_two etc.) and the result is returned in the argument "result".
+///   the same name to the three callouts (data_1 passed to hookpt_one, data_2
+///   to hookpt_two etc.) and the result is returned in the argument "result".
 
 #include <hooks/hooks.h>
 
@@ -108,6 +108,10 @@ version() {
 }
 
 int load(LibraryHandle& handle) {
+    // Initialize the user library if the main image was statically linked
+#ifdef USE_STATIC_LINK
+    isc::hooks::hooks_static_link_init();
+#endif
     // Register the non-standard functions
     handle.registerCallout("hookpt_two", hook_nonstandard_two);
     handle.registerCallout("hookpt_three", hook_nonstandard_three);

+ 5 - 0
src/lib/log/logger.cc

@@ -43,6 +43,11 @@ void Logger::initLoggerImpl() {
 
 Logger::~Logger() {
     delete loggerptr_;
+
+    // The next statement is required for the BIND 10 hooks framework, where
+    // a statically-linked BIND 10 loads and unloads multiple libraries. See
+    // the hooks documentation for more details.
+    loggerptr_ = 0;
 }
 
 // Get Name of Logger