I have figured out another solution to the problem that macOS SIP
defeats the use of DYLD_LIBRARY_PATH for running the temp-install
regression tests. It's not without problems either, but it might show a
path forward.
First of all, I think I now know the exact mechanism by which this
breakage happens.
The precise issue is that /bin/sh filters out DYLD_* environment
variables (presumably all, but at least the ones we care about) when it
starts. If you use a shell other than /bin/sh (say, a Homebrew
installation of bash or dash), there is no problem.
But /bin/sh is hardcoded in the system() library call, so in order to
fix that, you need to override that library call. Attached is a patch
that shows how this could be done. It uses the DYLD_INSERT_LIBRARIES
environment variable (equivalent to LD_PRELOAD) to substitute another
version of system(), which I hacked to allow overriding /bin/sh with
another shell using the environment variable PG_REGRESS_SHELL. That works.
There are also some other places where PostgreSQL code itself hardcodes
/bin/sh as part of system()-like functionality. These have to be fixed
up similarly, but that's easier.
The problem now is that DYLD_INSERT_LIBRARIES requires the "flat
namespace", which isn't the default. You can either build PostgreSQL
with -Wl,-flat_namespace, which works, but it's probably weird as a
proper solution, or you can set the environment variable
DYLD_FORCE_FLAT_NAMESPACE at run time, which also works but makes
everything brutally slow.
I think the way forward here is to get rid of all uses of system() for
calling between PostgreSQL programs. There are only a handful of those,
and we already have well-tested replacement code like spawn_process() in
pg_regress.c that could be used. (Perhaps we could also use that
opportunity to get rid of the need for shell quoting?)
There is a minor second issue, namely that /usr/bin/perl also filters
out DYLD_* environment variables. This can be worked around again by
using a third-party installation of Perl. You just need to make sure
that the "prove" program calls that installation instead of the system
one. (I just manually edited the shebang line. There is probably a
proper way to do it.)
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From e33718f57bfabc25b295a6fb159af70890eb9be9 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Tue, 10 Sep 2019 09:10:33 +0200
Subject: [PATCH] Make temp install tests work with macOS SIP
---
doc/src/sgml/regress.sgml | 31 +++++++++
src/bin/pg_ctl/pg_ctl.c | 6 +-
src/makefiles/Makefile.darwin | 11 +++
src/makefiles/pgxs.mk | 2 +-
src/test/perl/PostgresNode.pm | 1 +
src/test/regress/GNUmakefile | 14 ++++
src/test/regress/fake_system.c | 121 +++++++++++++++++++++++++++++++++
src/test/regress/pg_regress.c | 3 +
8 files changed, 187 insertions(+), 2 deletions(-)
create mode 100644 src/test/regress/fake_system.c
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index d98187c970..3040ee00d7 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -91,6 +91,37 @@ <title>Running the Tests Against a Temporary
Installation</title>
</screen>
runs no more than ten tests concurrently.
</para>
+
+ <sect3>
+ <title>macOS System Integrity Protection</title>
+
+ <para>
+ On macOS, if System Integrity Protection (SIP) is enabled, which has been
+ the default since version 10.11/El Capitan, then running the tests
+ against a temporary installation requires some additional setup. With
+ SIP enabled, the default shell <filename>/bin/sh</filename> and the
+ default Perl binary <filename>/usr/bin/perl</filename>> defeat the use of
+ the environment variable <envar>DYLD_LIBRARY_PATH</envar>, which is
+ used to let binaries find libraries in the temporary installation as
+ opposed to the final installation location. As a result, a test run will
+ likely crash because it cannot find missing libraries (or alternatively,
+ find the wrong ones).
+ </para>
+
+ <para>
+ One workaround is to run <literal>make install</literal> before
+ <literal>make check</literal>.
+ </para>
+
+ <para>
+ Another workaround is to set the environment variable
+ <envar>PG_REGRESS_SHELL</envar> to another shell installed via a
+ third-party package manager such as Homebrew or MacPorts, for example
+ <literal>PG_REGRESS_SHELL=/usr/local/bin/bash</literal>. For the TAP
+ tests, you also need to use a Perl binary other than the default for
+ running the <command>prove</command> program.
+ </para>
+ </sect3>
</sect2>
<sect2>
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index dd76be6dd2..b12f72667d 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -453,6 +453,7 @@ start_postmaster(void)
#ifndef WIN32
pgpid_t pm_pid;
+ char *shell;
/* Flush stdio channels just before fork, to avoid double-output
problems */
fflush(stdout);
@@ -501,7 +502,10 @@ start_postmaster(void)
snprintf(cmd, MAXPGPATH, "exec \"%s\" %s%s < \"%s\" 2>&1",
exec_path, pgdata_opt, post_opts, DEVNULL);
- (void) execl("/bin/sh", "/bin/sh", "-c", cmd, (char *) NULL);
+ shell = getenv("PG_CTL_SHELL");
+ if (!shell)
+ shell = "/bin/sh";
+ (void) execl(shell, "sh", "-c", cmd, (char *) NULL);
/* exec failed */
write_stderr(_("%s: could not start server: %s\n"),
diff --git a/src/makefiles/Makefile.darwin b/src/makefiles/Makefile.darwin
index b17598f058..08bdbf86c3 100644
--- a/src/makefiles/Makefile.darwin
+++ b/src/makefiles/Makefile.darwin
@@ -11,6 +11,17 @@ else
BE_DLLLIBS = -bundle_loader $(top_builddir)/src/backend/postgres
endif
+# On macOS with System Integrity Protection (SIP) enabled, /bin/sh
+# filters out DYLD_* environment variables, so our setting of
+# DYLD_LIBRARY_PATH won't survive a sytem() call. The workaround is
+# to write our own replacement system() that uses a different shell,
+# settable by the PG_REGRESS_SHELL environment variable, and then
+# preload that via DYLD_INSERT_LIBRARIES. DYLD_FORCE_FLAT_NAMESPACE
+# is required for DYLD_INSERT_LIBRARIES to work.
+ifdef PG_REGRESS_SHELL
+with_temp_install_extra = DYLD_FORCE_FLAT_NAMESPACE=1
DYLD_INSERT_LIBRARIES=$(abs_top_builddir)/src/test/regress/fake_system.dylib
+endif
+
# Rule for building a shared library from a single .o file
%.so: %.o
$(CC) $(CFLAGS) $< $(LDFLAGS) $(LDFLAGS_SL) -bundle $(BE_DLLLIBS) -o $@
diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk
index 271e7eaba8..29bee700d1 100644
--- a/src/makefiles/pgxs.mk
+++ b/src/makefiles/pgxs.mk
@@ -404,7 +404,7 @@ endif # REGRESS
.PHONY: submake
submake:
ifndef PGXS
- $(MAKE) -C $(top_builddir)/src/test/regress pg_regress$(X)
+ $(MAKE) -C $(top_builddir)/src/test/regress all
$(MAKE) -C $(top_builddir)/src/test/isolation all
endif
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 270bd6c856..d5501bce2d 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -122,6 +122,7 @@ INIT
$test_pghost = $use_tcp ? $test_localhost :
TestLib::tempdir_short;
$ENV{PGHOST} = $test_pghost;
$ENV{PGDATABASE} = 'postgres';
+ $ENV{PG_CTL_SHELL} = $ENV{PG_REGRESS_SHELL};
# Tracking of last port value assigned to accelerate free port lookup.
$last_port_assigned = int(rand() * 16384) + 49152;
diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index a24cfd4e01..62bc4fc744 100644
--- a/src/test/regress/GNUmakefile
+++ b/src/test/regress/GNUmakefile
@@ -75,6 +75,17 @@ input_files = $(patsubst $(srcdir)/input/%.source,sql/%.sql,
$(wildcard $(srcdir
output_files := $(patsubst $(srcdir)/output/%.source,expected/%.out,
$(wildcard $(srcdir)/output/*.source))
+# Workaround for macOS SIP: Replacement for system() library call that
+# does not filter DYLD_* environment variables
+
+ifeq ($(PORTNAME),darwin)
+all: fake_system.dylib
+
+fake_system.dylib: fake_system.c
+ $(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) -dynamiclib $< -o $@
+endif
+
+
# not installed by default
regress_data_files = \
@@ -167,6 +178,9 @@ clean distclean maintainer-clean: clean-lib
# things built by `all' target
rm -f $(OBJS) refint$(DLSUFFIX) autoinc$(DLSUFFIX)
rm -f pg_regress_main.o pg_regress.o pg_regress$(X)
+ifeq ($(PORTNAME),darwin)
+ rm -f fake_system.dylib
+endif
# things created by various check targets
rm -f $(output_files) $(input_files)
rm -rf testtablespace
diff --git a/src/test/regress/fake_system.c b/src/test/regress/fake_system.c
new file mode 100644
index 0000000000..eac4ba7834
--- /dev/null
+++ b/src/test/regress/fake_system.c
@@ -0,0 +1,121 @@
+/* $NetBSD: system.c,v 1.25 2015/01/20 18:31:25 christos Exp $ */
+
+/*
+ * Copyright (c) 1988, 1993
+ * The Regents of the University of California. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the University nor the names of its contributors
+ * may be used to endorse or promote products derived from this software
+ * without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#if defined(LIBC_SCCS) && !defined(lint)
+#if 0
+static char sccsid[] = "@(#)system.c 8.1 (Berkeley) 6/4/93";
+#else
+__RCSID("$NetBSD: system.c,v 1.25 2015/01/20 18:31:25 christos Exp $");
+#endif
+#endif /* LIBC_SCCS and not lint */
+
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <errno.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <stdio.h>
+
+extern char **environ;
+
+int
+system(const char *command)
+{
+ pid_t pid;
+ struct sigaction intsa, quitsa, sa;
+ sigset_t nmask, omask;
+ int pstat;
+ const char *argp[] = {"sh", "-c", NULL, NULL};
+ char *shell;
+
+ argp[2] = command;
+
+ shell = getenv("PG_REGRESS_SHELL");
+ if (!shell)
+ shell = "/bin/sh";
+
+ /*
+ * ISO/IEC 9899:1999 in 7.20.4.6 describes this special case.
+ * We need to check availability of a command interpreter.
+ */
+ if (command == NULL) {
+ if (access(shell, X_OK) == 0)
+ return 1;
+ return 0;
+ }
+
+ sa.sa_handler = SIG_IGN;
+ sigemptyset(&sa.sa_mask);
+ sa.sa_flags = 0;
+
+ if (sigaction(SIGINT, &sa, &intsa) == -1)
+ return -1;
+ if (sigaction(SIGQUIT, &sa, &quitsa) == -1) {
+ sigaction(SIGINT, &intsa, NULL);
+ return -1;
+ }
+
+ sigemptyset(&nmask);
+ sigaddset(&nmask, SIGCHLD);
+ if (sigprocmask(SIG_BLOCK, &nmask, &omask) == -1) {
+ sigaction(SIGINT, &intsa, NULL);
+ sigaction(SIGQUIT, &quitsa, NULL);
+ return -1;
+ }
+
+ switch(pid = vfork()) {
+ case -1: /* error */
+ sigaction(SIGINT, &intsa, NULL);
+ sigaction(SIGQUIT, &quitsa, NULL);
+ (void)sigprocmask(SIG_SETMASK, &omask, NULL);
+ return -1;
+ case 0: /* child */
+ sigaction(SIGINT, &intsa, NULL);
+ sigaction(SIGQUIT, &quitsa, NULL);
+ (void)sigprocmask(SIG_SETMASK, &omask, NULL);
+ execve(shell, (void *) argp, environ);
+ _exit(127);
+ }
+
+ while (waitpid(pid, &pstat, 0) == -1) {
+ if (errno != EINTR) {
+ pstat = -1;
+ break;
+ }
+ }
+
+ sigaction(SIGINT, &intsa, NULL);
+ sigaction(SIGQUIT, &quitsa, NULL);
+ (void)sigprocmask(SIG_SETMASK, &omask, NULL);
+
+ return (pstat);
+}
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index b4045abf21..c24d42b902 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1173,6 +1173,9 @@ spawn_process(const char *cmdline)
* parallel test case.
*/
char *cmdline2;
+ char *env = getenv("PG_REGRESS_SHELL");
+ if (env)
+ shellprog = env;
cmdline2 = psprintf("exec %s", cmdline);
execl(shellprog, shellprog, "-c", cmdline2, (char *) NULL);
--
2.22.0
