From 8bdba2edebeb203ab911ddad02ff1fedeb11cd10 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:09:21 +0200 Subject: [PATCH 1/2] Drop function wrapping as it is redundant for now As currently used in the BIND source tree, the --wrap linker option is redundant because: - static builds are no longer supported, - there is no need to wrap around existing functions - what is actually required (at least for now) is to replace them altogether in unit tests, - only functions exposed by shared libraries linked into unit test binaries are currently being replaced. Given the above, providing the alternative implementations of functions to be overridden in lib/ns/tests/nstest.c is a much simpler alternative to using the --wrap linker option. Drop the code detecting support for the latter from configure.ac, simplify the relevant Makefile.am, and remove lib/ns/tests/wrap.c, updating lib/ns/tests/nstest.c accordingly (it is harmless for unit tests which are not calling the overridden functions). --- configure.ac | 38 -------------------------------- lib/ns/tests/Makefile.am | 34 ++++++----------------------- lib/ns/tests/nstest.c | 9 ++------ lib/ns/tests/wrap.c | 47 ---------------------------------------- util/copyrights | 1 - 5 files changed, 9 insertions(+), 120 deletions(-) delete mode 100644 lib/ns/tests/wrap.c diff --git a/configure.ac b/configure.ac index be261f8646..441e150447 100644 --- a/configure.ac +++ b/configure.ac @@ -1354,44 +1354,6 @@ AC_SUBST([CMOCKA_LIBS]) AM_CONDITIONAL([HAVE_CMOCKA], [test "$with_cmocka" = "yes"]) -# -# Check for -Wl,--wrap= support -# - -LD_WRAP_TESTS=false -AC_MSG_CHECKING([for linker support for --wrap option]) -AX_SAVE_FLAGS([wrap]) -LDFLAGS="-Wl,--wrap,exit" -AC_RUN_IFELSE( - [AC_LANG_PROGRAM([[#include - void __real_exit (int status); - void __wrap_exit (int status) { __real_exit (0); } - ]], - [[exit (1);]])], - [LD_WRAP_TESTS=true - AC_MSG_RESULT([yes])], - [AC_MSG_RESULT([no])]) -AX_RESTORE_FLAGS([wrap]) - -AM_CONDITIONAL([HAVE_LD_WRAP], [test "$LD_WRAP_TESTS" = "true"]) - -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.am b/lib/ns/tests/Makefile.am index 38903266d5..31390fabfe 100644 --- a/lib/ns/tests/Makefile.am +++ b/lib/ns/tests/Makefile.am @@ -13,37 +13,17 @@ LDADD += \ $(LIBNS_LIBS) check_LTLIBRARIES = libnstest.la -libnstest_la_SOURCES = nstest.c nstest.h +libnstest_la_SOURCES = \ + nstest.c \ + nstest.h + check_PROGRAMS = \ listenlist_test \ - plugin_test - -TESTS = $(check_PROGRAMS) - -if HAVE_LD_WRAP - -check_PROGRAMS += \ - notify_test \ + notify_test \ + plugin_test \ query_test -notify_test_SOURCES = \ - notify_test.c \ - wrap.c - -notify_test_LDFLAGS = \ - $(LDFLAGS) \ - -Wl,--wrap=isc_nmhandle_attach \ - -Wl,--wrap=isc_nmhandle_detach - -query_test_SOURCES = \ - query_test.c \ - wrap.c - -query_test_LDFLAGS = \ - $(LDFLAGS) \ - -Wl,--wrap=isc_nmhandle_detach - -endif +TESTS = $(check_PROGRAMS) unit-local: check diff --git a/lib/ns/tests/nstest.c b/lib/ns/tests/nstest.c index 473c7a55b7..9b7d7e04e9 100644 --- a/lib/ns/tests/nstest.c +++ b/lib/ns/tests/nstest.c @@ -78,12 +78,7 @@ atomic_uint_fast32_t client_refs[32]; atomic_uintptr_t client_addrs[32]; void -__wrap_isc_nmhandle_attach(isc_nmhandle_t *source, isc_nmhandle_t **targetp); -void -__wrap_isc_nmhandle_detach(isc_nmhandle_t **handlep); - -void -__wrap_isc_nmhandle_attach(isc_nmhandle_t *source, isc_nmhandle_t **targetp) { +isc_nmhandle_attach(isc_nmhandle_t *source, isc_nmhandle_t **targetp) { ns_client_t *client = (ns_client_t *)source; int i; @@ -102,7 +97,7 @@ __wrap_isc_nmhandle_attach(isc_nmhandle_t *source, isc_nmhandle_t **targetp) { } void -__wrap_isc_nmhandle_detach(isc_nmhandle_t **handlep) { +isc_nmhandle_detach(isc_nmhandle_t **handlep) { isc_nmhandle_t *handle = *handlep; ns_client_t *client = (ns_client_t *)handle; int i; diff --git a/lib/ns/tests/wrap.c b/lib/ns/tests/wrap.c deleted file mode 100644 index 4fe6ad9183..0000000000 --- a/lib/ns/tests/wrap.c +++ /dev/null @@ -1,47 +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_attach/detach(), sending them to - * __wrap_isc_nmhandle_attach/detach() instead, when libtool is in use - * and LD_WRAP can't be used. - */ - -void -__wrap_isc_nmhandle_attach(isc_nmhandle_t *source, isc_nmhandle_t **targetp); -extern void -__wrap_isc_nmhandle_detach(isc_nmhandle_t **handlep); - -void -isc_nmhandle_attach(isc_nmhandle_t *source, isc_nmhandle_t **targetp) { - __wrap_isc_nmhandle_attach(source, targetp); -} - -void -isc_nmhandle_detach(isc_nmhandle_t **handlep) { - __wrap_isc_nmhandle_detach(handlep); -} diff --git a/util/copyrights b/util/copyrights index 95314a5679..e7594997b1 100644 --- a/util/copyrights +++ b/util/copyrights @@ -2124,7 +2124,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/win32/DLLMain.c C 2017,2018,2019,2020 ./lib/ns/win32/libns.def X 2017,2018,2019,2020 From b60d7345edae90560721c20d705ab32563abccb6 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:09:21 +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). --- Makefile.top | 7 +++++++ configure.ac | 4 ++++ fuzz/Makefile.am | 2 +- lib/bind9/Makefile.am | 1 + lib/dns/Makefile.am | 1 + lib/irs/Makefile.am | 1 + lib/isc/Makefile.am | 1 + lib/isccc/Makefile.am | 1 + lib/isccfg/Makefile.am | 1 + lib/ns/Makefile.am | 1 + 10 files changed, 19 insertions(+), 1 deletion(-) diff --git a/Makefile.top b/Makefile.top index 727f08bf49..66aaecf7bd 100644 --- a/Makefile.top +++ b/Makefile.top @@ -11,6 +11,13 @@ AM_CPPFLAGS = \ -include $(top_builddir)/config.h \ -I$(srcdir)/include +AM_LDFLAGS = + +if HOST_MACOS +AM_LDFLAGS += \ + -Wl,-flat_namespace +endif HOST_MACOS + if HAVE_GSSAPI AM_CPPFLAGS += \ $(GSSAPI_CFLAGS) diff --git a/configure.ac b/configure.ac index 441e150447..97286bcd98 100644 --- a/configure.ac +++ b/configure.ac @@ -150,6 +150,10 @@ AX_CHECK_COMPILE_FLAG([-Werror -fno-delete-null-pointer-checks], AX_CHECK_COMPILE_FLAG([-fdiagnostics-show-option], [STD_CFLAGS="$STD_CFLAGS -fdiagnostics-show-option"]) +host_macos=no +AS_CASE([$host],[*-darwin*],[host_macos=yes]) +AM_CONDITIONAL([HOST_MACOS], [test "$host_macos" = "yes"]) + # # Change defaults for developers if not explicity set. # Needs to be before the option is tested. diff --git a/fuzz/Makefile.am b/fuzz/Makefile.am index 4ba24e4029..bf5aa3eeb2 100644 --- a/fuzz/Makefile.am +++ b/fuzz/Makefile.am @@ -5,7 +5,7 @@ AM_CPPFLAGS += \ $(LIBDNS_CFLAGS) \ -DFUZZDIR=\"$(abs_srcdir)\" -AM_LDFLAGS = \ +AM_LDFLAGS += \ $(FUZZ_LDFLAGS) LDADD = \ diff --git a/lib/bind9/Makefile.am b/lib/bind9/Makefile.am index 3501cef49c..6b7edf8c9a 100644 --- a/lib/bind9/Makefile.am +++ b/lib/bind9/Makefile.am @@ -27,6 +27,7 @@ libbind9_la_LIBADD = \ $(LIBISCCFG_LIBS) libbind9_la_LDFLAGS = \ + $(AM_LDFLAGS) \ $(libbind9_VERSION_INFO) EXTRA_DIST = api diff --git a/lib/dns/Makefile.am b/lib/dns/Makefile.am index 78a2752c90..73e6bc2f73 100644 --- a/lib/dns/Makefile.am +++ b/lib/dns/Makefile.am @@ -287,6 +287,7 @@ libdns_la_CPPFLAGS = \ $(LIBLTDL_CFLAGS) libdns_la_LDFLAGS = \ + $(AM_LDFLAGS) \ $(libdns_VERSION_INFO) libdns_la_LIBADD = \ diff --git a/lib/irs/Makefile.am b/lib/irs/Makefile.am index 75d514cbcb..f7b82b47bc 100644 --- a/lib/irs/Makefile.am +++ b/lib/irs/Makefile.am @@ -23,6 +23,7 @@ libirs_la_LIBADD = \ $(LIBISCCFG_LIBS) libirs_la_LDFLAGS = \ + $(AM_LDFLAGS) \ $(libirs_VERSION_INFO) diff --git a/lib/isc/Makefile.am b/lib/isc/Makefile.am index cde0947bde..2c84285b16 100644 --- a/lib/isc/Makefile.am +++ b/lib/isc/Makefile.am @@ -227,6 +227,7 @@ libisc_la_CPPFLAGS = \ $(ZLIB_CFLAGS) libisc_la_LDFLAGS = \ + $(AM_LDFLAGS) \ $(libisc_VERSION_INFO) libisc_la_LIBADD = \ diff --git a/lib/isccc/Makefile.am b/lib/isccc/Makefile.am index 477cc525fb..aca06f777f 100644 --- a/lib/isccc/Makefile.am +++ b/lib/isccc/Makefile.am @@ -35,6 +35,7 @@ libisccc_la_LIBADD = \ $(LIBISC_LIBS) libisccc_la_LDFLAGS = \ + $(AM_LDFLAGS) \ $(libisccc_VERSION_INFO) if HAVE_CMOCKA diff --git a/lib/isccfg/Makefile.am b/lib/isccfg/Makefile.am index ef0cd16fc8..eb4b7d53d9 100644 --- a/lib/isccfg/Makefile.am +++ b/lib/isccfg/Makefile.am @@ -32,6 +32,7 @@ libisccfg_la_LIBADD = \ $(LIBDNS_LIBS) libisccfg_la_LDFLAGS = \ + $(AM_LDFLAGS) \ $(libisccfg_VERSION_INFO) if HAVE_CMOCKA diff --git a/lib/ns/Makefile.am b/lib/ns/Makefile.am index b2f81ccf08..7f79af4292 100644 --- a/lib/ns/Makefile.am +++ b/lib/ns/Makefile.am @@ -51,6 +51,7 @@ libns_la_LIBADD = \ $(LIBDNS_LIBS) libns_la_LDFLAGS = \ + $(AM_LDFLAGS) \ $(libns_VERSION_INFO) if HAVE_CMOCKA