Обсуждение: pg_preadv() and pg_pwritev()

Hello hackers,

I want to be able to do synchronous vectored file I/O, so I made
wrapper macros for preadv() and pwritev() with fallbacks for systems
that don't have them.  Following the precedent of the pg_pread() and
pg_pwrite() macros, the "pg_" prefix reflects a subtle contract
change: the fallback paths might have the side effect of changing the
file position.

They're non-standard system calls, but the BSDs and Linux have had
them for a long time, and for other systems we can use POSIX
readv()/writev() with an additional lseek().  The worst case is
Windows (and maybe our favourite antique Unix build farm animal?)
which has none of those things, so there is a further fallback to a
loop.  Windows does have ReadFileScatter() and WriteFileGather(), but
those only work for overlapped (= asynchronous), unbuffered, page
aligned access.  They'll very likely be useful for native AIO+DIO
support in the future, but don't fit the bill here.

This is part of a project to consolidate and offload I/O (about which
more soon), but seemed isolated enough to post separately and I guess
it could be independently useful.

Thomas Munro <thomas.munro@gmail.com> writes:
> I want to be able to do synchronous vectored file I/O, so I made
> wrapper macros for preadv() and pwritev() with fallbacks for systems
> that don't have them.  Following the precedent of the pg_pread() and
> pg_pwrite() macros, the "pg_" prefix reflects a subtle contract
> change: the fallback paths might have the side effect of changing the
> file position.

In a quick look, seems OK with some nits:

1. port.h cannot assume that <limits.h> has already been included;
nor do I want to fix that by including <limits.h> there.  Do we
really need to define a fallback value of IOV_MAX?  If so,
maybe the answer is to put the replacement struct iovec and
IOV_MAX in some new header.

2. I'm not really that happy about loading <sys/uio.h> into
every compilation we do, which would be another reason for a
new specialized header that either includes <sys/uio.h> or
provides fallback definitions.

3. The patch as given won't prove anything except that the code
compiles.  Is it worth fixing at least one code path to make
use of pg_preadv and pg_pwritev, so we can make sure this code
is tested before there's layers of other new code on top?

            regards, tom lane

On Sun, Dec 20, 2020 at 12:34 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Thomas Munro <thomas.munro@gmail.com> writes:
> > I want to be able to do synchronous vectored file I/O, so I made
> > wrapper macros for preadv() and pwritev() with fallbacks for systems
> > that don't have them.  Following the precedent of the pg_pread() and
> > pg_pwrite() macros, the "pg_" prefix reflects a subtle contract
> > change: the fallback paths might have the side effect of changing the
> > file position.
>
> In a quick look, seems OK with some nits:

Thanks for looking!

> 1. port.h cannot assume that <limits.h> has already been included;
> nor do I want to fix that by including <limits.h> there.  Do we
> really need to define a fallback value of IOV_MAX?  If so,
> maybe the answer is to put the replacement struct iovec and
> IOV_MAX in some new header.

Ok, I moved all this stuff into port/pg_uio.h.

> 2. I'm not really that happy about loading <sys/uio.h> into
> every compilation we do, which would be another reason for a
> new specialized header that either includes <sys/uio.h> or
> provides fallback definitions.

Ack.

> 3. The patch as given won't prove anything except that the code
> compiles.  Is it worth fixing at least one code path to make
> use of pg_preadv and pg_pwritev, so we can make sure this code
> is tested before there's layers of other new code on top?

OK, here's a patch to zero-fill fresh WAL segments with pwritev().

I'm drawing a blank on trivial candidate uses for preadv(), without
infrastructure from later patches.

Thomas Munro <thomas.munro@gmail.com> writes:
> OK, here's a patch to zero-fill fresh WAL segments with pwritev().
> I'm drawing a blank on trivial candidate uses for preadv(), without
> infrastructure from later patches.

This looks OK to me.  I tried it on prairiedog (has writev and
pwrite but not pwritev) as well as gaur (has only writev).
They seem happy.

One minor thought is that in

+        struct iovec iov[Min(IOV_MAX, 1024)];    /* cap stack space */

it seems like pretty much every use of IOV_MAX would want some
similar cap.  Should we centralize that idea with, say,

#define PG_IOV_MAX  Min(IOV_MAX, 1024)

?  Or will the plausible cap vary across uses?

            regards, tom lane

On Sun, Dec 20, 2020 at 8:07 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> One minor thought is that in
>
> +               struct iovec iov[Min(IOV_MAX, 1024)];   /* cap stack space */
>
> it seems like pretty much every use of IOV_MAX would want some
> similar cap.  Should we centralize that idea with, say,
>
> #define PG_IOV_MAX  Min(IOV_MAX, 1024)
>
> ?  Or will the plausible cap vary across uses?

Hmm.  For the real intended user of this, namely worker processes that
simulate AIO when native AIO isn't available, higher level code will
limit the iov count to much smaller numbers anyway.  It wants to try
to stay under typical device limits for vectored I/O, because split
requests would confound attempts to model and limit queue depth and
control latency.  In Andres's AIO prototype he currently has a macro
PGAIO_MAX_COMBINE set to 16 (meaning approximately 16 data block or
wal reads/writes = 128KB worth of scatter/gather per I/O request); I
guess it should really be Min(IOV_MAX, <something>), but I don't
currently have an opinion on the <something>, except that it should
surely be closer to 16 than 1024 (for example
/sys/block/nvme0n1/queue/max_segments is 33 here).  I mention all this
to explain that I don't think the code in patch 0002 is going to turn
out to be very typical: it's trying to minimise system calls by
staying under an API limit (though I cap it for allocation sanity),
whereas more typical code probably wants to stay under a device limit,
so I don't immediately have another use for eg PG_IOV_MAX.

Hi,

On 2020-12-20 16:26:42 +1300, Thomas Munro wrote:
> > 1. port.h cannot assume that <limits.h> has already been included;
> > nor do I want to fix that by including <limits.h> there.  Do we
> > really need to define a fallback value of IOV_MAX?  If so,
> > maybe the answer is to put the replacement struct iovec and
> > IOV_MAX in some new header.
> 
> Ok, I moved all this stuff into port/pg_uio.h.

Can we come up with a better name than 'uio'? I find that a not exactly
meaningful name.

Or perhaps we could just leave the functions in port/port.h, but extract
the value of the define at configure time? That way only pread.c /
pwrite.c would need it.


> > 3. The patch as given won't prove anything except that the code
> > compiles.  Is it worth fixing at least one code path to make
> > use of pg_preadv and pg_pwritev, so we can make sure this code
> > is tested before there's layers of other new code on top?
> 
> OK, here's a patch to zero-fill fresh WAL segments with pwritev().

I think that's a good idea. However, I see two small issues: 1) If we
write larger amounts at once, we need to handle partial writes. That's a
large enough amount of IO that it's much more likely to hit a memory
shortage or such in the kernel - we had to do that a while a go for WAL
writes (which can also be larger), if memory serves.

Perhaps we should have pg_pwritev/readv unconditionally go through
pwrite.c/pread.c and add support for "continuing" partial writes/reads
in one central place?


> I'm drawing a blank on trivial candidate uses for preadv(), without
> infrastructure from later patches.

Can't immediately think of something either.


Greetings,

Andres Freund

> > I'm drawing a blank on trivial candidate uses for preadv(), without
> > infrastructure from later patches.
>
> Can't immediately think of something either.

This might be not that trivial , but maybe acquire_sample_rows() from analyze.c ?

Please note however there's patch https://www.postgresql.org/message-id/20201109180644.GJ16415%40tamriel.snowman.net (
https://commitfest.postgresql.org/30/2799/) for adding fadvise, but maybe those two could be even combined so you would
bedoing e.g. 16x fadvise() and then grab 8 pages in one preadv() call ?  I'm find unlikely however that preadv give any
additionalperformance benefit there after having fadvise, but clearly a potential place to test. 

-J.

On Mon, Dec 21, 2020 at 8:25 PM Jakub Wartak <Jakub.Wartak@tomtom.com> wrote:
> > > I'm drawing a blank on trivial candidate uses for preadv(), without
> > > infrastructure from later patches.
> >
> > Can't immediately think of something either.
>
> This might be not that trivial , but maybe acquire_sample_rows() from analyze.c ?
>
> Please note however there's patch https://www.postgresql.org/message-id/20201109180644.GJ16415%40tamriel.snowman.net
(https://commitfest.postgresql.org/30/2799/ ) for adding fadvise, but maybe those two could be even combined so you
wouldbe doing e.g. 16x fadvise() and then grab 8 pages in one preadv() call ?  I'm find unlikely however that preadv
giveany additional performance benefit there after having fadvise, but clearly a potential place to test. 

Oh, interesting, that looks like another test case to study with the
AIO patch set, but I don't think it's worth trying to do a
simpler/half-baked version in the meantime.  (Since that ANALYZE patch
uses PrefetchBuffer() it should automatically benefit: the
posix_fadvise() calls will be replaced with consolidated preadv()
calls in a worker process or native AIO equivalent so that system
calls are mostly gone from the initiating process, and by the time you
try to access the buffer it'll hopefully see that it's finished
without any further system calls.  Refinements are possible though,
like making use of recent_buffer to avoid double-lookup, and
tuning/optimisation for how often IOs should be consolidated and
submitted.)

On Mon, Dec 21, 2020 at 11:40 AM Andres Freund <andres@anarazel.de> wrote:
> On 2020-12-20 16:26:42 +1300, Thomas Munro wrote:
> > > 1. port.h cannot assume that <limits.h> has already been included;
> > > nor do I want to fix that by including <limits.h> there.  Do we
> > > really need to define a fallback value of IOV_MAX?  If so,
> > > maybe the answer is to put the replacement struct iovec and
> > > IOV_MAX in some new header.
> >
> > Ok, I moved all this stuff into port/pg_uio.h.
>
> Can we come up with a better name than 'uio'? I find that a not exactly
> meaningful name.

Ok, let's try port/pg_iovec.h.

> Or perhaps we could just leave the functions in port/port.h, but extract
> the value of the define at configure time? That way only pread.c /
> pwrite.c would need it.

That won't work for the struct definition, so client code would need
to remember to do:

#ifdef HAVE_SYS_UIO_H
#include <sys/uio.h>
#endif

... which is a little tedious, or port.h would need to do that and
incur an overhead in every translation unit, which Tom objected to.
That's why I liked the separate header idea.

> > > 3. The patch as given won't prove anything except that the code
> > > compiles.  Is it worth fixing at least one code path to make
> > > use of pg_preadv and pg_pwritev, so we can make sure this code
> > > is tested before there's layers of other new code on top?
> >
> > OK, here's a patch to zero-fill fresh WAL segments with pwritev().
>
> I think that's a good idea. However, I see two small issues: 1) If we
> write larger amounts at once, we need to handle partial writes. That's a
> large enough amount of IO that it's much more likely to hit a memory
> shortage or such in the kernel - we had to do that a while a go for WAL
> writes (which can also be larger), if memory serves.
>
> Perhaps we should have pg_pwritev/readv unconditionally go through
> pwrite.c/pread.c and add support for "continuing" partial writes/reads
> in one central place?

Ok, here's a new version with the following changes:

1.  Define PG_IOV_MAX, a reasonable size for local variables, not
larger than IOV_MAX.
2   Use 32 rather than 1024, based on off-list complaint about 1024
potentially swamping the IO system unfairly.
3.  Add a wrapper pg_pwritev_retry() to retry automatically on short
writes.  (I didn't write pg_preadv_retry(), because I don't currently
need it for anything so I didn't want to have to think about EOF vs
short-reads-for-implementation-reasons.)
4.  I considered whether pg_pwrite() should have built-in retry
instead of a separate wrapper, but I thought of an argument against
hiding the "raw" version: the AIO patch set already understands short
reads/writes and knows how to retry at a higher level, as that's
needed for native AIO too, so I think it makes sense to be able to
keep things the same and not solve the same problem twice.  A counter
argument would be that you could get the retry underway sooner with a
tight loop, but I'm not expecting this to be common.

> > I'm drawing a blank on trivial candidate uses for preadv(), without
> > infrastructure from later patches.
>
> Can't immediately think of something either.

One idea I had for the future is for xlogreader.c to read the WAL into
a large multi-page circular buffer, which could wrap around using a
pair of iovecs, but that requires lots more work    .

On Wed, Dec 23, 2020 at 12:06 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Mon, Dec 21, 2020 at 11:40 AM Andres Freund <andres@anarazel.de> wrote:
> > Can we come up with a better name than 'uio'? I find that a not exactly
> > meaningful name.
>
> Ok, let's try port/pg_iovec.h.

I pushed it with that name, and a couple more cosmetic changes.  I'll
keep an eye on the build farm.

On Mon, Jan 11, 2021 at 3:34 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Wed, Dec 23, 2020 at 12:06 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> > On Mon, Dec 21, 2020 at 11:40 AM Andres Freund <andres@anarazel.de> wrote:
> > > Can we come up with a better name than 'uio'? I find that a not exactly
> > > meaningful name.
> >
> > Ok, let's try port/pg_iovec.h.
>
> I pushed it with that name, and a couple more cosmetic changes.  I'll
> keep an eye on the build farm.

Since only sifaka has managed to return a result so far (nice CPU), I
had plenty of time to notice that macOS Big Sur has introduced
preadv/pwritev.  They were missing on Catalina[1].

[1] https://cirrus-ci.com/task/6002157537198080

On Mon, Jan 11, 2021 at 3:59 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Mon, Jan 11, 2021 at 3:34 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> > I pushed it with that name, and a couple more cosmetic changes.  I'll
> > keep an eye on the build farm.
>
> Since only sifaka has managed to return a result so far (nice CPU), I
> had plenty of time to notice that macOS Big Sur has introduced
> preadv/pwritev.  They were missing on Catalina[1].

The rest of buildfarm was OK with it too, but I learned of a small
problem through CI testing of another patch: it's not OK for
src/port/pwrite.c to do this:

            if (part > 0)
                elog(ERROR, "unexpectedly wrote more than requested");

... because now when I try to use pg_pwrite() in pg_test_fsync,
Windows fails to link:

libpgport.lib(pwrite.obj) : error LNK2019: unresolved external symbol
errstart referenced in function pg_pwritev_with_retry
[C:\projects\postgresql\pg_test_fsync.vcxproj]

I'll go and replace that with an assertion.

On 11.01.2021 05:59, Thomas Munro wrote:
> Since only sifaka has managed to return a result so far (nice CPU), I
> had plenty of time to notice that macOS Big Sur has introduced
> preadv/pwritev.  They were missing on Catalina[1].
> 
> [1] https://cirrus-ci.com/task/6002157537198080

Hi, Thomas!

Indeed, pwritev is not available on macOS Catalina. So I get compiler 
warnings about that:

/Users/shinderuk/src/pgwork/devel/build/../src/port/pwrite.c:117:10: 
warning: 'pwritev' is only available on macOS 11.0 or newer 
[-Wunguarded-availability-new]
                 part = pg_pwritev(fd, iov, iovcnt, offset);
                        ^~~~~~~~~~
/Users/shinderuk/src/pgwork/devel/build/../src/include/port/pg_iovec.h:49:20: 
note: expanded from macro 'pg_pwritev'
#define pg_pwritev pwritev
                    ^~~~~~~

/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk/usr/include/sys/uio.h:104:9:

note: 'pwritev' has been marked as being introduced in macOS 11.0 here, 
but the deployment target is macOS
       10.15.0
ssize_t pwritev(int, const struct iovec *, int, off_t) 
__DARWIN_NOCANCEL(pwritev) __API_AVAILABLE(macos(11.0), ios(14.0), 
watchos(7.0), tvos(14.0));
         ^
/Users/shinderuk/src/pgwork/devel/build/../src/port/pwrite.c:117:10: 
note: enclose 'pwritev' in a __builtin_available check to silence this 
warning
                 part = pg_pwritev(fd, iov, iovcnt, offset);
                        ^~~~~~~~~~
/Users/shinderuk/src/pgwork/devel/build/../src/include/port/pg_iovec.h:49:20: 
note: expanded from macro 'pg_pwritev'
#define pg_pwritev pwritev
                    ^~~~~~~
1 warning generated.
(... several more warnings ...)


And initdb fails:

running bootstrap script ... dyld: lazy symbol binding failed: Symbol 
not found: _pwritev
   Referenced from: /Users/shinderuk/src/pgwork/devel/install/bin/postgres
   Expected in: /usr/lib/libSystem.B.dylib

dyld: Symbol not found: _pwritev
   Referenced from: /Users/shinderuk/src/pgwork/devel/install/bin/postgres
   Expected in: /usr/lib/libSystem.B.dylib


Regards.

-- 
Sergey Shinderuk
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

On Wed, Jan 13, 2021 at 10:40 PM Sergey Shinderuk
<s.shinderuk@postgrespro.ru> wrote:
>
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk/usr/include/sys/uio.h:104:9:
> note: 'pwritev' has been marked as being introduced in macOS 11.0 here,
> but the deployment target is macOS
>        10.15.0
> ssize_t pwritev(int, const struct iovec *, int, off_t)
> __DARWIN_NOCANCEL(pwritev) __API_AVAILABLE(macos(11.0), ios(14.0),
> watchos(7.0), tvos(14.0));
>          ^

Hrm...  So why did "configure" think you have pwritev, then?  It seems
like you must have been using different compilers or options at
configure time and compile time, no?

On 13.01.2021 12:56, Thomas Munro wrote:
> On Wed, Jan 13, 2021 at 10:40 PM Sergey Shinderuk
> <s.shinderuk@postgrespro.ru> wrote:
>>
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk/usr/include/sys/uio.h:104:9:
>> note: 'pwritev' has been marked as being introduced in macOS 11.0 here,
>> but the deployment target is macOS
>>         10.15.0
>> ssize_t pwritev(int, const struct iovec *, int, off_t)
>> __DARWIN_NOCANCEL(pwritev) __API_AVAILABLE(macos(11.0), ios(14.0),
>> watchos(7.0), tvos(14.0));
>>           ^
> 
> Hrm...  So why did "configure" think you have pwritev, then?  It seems
> like you must have been using different compilers or options at
> configure time and compile time, no?
> 

No, i've just rerun configure from clean checkout without any options. 
It does think that pwritev is available. I'll try to figure this out 
later and come back to you. Thanks.

Sergey Shinderuk <s.shinderuk@postgrespro.ru> writes:
> On 13.01.2021 12:56, Thomas Munro wrote:
>> On Wed, Jan 13, 2021 at 10:40 PM Sergey Shinderuk
>> <s.shinderuk@postgrespro.ru> wrote:
>>> note: 'pwritev' has been marked as being introduced in macOS 11.0 here,
>>> but the deployment target is macOS 10.15.0

>> Hrm...  So why did "configure" think you have pwritev, then?  It seems
>> like you must have been using different compilers or options at
>> configure time and compile time, no?

> No, i've just rerun configure from clean checkout without any options.
> It does think that pwritev is available. I'll try to figure this out
> later and come back to you. Thanks.

The symptoms sound consistent with using bleeding-edge Xcode on a
Catalina machine ... please report exact OS and Xcode versions.

I have a different complaint, using Big Sur and Xcode 12.3:

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file:
libpgport.a(pread.o)has no symbols 
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file:
libpgport_shlib.a(pread_shlib.o)has no symbols 
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file:
libpgport_srv.a(pread_srv.o)has no symbols 

Looks like we need to be more careful about not including pread.c
in the build unless it actually has code to contribute.

            regards, tom lane

I wrote:
> Sergey Shinderuk <s.shinderuk@postgrespro.ru> writes:
>>>> note: 'pwritev' has been marked as being introduced in macOS 11.0 here,
>>>> but the deployment target is macOS 10.15.0

> The symptoms sound consistent with using bleeding-edge Xcode on a
> Catalina machine ... please report exact OS and Xcode versions.

I can reproduce these warnings on Big Sur + Xcode 12.3 by doing

export MACOSX_DEPLOYMENT_TARGET=10.15

before building; however the executable runs anyway, which I guess
is unsurprising.  AFAICS from config.log, configure has no idea
that anything is wrong.

(BTW, at least the rather-old version of ccache I'm using does not
seem to realize that that environment variable is significant;
I had to clear ~/.ccache to get consistent results.)

We've had issues before with weird results from Xcode versions
newer than the underlying OS.  In the past we've been able to
work around that, but I'm not sure that I see a way here.

            regards, tom lane

Hmmm ... I can further report that on Catalina + Xcode 12.0,
everything seems fine.  configure correctly detects that preadv
and pwritev aren't there:

configure:15161: checking for preadv
configure:15161: ccache gcc -o conftest -Wall -Wmissing-prototypes -Wpointer-ar\
ith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-a\
ttribute -Wformat-security -fno-strict-aliasing -fwrapv -Wno-unused-command-lin\
e-argument -g -O2 -isysroot /Applications/Xcode.app/Contents/Developer/Platform\
s/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk   -isysroot /Applications/Xcod\
e.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.s\
dk   conftest.c -lz  -lm  >&5
Undefined symbols for architecture x86_64:
  "_preadv", referenced from:
      _main in conftest-fca7e9.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
configure:15161: $? = 1

So I'm a little confused as to why this test is failing to fail
with (I assume) newer Xcode.  Can we see the relevant part of
config.log on your machine?

            regards, tom lane

On Thu, Jan 14, 2021 at 5:13 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I have a different complaint, using Big Sur and Xcode 12.3:
>
> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file:
libpgport.a(pread.o)has no symbols
 
> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file:
libpgport_shlib.a(pread_shlib.o)has no symbols
 
> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file:
libpgport_srv.a(pread_srv.o)has no symbols
 
>
> Looks like we need to be more careful about not including pread.c
> in the build unless it actually has code to contribute.

I did it that way because it made it easy to test different
combinations of the replacements on computers that do actually have
pwrite and pwritev, just by tweaking pg_config.h.  Here's an attempt
to do it with AC_REPLACE_FUNCS, which avoids creating empty .o files.
It means that to test the replacements on modern systems you have to
tweak pg_config.h and also add the relevant .o files to LIBOBJS in
src/Makefile.global, but that seems OK.

Thomas Munro <thomas.munro@gmail.com> writes:
> On Thu, Jan 14, 2021 at 5:13 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Looks like we need to be more careful about not including pread.c
>> in the build unless it actually has code to contribute.

> I did it that way because it made it easy to test different
> combinations of the replacements on computers that do actually have
> pwrite and pwritev, just by tweaking pg_config.h.  Here's an attempt
> to do it with AC_REPLACE_FUNCS, which avoids creating empty .o files.
> It means that to test the replacements on modern systems you have to
> tweak pg_config.h and also add the relevant .o files to LIBOBJS in
> src/Makefile.global, but that seems OK.

Yeah, this looks better.  Two gripes, one major and one minor:

* You need to remove pread.o and pwrite.o from the hard-wired
part of the list in src/port/Makefile, else they get built
whether needed or not.

* I don't much like this in fd.h:

@@ -46,6 +46,7 @@
 #include <dirent.h>
 
 
+struct iovec;
 typedef int File;

because it makes it look like iovec and File are of similar
status, which they hardly are.  Perhaps more like

 #include <dirent.h>
+ 
+struct iovec;            /* avoid including sys/uio.h here */
 
 
 typedef int File;


I confirm clean builds on Big Sur and Catalina with this.

            regards, tom lane

On Thu, Jan 14, 2021 at 9:26 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> * You need to remove pread.o and pwrite.o from the hard-wired
> part of the list in src/port/Makefile, else they get built
> whether needed or not.

Right, done.

> * I don't much like this in fd.h:
>
> @@ -46,6 +46,7 @@
>  #include <dirent.h>
>
>
> +struct iovec;
>  typedef int File;
>
> because it makes it look like iovec and File are of similar
> status, which they hardly are.  Perhaps more like
>
>  #include <dirent.h>
> +
> +struct iovec;                  /* avoid including sys/uio.h here */

Done, except I wrote port/pg_iovec.h.

> I confirm clean builds on Big Sur and Catalina with this.

Thanks for checking.  I also checked on Windows via CI.  Pushed.

I wrote:
> So I'm a little confused as to why this test is failing to fail
> with (I assume) newer Xcode.  Can we see the relevant part of
> config.log on your machine?

After further digging I believe I understand what's happening,
and it's a bit surprising we've not been bit by it before.
If the compiler believes (thanks to __API_AVAILABLE macros in
Apple's system headers) that a given library symbol might not
exist in the lowest macOS version it is compiling for, it will
still emit a normal call to that function ... but it also emits

       .weak_reference _preadv

marking the call as a weak reference.  If the linker then fails
to link that call, it doesn't throw an error, it just replaces
the call instruction with a NOP :-(.  This is why configure's
test appears to succeed, since it only checks whether you can
link not whether the call would work at runtime.  Apple's
assumption evidently is that you'll guard the call with a run-time
check to see if the function exists before you use it, and you
don't want your link to fail if it doesn't.

The solution to this, according to "man ld", is

     -no_weak_imports
                 Error if any symbols are weak imports (i.e. allowed to be
                 unresolved (NULL) at runtime). Useful for config based
                 projects that assume they are built and run on the same OS
                 version.

I don't particularly care that Apple is looking down their nose
at people who don't want to make their builds run on multiple OS
versions, so I think we should just use this and call it good.

Attached is an untested quick hack to make that happen --- Sergey,
can you verify that this fixes configure's results on your setup?

(This is not quite committable as-is, it needs something to avoid
adding -Wl,-no_weak_imports on ancient macOS versions.  But it
will do to see if the fix works on modern versions.)

            regards, tom lane

diff --git a/src/template/darwin b/src/template/darwin
index 32414d21a9..22d04e7ba7 100644
--- a/src/template/darwin
+++ b/src/template/darwin
@@ -17,6 +17,9 @@ if test x"$PG_SYSROOT" != x"" ; then
   fi
 fi
 
+# Disable weak linking, else configure's checks will misbehave
+LDFLAGS="-Wl,-no_weak_imports $LDFLAGS"
+
 # Extra CFLAGS for code that will go into a shared library
 CFLAGS_SL=""

Tom Lane wrote:
> The symptoms sound consistent with using bleeding-edge Xcode on a
> Catalina machine ... please report exact OS and Xcode versions.

macOS 10.15.7 (19H2)
Xcode 12.3 (12C33)
macOS SDK 11.1 (20C63)


> Attached is an untested quick hack to make that happen --- Sergey,
> can you verify that this fixes configure's results on your setup?

"-no_weak_imports" doesn't help.

configure:15161: checking for pwritev
configure:15161: gcc -o conftest -Wall -Wmissing-prototypes 
-Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels 
-Wmissing-format-attribute -Wformat-security -fno-strict-aliasing 
-fwrapv -Wno-unused-command-line-argument -O2 -isysroot 
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk 
   -Wl,-no_weak_imports -isysroot 
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk 
   conftest.c -lz  -lm  >&5
configure:15161: $? = 0
configure:15161: result: yes


Then I get the same compiler warnings about pwritev and an unrelated 
link error:

ld: weak import of symbol '___darwin_check_fd_set_overflow' not 
supported because of option: -no_weak_imports for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see 
invocation)
make[2]: *** [postgres] Error 1
make[1]: *** [all-backend-recurse] Error 2
make: *** [all-src-recurse] Error 2

Please see the logs attached.

>> The symptoms sound consistent with using bleeding-edge Xcode on a
>> Catalina machine ... please report exact OS and Xcode versions.
> 
> macOS 10.15.7 (19H2)
> Xcode 12.3 (12C33)
> macOS SDK 11.1 (20C63)
> 

Everything is fine if I run "configure" with
PG_SYSROOT=/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk

I noticed that "cc" invoked from command line uses:
-isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk

But "xcodebuild -version -sdk macosx Path" invoked by "configure" when 
PG_SYSROOT is not provided gives:
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk

Now I'm confused about different SDK versions and locations used by 
Xcode and CommandLineTools :)

Sergey Shinderuk <s.shinderuk@postgrespro.ru> writes:
> I noticed that "cc" invoked from command line uses:
> -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk

Hm, how did you determine that exactly?

            regards, tom lane

On 14.01.2021 18:42, Tom Lane wrote:
>> I noticed that "cc" invoked from command line uses:
>> -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk
> 
> Hm, how did you determine that exactly?
> 

% echo 'int main(void){}' >tmp.c
% cc -v tmp.c
Apple clang version 12.0.0 (clang-1200.0.32.28)
Target: x86_64-apple-darwin19.6.0
Thread model: posix
InstalledDir: 
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
 
"/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang" 
-cc1 -triple x86_64-apple-macosx10.15.0 -Wdeprecated-objc-isa-usage 
-Werror=deprecated-objc-isa-usage -Werror=implicit-function-declaration 
-emit-obj -mrelax-all -disable-free -disable-llvm-verifier 
-discard-value-names -main-file-name tmp.c -mrelocation-model pic 
-pic-level 2 -mthread-model posix -mframe-pointer=all -fno-strict-return 
-masm-verbose -munwind-tables -target-sdk-version=10.15.6 
-fcompatibility-qualified-id-block-type-checking -target-cpu penryn 
-dwarf-column-info -debugger-tuning=lldb -target-linker-version 609.8 -v 
-resource-dir 
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/12.0.0 
-isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk 
-I/usr/local/include -internal-isystem 
/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/local/include 
-internal-isystem 
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/12.0.0/include 
-internal-externc-isystem 
/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/include 
-internal-externc-isystem 
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include 
-Wno-reorder-init-list -Wno-implicit-int-float-conversion 
-Wno-c99-designator -Wno-final-dtor-non-final-class -Wno-extra-semi-stmt 
-Wno-misleading-indentation -Wno-quoted-include-in-framework-header 
-Wno-implicit-fallthrough -Wno-enum-enum-conversion 
-Wno-enum-float-conversion -fdebug-compilation-dir /Users/shinderuk 
-ferror-limit 19 -fmessage-length 238 -stack-protector 1 -fstack-check 
-mdarwin-stkchk-strong-link -fblocks -fencode-extended-block-signature 
-fregister-global-dtors-with-atexit -fgnuc-version=4.2.1 
-fobjc-runtime=macosx-10.15.0 -fmax-type-align=16 
-fdiagnostics-show-option -fcolor-diagnostics -o 
/var/folders/8x/jvqv7hyd5h98m7tz2zm9r0yc0000gn/T/tmp-91fb5e.o -x c tmp.c
clang -cc1 version 12.0.0 (clang-1200.0.32.28) default target 
x86_64-apple-darwin19.6.0
ignoring nonexistent directory 
"/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/local/include"
ignoring nonexistent directory 
"/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/Library/Frameworks"
#include "..." search starts here:
#include <...> search starts here:
  /usr/local/include
 
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/12.0.0/include
  /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/include
 
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include
 
/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/System/Library/Frameworks 
(framework directory)
End of search list.
 
"/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ld" 
-demangle -lto_library 
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/libLTO.dylib 
-no_deduplicate -dynamic -arch x86_64 -platform_version macos 10.15.0 
10.15.6 -syslibroot 
/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk -o a.out 
-L/usr/local/lib 
/var/folders/8x/jvqv7hyd5h98m7tz2zm9r0yc0000gn/T/tmp-91fb5e.o -lSystem 

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/12.0.0/lib/darwin/libclang_rt.osx.a

Sergey Shinderuk <s.shinderuk@postgrespro.ru> writes:
> On 14.01.2021 18:42, Tom Lane wrote:
>>> I noticed that "cc" invoked from command line uses:
>>> -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk

>> Hm, how did you determine that exactly?

> % cc -v tmp.c
> ...
> -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk

Okay, interesting.  On my Catalina machine, I see

-isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk

which is also a 10.15 SDK, since I haven't upgraded Xcode past 12.0.
I wonder if that would change if I did upgrade (but I don't plan to
risk it, since this is my only remaining Catalina install).

After considerable playing around, I'm guessing that the reason
-no_weak_imports doesn't help is that it rejects calls that are
marked as weak references on the *calling* side.  Since AC_CHECK_FUNCS
doesn't bother to #include the relevant header file, the compiler
doesn't know that preadv() ought to be marked as a weak reference.
Then, when the test program gets linked against the stub libc that's
provided by the SDK, there is a version of preadv() there so no link
failure occurs.  (There are way more moving parts in this weak-reference
thing than I'd realized.)

It seems like the more productive approach would be to try to identify
the right sysroot to use.  I wonder if there is some less messy way
to find out the compiler's default sysroot than to scrape it out of
-v output.

Another thing I've been realizing while poking at this is that we
might not need to set -isysroot explicitly at all, which would then
lead to the compiler using its default sysroot automatically.
In some experimentation, it seems like what we need PG_SYSROOT for
is just for configure to be able to find tclConfig.sh and the Perl
header files.  So at this point I'm tempted to try ripping that
out altogether.  If you remove the lines in src/template/darwin
that inject PG_SYSROOT into CPPFLAGS and LDFLAGS, do things
work for you?

            regards, tom lane

I wrote:
> It seems like the more productive approach would be to try to identify
> the right sysroot to use.  I wonder if there is some less messy way
> to find out the compiler's default sysroot than to scrape it out of
> -v output.

This is, of course, not terribly well documented by Apple.  But
Mr. Google suggests that "xcrun --show-sdk-path" might serve.
What does that print for you?

            regards, tom lane

I borrowed my wife's Mac, which is still on Catalina and up to now
never had Xcode on it, and found some very interesting things.

Step 1: download/install Xcode 12.3, open it, agree to license,
wait for it to finish "installing components".

At this point, /Library/Developer/CommandLineTools doesn't exist,
and we have the following outputs from various probe commands:

% xcrun --show-sdk-path
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk
% xcrun --sdk macosx --show-sdk-path
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk
% xcodebuild -version -sdk macosx Path
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk

Also, cc -v reports
  -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk

Unsurprisingly, Xcode 12.3 itself only contains

% ls -l /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs
total 0
drwxr-xr-x  5 root  wheel  160 Nov 30 07:27 DriverKit20.2.sdk
drwxr-xr-x  7 root  wheel  224 Nov 30 07:27 MacOSX.sdk
lrwxr-xr-x  1 root  wheel   10 Jan 14 15:57 MacOSX11.1.sdk -> MacOSX.sdk

Step 2: install command line tools (I used "xcode-select --install"
to fire this off, rather than the Xcode menu item).

Now I have

% ls -l /Library/Developer/CommandLineTools/SDKs
total 0
lrwxr-xr-x  1 root  wheel   14 Jan 14 16:42 MacOSX.sdk -> MacOSX11.1.sdk
drwxr-xr-x  8 root  wheel  256 Jul  9  2020 MacOSX10.15.sdk
drwxr-xr-x  7 root  wheel  224 Nov 30 07:33 MacOSX11.1.sdk

which is pretty interesting in itself, because the same directory on
my recently-updated-to-Big-Sur Macs does NOT have the 11.1 SDK.
I wonder what determines which versions get installed here.

More interesting yet:

% xcrun --show-sdk-path
/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk
% xcrun --sdk macosx --show-sdk-path
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk
% xcodebuild -version -sdk macosx Path
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk

and cc -v reports
  -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk

So apparently, "xcrun --show-sdk-path" (without any -sdk option)
is the most authoritative guide to the compiler's default sysroot.

However, googling turns up various people reporting that "xcrun
--show-sdk-path" returns an empty string for them, and our last
major investigation into this [1] found that there are some system
states where the compiler appears to have no default sysroot,
which I bet is the same thing.  I do not at this point have a recipe
to reproduce such a state, but we'd be fools to imagine it's no
longer possible.  My guess about it is that Apple's processes for
updating the default sysroot during system updates are just plain
buggy, with various corner cases that have ill-understood causes.

Also, after re-reading [1] I am not at all excited about trying to
remove the -isysroot switches from our *FLAGS.  What I propose to do
is keep that, but improve our mechanism for choosing a default value
for PG_SYSROOT.  It looks like first trying "xcrun --show-sdk-path",
and falling back to "xcodebuild -version -sdk macosx Path" if that
doesn't yield a valid path, is more likely to give a working build
than relying entirely on xcodebuild.  Maybe there's a case for trying
"xcrun --sdk macosx --show-sdk-path" in between; in my tests that
seemed noticeably faster than invoking xcodebuild, and I've not yet
seen a case where it gave a different answer.

Thoughts?

            regards, tom lane

[1] https://www.postgresql.org/message-id/flat/20840.1537850987%40sss.pgh.pa.us

On 15.01.2021 01:13, Tom Lane wrote:
> I borrowed my wife's Mac, which is still on Catalina and up to now
> never had Xcode on it, and found some very interesting things.
> 
> Step 1: download/install Xcode 12.3, open it, agree to license,
> wait for it to finish "installing components".
> 
> At this point, /Library/Developer/CommandLineTools doesn't exist,
> and we have the following outputs from various probe commands:
> 
> % xcrun --show-sdk-path
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk
> % xcrun --sdk macosx --show-sdk-path
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk
> % xcodebuild -version -sdk macosx Path
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk
> 
> Also, cc -v reports
>    -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk
> 
> Unsurprisingly, Xcode 12.3 itself only contains
> 
> % ls -l /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs
> total 0
> drwxr-xr-x  5 root  wheel  160 Nov 30 07:27 DriverKit20.2.sdk
> drwxr-xr-x  7 root  wheel  224 Nov 30 07:27 MacOSX.sdk
> lrwxr-xr-x  1 root  wheel   10 Jan 14 15:57 MacOSX11.1.sdk -> MacOSX.sdk
> 
> Step 2: install command line tools (I used "xcode-select --install"
> to fire this off, rather than the Xcode menu item).
> 
> Now I have
> 
> % ls -l /Library/Developer/CommandLineTools/SDKs
> total 0
> lrwxr-xr-x  1 root  wheel   14 Jan 14 16:42 MacOSX.sdk -> MacOSX11.1.sdk
> drwxr-xr-x  8 root  wheel  256 Jul  9  2020 MacOSX10.15.sdk
> drwxr-xr-x  7 root  wheel  224 Nov 30 07:33 MacOSX11.1.sdk
> 
> which is pretty interesting in itself, because the same directory on
> my recently-updated-to-Big-Sur Macs does NOT have the 11.1 SDK.
> I wonder what determines which versions get installed here.
> 
> More interesting yet:
> 
> % xcrun --show-sdk-path
> /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk
> % xcrun --sdk macosx --show-sdk-path
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk
> % xcodebuild -version -sdk macosx Path
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk
> 
> and cc -v reports
>    -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk
> 
> So apparently, "xcrun --show-sdk-path" (without any -sdk option)
> is the most authoritative guide to the compiler's default sysroot.
> 
> However, googling turns up various people reporting that "xcrun
> --show-sdk-path" returns an empty string for them, and our last
> major investigation into this [1] found that there are some system
> states where the compiler appears to have no default sysroot,
> which I bet is the same thing.  I do not at this point have a recipe
> to reproduce such a state, but we'd be fools to imagine it's no
> longer possible.  My guess about it is that Apple's processes for
> updating the default sysroot during system updates are just plain
> buggy, with various corner cases that have ill-understood causes.
> 
> Also, after re-reading [1] I am not at all excited about trying to
> remove the -isysroot switches from our *FLAGS.  What I propose to do
> is keep that, but improve our mechanism for choosing a default value
> for PG_SYSROOT.  It looks like first trying "xcrun --show-sdk-path",
> and falling back to "xcodebuild -version -sdk macosx Path" if that
> doesn't yield a valid path, is more likely to give a working build
> than relying entirely on xcodebuild.  Maybe there's a case for trying
> "xcrun --sdk macosx --show-sdk-path" in between; in my tests that
> seemed noticeably faster than invoking xcodebuild, and I've not yet
> seen a case where it gave a different answer.
> 
> Thoughts?
> 
>             regards, tom lane
> 
> [1] https://www.postgresql.org/message-id/flat/20840.1537850987%40sss.pgh.pa.us
> 

Thanks for thorough investigation and sorry for the late reply.

I spent quite some time trying to understand / reverse engineer the 
logic behind xcrun's default SDK selection. Apparently, "man xcrun" is 
not accurate saying:

    The  SDK  which  will  be searched defaults to the most recent 
available...

I didn't find anything really useful or helpful. 
"/Library/Developer/CommandLineTools" is hardcoded into 
"libxcrun.dylib". On my machine xcrun scans

/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs

and

/Library/Developer/CommandLineTools/SDKs

in that order, and loads "SDKSettings.plist" from each subdirectory. I 
looked into plists, but couldn't find anything special about 
"MacOSX10.15.sdk".


Okay, here is what I have:

% ls -l 
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs
total 0
drwxr-xr-x  5 root  wheel  160 Nov 30 15:27 DriverKit20.2.sdk
drwxr-xr-x  7 root  wheel  224 Nov 30 15:27 MacOSX.sdk
lrwxr-xr-x  1 root  wheel   10 Dec 17 14:25 MacOSX11.1.sdk -> MacOSX.sdk

% ls -l /Library/Developer/CommandLineTools/SDKs
total 0
lrwxr-xr-x  1 root  wheel   14 Nov 17 02:21 MacOSX.sdk -> MacOSX11.0.sdk
drwxr-xr-x  8 root  wheel  256 Nov 17 02:22 MacOSX10.15.sdk
drwxr-xr-x  7 root  wheel  224 Oct 19 23:39 MacOSX11.0.sdk

% xcrun --show-sdk-path
/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk

% xcrun --sdk macosx --show-sdk-path
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk

Oh, that's weird! Nevertheless I like you suggestion to call "xcrun" 
from "configure".

Adding "--verbose" doesn't really explain anything, but just in case.

% xcrun --verbose --no-cache --find cc
xcrun: note: PATH = 
'/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Library/Apple/usr/bin'
xcrun: note: SDKROOT = 
'/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk'
xcrun: note: TOOLCHAINS = ''
xcrun: note: DEVELOPER_DIR = '/Applications/Xcode.app/Contents/Developer'
xcrun: note: XCODE_DEVELOPER_USR_PATH = ''
xcrun: note: xcrun_db = 
'/var/folders/8x/jvqv7hyd5h98m7tz2zm9r0yc0000gn/T/xcrun_db'
xcrun: note: xcrun via cc (xcrun)
xcrun: note: database key is: 
cc|/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk||/Applications/Xcode.app/Contents/Developer|
xcrun: note: looking up with 
'/Applications/Xcode.app/Contents/Developer/usr/bin/xcodebuild -sdk 
macosx -find cc 2> /dev/null'
xcrun: note: lookup resolved with 'xcodebuild -find' to 
'/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc'
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc


% xcrun --verbose --no-cache --sdk macosx --find cc
xcrun: note: looking up SDK with 
'/Applications/Xcode.app/Contents/Developer/usr/bin/xcodebuild -sdk 
macosx -version Path'
xcrun: note: PATH = 
'/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Library/Apple/usr/bin'
xcrun: note: SDKROOT = 'macosx'
xcrun: note: TOOLCHAINS = ''
xcrun: note: DEVELOPER_DIR = '/Applications/Xcode.app/Contents/Developer'
xcrun: note: XCODE_DEVELOPER_USR_PATH = ''
xcrun: note: xcrun_db = 
'/var/folders/8x/jvqv7hyd5h98m7tz2zm9r0yc0000gn/T/xcrun_db'
xcrun: note: lookup resolved to: 
'/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk'
xcrun: note: looking up SDK with 
'/Applications/Xcode.app/Contents/Developer/usr/bin/xcodebuild -sdk 
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk 
-version PlatformPath'
xcrun: note: PATH = 
'/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Library/Apple/usr/bin'
xcrun: note: SDKROOT = 
'/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk'
xcrun: note: TOOLCHAINS = ''
xcrun: note: DEVELOPER_DIR = '/Applications/Xcode.app/Contents/Developer'
xcrun: note: XCODE_DEVELOPER_USR_PATH = ''
xcrun: note: xcrun_db = 
'/var/folders/8x/jvqv7hyd5h98m7tz2zm9r0yc0000gn/T/xcrun_db'
xcrun: note: lookup resolved to:
'/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform'
xcrun: note: PATH = 
'/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Library/Apple/usr/bin'
xcrun: note: SDKROOT = 
'/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk'
xcrun: note: TOOLCHAINS = ''
xcrun: note: DEVELOPER_DIR = '/Applications/Xcode.app/Contents/Developer'
xcrun: note: XCODE_DEVELOPER_USR_PATH = ''
xcrun: note: xcrun_db = 
'/var/folders/8x/jvqv7hyd5h98m7tz2zm9r0yc0000gn/T/xcrun_db'
xcrun: note: xcrun via cc (xcrun)
xcrun: note: database key is: 

cc|/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk||/Applications/Xcode.app/Contents/Developer|
xcrun: note: looking up with 
'/Applications/Xcode.app/Contents/Developer/usr/bin/xcodebuild -sdk 
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk 
-find cc 2> /dev/null'
xcrun: note: lookup resolved with 'xcodebuild -find' to 
'/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc'
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc

On 14.01.2021 21:05, Tom Lane wrote:
> After considerable playing around, I'm guessing that the reason
> -no_weak_imports doesn't help is that it rejects calls that are
> marked as weak references on the *calling* side.  Since AC_CHECK_FUNCS
> doesn't bother to #include the relevant header file, the compiler
> doesn't know that preadv() ought to be marked as a weak reference.
> Then, when the test program gets linked against the stub libc that's
> provided by the SDK, there is a version of preadv() there so no link
> failure occurs.  (There are way more moving parts in this weak-reference
> thing than I'd realized.)
> 

Oh, that's interesting. I've just played with it a bit and it looks 
exactly as you say.

> Another thing I've been realizing while poking at this is that we
> might not need to set -isysroot explicitly at all, which would then
> lead to the compiler using its default sysroot automatically.
> In some experimentation, it seems like what we need PG_SYSROOT for
> is just for configure to be able to find tclConfig.sh and the Perl
> header files.  So at this point I'm tempted to try ripping that
> out altogether.  If you remove the lines in src/template/darwin
> that inject PG_SYSROOT into CPPFLAGS and LDFLAGS, do things
> work for you?

Yes, it works fine.

Sergey Shinderuk <s.shinderuk@postgrespro.ru> writes:
> On 15.01.2021 01:13, Tom Lane wrote:
>> Also, after re-reading [1] I am not at all excited about trying to
>> remove the -isysroot switches from our *FLAGS.  What I propose to do
>> is keep that, but improve our mechanism for choosing a default value
>> for PG_SYSROOT.  It looks like first trying "xcrun --show-sdk-path",
>> and falling back to "xcodebuild -version -sdk macosx Path" if that
>> doesn't yield a valid path, is more likely to give a working build
>> than relying entirely on xcodebuild.  Maybe there's a case for trying
>> "xcrun --sdk macosx --show-sdk-path" in between; in my tests that
>> seemed noticeably faster than invoking xcodebuild, and I've not yet
>> seen a case where it gave a different answer.

> I spent quite some time trying to understand / reverse engineer the 
> logic behind xcrun's default SDK selection.

Yeah, I wasted a fair amount of time on that too, going so far as
to ktrace xcrun (as I gather you did too).  I'm not any more
enlightened than you are about exactly how it's making the choice.

> Oh, that's weird! Nevertheless I like you suggestion to call "xcrun" 
> from "configure".

Anyway, after re-reading the previous thread, something I like about
the current behavior is that it tends to produce a version-numbered
sysroot path, ie something ending in "MacOSX11.1.sdk" or whatever.
One of the hazards we're trying to avoid is some parts of a PG
installation being built against one SDK version while other parts are
built against another.  The typical behavior of "xcrun --show-sdk-path"
seems to be to produce a path ending in "MacOSX.sdk", which defeats that.
So I think we should accept the path only if it contains a version number,
and otherwise move on to the other probe commands.

Hence, I propose the attached.  This works as far as I can tell
to fix the problem you're seeing.

            regards, tom lane

diff --git a/src/template/darwin b/src/template/darwin
index 32414d21a9..0c890482fe 100644
--- a/src/template/darwin
+++ b/src/template/darwin
@@ -3,11 +3,26 @@
 # Note: Darwin is the original code name for macOS, also known as OS X.
 # We still use "darwin" as the port name, partly because config.guess does.
 
-# Select where system include files should be sought.
+# Select where system include files should be sought, if user didn't say.
 if test x"$PG_SYSROOT" = x"" ; then
-  PG_SYSROOT=`xcodebuild -version -sdk macosx Path 2>/dev/null`
+  # This is far more complicated than it ought to be.  We first ask
+  # "xcrun --show-sdk-path", which seems to match the default -isysroot
+  # setting of Apple's compilers.  However, that may produce no result or
+  # a result that is not version-specific (i.e., just ".../SDKs/MacOSX.sdk").
+  # Having a version-specific sysroot seems desirable, so if there are not
+  # digits in the result string, try "xcrun --sdk macosx --show-sdk-path";
+  # and if that still doesn't work, fall back to asking xcodebuild.
+  PG_SYSROOT=`xcrun --show-sdk-path 2>/dev/null`
+  if expr x"$PG_SYSROOT" : '.*[0-9]\.[0-9]' >/dev/null ; then : okay
+  else
+    PG_SYSROOT=`xcrun --sdk macosx --show-sdk-path 2>/dev/null`
+    if expr x"$PG_SYSROOT" : '.*[0-9]\.[0-9]' >/dev/null ; then : okay
+    else
+      PG_SYSROOT=`xcodebuild -version -sdk macosx Path 2>/dev/null`
+    fi
+  fi
 fi
-# Old xcodebuild versions may produce garbage, so validate the result.
+# Validate the result: if it doesn't point at a directory, ignore it.
 if test x"$PG_SYSROOT" != x"" ; then
   if test -d "$PG_SYSROOT" ; then
     CPPFLAGS="-isysroot $PG_SYSROOT $CPPFLAGS"

On 15.01.2021 01:13, Tom Lane wrote:

> than relying entirely on xcodebuild.  Maybe there's a case for trying
> "xcrun --sdk macosx --show-sdk-path" in between; in my tests that
> seemed noticeably faster than invoking xcodebuild, and I've not yet
> seen a case where it gave a different answer.
> 

I see that "xcrun --sdk macosx --show-sdk-path" really calls
"/Applications/Xcode.app/Contents/Developer/usr/bin/xcodebuild -sdk 
macosx -version Path" under the hood.


% lldb -- xcrun --no-cache --sdk macosx --show-sdk-path
(lldb) target create "xcrun"
Current executable set to 'xcrun' (x86_64).
(lldb) settings set -- target.run-args  "--no-cache" "--sdk" "macosx" 
"--show-sdk-path"
(lldb) settings set target.unset-env-vars SDKROOT
(lldb) b popen
Breakpoint 1: where = libsystem_c.dylib`popen, address = 0x00007fff67265607
(lldb) r
Process 26857 launched: '/usr/bin/xcrun' (x86_64)
Process 26857 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
     frame #0: 0x00007fff6e313607 libsystem_c.dylib`popen
libsystem_c.dylib`popen:
->  0x7fff6e313607 <+0>: pushq  %rbp
     0x7fff6e313608 <+1>: movq   %rsp, %rbp
     0x7fff6e31360b <+4>: pushq  %r15
     0x7fff6e31360d <+6>: pushq  %r14
Target 0: (xcrun) stopped.
(lldb) p (char *)$arg1
(char *) $1 = 0x0000000100406960 
"/Applications/Xcode.app/Contents/Developer/usr/bin/xcodebuild -sdk 
macosx -version Path"


% sudo dtrace -n 'pid$target::popen:entry { trace(copyinstr(arg0)) }' -c 
'xcrun --sdk macosx --show-sdk-path'
dtrace: description 'pid$target::popen:entry ' matched 1 probe
CPU     ID                    FUNCTION:NAME
   0 413269                      popen:entry 
/Applications/Xcode.app/Contents/Developer/usr/bin/xcodebuild -sdk 
macosx -version Path
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk
dtrace: pid 26905 has exited

Sergey Shinderuk <s.shinderuk@postgrespro.ru> writes:
> I see that "xcrun --sdk macosx --show-sdk-path" really calls
> "/Applications/Xcode.app/Contents/Developer/usr/bin/xcodebuild -sdk
> macosx -version Path" under the hood.

Hmm.  I found something odd on my wife's Mac: although on my other
machines, I get something like

$ xcrun --verbose --no-cache --show-sdk-path
xcrun: note: looking up SDK with '/Applications/Xcode.app/Contents/Developer/usr/bin/xcodebuild -sdk
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk-version PlatformPath' 
xcrun: note: PATH =
'/Users/tgl/testversion/bin:/usr/local/autoconf-2.69/bin:/Users/tgl/bin:/usr/local/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/opt/X11/bin:/Library/Apple/usr/bin:/Library/Tcl/bin:/opt/X11/bin'
xcrun: note: SDKROOT = '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk'
xcrun: note: TOOLCHAINS = ''
xcrun: note: DEVELOPER_DIR = '/Applications/Xcode.app/Contents/Developer'
xcrun: note: XCODE_DEVELOPER_USR_PATH = ''
xcrun: note: xcrun_db = '/var/folders/3p/2bnrmypd17jcqbtzw79t9blw0000gn/T/xcrun_db'
xcrun: note: lookup resolved to: '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform'
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk

on her machine there's no detail at all:

% xcrun --verbose --no-cache --show-sdk-path
/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk

So I'm not sure what to make of that.  But I'm hesitant to assume
that xcrun is just a wrapper around xcodebuild.

            regards, tom lane

On 15.01.2021 04:53, Sergey Shinderuk wrote:

> I see that "xcrun --sdk macosx --show-sdk-path" really calls
> "/Applications/Xcode.app/Contents/Developer/usr/bin/xcodebuild -sdk 
> macosx -version Path" under the hood.
> 

... and caches the result. xcodebuild not called without --no-cache.
So it still make sense to fall back on xcodebuild.

% sudo dtrace -n 'pid$target::popen:entry { trace(copyinstr(arg0)) }' -c 
'xcrun --sdk macosx --show-sdk-path'
dtrace: description 'pid$target::popen:entry ' matched 1 probe
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk
dtrace: pid 26981 has exited

On 15.01.2021 05:04, Tom Lane wrote:
> 
> on her machine there's no detail at all:
> 
> % xcrun --verbose --no-cache --show-sdk-path
> /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk
> 

The same on my machine. I get details for --find, but not for 
--show-sdk-path.


> So I'm not sure what to make of that.  But I'm hesitant to assume
> that xcrun is just a wrapper around xcodebuild.
> 

I agree.

On 15.01.2021 04:45, Tom Lane wrote:
> Hence, I propose the attached.  This works as far as I can tell
> to fix the problem you're seeing.
Yes, it works fine. Thank you very much.

Sergey Shinderuk <s.shinderuk@postgrespro.ru> writes:
> On 15.01.2021 04:45, Tom Lane wrote:
>> Hence, I propose the attached.  This works as far as I can tell
>> to fix the problem you're seeing.

> Yes, it works fine. Thank you very much.

Great.  Pushed with a little more polishing.

            regards, tom lane

On Thu, Jan 14, 2021 at 6:45 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Sergey Shinderuk <s.shinderuk@postgrespro.ru> writes:
> > On 15.01.2021 01:13, Tom Lane wrote:
> >> Also, after re-reading [1] I am not at all excited about trying to
> >> remove the -isysroot switches from our *FLAGS.  What I propose to do
> >> is keep that, but improve our mechanism for choosing a default value
> >> for PG_SYSROOT.  It looks like first trying "xcrun --show-sdk-path",
> >> and falling back to "xcodebuild -version -sdk macosx Path" if that
> >> doesn't yield a valid path, is more likely to give a working build
> >> than relying entirely on xcodebuild.  Maybe there's a case for trying
> >> "xcrun --sdk macosx --show-sdk-path" in between; in my tests that
> >> seemed noticeably faster than invoking xcodebuild, and I've not yet
> >> seen a case where it gave a different answer.
>
> > I spent quite some time trying to understand / reverse engineer the
> > logic behind xcrun's default SDK selection.
>
> Yeah, I wasted a fair amount of time on that too, going so far as
> to ktrace xcrun (as I gather you did too).  I'm not any more
> enlightened than you are about exactly how it's making the choice.
>
> > Oh, that's weird! Nevertheless I like you suggestion to call "xcrun"
> > from "configure".
>
> Anyway, after re-reading the previous thread, something I like about
> the current behavior is that it tends to produce a version-numbered
> sysroot path, ie something ending in "MacOSX11.1.sdk" or whatever.
> One of the hazards we're trying to avoid is some parts of a PG
> installation being built against one SDK version while other parts are
> built against another.  The typical behavior of "xcrun --show-sdk-path"
> seems to be to produce a path ending in "MacOSX.sdk", which defeats that.
> So I think we should accept the path only if it contains a version number,
> and otherwise move on to the other probe commands.
I don't think enforcing a specific naming scheme makes sense, the minimum
OSX runtime version is effectively entirely separate from the SDK version.

The pwritev issue just seems to be caused by a broken configure check,
I've fixed that here:
https://postgr.es/m/20210119111625.20435-1-james.hilliard1%40gmail.com
>
> Hence, I propose the attached.  This works as far as I can tell
> to fix the problem you're seeing.
>
>                         regards, tom lane
>