From e05e5d7c1283f784199a1b9dee84ed53cf32728f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Mon, 28 Sep 2020 09:16:48 +0200 Subject: [PATCH 1/2] Clean up use of function wrapping Currently, building BIND using "--without-dlopen" universally breaks building unit tests which employ the --wrap linker option (because the replacement functions are put in a shared library and building shared objects requires "--with-dlopen"). Fix by moving the overridden symbol, isc_nmhandle_unref(), to lib/ns/tests/nstest.c and dropping lib/ns/tests/wrap.c altogether. This makes lib/ns/tests/Makefile.in simpler and prevents --without-dlopen from messing with the process of building unit tests. Remove parts of configure.ac which are made redundant by the above changes. Put the replacement definition of isc_nmhandle_unref() inside an #ifdef block, so that the build does not break for non-libtool builds (see below). These changes allow the broadest possible set of build variants to work while also simplifying the build process: - for libtool builds, overriding isc_nmhandle_unref() is done by placing that symbol directly in lib/ns/tests/nstest.c and relying on the dynamic linker to perform symbol resolution in the expected way when the test binary is run, - for non-libtool builds, overriding isc_nmhandle_unref() is done using the --wrap linker option (the libtool approach cannot be used in this case as multiple strong symbols with the same name cannot coexist in the same binary), - the "--without-dlopen" option no longer affects building unit tests. --- configure.ac | 17 ----------------- lib/ns/tests/Makefile.in | 38 +++++++++++++------------------------- lib/ns/tests/nstest.c | 7 +++++++ lib/ns/tests/wrap.c | 40 ---------------------------------------- util/copyrights | 1 - 5 files changed, 20 insertions(+), 83 deletions(-) delete mode 100644 lib/ns/tests/wrap.c diff --git a/configure.ac b/configure.ac index 690f763f54..f262a57c19 100644 --- a/configure.ac +++ b/configure.ac @@ -2260,23 +2260,6 @@ AX_RESTORE_FLAGS([wrap]) AC_SUBST([LD_WRAP_TESTS]) -WRAP_INTERPOSE= -AC_MSG_CHECKING([for linker support for '-z interpose' option]) -AX_SAVE_FLAGS([interpose]) -LDFLAGS="-Wl,-z,interpose" -AC_LINK_IFELSE( - [AC_LANG_PROGRAM([],[])], - [WRAP_INTERPOSE="-Wl,-z,interpose" - AC_MSG_RESULT([yes])], - [AC_MSG_RESULT([no])]) -AX_RESTORE_FLAGS([interpose]) - -AC_SUBST([WRAP_INTERPOSE]) - -WRAP_NAME='' -AS_CASE([$host],[*-darwin*],[WRAP_NAME='${WRAP_NAME}']) -AC_SUBST([WRAP_NAME]) - # # Check for i18n # diff --git a/lib/ns/tests/Makefile.in b/lib/ns/tests/Makefile.in index 7869c8e9cd..651b20033c 100644 --- a/lib/ns/tests/Makefile.in +++ b/lib/ns/tests/Makefile.in @@ -15,11 +15,7 @@ VERSION=@BIND9_VERSION@ @BIND9_MAKE_INCLUDES@ -WRAP_INTERPOSE= @WRAP_INTERPOSE@ WRAP_OPTIONS = -Wl,--wrap=isc_nmhandle_unref -WRAP_NAME = -Wl,-install_name,${top_builddir}/lib/ns/tests/$@ -WRAP_RPATH = -Wl,-rpath,${top_builddir}/lib/ns/tests -WRAP_LIB = -L${top_builddir}/lib/ns/tests -lwrap CINCLUDES = -I. -Iinclude ${NS_INCLUDES} ${DNS_INCLUDES} ${ISC_INCLUDES} \ ${OPENSSL_CFLAGS} \ @@ -38,10 +34,6 @@ LIBS = @LIBS@ @CMOCKA_LIBS@ SO_CFLAGS = @CFLAGS@ @SO_CFLAGS@ SO_LDFLAGS = @LDFLAGS@ @SO_LDFLAGS@ -SO_OBJS = wrap.@O@ -SO_SRCS = wrap.c -SO_TARGETS = libwrap.@SO@ - OBJS = nstest.@O@ SRCS = nstest.c \ listenlist_test.c \ @@ -53,39 +45,35 @@ SUBDIRS = TARGETS = listenlist_test@EXEEXT@ \ notify_test@EXEEXT@ \ plugin_test@EXEEXT@ \ - query_test@EXEEXT@ \ - @SO_TARGETS@ + query_test@EXEEXT@ LD_WRAP_TESTS=@LD_WRAP_TESTS@ @BIND9_MAKE_RULES@ -libwrap.@SO@: wrap.@O@ - ${LIBTOOL_MODE_LINK} @SO_LD@ ${SO_LDFLAGS} ${WRAP_INTERPOSE} @WRAP_NAME@ -o $@ wrap.@O@ ${LIBS} - -listenlist_test@EXEEXT@: listenlist_test.@O@ nstest.@O@ libwrap.@SO@ ${NSDEPLIBS} ${ISCDEPLIBS} ${DNSDEPLIBS} +listenlist_test@EXEEXT@: listenlist_test.@O@ nstest.@O@ ${NSDEPLIBS} ${ISCDEPLIBS} ${DNSDEPLIBS} if test "${LD_WRAP_TESTS}" = true -a -z "${LIBTOOL}"; then WRAP="${WRAP_OPTIONS}"; fi; \ ${LIBTOOL_MODE_LINK} ${PURIFY} ${CC} ${CFLAGS} \ - ${LDFLAGS} $${WRAP} ${WRAP_RPATH} -o $@ listenlist_test.@O@ nstest.@O@ \ - ${WRAP_LIB} ${NSLIBS} ${DNSLIBS} ${ISCLIBS} ${LIBS} + ${LDFLAGS} $${WRAP} -o $@ listenlist_test.@O@ nstest.@O@ \ + ${NSLIBS} ${DNSLIBS} ${ISCLIBS} ${LIBS} -notify_test@EXEEXT@: notify_test.@O@ nstest.@O@ libwrap.@SO@ ${NSDEPLIBS} ${ISCDEPLIBS} ${DNSDEPLIBS} +notify_test@EXEEXT@: notify_test.@O@ nstest.@O@ ${NSDEPLIBS} ${ISCDEPLIBS} ${DNSDEPLIBS} if test "${LD_WRAP_TESTS}" = true -a -z "${LIBTOOL}"; then WRAP="${WRAP_OPTIONS}"; fi; \ ${LIBTOOL_MODE_LINK} ${PURIFY} ${CC} ${CFLAGS} \ - ${LDFLAGS} $${WRAP} ${WRAP_RPATH} -o $@ notify_test.@O@ nstest.@O@ \ - ${WRAP_LIB} ${NSLIBS} ${DNSLIBS} ${ISCLIBS} ${LIBS} + ${LDFLAGS} $${WRAP} -o $@ notify_test.@O@ nstest.@O@ \ + ${NSLIBS} ${DNSLIBS} ${ISCLIBS} ${LIBS} -plugin_test@EXEEXT@: plugin_test.@O@ nstest.@O@ libwrap.@SO@ ${NSDEPLIBS} ${ISCDEPLIBS} ${DNSDEPLIBS} +plugin_test@EXEEXT@: plugin_test.@O@ nstest.@O@ ${NSDEPLIBS} ${ISCDEPLIBS} ${DNSDEPLIBS} if test "${LD_WRAP_TESTS}" = true -a -z "${LIBTOOL}"; then WRAP="${WRAP_OPTIONS}"; fi; \ ${LIBTOOL_MODE_LINK} ${PURIFY} ${CC} ${CFLAGS} \ - ${LDFLAGS} $${WRAP} ${WRAP_RPATH} -o $@ plugin_test.@O@ nstest.@O@ \ - ${WRAP_LIB} ${NSLIBS} ${DNSLIBS} ${ISCLIBS} ${LIBS} + ${LDFLAGS} $${WRAP} -o $@ plugin_test.@O@ nstest.@O@ \ + ${NSLIBS} ${DNSLIBS} ${ISCLIBS} ${LIBS} -query_test@EXEEXT@: query_test.@O@ nstest.@O@ libwrap.@SO@ ${NSDEPLIBS} ${ISCDEPLIBS} ${DNSDEPLIBS} +query_test@EXEEXT@: query_test.@O@ nstest.@O@ ${NSDEPLIBS} ${ISCDEPLIBS} ${DNSDEPLIBS} if test "${LD_WRAP_TESTS}" = true -a -z "${LIBTOOL}"; then WRAP="${WRAP_OPTIONS}"; fi; \ ${LIBTOOL_MODE_LINK} ${PURIFY} ${CC} ${CFLAGS} \ - ${LDFLAGS} $${WRAP} ${WRAP_RPATH} -o $@ query_test.@O@ nstest.@O@ \ - ${WRAP_LIB} ${NSLIBS} ${DNSLIBS} ${ISCLIBS} ${LIBS} + ${LDFLAGS} $${WRAP} -o $@ query_test.@O@ nstest.@O@ \ + ${NSLIBS} ${DNSLIBS} ${ISCLIBS} ${LIBS} unit:: sh ${top_builddir}/unit/unittest.sh diff --git a/lib/ns/tests/nstest.c b/lib/ns/tests/nstest.c index 5b5d2fd0ab..17eb44cf43 100644 --- a/lib/ns/tests/nstest.c +++ b/lib/ns/tests/nstest.c @@ -102,6 +102,13 @@ __wrap_isc_nmhandle_unref(isc_nmhandle_t *handle) { return; } +#ifdef USE_LIBTOOL +void +isc_nmhandle_unref(isc_nmhandle_t *handle) { + __wrap_isc_nmhandle_unref(handle); +} +#endif /* USE_LIBTOOL */ + /* * Logging categories: this needs to match the list in lib/ns/log.c. */ diff --git a/lib/ns/tests/wrap.c b/lib/ns/tests/wrap.c deleted file mode 100644 index 57e2e28535..0000000000 --- a/lib/ns/tests/wrap.c +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright (C) Internet Systems Consortium, Inc. ("ISC") - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, you can obtain one at https://mozilla.org/MPL/2.0/. - * - * See the COPYRIGHT file distributed with this work for additional - * information regarding copyright ownership. - */ - -/*! \file */ - -#include -#include -#include -#include -#include - -#include -#include -#include - -#include - -#include - -/* - * This overrides calls to isc_nmhandle_unref(), sending them to - * __wrap_isc_nmhandle_unref(), when libtool is in use and LD_WRAP - * can't be used. - */ - -extern void -__wrap_isc_nmhandle_unref(isc_nmhandle_t *handle); - -void -isc_nmhandle_unref(isc_nmhandle_t *handle) { - __wrap_isc_nmhandle_unref(handle); -} diff --git a/util/copyrights b/util/copyrights index 720b3f2f8a..15a820e986 100644 --- a/util/copyrights +++ b/util/copyrights @@ -2396,7 +2396,6 @@ ./lib/ns/tests/plugin_test.c C 2019,2020 ./lib/ns/tests/query_test.c C 2017,2018,2019,2020 ./lib/ns/tests/testdata/notify/notify1.msg X 2017,2018,2019,2020 -./lib/ns/tests/wrap.c C 2019,2020 ./lib/ns/update.c C 2017,2018,2019,2020 ./lib/ns/version.c C 2017,2018,2019,2020 ./lib/ns/win32/DLLMain.c C 2017,2018,2019,2020 From 87d9d10460290fa9273309b970ca0c1a95d42b33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Mon, 28 Sep 2020 09:16:48 +0200 Subject: [PATCH 2/2] Fix function overrides in unit tests on macOS Since Mac OS X 10.1, Mach-O object files are by default built with a so-called two-level namespace which prevents symbol lookups in BIND unit tests that attempt to override the implementations of certain library functions from working as intended. This feature can be disabled by passing the "-flat_namespace" flag to the linker. Fix unit tests affected by this issue on macOS by adding "-flat_namespace" to LDFLAGS used for building all object files on that operating system (it is not enough to only set that flag for the unit test executables). --- configure | 2 ++ configure.ac | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/configure b/configure index e5acb38794..deb7f36a75 100755 --- a/configure +++ b/configure @@ -13629,6 +13629,8 @@ $as_echo "$as_me: WARNING: When using GNU C Compiler on Solaris, -zrelax=transtl fi ;; #( + *-darwin*) : + LDFLAGS="$LDFLAGS -Wl,-flat_namespace" ;; #( *) : ;; esac diff --git a/configure.ac b/configure.ac index f262a57c19..4aaae5b048 100644 --- a/configure.ac +++ b/configure.ac @@ -382,7 +382,8 @@ AS_CASE([$host], [LDFLAGS="$LDFLAGS -zrelax=transtls" AC_MSG_WARN([When using GNU C Compiler on Solaris, -zrelax=transtls linker flag is used to fix bug in Thread Local Storage]) ]) - ]) + ], + [*-darwin*],[LDFLAGS="$LDFLAGS -Wl,-flat_namespace"]) # # CCNOOPT defaults to -O0 on gcc and disables optimization when is last