[SSSD] [PATCH] Check our libraries properly resolve symbols

Jakub Hrozek jhrozek at redhat.com
Mon Sep 9 08:33:50 UTC 2013


On Mon, Sep 09, 2013 at 09:52:34AM +0200, Lukas Slebodnik wrote:
> On (07/09/13 13:17), Simo Sorce wrote:
> >On Sat, 2013-09-07 at 12:39 -0400, Simo Sorce wrote:
> >> On Sat, 2013-09-07 at 11:42 +0200, Lukas Slebodnik wrote:
> >> > >+libdlopen_test_providers_la_LDFLAGS = \
> >> > >+    -module \
> >> > >+    -avoid-version \
> >> > >+    -Wl,--version-script,$(srcdir)/src/providers/sssd_be.exports \
> >> > add "-rpath $(abs_builddir) \"
> >> > libdlopen_test_providers will be dynamic library with this line.
> >> > 
> >> I had already tried this trick and it did not work, but the exact line I
> >> used was slightly different.
> >
> >Ah it did work with this syntax, I had tried with -Wl,-rpath previously.
> >Thanks a lot.
> >
> >> I'll retry and if it works I'll send an updated patch I do preferer
> >> check_LTLIBRARY as well, rather than the current hack.
> >
> >so now check_LTLIBRARY is used instead, moreover I added guards around
> >libraries that we compiled only conditionally, I had to add a couple of
> >statements in m4 files to make stuff appear in config.in.
> >Also I added the 2 missing python bindings.
> >
> >This should be final, it all works here.
> >
> >Simo.
> >
> >-- 
> >Simo Sorce * Red Hat, Inc * New York
> 
> >From a957e4642ec49a36831270c8007bcc8d7818db90 Mon Sep 17 00:00:00 2001
> >From: Simo Sorce <simo at redhat.com>
> >Date: Thu, 5 Sep 2013 11:52:08 -0400
> >Subject: [PATCH] tests: Add dlopen test to make sure modules works
> >
> >This tests dlopens and resolves all symbols to make sure there are no missing
> >symbols in our provider modules.
> >---
> > Makefile.am              |  31 ++++++++++
> > configure.ac             |   1 +
> > src/external/krb5.m4     |   3 +-
> > src/tests/dlopen-tests.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 189 insertions(+), 1 deletion(-)
> > create mode 100644 src/tests/dlopen-tests.c
> >
> >diff --git a/Makefile.am b/Makefile.am
> >index b913a12b895d68f1f3e23c185e493e576641d0e2..bddb403b373ecee21f6ad41f36e69338b4cbb5e2 100644
> >--- a/Makefile.am
> >+++ b/Makefile.am
> >@@ -114,6 +114,7 @@ endif
> > 
> > if HAVE_CHECK
> >     non_interactive_check_based_tests = \
> >+        dlopen-tests \
> >         sysdb-tests \
> >         strtonum-tests \
> >         resolv-tests \
> >@@ -886,6 +887,36 @@ if HAVE_CHECK
> > libsss_test_common_la_SOURCES += \
> >     src/tests/common_check.c
> > 
> >+check_LTLIBRARIES = \
> >+    libdlopen_test_providers.la
> >+
> >+libdlopen_test_providers_la_SOURCES = \
> >+    $(sssd_be_SOURCES)
> >+libdlopen_test_providers_la_CFLAGS = \
> >+    $(AM_CFLAGS) \
> >+    $(CHECK_CFLAGS) \
> >+    -DUNIT_TESTING
> >+libdlopen_test_providers_la_LIBADD = \
> >+    $(PAM_LIBS) \
> >+    $(SSSD_LIBS) \
> >+    $(CARES_LIBS) \
> >+    $(SSSD_INTERNAL_LTLIBS)
> >+libdlopen_test_providers_la_LDFLAGS = \
> >+    -module \
> >+    -avoid-version \
> >+    -Wl,--version-script,$(srcdir)/src/providers/sssd_be.exports \
> >+    -rpath $(abs_top_builddir) \
> >+    -export-dynamic
> >+
> >+dlopen_tests_SOURCES = \
> >+    src/tests/dlopen-tests.c
> >+dlopen_tests_CFLAGS = \
> >+    $(AM_CFLAGS) \
> >+    $(CHECK_CFLAGS)
> >+dlopen_tests_LDADD = \
> >+    -ldl \
> >+    $(CHECK_LIBS)
> >+
> > sysdb_tests_DEPENDENCIES = \
> >     $(ldblib_LTLIBRARIES)
> > sysdb_tests_SOURCES = \
> >diff --git a/configure.ac b/configure.ac
> >index 511e8d6c8afbfb0cbf5aca076b460aa4c881e252..586fe0bf9dbda5c65175d491e0044f8f09650b39 100644
> >--- a/configure.ac
> >+++ b/configure.ac
> >@@ -227,6 +227,7 @@ if test x$HAVE_PYTHON_BINDINGS != x; then
> >     AM_PATH_PYTHON([2.4])
> >     AM_CHECK_PYTHON_HEADERS([],
> >                             AC_MSG_ERROR([Could not find python headers]))
> >+    AC_DEFINE_UNQUOTED(HAVE_PYTHON_BINDINGS, 1, [Build with python bindings])
> I would prefer to have this line in the file conf_macros.m4
> directly after the line
>    src/conf_macros.m4:333: AM_CONDITIONAL([BUILD_PYTHON_BINDINGS],
> 
> In this file, there is a lot of similar patterns
> 
> 
> >     AM_PYTHON_CONFIG
> >     AM_CHECK_PYTHON_COMPAT
> > fi
> >diff --git a/src/external/krb5.m4 b/src/external/krb5.m4
> >index bdbd71ba1f395eb4f78047f75841729e0289ae81..a0da44a887329fbb2173a5849497d0b2cf1729a7 100644
> >--- a/src/external/krb5.m4
> >+++ b/src/external/krb5.m4
> >@@ -90,4 +90,5 @@ AC_CHECK_HEADER([krb5/locate_plugin.h],
> >                 ])
> > AM_CONDITIONAL([BUILD_KRB5_LOCATOR_PLUGIN],
> >                [test x$have_locate_plugin = xyes -a x$build_locator = xyes])
> >-
> >+AM_COND_IF([BUILD_KRB5_LOCATOR_PLUGIN],
> >+           [AC_DEFINE_UNQUOTED(HAVE_KRB5_LOCATOR_PLUGIN, 1, [Build with krb5 locator plugin])])
> >diff --git a/src/tests/dlopen-tests.c b/src/tests/dlopen-tests.c
> >new file mode 100644
> >index 0000000000000000000000000000000000000000..608b6106d6584a528a819cc2abdf1e21469d8d38
> >--- /dev/null
> >+++ b/src/tests/dlopen-tests.c
> >@@ -0,0 +1,155 @@
> >+/*
> >+    SSSD
> >+
> >+    debug-tests.c
> >+
> >+    Authors:
> >+        Simo Sorce <simo at redhat.com>
> >+
> >+    Copyright (C) 2013 Red Hat
> >+
> >+    This program is free software; you can redistribute it and/or modify
> >+    it under the terms of the GNU General Public License as published by
> >+    the Free Software Foundation; either version 3 of the License, or
> >+    (at your option) any later version.
> >+
> >+    This program is distributed in the hope that it will be useful,
> >+    but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >+    GNU General Public License for more details.
> >+
> >+    You should have received a copy of the GNU General Public License
> >+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
> >+*/
> >+
> >+#define _GNU_SOURCE
> >+#include <stdbool.h>
> >+#include <dlfcn.h>
> >+#include <stdio.h>
> >+#include <stdlib.h>
> >+#include <limits.h>
> >+#include <check.h>
> >+#include "tests/common.h"
> >+
> >+#define LIBPFX ABS_BUILD_DIR"/.libs/"
> >+
> >+struct so {
> >+    const char *name;
> >+    const char *libs[6];
> >+} so[] = {
> >+    { "libsss_debug.so", { LIBPFX"libsss_debug.so", NULL } },
> >+    { "libsss_sudo.so", { LIBPFX"libsss_sudo.so", NULL } },
> add check for BUILD_SUDO
> >+    { "libipa_hbac.so", { LIBPFX"libipa_hbac.so", NULL } },
> >+    { "libsss_autofs.so", { LIBPFX"libsss_autofs.so", NULL } },
> add check for BUILD_AUTOFS
> >+    { "libsss_idmap.so", { LIBPFX"libsss_idmap.so", NULL } },
> >+    { "libsss_nss_idmap.so", { LIBPFX"libsss_nss_idmap.so", NULL } },
> >+    { "libnss_sss.so", { LIBPFX"libnss_sss.so", NULL } },
> >+    { "pam_sss.so", { LIBPFX"pam_sss.so", NULL } },
> >+#ifdef HAVE_KRB5_LOCATOR_PLUGIN
> >+    { "sssd_krb5_locator_plugin.so", { LIBPFX"sssd_krb5_locator_plugin.so",
> >+                                       NULL } },
> >+#endif
> >+#ifdef HAVE_PAC_RESPONDER
> >+    { "sssd_pac_plugin.so", { LIBPFX"sssd_pac_plugin.so", NULL } },
> >+#endif
> >+    { "memberof.so", { LIBPFX"memberof.so", NULL } },
> >+    { "libsss_child.so", { "libtevent.so",
> >+                           LIBPFX"libsss_debug.so",
> >+                           LIBPFX"libsss_crypt.so",
> >+                           LIBPFX"libsss_util.so",
> >+                           LIBPFX"libsss_child.so", NULL } },
> >+    { "libsss_crypt.so", { "libtalloc.so",
> >+                           LIBPFX"libsss_debug.so",
> >+                           LIBPFX"libsss_crypt.so", NULL } },
> >+    { "libsss_util.so", { "libtalloc.so",
> >+                           LIBPFX"libsss_debug.so",
> >+                           LIBPFX"libsss_crypt.so",
> >+                           LIBPFX"libsss_util.so", NULL } },
> >+    { "libsss_simple.so", { LIBPFX"libdlopen_test_providers.so",
> >+                            LIBPFX"libsss_simple.so", NULL } },
> >+    { "libsss_ad.so", { LIBPFX"libdlopen_test_providers.so",
> >+                        LIBPFX"libsss_ad.so", NULL } },
> >+    { "libsss_ipa.so", { LIBPFX"libdlopen_test_providers.so",
> >+                         LIBPFX"libsss_ipa.so", NULL } },
> >+    { "libsss_krb5.so", { LIBPFX"libdlopen_test_providers.so",
> >+                          LIBPFX"libsss_krb5.so", NULL } },
> >+    { "libsss_ldap.so", { LIBPFX"libdlopen_test_providers.so",
> >+                          LIBPFX"libsss_ldap.so", NULL } },
> >+    { "libsss_proxy.so", { LIBPFX"libdlopen_test_providers.so",
> >+                           LIBPFX"libsss_proxy.so", NULL } },
> >+#ifdef BUILD_PYTHON_BINDINGS
> You defined macro HAVE_PYTHON_BINDINGS, but BUILD_PYTHON_BINDINGS is used here.
> This is a dead code.
> >+    { "pyhbac.so", { LIBPFX"pyhbac.so", NULL } },
> >+    { "pysss.so", { LIBPFX"pysss.so", NULL } },
> >+    { "pysss_murmur.so", { LIBPFX"pysss_murmur.so", NULL } },
> >+    { "pysss_nss_idmap.so", { LIBPFX"pysss_nss_idmap.so", NULL } },
> >+#endif
> 
> btw, BUILD_AUTOFS is used elsewhere in code.
>   src/tests/sysdb-tests.c:4707:#ifdef BUILD_AUTOFS
>   src/tests/sysdb-tests.c:4913:#endif /* BUILD_AUTOFS */
>   src/tests/sysdb-tests.c:5224:#ifdef BUILD_AUTOFS
> 
> So we can use either BUILD_PYTHON_BINDINGS, BUILD_KRB5_LOCATOR_PLUGIN
> or we should file a ticket to be consistent.
> 
> LS

These are valid comments, but I wonder if we can remove the conditional
altogether and always build the autofs responder? Then the distributions
can choose not to package it. The downside would be some dead code in
the LDAP provider.

The autofs code has no external dependencies, it just produces a shared
library that can be dlopen()-ed.



More information about the sssd-devel mailing list