Re: macOS SIP, next try

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: macOS SIP, next try
Date: 2021-01-05 03:54:02
Message-ID: 3349159.1609818842@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> writes:
> The solution here is to use install_name_tool to edit the shared library
> path in the binaries (programs and libraries) after installation into
> the temporary prefix.

Ah, this indeed seems like a promising direction.

> The solution is, I think, correct in principle, but the implementation
> is hackish, since it hardcodes the names and versions of the interesting
> shared libraries in an unprincipled way. More refinement is needed
> here. Ideas welcome.

Yeah, it's pretty hacky. I tried the patch as given, and "make check"
failed with

error: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/install_name_tool: can't open file: /Users/tgl/pgsql/tmp_install//Users/tgl/testversion/lib/*.so (No such file or directory)
make: *** [temp-install] Error 1

evidently because there are no files named *.so in the "lib" directory
in my build. I removed '$(abs_top_builddir)/tmp_install/$(libdir)'/*.so
from the "call temp_install_postprocess" invocation, and then it got
through "make temp-install", and most of the core regression tests worked.
But a couple of them failed with

+ERROR: could not load library "/Users/tgl/pgsql/tmp_install/Users/tgl/testversion/lib/postgresql/libpqwalreceiver.so": dlopen(/Users/tgl/pgsql/tmp_install/Users/tgl/testversion/lib/postgresql/libpqwalreceiver.so, 10): Library not loaded: /Users/tgl/testversion/lib/libpq.5.dylib
+ Referenced from: /Users/tgl/pgsql/tmp_install/Users/tgl/testversion/lib/postgresql/libpqwalreceiver.so
+ Reason: image not found

What this looks like to me is some confusion about whether "pgsql"
or "postgresql" has been injected into the install path or not.
(This build is using /Users/tgl/testversion as the install --prefix;
I surmise that your path had "pgsql" or "postgres" in it already.)

One plausible way to fix this is to use "find" on the install tree
to locate files to modify, instead of hard-wiring knowledge into
the "call temp_install_postprocess" invocation. A possibly better
way is to arrange to invoke install_name_tool once per installed
executable or shlib, at the time we do "make install" for it.
Not sure how hard the latter approach is given our Makefiles.

Another suggestion, after reading "man install_name_tool", is
that maybe you could avoid having to know the names of the specific
libraries if you didn't use per-library '-change' switches, but
instead installed the test-installation rpath with the -add_rpath
switch. This would be closer to the spirit of the existing test
process anyway.

Aside from removing that annoying requirement, this method would
bound the amount of extra header space needed, so instead of
"-headerpad_max_install_names" we could use "-headerpad N" with
some not-hugely-generous N. That ought to damp down the executable
bloat to the point where we'd not have to worry too much about it.

(Pokes around ... it appears that on my old dinosaur prairiedog,
install_name_tool exists and has the -change switch, but not
anything for rpath. So we'd better investigate how old -add_rpath
is. Maybe there's another way to set rpath that works further back?
At least the -headerpad switches exist there.)

I haven't experimented with any of these ideas, so maybe they won't
work for some reason. But I think we ought to push forward looking
for a solution, because it's getting harder and harder to disable
SIP and still have a useful macOS installation :-(

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2021-01-05 03:58:36 Cirrus CI (Windows help wanted)
Previous Message Bruce Momjian 2021-01-05 03:47:39 Re: Moving other hex functions to /common