Tarantool development patches archive
 help / color / mirror / Atom feed
From: Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Ostanevich <sergos@tarantool.org>,
	Sergey Kaplun <skaplun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH luajit 3/3] test: fix dynamic modules loading on MacOS
Date: Mon,  5 Apr 2021 20:11:40 +0300	[thread overview]
Message-ID: <6ca8010540a67a92b36327abf44b489ebddc5054.1617641697.git.imun@tarantool.org> (raw)
In-Reply-To: <cover.1617641697.git.imun@tarantool.org>

This patch resolves the issue with running the tests with auxiliary
dynamically loaded modules in case of out-of-source build.

The first issue relates to the way modules loaded at runtime are built
on MacOS. Since the auxiliary libraries are built as a dynamically
loaded modules on MacOS instead of shared libraries as it is done on
Linux and BSD, another environment variable should be used to guide
<ffi.load> while searching the extension. Hence the values collected in
scope of <BuildTestCLib> macro need to be set to DYLD_LIBRARY_PATH
variable instead of LD_LIBRARY_PATH on MacOS.

Unfortunately, this rather small change doesn't resolve the problem at
all and the root cause lies much deeper than it seems at the beginning.

Apple tries their best to "protect their users from malicious software".
As a result SIP[1] has been designed and released. Now, Apple developers
are *so protected*, that they can load nothing being not installed in
the system, since some programs sanitize the environment before they
start child processes. Specifically, environment variables starting with
DYLD_ and LD_ are unset for child process started by system programs[2].

That which does not kill us makes us stronger: fortunately, these
environment variables are used by FFI machinery to find the proper
shared library, hence we can still tweak testing environment before
calling <ffi.load>. However, the value can't be passed via the standard
environment variable, so we prepend TEST_ prefix to its name to get
around SIP magic tricks. Finally, to set the variable required by FFI
machinery the introduced <utils.tweakenv> routine is used. PROFIT!
Your move, Cupertino geniuses.

[1]: https://support.apple.com/en-us/HT204899
[2]: https://developer.apple.com/library/archive/documentation/Security/Conceptual/System_Integrity_Protection_Guide/RuntimeProtections/RuntimeProtections.html

Resolves tarantool/tarantool#5959
Follows up tarantool/tarantool#4862

Co-authored-by: Sergey Ostanevich <sergos@tarantool.org>
Co-authored-by: Mons Anderson <mons@cpan.org>
Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 test/tarantool-tests/CMakeLists.txt           | 39 +++++++++++++++++--
 .../gh-4427-ffi-sandwich.test.lua             |  4 ++
 .../lj-flush-on-trace.test.lua                |  4 ++
 test/tarantool-tests/utils.lua                | 39 ++++++++++++++++---
 4 files changed, 77 insertions(+), 9 deletions(-)

diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index 3e36ff86..c793ad60 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -69,11 +69,46 @@ set(LUA_PATH
 )
 set(LUA_TEST_SUFFIX .test.lua)
 set(LUA_TEST_FLAGS --failures --shuffle)
+set(LUA_TEST_ENV
+  "LUA_PATH=\"${LUA_PATH}\;\;\""
+  "LUA_CPATH=\"${LUA_CPATH}\;\;\""
+)
 
 if(CMAKE_VERBOSE_MAKEFILE)
   list(APPEND LUA_TEST_FLAGS --verbose)
 endif()
 
+# XXX: Since the auxiliary libraries are built as a dynamically
+# loaded modules on MacOS instead of shared libraries as it is
+# done on Linux and BSD, another environment variable should be
+# used to guide <ffi.load> while searching the extension.
+if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
+  # XXX: Apple tries their best to "protect their users from
+  # malware". As a result SIP (see the link[1] below) has been
+  # designed and released. Now, Apple developers are so protected,
+  # that they can load nothing being not installed in the system,
+  # since some programs sanitize the environment before they start
+  # child processes. Specifically, environment variables starting
+  # with DYLD_ and LD_ are unset for child process started by
+  # system programs (like /usr/bin/env used for preparing testing
+  # environment). For more info, see the docs[2] below.
+  #
+  # That which does not kill us makes us stronger: fortunately,
+  # these environment variables are used by FFI machinery to find
+  # the proper shared library, hence we can still tweak testing
+  # environment before calling <ffi.load>. However, the value
+  # can't be passed via the standard environment variable, so we
+  # prepend TEST_ prefix to its name to get around SIP magic
+  # tricks. Finally, to set the variable required by FFI machinery
+  # the introduced <utils.tweakenv> routine is used.
+  #
+  # [1]: https://support.apple.com/en-us/HT204899
+  # [2]: https://developer.apple.com/library/archive/documentation/Security/Conceptual/System_Integrity_Protection_Guide/RuntimeProtections/RuntimeProtections.html
+  list(APPEND LUA_TEST_ENV TEST_DYLD_LIBRARY_PATH="${LD_LIBRARY_PATH}")
+else()
+  list(APPEND LUA_TEST_ENV LD_LIBRARY_PATH="${LD_LIBRARY_PATH}")
+endif()
+
 # LUA_CPATH and LD_LIBRARY_PATH variables and also TESTLIBS list
 # with dependecies are set in scope of BuildTestLib macro.
 add_custom_target(tarantool-tests
@@ -83,9 +118,7 @@ add_custom_command(TARGET tarantool-tests
   COMMENT "Running Tarantool tests"
   COMMAND
   env
-    LUA_PATH="${LUA_PATH}\;\;"
-    LUA_CPATH="${LUA_CPATH}\;\;"
-    LD_LIBRARY_PATH="${LD_LIBRARY_PATH}"
+    ${LUA_TEST_ENV}
     ${PROVE} ${CMAKE_CURRENT_SOURCE_DIR}
       --exec '${LUAJIT_TEST_COMMAND}'
       --ext ${LUA_TEST_SUFFIX}
diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
index 64df5dbd..651dc3f4 100644
--- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
+++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
@@ -3,6 +3,10 @@ local utils = require('utils')
 -- Disabled on *BSD due to #4819.
 utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')
 
+-- XXX: Tweak the process environment to get around SIP.
+-- See the comment in suite CMakeLists.txt for more info.
+utils.tweakenv(jit.os == 'OSX', 'DYLD_LIBRARY_PATH')
+
 utils.selfrun(arg, {
   {
     arg = {
diff --git a/test/tarantool-tests/lj-flush-on-trace.test.lua b/test/tarantool-tests/lj-flush-on-trace.test.lua
index edc5cf61..1ad4f832 100644
--- a/test/tarantool-tests/lj-flush-on-trace.test.lua
+++ b/test/tarantool-tests/lj-flush-on-trace.test.lua
@@ -3,6 +3,10 @@ local utils = require('utils')
 -- Disabled on *BSD due to #4819.
 utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')
 
+-- XXX: Tweak the process environment to get around SIP.
+-- See the comment in suite CMakeLists.txt for more info.
+utils.tweakenv(jit.os == 'OSX', 'DYLD_LIBRARY_PATH')
+
 utils.selfrun(arg, {
   {
     arg = {
diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
index d2dd71b0..9bdb71ec 100644
--- a/test/tarantool-tests/utils.lua
+++ b/test/tarantool-tests/utils.lua
@@ -1,7 +1,12 @@
 local M = {}
 
+local ffi = require('ffi')
 local tap = require('tap')
 
+ffi.cdef([[
+  int setenv(const char *name, const char *value, int overwrite);
+]])
+
 local function luacmd(args)
   -- arg[-1] is guaranteed to be not nil.
   local idx = -2
@@ -13,6 +18,12 @@ local function luacmd(args)
   return table.concat(args, ' ', idx + 1, -1)
 end
 
+local function unshiftenv(variable, value, sep)
+  local envvar = os.getenv(variable)
+  return ('%s="%s%s"'):format(variable, value,
+                              envvar and ('%s%s'):format(sep, envvar) or '')
+end
+
 function M.selfrun(arg, checks)
   -- If TEST_SELFRUN is set, just execute the test payload below
   -- <selfrun> call, ...
@@ -24,18 +35,22 @@ function M.selfrun(arg, checks)
 
   test:plan(#checks)
 
+  local libext = package.cpath:match('?.(%a+);')
   local vars = {
     LUABIN = luacmd(arg),
     SCRIPT = arg[0],
     PATH   = arg[0]:gsub('%.test%.lua', ''),
-    SUFFIX = package.cpath:match('?.(%a+);'),
+    SUFFIX = libext,
+    ENV = table.concat({
+      unshiftenv('LUA_PATH', '<PATH>/?.lua', ';'),
+      unshiftenv('LUA_CPATH', '<PATH>/?.<SUFFIX>', ';'),
+      unshiftenv((libext == 'dylib' and 'DYLD' or 'LD') .. '_LIBRARY_PATH',
+                 '<PATH>', ':'),
+      'TEST_SELFRUN=1',
+    }, ' '),
   }
 
-  local cmd = string.gsub('LUA_PATH="<PATH>/?.lua;$LUA_PATH" ' ..
-                          'LUA_CPATH="<PATH>/?.<SUFFIX>;$LUA_CPATH" ' ..
-                          'LD_LIBRARY_PATH=<PATH>:$LD_LIBRARY_PATH ' ..
-                          'TEST_SELFRUN=1' ..
-                          '<LUABIN> 2>&1 <SCRIPT>', '%<(%w+)>', vars)
+  local cmd = string.gsub('<ENV> <LUABIN> 2>&1 <SCRIPT>', '%<(%w+)>', vars)
 
   for _, ch in pairs(checks) do
     local testf = test[ch.test]
@@ -58,4 +73,16 @@ function M.skipcond(condition, message)
   os.exit(test:check() and 0 or 1)
 end
 
+function M.tweakenv(condition, variable)
+  if not condition or os.getenv(variable) then return end
+  local testvar = assert(os.getenv('TEST_' .. variable),
+                         ('Neither %s nor auxiliary TEST_%s variables are set')
+                         :format(variable, variable))
+  -- XXX: The third argument of setenv(3) is set to zero to forbid
+  -- overwriting the <variable>. Since there is the check above
+  -- whether this <variable> is set in the process environment, it
+  -- just makes this solution foolproof.
+  ffi.C.setenv(variable, testvar, 0)
+end
+
 return M
-- 
2.25.0


  parent reply	other threads:[~2021-04-05 17:13 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-05 17:11 [Tarantool-patches] [PATCH luajit 0/3] Fix out-of-source testing " Igor Munkin via Tarantool-patches
2021-04-05 17:11 ` [Tarantool-patches] [PATCH luajit 1/3] test: remove excess dependency for tests target Igor Munkin via Tarantool-patches
2021-04-06  7:38   ` Sergey Kaplun via Tarantool-patches
2021-04-06  8:02     ` Igor Munkin via Tarantool-patches
2021-04-06  9:51       ` Sergey Kaplun via Tarantool-patches
2021-04-06 15:38   ` Sergey Ostanevich via Tarantool-patches
2021-04-06 16:19     ` Igor Munkin via Tarantool-patches
2021-04-05 17:11 ` [Tarantool-patches] [PATCH luajit 2/3] test: make utils.selfrun usage easier Igor Munkin via Tarantool-patches
2021-04-06 13:01   ` Sergey Kaplun via Tarantool-patches
2021-04-06 13:35     ` Igor Munkin via Tarantool-patches
2021-04-06 16:22   ` Sergey Ostanevich via Tarantool-patches
2021-04-06 18:32     ` Igor Munkin via Tarantool-patches
2021-04-07 12:16       ` Sergey Ostanevich via Tarantool-patches
2021-04-05 17:11 ` Igor Munkin via Tarantool-patches [this message]
2021-04-06 17:02   ` [Tarantool-patches] [PATCH luajit 3/3] test: fix dynamic modules loading on MacOS Sergey Ostanevich via Tarantool-patches
2021-04-06 18:05     ` Igor Munkin via Tarantool-patches
2021-04-07 16:38   ` Sergey Kaplun via Tarantool-patches
2021-04-07 17:33     ` Igor Munkin via Tarantool-patches
2021-04-07 21:17 ` [Tarantool-patches] [PATCH luajit 0/3] Fix out-of-source testing " Igor Munkin via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6ca8010540a67a92b36327abf44b489ebddc5054.1617641697.git.imun@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imun@tarantool.org \
    --cc=sergos@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit 3/3] test: fix dynamic modules loading on MacOS' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox