pqsignal() already does a similar check, but strange Valgrind
reports have us wondering if wrapper_handler() is somehow getting
called with an invalid signal number.
Reported-by: Tomas Vondra <tomas@vondra.me>
Suggested-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/ace01111-f9ac-4f61-b1b1-8e9379415444%40vondra.me
Backpatch-through: 17
We had managed not to notice this simple oversight because none
of our calls exercised the case --- until commit 8f427187d.
That led to pg_dump crashing on any platform that uses this code
(currently Cygwin and Mingw).
Even though there's no immediate bug in the back branches, backpatch,
because a non-POSIX-compliant strtof() substitute is trouble waiting
to happen for extensions or future back-patches.
Diagnosed-by: Alexander Lakhin <exclusion@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/339b3902-4e98-4e31-a744-94e43b7b9292@gmail.com
Backpatch-through: 13
When running on Windows, canonicalize_path() converts '\' to '/'
to prevent confusing the Windows command processor. It was
doing that in a non-encoding-aware fashion; but in SJIS there
are valid two-byte characters whose second byte matches '\'.
So encoding corruption ensues if such a character is used in
the path.
We can fairly easily fix this if we know which encoding is
in use, but a lot of our utilities don't have much of a clue
about that. After some discussion we decided we'd settle for
fixing this only in psql, and assuming that its value of
client_encoding matches what the user is typing.
It seems hopeless to get the server to deal with the problematic
characters in database path names, so we'll just declare that
case to be unsupported. That means nothing need be done in
the server, nor in utility programs whose only contact with
file path names is for database paths. But psql frequently
deals with client-side file paths, so it'd be good if it
didn't mess those up.
Bug: #18735
Reported-by: Koichi Suzuki <koichi.suzuki@enterprisedb.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Koichi Suzuki <koichi.suzuki@enterprisedb.com>
Discussion: https://postgr.es/m/18735-4acdb3998bb9f2b1@postgresql.org
Backpatch-through: 13
The protections added by commit 3b00fdba9f introduced race
conditions to this function that can lead to bogus return values.
Since nobody seems to inspect the return value, this is of little
consequence, but it would have been nice to convert it to a void
function to avoid any possibility of a bogus return value. I
originally thought that doing so would have required also modifying
legacy-pqsignal.c's version of the function (which would've
required an SONAME bump), but commit 9a45a89c38 gave
legacy-pqsignal.c its own dedicated extern for pqsignal(), thereby
decoupling it enough that libpgport's pqsignal() can be modified.
This commit also adds an assertion for the return value of
sigaction()/signal(). Since a failure most likely indicates a
coding error, and nobody has ever bothered to check pqsignal()'s
return value, it's probably not worth the effort to do anything
fancier.
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/Z4chOKfnthRH71mw%40nathan
In the name of ABI stability (that is, to avoid a library major
version bump for libpq), libpq still exports a version of pqsignal()
that we no longer want to use ourselves. However, since that has
the same link name as the function exported by src/port/pqsignal.c,
there is a link ordering dependency determining which version will
actually get used by code that uses libpq as well as libpgport.a.
It now emerges that the wrong version has been used by pgbench and
psql since commit 06843df4a rearranged their link commands. This
can result in odd failures in pgbench with the -T switch, since its
SIGALRM handler will now not be marked SA_RESTART. psql may have
some edge-case problems in \watch, too.
Since we don't want to depend on link ordering effects anymore,
let's fix this in the same spirit as b6c7cfac8: use macros to change
the actual link names of the competing functions. We cannot change
legacy-pqsignal.c's exported name of course, so the victim has to be
src/port/pqsignal.c.
In master, rename its exported name to be pqsignal_fe in frontend or
pqsignal_be in backend. (We could perhaps have gotten away with using
the same symbol in both cases, but since the FE and BE versions now
work a little differently, it seems advisable to use different names.)
In back branches, rename to pqsignal_fe in frontend but keep it as
pqsignal in backend. The frontend change could affect third-party
code that is calling pqsignal from libpgport.a or libpgport_shlib.a,
but only if the code is compiled against port.h from a different minor
release than libpgport. Since we don't support using libpgport as a
shared library, it seems unlikely that there will be such a problem.
I left the backend symbol unchanged to avoid an ABI break for
extensions. This means that the link ordering hazard still exists
for any extension that links against libpq. However, none of our own
extensions use both pqsignal() and libpq, and we're not making things
any worse for third-party extensions that do.
Report from Andy Fan, diagnosis by Fujii Masao, patch by me.
Back-patch to all supported branches, as 06843df4a was.
Discussion: https://postgr.es/m/87msfz5qv2.fsf@163.com
This fixes "unresolved external symbol" errors with extensions that
use functions from libpgport that need special CFLAGS to
compile. Currently, that includes the CRC-32 functions.
Commit 2571c1d5cc did this for libcommon, but I missed that libpqport
has the same issue.
Reported-by: Tom Lane
Backpatch-through: 16, where Meson was introduced
Discussion: https://www.postgresql.org/message-id/CAOdR5yF0krWrxycA04rgUKCgKugRvGWzzGLAhDZ9bzNv8g0Lag@mail.gmail.com
Since commit 97550c0711, these failed with
"PANIC: proc_exit() called in child process" due to uninitialized or
stale MyProcPid. That was reachable if close() failed in
ClosePostmasterPorts() or setlocale(category, "C") failed, both
unlikely. Back-patch to v13 (all supported versions).
Discussion: https://postgr.es/m/20241208034614.45.nmisch@google.com
Redefine our exact width types with standard C99 types and macros,
including int64_t, INT64_MAX, INT64_C(), PRId64 etc. We were already
using <stdint.h> types in a few places.
One complication is that Windows' <inttypes.h> uses format strings like
"%I64d", "%I32", "%I" for PRI*64, PRI*32, PTR*PTR, instead of mapping to
other standardized format strings like "%lld" etc as seen on other known
systems. Teach our snprintf.c to understand them.
This removes a lot of configure clutter, and should also allow 64-bit
numbers and other standard types to be used in localized messages
without casting.
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/ME3P282MB3166F9D1F71F787929C0C7E7B6312%40ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM
Presently, we check for compiler support for the required
intrinsics both with and without the -msse4.2 compiler flag, and
then depending on the results of those checks, we pick which files
to compile with which flags. This is tedious and complicated, and
it results in unsustainable coding patterns such as separate files
for each portion of code that may need to be built with different
compiler flags.
This commit makes use of the newly-added support for
__attribute__((target(...))) in the SSE4.2 CRC-32C code. This
simplifies both the configure-time checks and the build scripts,
and it allows us to place the functions that use the intrinsics in
files that we otherwise do not want to build with special CPU
instructions (although this commit refrains from doing so). This
is also preparatory work for a proposed follow-up commit that will
further optimize the CRC-32C code with AVX-512 instructions.
While at it, this commit modifies meson's checks for SSE4.2 CRC
support to be the same as autoconf's. meson was choosing whether
to use a runtime check based purely on whether -msse4.2 is
required, while autoconf has long checked for the __SSE4_2__
preprocessor symbol to decide. meson's previous approach seems to
work just fine, but this change avoids needing to build multiple
test programs and to keep track of whether to actually use
pg_attribute_target().
Ideally we'd use __attribute__((target(...))) for ARMv8 CRC
support, too, but there's little point in doing so because until
clang 16, using the ARM intrinsics still requires special compiler
flags. Perhaps we can re-evaluate this decision after some time
has passed.
Author: Raghuveer Devulapalli
Discussion: https://postgr.es/m/PH8PR11MB8286BE735A463468415D46B5FB5C2%40PH8PR11MB8286.namprd11.prod.outlook.com
Previously we probed for CRC32 instructions by testing if they caused
SIGILL. Some have expressed doubts about that technique, the Linux
documentation advises not to use it, and it's not exactly beautiful.
Now that more operating systems expose CPU features to userspace via the
ELF loader in approximately the same way, let's use that instead.
This is expected to work on Linux, FreeBSD and recent OpenBSD.
OpenBSD/ARM has not been tested and is not present in our build farm,
but the API matches FreeBSD.
On macOS, compilers use a more recent baseline ISA so the runtime test
mechanism isn't reached. (A similar situation is expected for
Windows/ARM when that port lands.)
On NetBSD, runtime feature probing is lost for armv8-a builds. It looks
potentially doable with sysctl following the example of the cpuctl
program; patches are welcome.
No back-patch for now, since we don't have any evidence of actual
breakage from the previous technique.
Suggested-by: Bastien Roucariès <rouca@debian.org>
Discussion: https://postgr.es/m/4496616.iHFcN1HehY%40portable-bastien
Unlike TRY_POPCNT_FAST, which is defined in pg_bitutils.h, this
macro is defined in c.h (via pg_config.h), so we can check for it
earlier and avoid some unnecessary #includes on systems that lack
AVX-512 support.
Oversight in commit f78667bd91.
Discussion: https://postgr.es/m/Zy5K5Qmlb3Z4dsd4%40nathan
The commonly supported way to specify multiple target options is to
surround the entire list with quotes and to use a comma (with no
extra spaces) as the delimiter.
Oversight in commit f78667bd91.
Discussion: https://postgr.es/m/Zy0jya8nF8CPpv3B%40nathan
Presently, we check for compiler support for the required
intrinsics both with and without extra compiler flags (e.g.,
-mxsave), and then depending on the results of those checks, we
pick which files to compile with which flags. This is tedious and
complicated, and it results in unsustainable coding patterns such
as separate files for each portion of code may need to be built
with different compiler flags.
This commit introduces support for __attribute__((target(...))) and
uses it for the AVX-512 code. This simplifies both the
configure-time checks and the build scripts, and it allows us to
place the functions that use the intrinsics in files that we
otherwise do not want to build with special CPU instructions. We
are careful to avoid using __attribute__((target(...))) on
compilers that do not understand it, but we still perform the
configure-time checks in case the compiler allows using the
intrinsics without it (e.g., MSVC).
A similar change could likely be made for some of the CRC-32C code,
but that is left as a future exercise.
Suggested-by: Andres Freund
Reviewed-by: Raghuveer Devulapalli, Andres Freund
Discussion: https://postgr.es/m/20240731205254.vfpap7uxwmebqeaf%40awork3.anarazel.de
This was introduced in commit bfa2cee784, which replaced the old
bsearch_cmp() function we had in extended_stats.c with the current
implementation. The original discussion or commit message of
bfa2cee784 didn't mention where the new implementation came from, but
based on some googling, I'm guessing *BSD or libiberty, all of which
share this same code, with or without this fix.
Author: Ranier Vilela
Reviewed-by: Nathan Bossart
Backpatch-through: 14
Discussion: https://www.postgresql.org/message-id/CAEudQAp34o_8u6sGSVraLwuMv9F7T9hyHpePXHmRaxR2Aboi%2Bw%40mail.gmail.com
Commit a70e01d430 retired support for OpenSSL 1.0.2 in order to get
rid of the need for manual initialization of the library. This left our
API usage compatible with 1.1.0 which was defined as the minimum required
version. Also mention that 3.4 is the minimum version required when using
LibreSSL.
An upcoming commit will introduce support for configuring TLSv1.3 cipher
suites which require an API call in OpenSSL 1.1.1 and onwards. In order
to support this setting this commit will set v1.1.1 as the new minimum
required version. The version-specific call for randomness init added
in commit c3333dbc0c is removed as it's no longer needed.
Author: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/909A668B-06AD-47D1-B8EB-A164211AAD16@yesql.se
Discussion: https://postgr.es/m/tencent_063F89FA72CCF2E48A0DF5338841988E9809@qq.com
Instead of XXX_IN_XLOCALE_H for several features XXX, let's just
include <xlocale.h> if HAVE_XLOCALE_H. The reason for the extra
complication was apparently that some old glibc systems also had an
<xlocale.h>, and you weren't supposed to include it directly, but it's
gone now (as far as I can tell it was harmless to do so anyway).
Author: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/CWZBBRR6YA8D.8EHMDRGLCKCD%40neon.tech
The random number generator in OpenSSL 1.1.1 was redesigned to provide
fork safety by default, thus removing the need for calling RAND_poll
after forking to ensure that two processes cannot share the same state.
Since we now support 1.1.0 as the minumum version, and 1.1.0 is being
increasingly phased out from production use, only perform the RAND_poll
initialization for installations running 1.1.0 by checking the OpenSSL
version number.
LibreSSL changed random number generator when forking OpenSSL and has
provided fork safety since version 2.0.2.
This removes the overhead of initializing the RNG for strong random
for the vast majority of users for whom it is no longer required.
Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CA+hUKGKh7QrYzu=8yWEUJvXtMVm_CNWH1L_TLWCbZMwbi1XP2Q@mail.gmail.com
Remove src/port/user.c, call getpwuid_r() directly. This reduces some
complexity and allows better control of the error behavior. For
example, the old code would in some circumstances silently truncate
the result string, or produce error message strings that the caller
wouldn't use.
src/port/user.c used to be called src/port/thread.c and contained
various portability complications to support thread-safety. These are
all obsolete, and all but the user-lookup functions have already been
removed. This patch completes this by also removing the user-lookup
functions.
Also convert src/backend/libpq/auth.c to use getpwuid_r() for
thread-safety.
Originally, I tried to be overly correct by using
sysconf(_SC_GETPW_R_SIZE_MAX) to get the buffer size for getpwuid_r(),
but that doesn't work on FreeBSD. All the OS where I could find the
source code internally use 1024 as the suggested buffer size, so I
just ended up hardcoding that. The previous code used BUFSIZ, which
is an unrelated constant from stdio.h, so its use seemed
inappropriate.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://www.postgresql.org/message-id/flat/5f293da9-ceb4-4937-8e52-82c25db8e4d3%40eisentraut.org
Historically, MinGW environments lacked some Windows API calls, so we
took a different code path in win32_langinfo(). Somehow, the code
change in commit 35eeea62 (removing setlocale() calls) caused one
particular 001_initdb.pl test to fail on MinGW + ICU builds, because
pg_import_system_collations() found no collations. It might take a
MinGW user to discover the exact reason.
Updating that function to use the same code as MSVC seems to fix that
test, so lets do that. (There are plenty more places that test for MSVC
unnecessarily, to be investigated later.)
While here, also rename the helper function win32_langinfo() to
win32_get_codeset(), to explain what it does less confusingly; it's not
really a general langinfo() substitute.
Noticed by triggering the optional MinGW CI task; no build farm animals
failed.
Discussion: https://postgr.es/m/CA%2BhUKGKBWfhXQ3J%2B2Lj5PhKvQnGD%3DsywA0XQcb7boTCf%3DerVLg%40mail.gmail.com
Commit 35eeea62 forgot to include <xlocale.h> when using locale_t
(which didn't seem to be required on newer Apple SDK as used by CI,
hence mistake). Let's see if this fixes build farm animals longfin and
sifika.
This gets rid of some setlocale() calls. The remaining call to
setlocale() in pg_get_encoding_from_locale() is a query of the name
of the current locale when none was provided (in a multi-threaded future
that would need more work).
All known non-Windows targets have nl_langinfo_l(), from POSIX 2008, and
for Windows we already do something thread-safe.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/CA%2BhUKGJqVe0%2BPv9dvC9dSums_PXxGo9SWcxYAMBguWJUGbWz-A%40mail.gmail.com
Run pgindent, pgperltidy, and reformat-dat-files.
The pgindent part of this is pretty small, consisting mainly of
fixing up self-inflicted formatting damage from patches that
hadn't bothered to add their new typedefs to typedefs.list.
In order to keep it from making anything worse, I manually added
a dozen or so typedefs that appeared in the existing typedefs.list
but not in the buildfarm's list. Perhaps we should formalize that,
or better find a way to get those typedefs into the automatic list.
pgperltidy is as opinionated as always, and reformat-dat-files too.
This commit fixes a few things:
* Instead of checking for CPU support of the "xsave" extension, we
need to check for OS support of XGETBV instructions via the
"osxsave" flag.
* We must check that additional XCR0 bits are set to be sure the
ZMM registers are fully enabled.
* We should use the recommended ordering of steps. Specifically,
we need to check that the ZMM registers are enabled prior to
checking for AVX-512 via CPUID.
In passing, split this code into separate functions to improve
readability.
Reported-by: Andrew Kane
Reviewed-by: Akash Shankaran, Raghuveer Devulapalli
Discussion: https://postgr.es/m/20240418024459.GA3385227%40nathanxps13
Commit 792752af4e added infrastructure for using AVX-512 intrinsic
functions, and this commit uses that infrastructure to optimize
visibilitymap_count(). Specificially, a new pg_popcount_masked()
function is introduced that applies a bitmask to every byte in the
buffer prior to calculating the population count, which is used to
filter out the all-visible or all-frozen bits as needed. Platforms
without AVX-512 support should also see a nice speedup due to the
reduced number of calls to a function pointer.
Co-authored-by: Ants Aasma
Discussion: https://postgr.es/m/BL1PR11MB5304097DF7EA81D04C33F3D1DCA6A%40BL1PR11MB5304.namprd11.prod.outlook.com
Presently, pg_popcount() processes data in 32-bit or 64-bit chunks
when possible. Newer hardware that supports AVX-512 instructions
can use 512-bit chunks, which provides a nice speedup, especially
for larger buffers. This commit introduces the infrastructure
required to detect compiler and CPU support for the required
AVX-512 intrinsic functions, and it adds a new pg_popcount()
implementation that uses these functions. If CPU support for this
optimized implementation is detected at runtime, a function pointer
is updated so that it is used by subsequent calls to pg_popcount().
Most of the existing in-tree calls to pg_popcount() should benefit
from these instructions, and calls with smaller buffers should at
least not regress compared to v16. The new infrastructure
introduced by this commit can also be used to optimize
visibilitymap_count(), but that is left for a follow-up commit.
Co-authored-by: Paul Amonson, Ants Aasma
Reviewed-by: Matthias van de Meent, Tom Lane, Noah Misch, Akash Shankaran, Alvaro Herrera, Andres Freund, David Rowley
Discussion: https://postgr.es/m/BL1PR11MB5304097DF7EA81D04C33F3D1DCA6A%40BL1PR11MB5304.namprd11.prod.outlook.com
If there aren't many bytes to process, the function call overhead
of the optimized implementation isn't worth taking, so instead we
inline a loop that consults pg_number_of_ones in that case. If
there are many bytes to process, we accept the function call
overhead because the optimized versions are likely to be faster.
The threshold at which we use the optimized implementation is set
to the smallest amount of data required to use special popcount
instructions.
Reviewed-by: Alvaro Herrera, Tom Lane
Discussion: https://postgr.es/m/20240402155301.GA2750455%40nathanxps13
On some systems, calls to pg_popcount{32,64} are indirected through
a function pointer. This commit converts pg_popcount() to a
function pointer on those systems so that we can inline the
appropriate pg_popcount{32,64} implementations into each of the
pg_popcount() implementations. Since pg_popcount() may call
pg_popcount{32,64} several times, this can significantly improve
its performance.
Suggested-by: David Rowley
Reviewed-by: David Rowley
Discussion: https://postgr.es/m/CAApHDvrb7MJRB6JuKLDEY2x_LKdFHwVbogpjZBCX547i5%2BrXOQ%40mail.gmail.com
Most callers of strerror() are removed from the backend code. The
remaining callers require special handling with a saved errno from a
previous system call. The frontend code still needs strerror() where
error states need to be handled outside of fprintf.
Note that pg_regress is not changed to use %m as the TAP output may
clobber errno, since those functions call fprintf() and friends before
evaluating the format string.
Support for %m in src/port/snprintf.c has been added in d6c55de1f9,
hence all the stable branches currently supported include it.
Author: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/87sf13jhuw.fsf@wibble.ilmari.org
When calling the Windows file I/O APIs there is an implicit conversion
from size_t to DWORD, which could overflow. Clamp the size at 1GB to
avoid that.
Not a really a live bug as we don't expect anything in PostgreSQL to
call with such large values.
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/1672202.1703441340%40sss.pgh.pa.us
There isn't a lot of user demand for AIX support, we have a bunch of
hacks to work around AIX-specific compiler bugs and idiosyncrasies,
and no one has stepped up to the plate to properly maintain it.
Remove support for AIX to get rid of that maintenance overhead. It's
still supported for stable versions.
The acute issue that triggered this decision was that after commit
8af2565248, the AIX buildfarm members have been hitting this
assertion:
TRAP: failed Assert("(uintptr_t) buffer == TYPEALIGN(PG_IO_ALIGN_SIZE, buffer)"), File: "md.c", Line: 472, PID: 2949728
Apperently the "pg_attribute_aligned(a)" attribute doesn't work on AIX
for values larger than PG_IO_ALIGN_SIZE, for a static const variable.
That could be worked around, but we decided to just drop the AIX support
instead.
Discussion: https://www.postgresql.org/message-id/20240224172345.32@rfd.leadboat.com
Reviewed-by: Andres Freund, Noah Misch, Thomas Munro
Presently, we rely on each individual signal handler to save the
initial value of errno and then restore it before returning if
needed. This is easily forgotten and, if missed, often goes
undetected for a long time.
In commit 3b00fdba9f, we introduced a wrapper signal handler
function that checks whether MyProcPid matches getpid(). This
commit moves the aforementioned errno restoration code from the
individual signal handlers to the new wrapper handler so that we no
longer need to worry about missing it.
Reviewed-by: Andres Freund, Noah Misch
Discussion: https://postgr.es/m/20231121212008.GA3742740%40nathanxps13
In commit 97550c0711, we added a similar check to the SIGTERM
handler for the startup process. This commit adds this check to
backend signal handlers installed with pqsignal(). This is done by
using a wrapper function that performs the check before calling the
actual handler.
The hope is that this will offer more general protection against
child processes of Postgres backends inadvertently modifying shared
memory due to inherited signal handlers. Another potential
follow-up improvement is to use this wrapper handler function to
restore errno instead of relying on each individual handler
function to do so.
This commit makes the changes in commit 97550c0711 obsolete but
leaves reverting it for a follow-up commit.
Reviewed-by: Andres Freund, Noah Misch
Discussion: https://postgr.es/m/20231121212008.GA3742740%40nathanxps13
Commit 5579388d removed code that supplied a fallback implementation of
getaddrinfo(), which was dead code on modern systems. One tiny piece of
the removed code was still doing something useful on Windows, though:
that OS's own gai_strerror()/gai_strerrorA() function returns a pointer
to a static buffer that it overwrites each time, so it's not
thread-safe. In rare circumstances, a multi-threaded client program
could get an incorrect or corrupted error message.
Restore the replacement gai_strerror() function, though now that it's
only for Windows we can put it into a win32-specific file and cut it
down to the errors that Windows documents. The error messages here are
taken from FreeBSD, because Windows' own messages seemed too verbose.
Back-patch to 16.
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKGKz%2BF9d2PTiXwfYV7qJw%2BWg2jzACgSDgPizUw7UG%3Di58A%40mail.gmail.com
Fix pthread-win32.h and pthread-win32.c to provide a more complete
emulation of POSIX pthread mutexes: define PTHREAD_MUTEX_INITIALIZER
and make sure that pthread_mutex_lock() can operate on a mutex
object that's been initialized that way. Then we don't need the
duplicative platform-specific logic in default_threadlock() and
pgtls_init(), which we'd otherwise need yet a third copy of for
an upcoming bug fix.
Also, since default_threadlock() supposes that pthread_mutex_lock()
cannot fail, try to ensure that that's actually true, by getting
rid of the malloc call that was formerly involved in initializing
an emulated mutex. We can define an extra state for the spinlock
field instead.
Also, replace the similar code in ecpglib/misc.c with this version.
While ecpglib's version at least had a POSIX-compliant API, it
also had the potential of failing during mutex init (but here,
because of CreateMutex failure rather than malloc failure). Since
all of misc.c's callers ignore failures, it seems like a wise idea
to avoid failures here too.
A further improvement in this area could be to unify libpq's and
ecpglib's implementations into a src/port/pthread-win32.c file.
But that doesn't seem like a bug fix, so I'll desist for now.
In preparation for the aforementioned bug fix, back-patch to all
supported branches.
Discussion: https://postgr.es/m/264860.1707163416@sss.pgh.pa.us
This commit removes all the scripts located in src/tools/msvc/ to build
PostgreSQL with Visual Studio on Windows, meson becoming the recommended
way to achieve that. The scripts held some information that is still
relevant with meson, information kept and moved to better locations.
Comments that referred directly to the scripts are removed.
All the documentation still relevant that was in install-windows.sgml
has been moved to installation.sgml under a new subsection for Visual.
All the content specific to the scripts is removed. Some adjustments
for the documentation are planned in a follow-up set of changes.
Author: Michael Paquier
Reviewed-by: Peter Eisentraut, Andres Freund
Discussion: https://postgr.es/m/ZQzp_VMJcerM1Cs_@paquier.xyz
For the trivial case of iovcnt == 1, kernels are measurably slower at
dealing with the more complex arguments of preadv/pwritev than the
equivalent plain old pread/pwrite. The overheads are worth it for
iovcnt > 1, but for 1 let's just redirect to the cheaper calls. While
we could leave it to callers to worry about that, we already have to
have our own pg_ wrappers for portability reasons so it seems
reasonable to centralize this knowledge there (thanks to Heikki for this
suggestion). Try to avoid function call overheads by making them
inlinable, which might also allow the compiler to avoid the branch in
some cases. For systems that don't have preadv and pwritev (currently:
Windows and [closed] Solaris), we might as well pull the replacement
functions up into the static inline functions too.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/CA+hUKGJkOiOCa+mag4BF+zHo7qo=o9CFheB8=g6uT5TUm2gkvA@mail.gmail.com
Explain where pqsignal() came from, what problem it originally solved
without assuming the reader is familiar with historical Unixen, why we
still need it, what it does for us now, and the key differences in
frontend code on Windows.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/CA%2BhUKG%2BRst1h3uo%2BXRgdRVnWHBa4mmj5gFbmCzZr73s-Fh_5JA%40mail.gmail.com
A PostgreSQL release tarball contains a number of prebuilt files, in
particular files produced by bison, flex, perl, and well as html and
man documentation. We have done this consistent with established
practice at the time to not require these tools for building from a
tarball. Some of these tools were hard to get, or get the right
version of, from time to time, and shipping the prebuilt output was a
convenience to users.
Now this has at least two problems:
One, we have to make the build system(s) work in two modes: Building
from a git checkout and building from a tarball. This is pretty
complicated, but it works so far for autoconf/make. It does not
currently work for meson; you can currently only build with meson from
a git checkout. Making meson builds work from a tarball seems very
difficult or impossible. One particular problem is that since meson
requires a separate build directory, we cannot make the build update
files like gram.h in the source tree. So if you were to build from a
tarball and update gram.y, you will have a gram.h in the source tree
and one in the build tree, but the way things work is that the
compiler will always use the one in the source tree. So you cannot,
for example, make any gram.y changes when building from a tarball.
This seems impossible to fix in a non-horrible way.
Second, there is increased interest nowadays in precisely tracking the
origin of software. We can reasonably track contributions into the
git tree, and users can reasonably track the path from a tarball to
packages and downloads and installs. But what happens between the git
tree and the tarball is obscure and in some cases non-reproducible.
The solution for both of these issues is to get rid of the step that
adds prebuilt files to the tarball. The tarball now only contains
what is in the git tree (*). Getting the additional build
dependencies is no longer a problem nowadays, and the complications to
keep these dual build modes working are significant. And of course we
want to get the meson build system working universally.
This commit removes the make distprep target altogether. The make
dist target continues to do its job, it just doesn't call distprep
anymore.
(*) - The tarball also contains the INSTALL file that is built at make
dist time, but not by distprep. This is unchanged for now.
The make maintainer-clean target, whose job it is to remove the
prebuilt files in addition to what make distclean does, is now just an
alias to make distprep. (In practice, it is probably obsolete given
that git clean is available.)
The following programs are now hard build requirements in configure
(they were already required by meson.build):
- bison
- flex
- perl
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/e07408d9-e5f2-d9fd-5672-f53354e9305e@eisentraut.org
Since C99, there can be a trailing comma after the last value in an
enum definition. A lot of new code has been introducing this style on
the fly. Some new patches are now taking an inconsistent approach to
this. Some add the last comma on the fly if they add a new last
value, some are trying to preserve the existing style in each place,
some are even dropping the last comma if there was one. We could
nudge this all in a consistent direction if we just add the trailing
commas everywhere once.
I omitted a few places where there was a fixed "last" value that will
always stay last. I also skipped the header files of libpq and ecpg,
in case people want to use those with older compilers. There were
also a small number of cases where the enum type wasn't used anywhere
(but the enum values were), which ended up confusing pgindent a bit,
so I left those alone.
Discussion: https://www.postgresql.org/message-id/flat/386f8c45-c8ac-4681-8add-e3b0852c1620%40eisentraut.org
The meson build scripts attempt to find files left over from configure
and fail, mentioning that "make maintainer-clean" should be run to remove
these. This seems to have been done for files generated from configure.
pg_config_paths.h is generated during the actual make build, so seems to
have been missed. This would result in compilation using the wrong
pg_config_paths.h file.
Here we just add this file to generated_sources_ac so that meson errors
out if pg_config_paths.h exists.
Likely this wasn't noticed before because make maintainer-clean will
remove pg_config_paths.h, however, people using the MSVC build scripts
are more likely to run into issues and they have to manually remove
these files and pg_config_paths.h wasn't listed as a conflicting file to
remove in the meson log.
Backpatch-through: 16, where meson support was added
Discussion: https://postgr.es/m/CAApHDvqjYOxZfmLKAOWKFEE7LOr9_E6UA6YNmx9r8nxStcS3gg@mail.gmail.com
