public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v6] pg_rewind: options to use restore_command from command line or cluster config
19+ messages / 5 participants
[nested] [flat]
* [PATCH v6] pg_rewind: options to use restore_command from command line or cluster config
@ 2019-02-19 16:14 Alexey Kondratov <[email protected]>
0 siblings, 0 replies; 19+ messages in thread
From: Alexey Kondratov @ 2019-02-19 16:14 UTC (permalink / raw)
Previously, when pg_rewind could not find required WAL files in the
target data directory the rewind process would fail. One had to
manually figure out which of required WAL files have already moved to
the archival storage and copy them back.
This patch adds possibility to specify restore_command via command
line option or use one specified inside postgresql.conf. Specified
restore_command will be used for automatic retrieval of missing WAL
files from archival storage.
---
doc/src/sgml/ref/pg_rewind.sgml | 30 ++++-
src/bin/pg_rewind/parsexlog.c | 167 +++++++++++++++++++++++++-
src/bin/pg_rewind/pg_rewind.c | 96 ++++++++++++++-
src/bin/pg_rewind/pg_rewind.h | 7 +-
src/bin/pg_rewind/t/001_basic.pl | 4 +-
src/bin/pg_rewind/t/002_databases.pl | 4 +-
src/bin/pg_rewind/t/003_extrafiles.pl | 4 +-
src/bin/pg_rewind/t/RewindTest.pm | 84 ++++++++++++-
8 files changed, 376 insertions(+), 20 deletions(-)
diff --git a/doc/src/sgml/ref/pg_rewind.sgml b/doc/src/sgml/ref/pg_rewind.sgml
index 53a64ee29e..90e3f22f97 100644
--- a/doc/src/sgml/ref/pg_rewind.sgml
+++ b/doc/src/sgml/ref/pg_rewind.sgml
@@ -67,8 +67,10 @@ PostgreSQL documentation
ancestor. In the typical failover scenario where the target cluster was
shut down soon after the divergence, this is not a problem, but if the
target cluster ran for a long time after the divergence, the old WAL
- files might no longer be present. In that case, they can be manually
- copied from the WAL archive to the <filename>pg_wal</filename> directory, or
+ files might no longer be present. In that case, they can be automatically
+ copied by <application>pg_rewind</application> from the WAL archive to the
+ <filename>pg_wal</filename> directory if either <literal>-r</literal> or
+ <literal>-R</literal> option is specified, or
fetched on startup by configuring <xref linkend="guc-primary-conninfo"/> or
<xref linkend="guc-restore-command"/>. The use of
<application>pg_rewind</application> is not limited to failover, e.g. a standby
@@ -200,6 +202,30 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>-r</option></term>
+ <term><option>--use-postgresql-conf</option></term>
+ <listitem>
+ <para>
+ Use restore_command in the <filename>postgresql.conf</filename> to
+ retrieve missing in the target <filename>pg_wal</filename> directory
+ WAL files from the WAL archive.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><option>-R <replaceable class="parameter">restore_command</replaceable></option></term>
+ <term><option>--restore-command=<replaceable class="parameter">restore_command</replaceable></option></term>
+ <listitem>
+ <para>
+ Specifies the restore_command to use for retrieval of the missing
+ in the target <filename>pg_wal</filename> directory WAL files from
+ the WAL archive.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>--debug</option></term>
<listitem>
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index e19c265cbb..3dc110be4e 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -12,6 +12,7 @@
#include "postgres_fe.h"
#include <unistd.h>
+#include <sys/stat.h>
#include "pg_rewind.h"
#include "filemap.h"
@@ -45,6 +46,7 @@ static char xlogfpath[MAXPGPATH];
typedef struct XLogPageReadPrivate
{
const char *datadir;
+ const char *restoreCommand;
int tliIndex;
} XLogPageReadPrivate;
@@ -53,6 +55,9 @@ static int SimpleXLogPageRead(XLogReaderState *xlogreader,
int reqLen, XLogRecPtr targetRecPtr, char *readBuf,
TimeLineID *pageTLI);
+static int RestoreArchivedWAL(const char *path, const char *xlogfname,
+ off_t expectedSize, const char *restoreCommand);
+
/*
* Read WAL from the datadir/pg_wal, starting from 'startpoint' on timeline
* index 'tliIndex' in target timeline history, until 'endpoint'. Make note of
@@ -60,7 +65,7 @@ static int SimpleXLogPageRead(XLogReaderState *xlogreader,
*/
void
extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
- XLogRecPtr endpoint)
+ XLogRecPtr endpoint, const char *restore_command)
{
XLogRecord *record;
XLogReaderState *xlogreader;
@@ -69,6 +74,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
private.datadir = datadir;
private.tliIndex = tliIndex;
+ private.restoreCommand = restore_command;
xlogreader = XLogReaderAllocate(WalSegSz, &SimpleXLogPageRead,
&private);
if (xlogreader == NULL)
@@ -156,7 +162,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
void
findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
XLogRecPtr *lastchkptrec, TimeLineID *lastchkpttli,
- XLogRecPtr *lastchkptredo)
+ XLogRecPtr *lastchkptredo, const char *restoreCommand)
{
/* Walk backwards, starting from the given record */
XLogRecord *record;
@@ -181,6 +187,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
private.datadir = datadir;
private.tliIndex = tliIndex;
+ private.restoreCommand = restoreCommand;
xlogreader = XLogReaderAllocate(WalSegSz, &SimpleXLogPageRead,
&private);
if (xlogreader == NULL)
@@ -291,9 +298,30 @@ SimpleXLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
if (xlogreadfd < 0)
{
- printf(_("could not open file \"%s\": %s\n"), xlogfpath,
- strerror(errno));
- return -1;
+ /*
+ * If we have no restore_command to execute, then exit.
+ */
+ if (private->restoreCommand == NULL)
+ {
+ printf(_("could not open file \"%s\": %s\n"), xlogfpath,
+ strerror(errno));
+ return -1;
+ }
+
+ /*
+ * Since we have restore_command to execute, then try to retrieve
+ * missing WAL file from the archive.
+ */
+ xlogreadfd = RestoreArchivedWAL(private->datadir,
+ xlogfname,
+ WalSegSz,
+ private->restoreCommand);
+
+ if (xlogreadfd < 0)
+ return -1;
+ else
+ pg_log(PG_DEBUG, "using restored from archive version of file \"%s\"\n",
+ xlogfpath);
}
}
@@ -409,3 +437,132 @@ extractPageInfo(XLogReaderState *record)
process_block_change(forknum, rnode, blkno);
}
}
+
+/*
+ * Attempt to retrieve the specified file from off-line archival storage.
+ * If successful return a file descriptor of restored WAL file, else
+ * return -1.
+ *
+ * For fixed-size files, the caller may pass the expected size as an
+ * additional crosscheck on successful recovery. If the file size is not
+ * known, set expectedSize = 0.
+ */
+static int
+RestoreArchivedWAL(const char *path, const char *xlogfname,
+ off_t expectedSize, const char *restoreCommand)
+{
+ char xlogpath[MAXPGPATH],
+ xlogRestoreCmd[MAXPGPATH],
+ *dp,
+ *endp;
+ const char *sp;
+ int rc,
+ xlogfd;
+ struct stat stat_buf;
+
+ snprintf(xlogpath, MAXPGPATH, "%s/" XLOGDIR "/%s", path, xlogfname);
+
+ /*
+ * Construct the command to be executed.
+ */
+ dp = xlogRestoreCmd;
+ endp = xlogRestoreCmd + MAXPGPATH - 1;
+ *endp = '\0';
+
+ for (sp = restoreCommand; *sp; sp++)
+ {
+ if (*sp == '%')
+ {
+ switch (sp[1])
+ {
+ case 'p':
+ /* %p: relative path of target file */
+ sp++;
+ StrNCpy(dp, xlogpath, endp - dp);
+ make_native_path(dp);
+ dp += strlen(dp);
+ break;
+ case 'f':
+ /* %f: filename of desired file */
+ sp++;
+ StrNCpy(dp, xlogfname, endp - dp);
+ dp += strlen(dp);
+ break;
+ case 'r':
+ /* %r: filename of last restartpoint */
+ pg_fatal("restore_command with %%r cannot be used with pg_rewind.\n");
+ break;
+ case '%':
+ /* convert %% to a single % */
+ sp++;
+ if (dp < endp)
+ *dp++ = *sp;
+ break;
+ default:
+ /* otherwise treat the % as not special */
+ if (dp < endp)
+ *dp++ = *sp;
+ break;
+ }
+ }
+ else
+ {
+ if (dp < endp)
+ *dp++ = *sp;
+ }
+ }
+ *dp = '\0';
+
+ /*
+ * Execute restore_command, which should copy
+ * the missing WAL file from archival storage.
+ */
+ rc = system(xlogRestoreCmd);
+
+ if (rc == 0)
+ {
+ /*
+ * Command apparently succeeded, but let's make sure the file is
+ * really there now and has the correct size.
+ */
+ if (stat(xlogpath, &stat_buf) == 0)
+ {
+ if (expectedSize > 0 && stat_buf.st_size != expectedSize)
+ {
+ printf(_("archive file \"%s\" has wrong size: %lu instead of %lu, %s"),
+ xlogfname, (unsigned long) stat_buf.st_size,
+ (unsigned long) expectedSize, strerror(errno));
+ }
+ else
+ {
+ xlogfd = open(xlogpath, O_RDONLY | PG_BINARY, 0);
+
+ if (xlogfd < 0)
+ printf(_("could not open restored from archive file \"%s\": %s\n"),
+ xlogpath, strerror(errno));
+ else
+ return xlogfd;
+ }
+ }
+ else
+ {
+ /* Stat failed */
+ printf(_("could not stat file \"%s\": %s"),
+ xlogpath, strerror(errno));
+ }
+ }
+
+ /*
+ * If the failure was due to any sort of signal, then it will be
+ * misleading to return message 'could not restore file...' and
+ * propagate result to the upper levels. We should exit right now.
+ */
+ if (wait_result_is_any_signal(rc, false))
+ pg_fatal("restore_command failed due to the signal: %s\n",
+ wait_result_to_str(rc));
+
+ printf(_("could not restore file \"%s\" from archive\n"),
+ xlogfname);
+
+ return -1;
+}
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 3dcadb9b40..344b67f99b 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -52,11 +52,13 @@ int WalSegSz;
char *datadir_target = NULL;
char *datadir_source = NULL;
char *connstr_source = NULL;
+char *restore_command = NULL;
bool debug = false;
bool showprogress = false;
bool dry_run = false;
bool do_sync = true;
+bool restore_wals = false;
/* Target history */
TimeLineHistoryEntry *targetHistory;
@@ -75,6 +77,9 @@ usage(const char *progname)
printf(_(" -N, --no-sync do not wait for changes to be written\n"));
printf(_(" safely to disk\n"));
printf(_(" -P, --progress write progress messages\n"));
+ printf(_(" -r, --use-postgresql-conf use restore_command in the postgresql.conf to\n"));
+ printf(_(" retrieve WALs from archive\n"));
+ printf(_(" -R, --restore-command=COMMAND restore_command\n"));
printf(_(" --debug write a lot of debug messages\n"));
printf(_(" -V, --version output version information, then exit\n"));
printf(_(" -?, --help show this help, then exit\n"));
@@ -94,6 +99,8 @@ main(int argc, char **argv)
{"dry-run", no_argument, NULL, 'n'},
{"no-sync", no_argument, NULL, 'N'},
{"progress", no_argument, NULL, 'P'},
+ {"use-postgresql-conf", no_argument, NULL, 'r'},
+ {"restore-command", required_argument, NULL, 'R'},
{"debug", no_argument, NULL, 3},
{NULL, 0, NULL, 0}
};
@@ -129,7 +136,7 @@ main(int argc, char **argv)
}
}
- while ((c = getopt_long(argc, argv, "D:nNP", long_options, &option_index)) != -1)
+ while ((c = getopt_long(argc, argv, "D:nNPR:r", long_options, &option_index)) != -1)
{
switch (c)
{
@@ -141,6 +148,10 @@ main(int argc, char **argv)
showprogress = true;
break;
+ case 'r':
+ restore_wals = true;
+ break;
+
case 'n':
dry_run = true;
break;
@@ -157,6 +168,10 @@ main(int argc, char **argv)
datadir_target = pg_strdup(optarg);
break;
+ case 'R':
+ restore_command = pg_strdup(optarg);
+ break;
+
case 1: /* --source-pgdata */
datadir_source = pg_strdup(optarg);
break;
@@ -223,6 +238,78 @@ main(int argc, char **argv)
umask(pg_mode_mask);
+ if (restore_command != NULL)
+ {
+ if (restore_wals)
+ {
+ fprintf(stderr, _("%s: conflicting options: both -r and -R are specified\n"),
+ progname);
+ fprintf(stderr, _("You must run %s with either -r/--use-postgresql-conf "
+ "or -R/--restore-command.\n"), progname);
+ exit(1);
+ }
+
+ pg_log(PG_DEBUG, "using command line restore_command=\'%s\'.\n", restore_command);
+ }
+ else if (restore_wals)
+ {
+ int rc;
+ char postgres_exec_path[MAXPGPATH],
+ postgres_cmd[MAXPGPATH],
+ cmd_output[MAXPGPATH];
+ FILE *output_fp;
+
+ /* Find postgres executable. */
+ rc = find_other_exec(argv[0], "postgres",
+ PG_BACKEND_VERSIONSTR,
+ postgres_exec_path);
+
+ if (rc < 0)
+ {
+ char full_path[MAXPGPATH];
+
+ if (find_my_exec(argv[0], full_path) < 0)
+ strlcpy(full_path, progname, sizeof(full_path));
+
+ if (rc == -1)
+ fprintf(stderr,
+ _("the program \"postgres\" is needed by %s "
+ "but was not found in the\n"
+ "same directory as \"%s\".\n"
+ "Check your installation.\n"),
+ progname, full_path);
+ else
+ fprintf(stderr,
+ _("the program \"postgres\" was found by \"%s\"\n"
+ "but was not the same version as %s.\n"
+ "Check your installation.\n"),
+ full_path, progname);
+ exit(1);
+ }
+
+ /* Build a command to execute for restore_command GUC retrieval if set. */
+ snprintf(postgres_cmd, sizeof(postgres_cmd), "%s -D %s -C restore_command",
+ postgres_exec_path, datadir_target);
+
+ if ((output_fp = popen(postgres_cmd, "r")) == NULL ||
+ fgets(cmd_output, sizeof(cmd_output), output_fp) == NULL)
+ pg_fatal("could not get restore_command using %s: %s\n",
+ postgres_cmd, strerror(errno));
+
+ pclose(output_fp);
+
+ /* Remove trailing newline */
+ if (strchr(cmd_output, '\n') != NULL)
+ *strchr(cmd_output, '\n') = '\0';
+
+ if (!strcmp(cmd_output, ""))
+ pg_fatal("restore_command is not set on the target cluster\n");
+
+ restore_command = pg_strdup(cmd_output);
+
+ pg_log(PG_DEBUG, "using config variable restore_command=\'%s\'.\n", restore_command);
+ }
+
/* Connect to remote server */
if (connstr_source)
libpqConnect(connstr_source);
@@ -294,9 +381,8 @@ main(int argc, char **argv)
exit(0);
}
- findLastCheckpoint(datadir_target, divergerec,
- lastcommontliIndex,
- &chkptrec, &chkpttli, &chkptredo);
+ findLastCheckpoint(datadir_target, divergerec, lastcommontliIndex,
+ &chkptrec, &chkpttli, &chkptredo, restore_command);
printf(_("rewinding from last common checkpoint at %X/%X on timeline %u\n"),
(uint32) (chkptrec >> 32), (uint32) chkptrec,
chkpttli);
@@ -319,7 +405,7 @@ main(int argc, char **argv)
*/
pg_log(PG_PROGRESS, "reading WAL in target\n");
extractPageMap(datadir_target, chkptrec, lastcommontliIndex,
- ControlFile_target.checkPoint);
+ ControlFile_target.checkPoint, restore_command);
filemap_finalize();
if (showprogress)
diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h
index 83b2898b8b..08a753475c 100644
--- a/src/bin/pg_rewind/pg_rewind.h
+++ b/src/bin/pg_rewind/pg_rewind.h
@@ -32,11 +32,10 @@ extern int targetNentries;
/* in parsexlog.c */
extern void extractPageMap(const char *datadir, XLogRecPtr startpoint,
- int tliIndex, XLogRecPtr endpoint);
+ int tliIndex, XLogRecPtr endpoint, const char *restoreCommand);
extern void findLastCheckpoint(const char *datadir, XLogRecPtr searchptr,
- int tliIndex,
- XLogRecPtr *lastchkptrec, TimeLineID *lastchkpttli,
- XLogRecPtr *lastchkptredo);
+ int tliIndex, XLogRecPtr *lastchkptrec, TimeLineID *lastchkpttli,
+ XLogRecPtr *lastchkptredo, const char *restoreCommand);
extern XLogRecPtr readOneRecord(const char *datadir, XLogRecPtr ptr,
int tliIndex);
diff --git a/src/bin/pg_rewind/t/001_basic.pl b/src/bin/pg_rewind/t/001_basic.pl
index 115192170e..8a6fa33016 100644
--- a/src/bin/pg_rewind/t/001_basic.pl
+++ b/src/bin/pg_rewind/t/001_basic.pl
@@ -1,7 +1,7 @@
use strict;
use warnings;
use TestLib;
-use Test::More tests => 10;
+use Test::More tests => 20;
use FindBin;
use lib $FindBin::RealBin;
@@ -106,5 +106,7 @@ in master, before promotion
# Run the test in both modes
run_test('local');
run_test('remote');
+run_test('archive');
+run_test('archive_conf');
exit(0);
diff --git a/src/bin/pg_rewind/t/002_databases.pl b/src/bin/pg_rewind/t/002_databases.pl
index 0562c21549..f42fb5a068 100644
--- a/src/bin/pg_rewind/t/002_databases.pl
+++ b/src/bin/pg_rewind/t/002_databases.pl
@@ -1,7 +1,7 @@
use strict;
use warnings;
use TestLib;
-use Test::More tests => 6;
+use Test::More tests => 12;
use FindBin;
use lib $FindBin::RealBin;
@@ -70,5 +70,7 @@ template1
# Run the test in both modes.
run_test('local');
run_test('remote');
+run_test('archive');
+run_test('archive_conf');
exit(0);
diff --git a/src/bin/pg_rewind/t/003_extrafiles.pl b/src/bin/pg_rewind/t/003_extrafiles.pl
index c4040bd562..24cec256de 100644
--- a/src/bin/pg_rewind/t/003_extrafiles.pl
+++ b/src/bin/pg_rewind/t/003_extrafiles.pl
@@ -3,7 +3,7 @@
use strict;
use warnings;
use TestLib;
-use Test::More tests => 4;
+use Test::More tests => 8;
use File::Find;
@@ -90,5 +90,7 @@ sub run_test
# Run the test in both modes.
run_test('local');
run_test('remote');
+run_test('archive');
+run_test('archive_conf');
exit(0);
diff --git a/src/bin/pg_rewind/t/RewindTest.pm b/src/bin/pg_rewind/t/RewindTest.pm
index 900d452d8b..3a58376347 100644
--- a/src/bin/pg_rewind/t/RewindTest.pm
+++ b/src/bin/pg_rewind/t/RewindTest.pm
@@ -39,7 +39,9 @@ use Carp;
use Config;
use Exporter 'import';
use File::Copy;
-use File::Path qw(rmtree);
+use File::Glob ':bsd_glob';
+use File::Path qw(remove_tree make_path);
+use File::Spec::Functions qw(catdir catfile);
use IPC::Run qw(run);
use PostgresNode;
use TestLib;
@@ -199,6 +201,38 @@ sub promote_standby
return;
}
+# Moves WAL files to the temporary location and returns restore_command
+# to get them back.
+sub move_wals
+{
+ my $tmp_dir = shift;
+ my $master_pgdata = shift;
+ my $wals_archive_dir = catdir($tmp_dir, "master_wals_archive");
+ my @wal_files = bsd_glob catfile($master_pgdata, "pg_wal", "0000000*");
+ my $restore_command;
+
+ remove_tree($wals_archive_dir);
+ make_path($wals_archive_dir) or die;
+
+ # Move all old master WAL files to the archive.
+ # Old master should be stopped at this point.
+ foreach my $wal_file (@wal_files)
+ {
+ move($wal_file, "$wals_archive_dir") or die;
+ }
+
+ if ($windows_os)
+ {
+ $restore_command = "copy $wals_archive_dir\\\%f \%p";
+ }
+ else
+ {
+ $restore_command = "cp $wals_archive_dir/\%f \%p";
+ }
+
+ return $restore_command;
+}
+
sub run_pg_rewind
{
my $test_mode = shift;
@@ -251,6 +285,54 @@ sub run_pg_rewind
],
'pg_rewind remote');
}
+ elsif ($test_mode eq "archive")
+ {
+
+ # Do rewind using a local pgdata as source and
+ # specified directory with target WALs archive.
+ my $restore_command = move_wals($tmp_folder, $master_pgdata);
+
+ # Stop the new master and be ready to perform the rewind.
+ $node_standby->stop;
+
+ command_ok(
+ [
+ 'pg_rewind',
+ "--debug",
+ "--source-pgdata=$standby_pgdata",
+ "--target-pgdata=$master_pgdata",
+ "--no-sync",
+ "--restore-command=$restore_command"
+ ],
+ 'pg_rewind archive');
+ }
+ elsif ($test_mode eq "archive_conf")
+ {
+
+ # Do rewind using a local pgdata as source and
+ # specified directory with target WALs archive.
+ my $master_conf_path = catfile($master_pgdata, 'postgresql.conf');
+ my $restore_command = move_wals($tmp_folder, $master_pgdata);
+
+ # Stop the new master and be ready to perform the rewind.
+ $node_standby->stop;
+
+ # Add restore_command to postgresql.conf of target cluster.
+ open(my $conf_fd, ">>", $master_conf_path) or die;
+ print $conf_fd "\nrestore_command='$restore_command'";
+ close $conf_fd;
+
+ command_ok(
+ [
+ 'pg_rewind',
+ "--debug",
+ "--source-pgdata=$standby_pgdata",
+ "--target-pgdata=$master_pgdata",
+ "--no-sync",
+ "-r"
+ ],
+ 'pg_rewind archive_conf');
+ }
else
{
base-commit: c8c885b7a5c8c1175288de1d8aaec3b4ae9050e1
--
2.17.1
--------------90C404DEB1DBAB9F51337B62--
^ permalink raw reply [nested|flat] 19+ messages in thread
* [PATCH v37 11/11] Add documentations about Incremental View Maintenance
@ 2019-12-20 01:25 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 19+ messages in thread
From: Yugo Nagata @ 2019-12-20 01:25 UTC (permalink / raw)
---
doc/src/sgml/catalogs.sgml | 9 +
.../sgml/ref/create_materialized_view.sgml | 124 ++++-
.../sgml/ref/refresh_materialized_view.sgml | 8 +-
doc/src/sgml/rules.sgml | 437 ++++++++++++++++++
doc/src/sgml/system-views.sgml | 9 +
5 files changed, 583 insertions(+), 4 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 4b474c13917..9b1c88161a7 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -2276,6 +2276,15 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
</para></entry>
</row>
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>relisivm</structfield> <type>bool</type>
+ </para>
+ <para>
+ True if relation is incrementally maintainable materialized view
+ </para></entry>
+ </row>
+
<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>relrewrite</structfield> <type>oid</type>
diff --git a/doc/src/sgml/ref/create_materialized_view.sgml b/doc/src/sgml/ref/create_materialized_view.sgml
index 62d897931c3..f49db209558 100644
--- a/doc/src/sgml/ref/create_materialized_view.sgml
+++ b/doc/src/sgml/ref/create_materialized_view.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
<refsynopsisdiv>
<synopsis>
-CREATE MATERIALIZED VIEW [ IF NOT EXISTS ] <replaceable>table_name</replaceable>
+CREATE [ INCREMENTAL ] MATERIALIZED VIEW [ IF NOT EXISTS ] <replaceable>table_name</replaceable>
[ (<replaceable>column_name</replaceable> [, ...] ) ]
[ USING <replaceable class="parameter">method</replaceable> ]
[ WITH ( <replaceable class="parameter">storage_parameter</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] ) ]
@@ -60,6 +60,125 @@ CREATE MATERIALIZED VIEW [ IF NOT EXISTS ] <replaceable>table_name</replaceable>
<title>Parameters</title>
<variablelist>
+ <varlistentry>
+ <term><literal>INCREMENTAL</literal></term>
+ <listitem>
+ <para>
+ If specified, some triggers are automatically created so that the rows
+ of the materialized view are immediately updated when base tables of the
+ materialized view are updated. In general, this allows faster update of
+ the materialized view at a price of slower update of the base tables
+ because the triggers will be invoked. We call this form of materialized
+ view as "Incrementally Maintainable Materialized View" (IMMV).
+ </para>
+ <para>
+ When <acronym>IMMV</acronym> is defined without using <command>WITH NO DATA</command>,
+ a unique index is created on the view automatically if possible. If the view
+ definition query has a GROUP BY clause, a unique index is created on the columns
+ of GROUP BY expressions. Also, if the view has DISTINCT clause, a unique index
+ is created on all columns in the target list. Otherwise, if the view contains all
+ primary key attritubes of its base tables in the target list, a unique index is
+ created on these attritubes. In other cases, no index is created.
+ </para>
+ <para>
+ There are restrictions of query definitions allowed to use this
+ option. The following are supported in query definitions for IMMV:
+ <itemizedlist>
+
+ <listitem>
+ <para>
+ Inner joins (including self-joins).
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ Some built-in aggregate functions (count, sum, avg, min, max) without a HAVING
+ clause.
+ </para>
+ </listitem>
+ </itemizedlist>
+
+ Unsupported queries with this option include the following:
+
+ <itemizedlist>
+ <listitem>
+ <para>
+ Outer joins.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ Sub-queries.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ Aggregate functions other than built-in count, sum, avg, min and max.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ Aggregate functions with a HAVING clause.
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ DISTINCT ON, WINDOW, VALUES, LIMIT and OFFSET clause.
+ </para>
+ </listitem>
+ </itemizedlist>
+
+ Other restrictions include:
+ <itemizedlist>
+
+ <listitem>
+ <para>
+ IMMVs must be based on simple base tables. It's not supported to
+ create them on top of views or materialized views.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ It is not supported to include system columns in an IMMV.
+ <programlisting>
+CREATE INCREMENTAL MATERIALIZED VIEW mv_ivm02 AS SELECT i,j FROM mv_base_a WHERE xmin = '610';
+ERROR: system column is not supported with IVM
+ </programlisting>
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ Non-immutable functions are not supported.
+ <programlisting>
+CREATE INCREMENTAL MATERIALIZED VIEW mv_ivm12 AS SELECT i,j FROM mv_base_a WHERE i = random()::int;
+ERROR: functions in IMMV must be marked IMMUTABLE
+ </programlisting>
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ IMMVs do not support expressions that contains aggregates
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ Logical replication does not support IMMVs.
+ </para>
+ </listitem>
+
+ </itemizedlist>
+
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal>IF NOT EXISTS</literal></term>
<listitem>
@@ -157,7 +276,8 @@ CREATE MATERIALIZED VIEW [ IF NOT EXISTS ] <replaceable>table_name</replaceable>
This clause specifies whether or not the materialized view should be
populated at creation time. If not, the materialized view will be
flagged as unscannable and cannot be queried until <command>REFRESH
- MATERIALIZED VIEW</command> is used.
+ MATERIALIZED VIEW</command> is used. Also, if the view is IMMV,
+ triggers for maintaining the view are not created.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/refresh_materialized_view.sgml b/doc/src/sgml/ref/refresh_materialized_view.sgml
index 8ed43ade803..a4d729bdf0f 100644
--- a/doc/src/sgml/ref/refresh_materialized_view.sgml
+++ b/doc/src/sgml/ref/refresh_materialized_view.sgml
@@ -36,9 +36,13 @@ REFRESH MATERIALIZED VIEW [ CONCURRENTLY ] <replaceable class="parameter">name</
privilege on the materialized view. The old contents are discarded. If
<literal>WITH DATA</literal> is specified (or defaults) the backing query
is executed to provide the new data, and the materialized view is left in a
- scannable state. If <literal>WITH NO DATA</literal> is specified no new
+ scannable state. If the view is an incrementally maintainable materialized
+ view (IMMV) and was unpopulated, triggers for maintaining the view are
+ created. Also, a unique index is created for IMMV if it is possible and the
+ view doesn't have that yet.
+ If <literal>WITH NO DATA</literal> is specified no new
data is generated and the materialized view is left in an unscannable
- state.
+ state. If the view is IMMV, the triggers are dropped.
</para>
<para>
<literal>CONCURRENTLY</literal> and <literal>WITH NO DATA</literal> may not
diff --git a/doc/src/sgml/rules.sgml b/doc/src/sgml/rules.sgml
index 7f23962f524..da9ad3c6316 100644
--- a/doc/src/sgml/rules.sgml
+++ b/doc/src/sgml/rules.sgml
@@ -1103,6 +1103,443 @@ SELECT word FROM words ORDER BY word <-> 'caterpiler' LIMIT 10;
</sect1>
+<sect1 id="rules-ivm">
+<title>Incremental View Maintenance</title>
+
+<indexterm zone="rules-ivm">
+ <primary>incremental view maintenance</primary>
+</indexterm>
+
+<indexterm zone="rules-ivm">
+ <primary>materialized view</primary>
+ <secondary>incremental view maintenance</secondary>
+</indexterm>
+
+<indexterm zone="rules-ivm">
+ <primary>view</primary>
+ <secondary>incremental view maintenance</secondary>
+</indexterm>
+
+<sect2 id="rules-ivm-overview">
+<title>Overview</title>
+
+<para>
+ Incremental View Maintenance (<acronym>IVM</acronym>) is a way to make
+ materialized views up-to-date in which only incremental changes are computed
+ and applied on views rather than recomputing the contents from scratch as
+ <command>REFRESH MATERIALIZED VIEW</command> does. <acronym>IVM</acronym>
+ can update materialized views more efficiently than recomputation when only
+ small parts of the view are changed.
+</para>
+
+<para>
+ There are two approaches with regard to timing of view maintenance:
+ immediate and deferred. In immediate maintenance, views are updated in the
+ same transaction that its base table is modified. In deferred maintenance,
+ views are updated after the transaction is committed, for example, when the
+ view is accessed, as a response to user command like <command>REFRESH
+ MATERIALIZED VIEW</command>, or periodically in background, and so on.
+ <productname>PostgreSQL</productname> currently implements only a kind of
+ immediate maintenance, in which materialized views are updated immediately
+ in AFTER triggers when a base table is modified.
+</para>
+
+<para>
+ To create materialized views supporting <acronym>IVM</acronym>, use the
+ <command>CREATE INCREMENTAL MATERIALIZED VIEW</command>, for example:
+<programlisting>
+CREATE <emphasis>INCREMENTAL</emphasis> MATERIALIZED VIEW mymatview AS SELECT * FROM mytab;
+</programlisting>
+ When a materialized view is created with the <literal>INCREMENTAL</literal>
+ keyword, some triggers are automatically created so that the view's contents are
+ immediately updated when its base tables are modified. We call this form
+ of materialized view an Incrementally Maintainable Materialized View
+ (<acronym>IMMV</acronym>).
+<programlisting>
+postgres=# CREATE INCREMENTAL MATERIALIZED VIEW m AS SELECT * FROM t0;
+NOTICE: could not create an index on materialized view "m" automatically
+HINT: Create an index on the materialized view for effcient incremental maintenance.
+SELECT 3
+postgres=# SELECT * FROM m;
+ i
+---
+ 1
+ 2
+ 3
+(3 rows)
+
+postgres=# INSERT INTO t0 VALUES (4);
+INSERT 0 1
+postgres=# SELECT * FROM m; -- automatically updated
+ i
+---
+ 1
+ 2
+ 3
+ 4
+(4 rows)
+</programlisting>
+</para>
+
+<para>
+ Some <acronym>IMMV</acronym>s have hidden columns which are added
+ automatically when a materialized view is created. Their name starts
+ with <literal>__ivm_</literal> and they contain information required
+ for maintaining the <acronym>IMMV</acronym>. Such columns are not visible
+ when the <acronym>IMMV</acronym> is accessed by <literal>SELECT *</literal>
+ but are visible if the column name is explicitly specified in the target
+ list. We can also see the hidden columns in <literal>\d</literal>
+ meta-commands of <command>psql</command> commands.
+</para>
+
+<para>
+ In general, <acronym>IMMV</acronym>s allow faster updates of materialized
+ views at the price of slower updates to their base tables. Updates of
+ <acronym>IMMV</acronym> is slower because triggers will be invoked and the
+ view is updated in triggers per modification statement.
+</para>
+
+<para>
+ For example, suppose a normal materialized view defined as below:
+
+<programlisting>
+test=# CREATE INCREMENTAL MATERIALIZED VIEW mv_ivm AS
+ SELECT a.aid, b.bid, a.abalance, b.bbalance
+ FROM pgbench_accounts a JOIN pgbench_branches b USING(bid);
+SELECT 10000000
+
+</programlisting>
+
+ Updating a tuple in a base table of this materialized view is rapid but the
+ <command>REFRESH MATERIALIZED VIEW</command> command on this view takes a long time:
+
+<programlisting>
+test=# UPDATE pgbench_accounts SET abalance = 1000 WHERE aid = 1;
+UPDATE 1
+Time: 0.990 ms
+
+test=# REFRESH MATERIALIZED VIEW mv_normal ;
+REFRESH MATERIALIZED VIEW
+Time: 33533.952 ms (00:33.534)
+</programlisting>
+</para>
+
+<para>
+ On the other hand, after creating <acronym>IMMV</acronym> with the same view
+ definition as below:
+
+<programlisting>
+test=# CREATE INCREMENTAL MATERIALIZED VIEW mv_ivm AS
+ SELECT a.aid, b.bid, a.abalance, b.bbalance
+ FROM pgbench_accounts a JOIN pgbench_branches b USING(bid);
+test=# UPDATE pgbench_accounts SET abalance = 1000 WHERE aid = 1;
+NOTICE: created index "mv_ivm_index" on materialized view "mv_ivm"
+</programlisting>
+
+ updating a tuple in a base table takes more than the normal view,
+ but its content is updated automatically and this is faster than the
+ <command>REFRESH MATERIALIZED VIEW</command> command.
+
+<programlisting>
+test=# UPDATE pgbench_accounts SET abalance = 1000 WHERE aid = 1;
+UPDATE 1
+Time: 13.068 ms
+</programlisting>
+
+</para>
+
+<para>
+ Appropriate indexes on <acronym>IMMV</acronym>s are necessary for
+ efficient <acronym>IVM</acronym> because it looks for tuples to be
+ updated in <acronym>IMMV</acronym>. If there are no indexes, it
+ will take a long time.
+</para>
+
+<para>
+ Therefore, when <acronym>IMMV</acronym> is defined, a unique index is created on the view
+ automatically if possible. If the view definition query has a GROUP BY clause, a unique
+ index is created on the columns of GROUP BY expressions. Also, if the view has DISTINCT
+ clause, a unique index is created on all columns in the target list. Otherwise, if the
+ view contains all primary key attritubes of its base tables in the target list, a unique
+ index is created on these attritubes. In other cases, no index is created.
+</para>
+
+<para>
+ In the previous example, a unique index "mv_ivm_index" is created on aid and bid
+ columns of materialized view "mv_ivm", and this enables the rapid update of the view.
+ Dropping this index make updating the view take a loger time.
+<programlisting>
+test=# DROP INDEX mv_ivm_index;
+DROP INDEX
+Time: 67.081 ms
+
+test=# UPDATE pgbench_accounts SET abalance = 1000 WHERE aid = 1;
+UPDATE 1
+Time: 16386.245 ms (00:16.386)
+</programlisting>
+
+</para>
+
+<para>
+ <acronym>IVM</acronym> is effective when we want to keep a materialized
+ view up-to-date and small fraction of a base table is modified
+ infrequently. Due to the overhead of immediate maintenance, <acronym>IVM</acronym>
+ is not effective when a base table is modified frequently. Also, when a
+ large part of a base table is modified or large data is inserted into a
+ base table, <acronym>IVM</acronym> is not effective and the cost of
+ maintenance can be larger than the <command>REFRESH MATERIALIZED VIEW</command>
+ command. In such situation, we can use <command>REFRESH MATERIALIZED VIEW</command>
+ and specify <literal>WITH NO DATA</literal> to disable immediate
+ maintenance before modifying a base table. After a base table modification,
+ execute the <command>REFRESH MATERIALIZED VIEW</command> (with <literal>WITH DATA</literal>)
+ command to refresh the view data and enable immediate maintenance.
+</para>
+
+</sect2>
+
+<sect2 id="rules-ivm-support">
+<title>Supported View Definitions and Restrictions</title>
+
+<para>
+ Currently, we can create <acronym>IMMV</acronym>s using inner joins, and some
+ aggregates. However, several restrictions apply to the definition of IMMV.
+</para>
+
+<sect3 id="rules-ivm-support-joins">
+<title>Joins</title>
+<para>
+ Inner joins including self-join are supported. Outer joins are not supported.
+</para>
+</sect3>
+
+<sect3 id="rules-ivm-support-aggregates">
+<title>Aggregates</title>
+<para>
+ Supported aggregate functions are <function>count</function>, <function>sum</function>,
+ <function>avg</function>, <function>min</function>, and <function>max</function>.
+ Currently, only built-in aggregate functions are supported and user defined
+ aggregates cannot be used. When a base table is modified, the new aggregated
+ values are incrementally calculated using the old aggregated values and values
+ of related hidden columns stored in <acronym>IMMV</acronym>.
+</para>
+
+<para>
+ Note that for <function>min</function> or <function>max</function>, the new values
+ could be re-calculated from base tables with regard to the affected groups when a
+ tuple containing the current minimal or maximal values are deleted from a base table.
+ Therefore, it can takes a long time to update an <acronym>IMMV</acronym> containing
+ these functions.
+</para>
+
+<para>
+ Also note that using <function>sum</function> or <function>avg</function> on
+ <type>real</type> (<type>float4</type>) type or <type>double precision</type>
+ (<type>float8</type>) type in <acronym>IMMV</acronym> is unsafe. This is
+ because aggregated values in <acronym>IMMV</acronym> can become different from
+ results calculated from base tables due to the limited precision of these types.
+ To avoid this problem, use the <type>numeric</type> type instead.
+</para>
+
+ <sect4 id="rules-ivm-restrictions-aggregates">
+ <title>Restrictions on Aggregates</title>
+ <para>
+ There are the following restrictions:
+ <itemizedlist>
+ <listitem>
+ <para>
+ If we have a <literal>GROUP BY</literal> clause, expressions specified in
+ <literal>GROUP BY</literal> must appear in the target list. This is
+ how tuples to be updated in the <acronym>IMMV</acronym> are identified.
+ These attributes are used as scan keys for searching tuples in the
+ <acronym>IMMV</acronym>, so indexes on them are required for efficient
+ <acronym>IVM</acronym>.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ <literal>HAVING</literal> clause cannot be used.
+ </para>
+ </listitem>
+ </itemizedlist>
+ </para>
+ </sect4>
+</sect3>
+
+<sect3 id="rules-ivm-general-restricitons">
+<title>Other General Restrictions</title>
+<para>
+ There are other restrictions which generally apply to <acronym>IMMV</acronym>:
+ <itemizedlist>
+ <listitem>
+ <para>
+ Sub-queries cannot be used.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ CTEs cannot be used.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ Window functions cannot be used.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ <acronym>IMMV</acronym>s must be based on simple base tables. It's not
+ supported to create them on top of views, materialized views, foreign tables, inhe.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ LIMIT and OFFSET clauses cannot be used.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ <acronym>IMMV</acronym>s cannot contain system columns.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ <acronym>IMMV</acronym>s cannot contain non-immutable functions.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ UNION/INTERSECT/EXCEPT clauses cannnot be used.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ DISTINCT ON clauses cannot be used.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ TABLESAMPLE parameter cannot be used.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ inheritance parent tables cannnot be used.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ VALUES clause cannnot be used.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ <literal>GROUPING SETS</literal> and <literal>FILTER</literal> clauses cannot be used.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ FOR UPDATE/SHARE cannot be used.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ targetlist cannot contain columns whose name start with <literal>__ivm_</literal>.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ targetlist cannot contain expressions which contain an aggregate in it.
+ </para>
+ </listitem>
+
+ <listitem>
+ <para>
+ Logical replication is not supported, that is, even when a base table
+ at a publisher node is modified, <acronym>IMMV</acronym>s at subscriber
+ nodes are not updated.
+ </para>
+ </listitem>
+
+ </itemizedlist>
+</para>
+</sect3>
+
+</sect2>
+
+<sect2 id="rules-ivm-distinct">
+<title><literal>DISTINCT</literal></title>
+
+<para>
+ <productname>PostgreSQL</productname> supports <acronym>IMMV</acronym> with
+ <literal>DISTINCT</literal>. For example, suppose a <acronym>IMMV</acronym>
+ defined with <literal>DISTINCT</literal> on a base table containing duplicate
+ tuples. When tuples are deleted from the base table, a tuple in the view is
+ deleted if and only if the multiplicity of the tuple becomes zero. Moreover,
+ when tuples are inserted into the base table, a tuple is inserted into the
+ view only if the same tuple doesn't already exist in it.
+</para>
+
+<para>
+ Physically, an <acronym>IMMV</acronym> defined with <literal>DISTINCT</literal>
+ contains tuples after eliminating duplicates, and the multiplicity of each tuple
+ is stored in a hidden column named <literal>__ivm_count__</literal>.
+</para>
+</sect2>
+
+<sect2 id="rules-ivm-concurrent-transactions">
+<title>Concurrent Transactions</title>
+<para>
+ Suppose an <acronym>IMMV</acronym> is defined on two base tables and each
+ table was modified in different a concurrent transaction simultaneously.
+ In the transaction which was committed first, <acronym>IMMV</acronym> can
+ be updated considering only the change which happened in this transaction.
+ On the other hand, in order to update the view correctly in the transaction
+ which was committed later, we need to know the changes occurred in
+ both transactions. For this reason, <literal>ExclusiveLock</literal>
+ is held on an <acronym>IMMV</acronym> immediately after a base table is
+ modified in <literal>READ COMMITTED</literal> mode to make sure that
+ the <acronym>IMMV</acronym> is updated in the latter transaction after
+ the former transaction is committed. In <literal>REPEATABLE READ</literal>
+ or <literal>SERIALIZABLE</literal> mode, an error is raised immediately
+ if lock acquisition fails because any changes which occurred in
+ other transactions are not be visible in these modes and
+ <acronym>IMMV</acronym> cannot be updated correctly in such situations.
+ However, as an exception if the view has only one base table and
+ <command>INSERT</command> is performed on the table,
+ the lock held on thew view is <literal>RowExclusiveLock</literal>.
+</para>
+</sect2>
+
+<sect2 id="rules-ivm-rls">
+<title>Row Level Security</title>
+<para>
+ If some base tables have row level security policy, rows that are not visible
+ to the materialized view's owner are excluded from the result. In addition, such
+ rows are excluded as well when views are incrementally maintained. However, if a
+ new policy is defined or policies are changed after the materialized view was created,
+ the new policy will not be applied to the view contents. To apply the new policy,
+ you need to refresh materialized views.
+</para>
+</sect2>
+
+</sect1>
+
<sect1 id="rules-update">
<title>Rules on <command>INSERT</command>, <command>UPDATE</command>, and <command>DELETE</command></title>
diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 2ebec6928d5..65d1e2041d2 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -2231,6 +2231,15 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
</para></entry>
</row>
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>isimmv</structfield> <type>bool</type>
+ </para>
+ <para>
+ True if materialized view is incrementally maintainable
+ </para></entry>
+ </row>
+
<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>definition</structfield> <type>text</type>
--
2.43.0
--Multipart=_Fri__29_May_2026_23_14_17_+0900_Te0o73X2VqYK57Gd
Content-Type: text/x-diff;
name="v37-0010-Add-regression-tests-for-Incremental-View-Mainte.patch"
Content-Disposition: attachment;
filename="v37-0010-Add-regression-tests-for-Incremental-View-Mainte.patch"
Content-Transfer-Encoding: 7bit
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Remove dependence on integer wrapping
@ 2024-08-16 16:52 Nathan Bossart <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Nathan Bossart @ 2024-08-16 16:52 UTC (permalink / raw)
To: Joseph Koshakow <[email protected]>; +Cc: jian he <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Alexander Lakhin <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
On Thu, Aug 15, 2024 at 10:49:46PM -0400, Joseph Koshakow wrote:
> This updated version LGTM, I agree it's a good use of pg_abs_s32().
Committed.
--
nathan
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Remove dependence on integer wrapping
@ 2024-08-16 18:00 Alexander Lakhin <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Alexander Lakhin @ 2024-08-16 18:00 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; Joseph Koshakow <[email protected]>; +Cc: jian he <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
Hello Nathan and Joe,
16.08.2024 19:52, Nathan Bossart wrote:
> On Thu, Aug 15, 2024 at 10:49:46PM -0400, Joseph Koshakow wrote:
>> This updated version LGTM, I agree it's a good use of pg_abs_s32().
> Committed.
Thank you for working on that issue!
I've tried `make check` with CC=gcc-13 CPPFLAGS="-O0 -ftrapv" and got a
server crash:
2024-08-16 17:14:36.102 UTC postmaster[1982703] LOG:Â Â (PID 1983867) was terminated by signal 6: Aborted
2024-08-16 17:14:36.102 UTC postmaster[1982703] DETAIL:Â Failed process was running: select '[]'::jsonb ->> -2147483648;
with the stack trace
...
#5Â 0x0000556aec224a11 in __negvsi2 ()
#6Â 0x0000556aec046238 in jsonb_array_element_text (fcinfo=0x556aedd70240) at jsonfuncs.c:993
#7Â 0x0000556aebc90b68 in ExecInterpExpr (state=0x556aedd70160, econtext=0x556aedd706a0, isnull=0x7ffdf82211e4)
   at execExprInterp.c:765
...
(gdb) f 6
#6Â 0x0000556aec046238 in jsonb_array_element_text (fcinfo=0x556aedd70240) at jsonfuncs.c:993
993Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (-element > nelements)
(gdb) p element
$1 = -2147483648
Sp it looks like jsonb_array_element_text() still needs the same
treatment as jsonb_array_element().
Moreover, I tried to use "-ftrapv" on 32-bit Debian and came across
another failure:
select '9223372036854775807'::int8 * 2147483648::int8;
server closed the connection unexpectedly
...
#4Â 0xb722226a in __GI_abort () at ./stdlib/abort.c:79
#5Â 0x004cb2e1 in __mulvdi3.cold ()
#6Â 0x00abe7ab in pg_mul_s64_overflow (a=9223372036854775807, b=2147483648, result=0xbff1da68)
   at ../../../../src/include/common/int.h:264
#7Â 0x00abfbff in int8mul (fcinfo=0x14d9d04) at int8.c:496
#8Â 0x00782675 in ExecInterpExpr (state=0x14d9c4c, econtext=0x14da15c, isnull=0xbff1dc3f) at execExprInterp.c:765
Whilst
select '9223372036854775807'::int8 * 2147483647::int8;
emits
ERROR:Â bigint out of range
I've also discovered another trap-triggering case for a 64-bit platform:
select 1 union all select 1 union all select 1 union all select 1 union all
select 1 union all select 1 union all select 1 union all select 1 union all
select 1 union all select 1 union all select 1 union all select 1 union all
select 1 union all select 1 union all select 1 union all select 1 union all
select 1 union all select 1 union all select 1 union all select 1 union all
select 1 union all select 1 union all select 1 union all select 1 union all
select 1 union all select 1 union all select 1 union all select 1 union all
select 1 union all select 1 union all select 1;
server closed the connection unexpectedly
...
#5Â 0x00005576cfb1c9f3 in __negvdi2 ()
#6Â 0x00005576cf627c68 in bms_singleton_member (a=0x5576d09f7fb0) at bitmapset.c:691
#7Â 0x00005576cf72be0f in fix_append_rel_relids (root=0x5576d09df198, varno=31, subrelids=0x5576d09f7fb0)
   at prepjointree.c:3830
#8Â 0x00005576cf7278c2 in pull_up_simple_subquery (root=0x5576d09df198, jtnode=0x5576d09f7470, rte=0x5576d09de300,
   lowest_outer_join=0x0, containing_appendrel=0x5576d09f7368) at prepjointree.c:1277
...
(gdb) f 6
#6Â 0x00005576cf627c68 in bms_singleton_member (a=0x5576d09f7fb0) at bitmapset.c:691
691Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (result >= 0 || HAS_MULTIPLE_ONES(w))
(gdb) p/x w
$1 = 0x8000000000000000
Best regards,
Alexander
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Remove dependence on integer wrapping
@ 2024-08-16 18:35 Nathan Bossart <[email protected]>
parent: Alexander Lakhin <[email protected]>
0 siblings, 2 replies; 19+ messages in thread
From: Nathan Bossart @ 2024-08-16 18:35 UTC (permalink / raw)
To: Alexander Lakhin <[email protected]>; +Cc: Joseph Koshakow <[email protected]>; jian he <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
On Fri, Aug 16, 2024 at 09:00:00PM +0300, Alexander Lakhin wrote:
> Sp it looks like jsonb_array_element_text() still needs the same
> treatment as jsonb_array_element().
D'oh. I added a test for that but didn't actually fix the code. I think
we just need something like the following.
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 1f8ea51e6a..69cdd84393 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -990,7 +990,7 @@ jsonb_array_element_text(PG_FUNCTION_ARGS)
{
uint32 nelements = JB_ROOT_COUNT(jb);
- if (-element > nelements)
+ if (pg_abs_s32(element) > nelements)
PG_RETURN_NULL();
else
element += nelements;
> Moreover, I tried to use "-ftrapv" on 32-bit Debian and came across
> another failure:
> select '9223372036854775807'::int8 * 2147483648::int8;
> server closed the connection unexpectedly
> ...
> #4Â 0xb722226a in __GI_abort () at ./stdlib/abort.c:79
> #5Â 0x004cb2e1 in __mulvdi3.cold ()
> #6Â 0x00abe7ab in pg_mul_s64_overflow (a=9223372036854775807, b=2147483648, result=0xbff1da68)
> Â Â Â at ../../../../src/include/common/int.h:264
> #7Â 0x00abfbff in int8mul (fcinfo=0x14d9d04) at int8.c:496
> #8Â 0x00782675 in ExecInterpExpr (state=0x14d9c4c, econtext=0x14da15c, isnull=0xbff1dc3f) at execExprInterp.c:765
Hm. It looks like that is pointing to __builtin_mul_overflow(), which
seems strange.
> #6Â 0x00005576cf627c68 in bms_singleton_member (a=0x5576d09f7fb0) at bitmapset.c:691
> 691Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (result >= 0 || HAS_MULTIPLE_ONES(w))
At a glance, this appears to be caused by the RIGHTMOST_ONE macro:
#define RIGHTMOST_ONE(x) ((signedbitmapword) (x) & -((signedbitmapword) (x)))
--
nathan
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Remove dependence on integer wrapping
@ 2024-08-16 18:56 Nathan Bossart <[email protected]>
parent: Nathan Bossart <[email protected]>
1 sibling, 0 replies; 19+ messages in thread
From: Nathan Bossart @ 2024-08-16 18:56 UTC (permalink / raw)
To: Alexander Lakhin <[email protected]>; +Cc: Joseph Koshakow <[email protected]>; jian he <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
On Fri, Aug 16, 2024 at 01:35:01PM -0500, Nathan Bossart wrote:
> On Fri, Aug 16, 2024 at 09:00:00PM +0300, Alexander Lakhin wrote:
>> #6Â 0x00005576cf627c68 in bms_singleton_member (a=0x5576d09f7fb0) at bitmapset.c:691
>> 691Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (result >= 0 || HAS_MULTIPLE_ONES(w))
>
> At a glance, this appears to be caused by the RIGHTMOST_ONE macro:
>
> #define RIGHTMOST_ONE(x) ((signedbitmapword) (x) & -((signedbitmapword) (x)))
I believe hand-rolling the two's complement calculation should be
sufficient to avoid depending on -fwrapv here. godbolt.org indicates that
it produces roughly the same code, too.
diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index cd05c642b0..d37a997c0e 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -67,7 +67,7 @@
* we get zero.
*----------
*/
-#define RIGHTMOST_ONE(x) ((signedbitmapword) (x) & -((signedbitmapword) (x)))
+#define RIGHTMOST_ONE(x) ((bitmapword) (x) & (~((bitmapword) (x)) + 1))
#define HAS_MULTIPLE_ONES(x) ((bitmapword) RIGHTMOST_ONE(x) != (x))
--
nathan
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Remove dependence on integer wrapping
@ 2024-08-16 20:11 Nathan Bossart <[email protected]>
parent: Nathan Bossart <[email protected]>
1 sibling, 1 reply; 19+ messages in thread
From: Nathan Bossart @ 2024-08-16 20:11 UTC (permalink / raw)
To: Alexander Lakhin <[email protected]>; +Cc: Joseph Koshakow <[email protected]>; jian he <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
On Fri, Aug 16, 2024 at 01:35:01PM -0500, Nathan Bossart wrote:
> On Fri, Aug 16, 2024 at 09:00:00PM +0300, Alexander Lakhin wrote:
>> Sp it looks like jsonb_array_element_text() still needs the same
>> treatment as jsonb_array_element().
>
> D'oh. I added a test for that but didn't actually fix the code. I think
> we just need something like the following.
>
> diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
> index 1f8ea51e6a..69cdd84393 100644
> --- a/src/backend/utils/adt/jsonfuncs.c
> +++ b/src/backend/utils/adt/jsonfuncs.c
> @@ -990,7 +990,7 @@ jsonb_array_element_text(PG_FUNCTION_ARGS)
> {
> uint32 nelements = JB_ROOT_COUNT(jb);
>
> - if (-element > nelements)
> + if (pg_abs_s32(element) > nelements)
> PG_RETURN_NULL();
> else
> element += nelements;
I've committed this one.
--
nathan
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Remove dependence on integer wrapping
@ 2024-08-17 19:16 Joseph Koshakow <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 2 replies; 19+ messages in thread
From: Joseph Koshakow @ 2024-08-17 19:16 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Alexander Lakhin <[email protected]>; jian he <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
Hi,
I wanted to take this opportunity to provide a brief summary of
outstanding work.
> Also there are several trap-producing cases with date types:
> SELECT to_date('100000000', 'CC');
> SELECT to_timestamp('1000000000,999', 'Y,YYY');
> SELECT make_date(-2147483648, 1, 1);
This is resolved with Matthew's patches, which I've rebased, squashed
and attached to this email. They still require a review.
----
> SET temp_buffers TO 1000000000;
>
> CREATE TEMP TABLE t(i int PRIMARY KEY);
> INSERT INTO t VALUES(1);
>
> #4 0x00007f385cdd37f3 in __GI_abort () at ./stdlib/abort.c:79
> #5 0x00005620071c4f51 in __addvsi3 ()
> #6 0x0000562007143f3c in init_htab (hashp=0x562008facb20,
nelem=610070812) at dynahash.c:720
>
> (gdb) f 6
> #6 0x0000560915207f3c in init_htab (hashp=0x560916039930,
nelem=1000000000) at dynahash.c:720
> 720 hctl->high_mask = (nbuckets << 1) - 1;
> (gdb) p nbuckets
> $1 = 1073741824
I've taken a look at this and my current proposal is to convert
`nbuckets` to 64 bit integer which would prevent the overflow. I'm
hoping to look into if this is feasible soon.
----
> CREATE FUNCTION check_foreign_key () RETURNS trigger AS .../refint.so'
LANGUAGE C;
> CREATE TABLE t (i int4 NOT NULL);
> CREATE TRIGGER check_fkey BEFORE DELETE ON t FOR EACH ROW EXECUTE
PROCEDURE
> check_foreign_key (2147483647, 'cascade', 'i', "ft", "i");
> INSERT INTO t VALUES (1);
> DELETE FROM t;
>
> #4 0x00007f57f0bef7f3 in __GI_abort () at ./stdlib/abort.c:79
> #5 0x00007f57f1671351 in __addvsi3 () from .../src/test/regress/refint.so
> #6 0x00007f57f1670234 in check_foreign_key (fcinfo=0x7ffebf523650) at
refint.c:321
>
> (gdb) f 6
> #6 0x00007f3400ef9234 in check_foreign_key (fcinfo=0x7ffd6e16a600) at
refint.c:321
> 321 nkeys = (nargs - nrefs) / (nrefs + 1);
> (gdb) p nargs
> $1 = 3
> (gdb) p nrefs
> $2 = 2147483647
I have not looked into this yet, though I was unable to reproduce it
immediately.
test=# CREATE FUNCTION check_foreign_key () RETURNS trigger AS
'.../refint.so' LANGUAGE C;
ERROR: could not access file ".../refint.so": No such file or directory
I think I just have to play around with the path.
----
>> Moreover, I tried to use "-ftrapv" on 32-bit Debian and came across
>> another failure:
>> select '9223372036854775807'::int8 * 2147483648::int8;
>> server closed the connection unexpectedly
>> ...
>> #4 0xb722226a in __GI_abort () at ./stdlib/abort.c:79
>> #5 0x004cb2e1 in __mulvdi3.cold ()
>> #6 0x00abe7ab in pg_mul_s64_overflow (a=9223372036854775807,
b=2147483648, result=0xbff1da68)
>> at ../../../../src/include/common/int.h:264
>> #7 0x00abfbff in int8mul (fcinfo=0x14d9d04) at int8.c:496
>> #8 0x00782675 in ExecInterpExpr (state=0x14d9c4c, econtext=0x14da15c,
isnull=0xbff1dc3f) at execExprInterp.c:765
>
> Hm. It looks like that is pointing to __builtin_mul_overflow(), which
> seems strange.
Agreed that this looks strange. The docs [0] seem to indicate that this
shouldn't happen.
> These built-in functions promote the first two operands into infinite
> precision signed type and perform addition on those promoted
> operands.
...
> As the addition is performed in infinite signed precision, these
> built-in functions have fully defined behavior for all argument
> values.
...
> The first built-in function allows arbitrary integral types for
> operands and the result type must be pointer to some integral type
> other than enumerated or boolean type
The docs for the mul functions say that they behave the same as
addition. Alexander, is it possible that you're compiling with
something other than GCC?
----
>>> #6 0x00005576cf627c68 in bms_singleton_member (a=0x5576d09f7fb0) at
bitmapset.c:691
>>> 691 if (result >= 0 || HAS_MULTIPLE_ONES(w))
>>
>> At a glance, this appears to be caused by the RIGHTMOST_ONE macro:
>>
>> #define RIGHTMOST_ONE(x) ((signedbitmapword) (x) &
-((signedbitmapword) (x)))
>
> I believe hand-rolling the two's complement calculation should be
> sufficient to avoid depending on -fwrapv here. godbolt.org indicates that
> it produces roughly the same code, too.
>
> diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
> index cd05c642b0..d37a997c0e 100644
> --- a/src/backend/nodes/bitmapset.c
> +++ b/src/backend/nodes/bitmapset.c
> @@ -67,7 +67,7 @@
> * we get zero.
> *----------
> */
> -#define RIGHTMOST_ONE(x) ((signedbitmapword) (x) & -((signedbitmapword)
(x)))
> +#define RIGHTMOST_ONE(x) ((bitmapword) (x) & (~((bitmapword) (x)) + 1))
This approach seems to resolve the issue locally for me, and I think it
falls out cleanly from the comment in the code above.
Thanks,
Joseph Koshakow
[0] https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html
Attachments:
[application/x-patch] v24-0001-Remove-dependence-on-fwrapv-semantics-in-some-da.patch (6.7K, ../../CAAvxfHd43nT6Va20wgftJ9XQuNPpmYSjR4WRc1jxTv=U4qOZgg@mail.gmail.com/3-v24-0001-Remove-dependence-on-fwrapv-semantics-in-some-da.patch)
download | inline diff:
From 15e5a79f198a029fc6436c409a00a1592e23a46d Mon Sep 17 00:00:00 2001
From: Matthew Kim <[email protected]>
Date: Tue, 9 Jul 2024 18:25:10 -0400
Subject: [PATCH v24] Remove dependence on -fwrapv semantics in some date/time
functions.
This commit updates a few date/time functions, such as
to_timestamp() and make_date(), to no longer rely on signed integer
wrapping for correctness. This is intended to move us closer towards
removing -fwrapv, which may enable some compiler optimizations.
However, there is presently no plan to actually remove that compiler
option in the near future.
---
src/backend/utils/adt/date.c | 5 +++-
src/backend/utils/adt/formatting.c | 35 +++++++++++++++++++++++---
src/test/regress/expected/date.out | 2 ++
src/test/regress/expected/horology.out | 4 +++
src/test/regress/sql/date.sql | 1 +
src/test/regress/sql/horology.sql | 2 ++
6 files changed, 45 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 9c854e0e5c..baa677fb96 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -257,7 +257,10 @@ make_date(PG_FUNCTION_ARGS)
if (tm.tm_year < 0)
{
bc = true;
- tm.tm_year = -tm.tm_year;
+ if (pg_mul_s32_overflow(tm.tm_year, -1, &tm.tm_year))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("date field value out of range")));
}
dterr = ValidateDate(DTK_DATE_M, false, false, bc, &tm);
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 68069fcfd3..e2cb57a9e1 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -77,6 +77,7 @@
#include "catalog/pg_collation.h"
#include "catalog/pg_type.h"
+#include "common/int.h"
#include "common/unicode_case.h"
#include "common/unicode_category.h"
#include "mb/pg_wchar.h"
@@ -3809,7 +3810,15 @@ DCH_from_char(FormatNode *node, const char *in, TmFromChar *out,
ereturn(escontext,,
(errcode(ERRCODE_INVALID_DATETIME_FORMAT),
errmsg("invalid input string for \"Y,YYY\"")));
- years += (millennia * 1000);
+ if (pg_mul_s32_overflow(millennia, 1000, &millennia))
+ ereturn(escontext,,
+ (errcode(ERRCODE_INVALID_DATETIME_FORMAT),
+ errmsg("invalid input string for \"Y,YYY\"")));
+ if (pg_add_s32_overflow(years, millennia, &years))
+ ereturn(escontext,,
+ (errcode(ERRCODE_INVALID_DATETIME_FORMAT),
+ errmsg("invalid input string for \"Y,YYY\"")));
+
if (!from_char_set_int(&out->year, years, n, escontext))
return;
out->yysz = 4;
@@ -4797,11 +4806,31 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
if (tmfc.bc)
tmfc.cc = -tmfc.cc;
if (tmfc.cc >= 0)
+ {
/* +1 because 21st century started in 2001 */
- tm->tm_year = (tmfc.cc - 1) * 100 + 1;
+ /* tm->tm_year = (tmfc.cc - 1) * 100 + 1; */
+ if (pg_mul_s32_overflow((tmfc.cc - 1), 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("date out of range: \"%s\"",
+ text_to_cstring(date_txt))));
+ }
+ }
else
+ {
/* +1 because year == 599 is 600 BC */
- tm->tm_year = tmfc.cc * 100 + 1;
+ /* tm->tm_year = tmfc.cc * 100 + 1; */
+ if (pg_mul_s32_overflow(tmfc.cc, 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("date out of range: \"%s\"",
+ text_to_cstring(date_txt))));
+ }
+ }
fmask |= DTK_M(YEAR);
}
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index f5949f3d17..9293e045b0 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1532,3 +1532,5 @@ select make_time(10, 55, 100.1);
ERROR: time field value out of range: 10:55:100.1
select make_time(24, 0, 2.1);
ERROR: time field value out of range: 24:00:2.1
+SELECT make_date(-2147483648, 1, 1);
+ERROR: date field value out of range
diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out
index 241713cc51..df02d268c0 100644
--- a/src/test/regress/expected/horology.out
+++ b/src/test/regress/expected/horology.out
@@ -3448,6 +3448,8 @@ SELECT i, to_timestamp('2018-11-02 12:34:56.123456', 'YYYY-MM-DD HH24:MI:SS.FF'
SELECT i, to_timestamp('2018-11-02 12:34:56.123456789', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
ERROR: date/time field value out of range: "2018-11-02 12:34:56.123456789"
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
+ERROR: invalid input string for "Y,YYY"
SELECT to_date('1 4 1902', 'Q MM YYYY'); -- Q is ignored
to_date
------------
@@ -3778,6 +3780,8 @@ SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
02-01-0001 BC
(1 row)
+SELECT to_date('100000000', 'CC');
+ERROR: date out of range: "100000000"
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
to_char
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index 1c58ff6966..9a4e5832b9 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -373,3 +373,4 @@ select make_date(2013, 13, 1);
select make_date(2013, 11, -1);
select make_time(10, 55, 100.1);
select make_time(24, 0, 2.1);
+SELECT make_date(-2147483648, 1, 1);
diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql
index e5cf12ff63..db532ee3c0 100644
--- a/src/test/regress/sql/horology.sql
+++ b/src/test/regress/sql/horology.sql
@@ -558,6 +558,7 @@ SELECT i, to_timestamp('2018-11-02 12:34:56.1234', 'YYYY-MM-DD HH24:MI:SS.FF' ||
SELECT i, to_timestamp('2018-11-02 12:34:56.12345', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
SELECT i, to_timestamp('2018-11-02 12:34:56.123456', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
SELECT i, to_timestamp('2018-11-02 12:34:56.123456789', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
SELECT to_date('1 4 1902', 'Q MM YYYY'); -- Q is ignored
SELECT to_date('3 4 21 01', 'W MM CC YY');
@@ -660,6 +661,7 @@ SELECT to_date('2016 365', 'YYYY DDD'); -- ok
SELECT to_date('2016 366', 'YYYY DDD'); -- ok
SELECT to_date('2016 367', 'YYYY DDD');
SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
+SELECT to_date('100000000', 'CC');
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
--
2.34.1
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Remove dependence on integer wrapping
@ 2024-08-17 21:52 Joseph Koshakow <[email protected]>
parent: Joseph Koshakow <[email protected]>
1 sibling, 1 reply; 19+ messages in thread
From: Joseph Koshakow @ 2024-08-17 21:52 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Alexander Lakhin <[email protected]>; jian he <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
>>> SET temp_buffers TO 1000000000;
>>>
>>> CREATE TEMP TABLE t(i int PRIMARY KEY);
>>> INSERT INTO t VALUES(1);
>>>
>>> #4 0x00007f385cdd37f3 in __GI_abort () at ./stdlib/abort.c:79
>>> #5 0x00005620071c4f51 in __addvsi3 ()
>>> #6 0x0000562007143f3c in init_htab (hashp=0x562008facb20,
nelem=610070812) at dynahash.c:720
>>>
>>> (gdb) f 6
>>> #6 0x0000560915207f3c in init_htab (hashp=0x560916039930,
nelem=1000000000) at dynahash.c:720
>>> 720 hctl->high_mask = (nbuckets << 1) - 1;
>>> (gdb) p nbuckets
>>> $1 = 1073741824
>>
>> Here's what it looks like is happening:
>>
>> 1. When inserting into the table, we create a new dynamic hash table
>> and set `nelem` equal to `temp_buffers`, which is 1000000000.
>>
>> 2. `nbuckets` is then set to the the next highest power of 2 from
>> `nelem`, which is 1073741824.
>>
>> /*
>> * Allocate space for the next greater power of two number of
buckets,
>> * assuming a desired maximum load factor of 1.
>> */
>> nbuckets = next_pow2_int(nelem);
>>
>> 3. Shift `nbuckets` to the left by 1. This would equal 2147483648,
>> which is larger than `INT_MAX`, which causes an overflow.
>>
>> hctl->high_mask = (nbuckets << 1) - 1;
>>
>> The max value allowed for `temp_buffers` is `INT_MAX / 2` (1073741823),
>> So any value of `temp_buffers` in the range (536870912, 1073741823]
>> would cause this overflow. Without `-ftrapv`, `nbuckets` would wrap
>> around to -2147483648, which is likely to cause all sorts of havoc, I'm
>> just not sure what exactly.
>>
>> Also, `nbuckets = next_pow2_int(nelem);`, by itself is a bit sketchy
>> considering that `nelem` is a `long` and `nbuckets` is an `int`.
>> Potentially, the fix here is to just convert `nbuckets` to a `long`. >>
I plan on checking if that's feasible.
> Yeah, the minimum value that triggers the trap is 536870913 and the
maximum
> accepted is 1073741823.
>
> Without -ftrapv, hctl->high_mask is set to 2147483647 on my machine,
> when nbuckets is 1073741824, and the INSERT apparently succeeds.
> I've taken a look at this and my current proposal is to convert
> `nbuckets` to 64 bit integer which would prevent the overflow. I'm
> hoping to look into if this is feasible soon.
I've both figured out why the INSERT still succeeds and a simple
solution to this. After `nbuckets` wraps around to -2147483648, we
subtract 1 which causes it to wrap back around to 2147483647. Which
explains the result seen by Alexander.
By the way,
>> Also, `nbuckets = next_pow2_int(nelem);`, by itself is a bit sketchy
>> considering that `nelem` is a `long` and `nbuckets` is an `int`.
It turns out I was wrong about this, `next_pow2_int` will always return
a value that fits into an `int`.
hctl->high_mask = (nbuckets << 1) - 1;
This calculation is used to ultimately populate the field
`uint32 high_mask`. I'm not very familiar with this hash table
implementation and I'm not entirely sure what it would take to convert
this to a `uint64`, but from poking around it looks like it would have
a huge blast radius.
The largest possible (theoretical) value for `nbuckets` is
`1073741824`, the largest power of 2 that fits into an `int`. So, the
largest possible value for `nbuckets << 1` is `2147483648`. This can
fully fit in a `uint32`, so the simple fix for this case is to cast
`nbuckets` to a `uint32` before shifting. I've attached this fix,
Alexander if you have time I would appreciate if you were able to test
it.
I noticed another potential issue with next_pow2_int. The
implementation is in dynahash.c and is as follows
/* calculate first power of 2 >= num, bounded to what will fit in an
int */
static int
next_pow2_int(long num)
{
if (num > INT_MAX / 2)
num = INT_MAX / 2;
return 1 << my_log2(num);
}
I'm pretty sure that `INT_MAX / 2` is not a power of 2, as `INT_MAX`
is not a power of 2. It should be `num = INT_MAX / 2 + 1;` I've also
attached a patch with this fix.
Thanks,
Joseph Koshakow
Attachments:
[text/x-patch] v25-0003-Fix-next_pow2_int.patch (813B, ../../CAAvxfHeWeETpiwxn11NvWkWd4o0curubg4cTXuk6yOGtixzAdg@mail.gmail.com/3-v25-0003-Fix-next_pow2_int.patch)
download | inline diff:
From 529452218496b9cd89464e1fad9e48c6279cef86 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <[email protected]>
Date: Sat, 17 Aug 2024 17:50:11 -0400
Subject: [PATCH v25 3/3] Fix next_pow2_int
This commit fixes the `next_pow2_int` function so that it always
returns a power of 2.
---
src/backend/utils/hash/dynahash.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 5aed981085..ee2bbe5dc8 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -1819,8 +1819,8 @@ next_pow2_long(long num)
static int
next_pow2_int(long num)
{
- if (num > INT_MAX / 2)
- num = INT_MAX / 2;
+ if (num > INT_MAX / 2 + 1)
+ num = INT_MAX / 2 + 1;
return 1 << my_log2(num);
}
--
2.34.1
[text/x-patch] v25-0002-Remove-dependence-on-fwrapv-semantics-in-dynahas.patch (956B, ../../CAAvxfHeWeETpiwxn11NvWkWd4o0curubg4cTXuk6yOGtixzAdg@mail.gmail.com/4-v25-0002-Remove-dependence-on-fwrapv-semantics-in-dynahas.patch)
download | inline diff:
From 397669bb89e51610928e2404c02a467ac431b280 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <[email protected]>
Date: Sat, 17 Aug 2024 17:47:10 -0400
Subject: [PATCH v25 2/3] Remove dependence on -fwrapv semantics in dynahash
This commit updates the logic for creating a new dynamic hash table to
no longer rely on integer wrapping for correctness.
---
src/backend/utils/hash/dynahash.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 5d9c62b652..5aed981085 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -717,7 +717,7 @@ init_htab(HTAB *hashp, long nelem)
nbuckets <<= 1;
hctl->max_bucket = hctl->low_mask = nbuckets - 1;
- hctl->high_mask = (nbuckets << 1) - 1;
+ hctl->high_mask = ((uint32) nbuckets << 1) - 1;
/*
* Figure number of directory segments needed, round up to a power of 2
--
2.34.1
[text/x-patch] v25-0001-Remove-dependence-on-fwrapv-semantics-in-some-da.patch (6.7K, ../../CAAvxfHeWeETpiwxn11NvWkWd4o0curubg4cTXuk6yOGtixzAdg@mail.gmail.com/5-v25-0001-Remove-dependence-on-fwrapv-semantics-in-some-da.patch)
download | inline diff:
From 15e5a79f198a029fc6436c409a00a1592e23a46d Mon Sep 17 00:00:00 2001
From: Matthew Kim <[email protected]>
Date: Tue, 9 Jul 2024 18:25:10 -0400
Subject: [PATCH v25 1/3] Remove dependence on -fwrapv semantics in some
date/time functions.
This commit updates a few date/time functions, such as
to_timestamp() and make_date(), to no longer rely on signed integer
wrapping for correctness. This is intended to move us closer towards
removing -fwrapv, which may enable some compiler optimizations.
However, there is presently no plan to actually remove that compiler
option in the near future.
---
src/backend/utils/adt/date.c | 5 +++-
src/backend/utils/adt/formatting.c | 35 +++++++++++++++++++++++---
src/test/regress/expected/date.out | 2 ++
src/test/regress/expected/horology.out | 4 +++
src/test/regress/sql/date.sql | 1 +
src/test/regress/sql/horology.sql | 2 ++
6 files changed, 45 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 9c854e0e5c..baa677fb96 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -257,7 +257,10 @@ make_date(PG_FUNCTION_ARGS)
if (tm.tm_year < 0)
{
bc = true;
- tm.tm_year = -tm.tm_year;
+ if (pg_mul_s32_overflow(tm.tm_year, -1, &tm.tm_year))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("date field value out of range")));
}
dterr = ValidateDate(DTK_DATE_M, false, false, bc, &tm);
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 68069fcfd3..e2cb57a9e1 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -77,6 +77,7 @@
#include "catalog/pg_collation.h"
#include "catalog/pg_type.h"
+#include "common/int.h"
#include "common/unicode_case.h"
#include "common/unicode_category.h"
#include "mb/pg_wchar.h"
@@ -3809,7 +3810,15 @@ DCH_from_char(FormatNode *node, const char *in, TmFromChar *out,
ereturn(escontext,,
(errcode(ERRCODE_INVALID_DATETIME_FORMAT),
errmsg("invalid input string for \"Y,YYY\"")));
- years += (millennia * 1000);
+ if (pg_mul_s32_overflow(millennia, 1000, &millennia))
+ ereturn(escontext,,
+ (errcode(ERRCODE_INVALID_DATETIME_FORMAT),
+ errmsg("invalid input string for \"Y,YYY\"")));
+ if (pg_add_s32_overflow(years, millennia, &years))
+ ereturn(escontext,,
+ (errcode(ERRCODE_INVALID_DATETIME_FORMAT),
+ errmsg("invalid input string for \"Y,YYY\"")));
+
if (!from_char_set_int(&out->year, years, n, escontext))
return;
out->yysz = 4;
@@ -4797,11 +4806,31 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
if (tmfc.bc)
tmfc.cc = -tmfc.cc;
if (tmfc.cc >= 0)
+ {
/* +1 because 21st century started in 2001 */
- tm->tm_year = (tmfc.cc - 1) * 100 + 1;
+ /* tm->tm_year = (tmfc.cc - 1) * 100 + 1; */
+ if (pg_mul_s32_overflow((tmfc.cc - 1), 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("date out of range: \"%s\"",
+ text_to_cstring(date_txt))));
+ }
+ }
else
+ {
/* +1 because year == 599 is 600 BC */
- tm->tm_year = tmfc.cc * 100 + 1;
+ /* tm->tm_year = tmfc.cc * 100 + 1; */
+ if (pg_mul_s32_overflow(tmfc.cc, 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("date out of range: \"%s\"",
+ text_to_cstring(date_txt))));
+ }
+ }
fmask |= DTK_M(YEAR);
}
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index f5949f3d17..9293e045b0 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1532,3 +1532,5 @@ select make_time(10, 55, 100.1);
ERROR: time field value out of range: 10:55:100.1
select make_time(24, 0, 2.1);
ERROR: time field value out of range: 24:00:2.1
+SELECT make_date(-2147483648, 1, 1);
+ERROR: date field value out of range
diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out
index 241713cc51..df02d268c0 100644
--- a/src/test/regress/expected/horology.out
+++ b/src/test/regress/expected/horology.out
@@ -3448,6 +3448,8 @@ SELECT i, to_timestamp('2018-11-02 12:34:56.123456', 'YYYY-MM-DD HH24:MI:SS.FF'
SELECT i, to_timestamp('2018-11-02 12:34:56.123456789', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
ERROR: date/time field value out of range: "2018-11-02 12:34:56.123456789"
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
+ERROR: invalid input string for "Y,YYY"
SELECT to_date('1 4 1902', 'Q MM YYYY'); -- Q is ignored
to_date
------------
@@ -3778,6 +3780,8 @@ SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
02-01-0001 BC
(1 row)
+SELECT to_date('100000000', 'CC');
+ERROR: date out of range: "100000000"
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
to_char
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index 1c58ff6966..9a4e5832b9 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -373,3 +373,4 @@ select make_date(2013, 13, 1);
select make_date(2013, 11, -1);
select make_time(10, 55, 100.1);
select make_time(24, 0, 2.1);
+SELECT make_date(-2147483648, 1, 1);
diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql
index e5cf12ff63..db532ee3c0 100644
--- a/src/test/regress/sql/horology.sql
+++ b/src/test/regress/sql/horology.sql
@@ -558,6 +558,7 @@ SELECT i, to_timestamp('2018-11-02 12:34:56.1234', 'YYYY-MM-DD HH24:MI:SS.FF' ||
SELECT i, to_timestamp('2018-11-02 12:34:56.12345', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
SELECT i, to_timestamp('2018-11-02 12:34:56.123456', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
SELECT i, to_timestamp('2018-11-02 12:34:56.123456789', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
SELECT to_date('1 4 1902', 'Q MM YYYY'); -- Q is ignored
SELECT to_date('3 4 21 01', 'W MM CC YY');
@@ -660,6 +661,7 @@ SELECT to_date('2016 365', 'YYYY DDD'); -- ok
SELECT to_date('2016 366', 'YYYY DDD'); -- ok
SELECT to_date('2016 367', 'YYYY DDD');
SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
+SELECT to_date('100000000', 'CC');
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
--
2.34.1
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Remove dependence on integer wrapping
@ 2024-08-18 03:00 Alexander Lakhin <[email protected]>
parent: Joseph Koshakow <[email protected]>
1 sibling, 0 replies; 19+ messages in thread
From: Alexander Lakhin @ 2024-08-18 03:00 UTC (permalink / raw)
To: Joseph Koshakow <[email protected]>; Nathan Bossart <[email protected]>; +Cc: jian he <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
Hello Joe,
17.08.2024 22:16, Joseph Koshakow wrote:
> Hi,
>
> I wanted to take this opportunity to provide a brief summary of
> outstanding work.
>
> > Also there are several trap-producing cases with date types:
> > SELECT to_date('100000000', 'CC');
> > SELECT to_timestamp('1000000000,999', 'Y,YYY');
> > SELECT make_date(-2147483648, 1, 1);
>
> This is resolved with Matthew's patches, which I've rebased, squashed
> and attached to this email. They still require a review.
>
I've filed a separate bug report about date/time conversion issues
yesterday. Maybe it was excessive, but it also demonstrates other
problematic cases:
https://www.postgresql.org/message-id/18585-db646741dd649abd%40postgresql.org
Best regards,
Alexander
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Remove dependence on integer wrapping
@ 2024-08-18 12:00 Alexander Lakhin <[email protected]>
parent: Joseph Koshakow <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Alexander Lakhin @ 2024-08-18 12:00 UTC (permalink / raw)
To: Joseph Koshakow <[email protected]>; Nathan Bossart <[email protected]>; +Cc: jian he <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
18.08.2024 00:52, Joseph Koshakow wrote:
> The largest possible (theoretical) value for `nbuckets` is
> `1073741824`, the largest power of 2 that fits into an `int`. So, the
> largest possible value for `nbuckets << 1` is `2147483648`. This can
> fully fit in a `uint32`, so the simple fix for this case is to cast
> `nbuckets` to a `uint32` before shifting. I've attached this fix,
> Alexander if you have time I would appreciate if you were able to test
> it.
>
Yes, I've tested v25-0002-*.patch and can confirm that this fix works
as well.
Best regards,
Alexander
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Remove dependence on integer wrapping
@ 2024-08-20 21:21 Nathan Bossart <[email protected]>
parent: Alexander Lakhin <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Nathan Bossart @ 2024-08-20 21:21 UTC (permalink / raw)
To: Alexander Lakhin <[email protected]>; +Cc: Joseph Koshakow <[email protected]>; jian he <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
I've combined all the current proposed changes into one patch. I've also
introduced signed versions of the negation functions into int.h to avoid
relying on multiplication.
--
nathan
From 2364ba4028f879a22b9f69f999aee3ea9c013ec0 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Tue, 20 Aug 2024 16:12:39 -0500
Subject: [PATCH v26 1/1] Remove dependence on -fwrapv semantics in more
places.
---
src/backend/nodes/bitmapset.c | 2 +-
src/backend/utils/adt/date.c | 6 +++-
src/backend/utils/adt/formatting.c | 28 +++++++++++++--
src/backend/utils/hash/dynahash.c | 6 ++--
src/include/common/int.h | 48 ++++++++++++++++++++++++++
src/test/regress/expected/date.out | 2 ++
src/test/regress/expected/horology.out | 4 +++
src/test/regress/expected/union.out | 43 +++++++++++++++++++++++
src/test/regress/sql/date.sql | 1 +
src/test/regress/sql/horology.sql | 2 ++
src/test/regress/sql/union.sql | 9 +++++
11 files changed, 143 insertions(+), 8 deletions(-)
diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index cd05c642b0..d37a997c0e 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -67,7 +67,7 @@
* we get zero.
*----------
*/
-#define RIGHTMOST_ONE(x) ((signedbitmapword) (x) & -((signedbitmapword) (x)))
+#define RIGHTMOST_ONE(x) ((bitmapword) (x) & (~((bitmapword) (x)) + 1))
#define HAS_MULTIPLE_ONES(x) ((bitmapword) RIGHTMOST_ONE(x) != (x))
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 9c854e0e5c..0782e84776 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -257,7 +257,11 @@ make_date(PG_FUNCTION_ARGS)
if (tm.tm_year < 0)
{
bc = true;
- tm.tm_year = -tm.tm_year;
+ if (pg_neg_s32_overflow(tm.tm_year, &tm.tm_year))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW),
+ errmsg("date field value out of range: %d-%02d-%02d",
+ tm.tm_year, tm.tm_mon, tm.tm_mday)));
}
dterr = ValidateDate(DTK_DATE_M, false, false, bc, &tm);
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 68069fcfd3..76bb3a79b5 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -77,6 +77,7 @@
#include "catalog/pg_collation.h"
#include "catalog/pg_type.h"
+#include "common/int.h"
#include "common/unicode_case.h"
#include "common/unicode_category.h"
#include "mb/pg_wchar.h"
@@ -3809,7 +3810,12 @@ DCH_from_char(FormatNode *node, const char *in, TmFromChar *out,
ereturn(escontext,,
(errcode(ERRCODE_INVALID_DATETIME_FORMAT),
errmsg("invalid input string for \"Y,YYY\"")));
- years += (millennia * 1000);
+ if (pg_mul_s32_overflow(millennia, 1000, &millennia) ||
+ pg_add_s32_overflow(years, millennia, &years))
+ ereturn(escontext,,
+ (errcode(ERRCODE_INVALID_DATETIME_FORMAT),
+ errmsg("invalid input string for \"Y,YYY\"")));
+
if (!from_char_set_int(&out->year, years, n, escontext))
return;
out->yysz = 4;
@@ -4797,11 +4803,27 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
if (tmfc.bc)
tmfc.cc = -tmfc.cc;
if (tmfc.cc >= 0)
+ {
/* +1 because 21st century started in 2001 */
- tm->tm_year = (tmfc.cc - 1) * 100 + 1;
+ /* tm->tm_year = (tmfc.cc - 1) * 100 + 1 */
+ if (pg_mul_s32_overflow(tmfc.cc - 1, 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("date out of range: \"%s\"",
+ text_to_cstring(date_txt))));
+ }
else
+ {
/* +1 because year == 599 is 600 BC */
- tm->tm_year = tmfc.cc * 100 + 1;
+ /* tm->tm_year = tmfc.cc * 100 + 1 */
+ if (pg_mul_s32_overflow(tmfc.cc, 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("date out of range: \"%s\"",
+ text_to_cstring(date_txt))));
+ }
fmask |= DTK_M(YEAR);
}
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 5d9c62b652..ee2bbe5dc8 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -717,7 +717,7 @@ init_htab(HTAB *hashp, long nelem)
nbuckets <<= 1;
hctl->max_bucket = hctl->low_mask = nbuckets - 1;
- hctl->high_mask = (nbuckets << 1) - 1;
+ hctl->high_mask = ((uint32) nbuckets << 1) - 1;
/*
* Figure number of directory segments needed, round up to a power of 2
@@ -1819,8 +1819,8 @@ next_pow2_long(long num)
static int
next_pow2_int(long num)
{
- if (num > INT_MAX / 2)
- num = INT_MAX / 2;
+ if (num > INT_MAX / 2 + 1)
+ num = INT_MAX / 2 + 1;
return 1 << my_log2(num);
}
diff --git a/src/include/common/int.h b/src/include/common/int.h
index 3b1590d676..6b50aa67b9 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -117,6 +117,22 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
#endif
}
+static inline bool
+pg_neg_s16_overflow(int16 a, int16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT16_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint16
pg_abs_s16(int16 a)
{
@@ -185,6 +201,22 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
#endif
}
+static inline bool
+pg_neg_s32_overflow(int32 a, int32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT32_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint32
pg_abs_s32(int32 a)
{
@@ -300,6 +332,22 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
#endif
}
+static inline bool
+pg_neg_s64_overflow(int64 a, int64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT64_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint64
pg_abs_s64(int64 a)
{
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index f5949f3d17..c8f76c205d 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1532,3 +1532,5 @@ select make_time(10, 55, 100.1);
ERROR: time field value out of range: 10:55:100.1
select make_time(24, 0, 2.1);
ERROR: time field value out of range: 24:00:2.1
+SELECT make_date(-2147483648, 1, 1);
+ERROR: date field value out of range: -2147483648-01-01
diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out
index 241713cc51..df02d268c0 100644
--- a/src/test/regress/expected/horology.out
+++ b/src/test/regress/expected/horology.out
@@ -3448,6 +3448,8 @@ SELECT i, to_timestamp('2018-11-02 12:34:56.123456', 'YYYY-MM-DD HH24:MI:SS.FF'
SELECT i, to_timestamp('2018-11-02 12:34:56.123456789', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
ERROR: date/time field value out of range: "2018-11-02 12:34:56.123456789"
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
+ERROR: invalid input string for "Y,YYY"
SELECT to_date('1 4 1902', 'Q MM YYYY'); -- Q is ignored
to_date
------------
@@ -3778,6 +3780,8 @@ SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
02-01-0001 BC
(1 row)
+SELECT to_date('100000000', 'CC');
+ERROR: date out of range: "100000000"
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
to_char
diff --git a/src/test/regress/expected/union.out b/src/test/regress/expected/union.out
index 0fd0e1c38b..444744848e 100644
--- a/src/test/regress/expected/union.out
+++ b/src/test/regress/expected/union.out
@@ -59,6 +59,49 @@ SELECT 1.1 AS two UNION SELECT 2.2 ORDER BY 1;
2.2
(2 rows)
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1;
+ ?column?
+----------
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+(31 rows)
+
-- Mixed types
SELECT 1.1 AS two UNION SELECT 2 ORDER BY 1;
two
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index 1c58ff6966..9a4e5832b9 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -373,3 +373,4 @@ select make_date(2013, 13, 1);
select make_date(2013, 11, -1);
select make_time(10, 55, 100.1);
select make_time(24, 0, 2.1);
+SELECT make_date(-2147483648, 1, 1);
diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql
index e5cf12ff63..db532ee3c0 100644
--- a/src/test/regress/sql/horology.sql
+++ b/src/test/regress/sql/horology.sql
@@ -558,6 +558,7 @@ SELECT i, to_timestamp('2018-11-02 12:34:56.1234', 'YYYY-MM-DD HH24:MI:SS.FF' ||
SELECT i, to_timestamp('2018-11-02 12:34:56.12345', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
SELECT i, to_timestamp('2018-11-02 12:34:56.123456', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
SELECT i, to_timestamp('2018-11-02 12:34:56.123456789', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
SELECT to_date('1 4 1902', 'Q MM YYYY'); -- Q is ignored
SELECT to_date('3 4 21 01', 'W MM CC YY');
@@ -660,6 +661,7 @@ SELECT to_date('2016 365', 'YYYY DDD'); -- ok
SELECT to_date('2016 366', 'YYYY DDD'); -- ok
SELECT to_date('2016 367', 'YYYY DDD');
SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
+SELECT to_date('100000000', 'CC');
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
diff --git a/src/test/regress/sql/union.sql b/src/test/regress/sql/union.sql
index f8826514e4..e1954a92d2 100644
--- a/src/test/regress/sql/union.sql
+++ b/src/test/regress/sql/union.sql
@@ -20,6 +20,15 @@ SELECT 1 AS three UNION SELECT 2 UNION ALL SELECT 2 ORDER BY 1;
SELECT 1.1 AS two UNION SELECT 2.2 ORDER BY 1;
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1;
+
-- Mixed types
SELECT 1.1 AS two UNION SELECT 2 ORDER BY 1;
--
2.39.3 (Apple Git-146)
Attachments:
[text/plain] v26-0001-Remove-dependence-on-fwrapv-semantics-in-more-pl.patch (11.4K, ../../ZsUI3QJdUOIpPkZ5@nathan/2-v26-0001-Remove-dependence-on-fwrapv-semantics-in-more-pl.patch)
download | inline diff:
From 2364ba4028f879a22b9f69f999aee3ea9c013ec0 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Tue, 20 Aug 2024 16:12:39 -0500
Subject: [PATCH v26 1/1] Remove dependence on -fwrapv semantics in more
places.
---
src/backend/nodes/bitmapset.c | 2 +-
src/backend/utils/adt/date.c | 6 +++-
src/backend/utils/adt/formatting.c | 28 +++++++++++++--
src/backend/utils/hash/dynahash.c | 6 ++--
src/include/common/int.h | 48 ++++++++++++++++++++++++++
src/test/regress/expected/date.out | 2 ++
src/test/regress/expected/horology.out | 4 +++
src/test/regress/expected/union.out | 43 +++++++++++++++++++++++
src/test/regress/sql/date.sql | 1 +
src/test/regress/sql/horology.sql | 2 ++
src/test/regress/sql/union.sql | 9 +++++
11 files changed, 143 insertions(+), 8 deletions(-)
diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index cd05c642b0..d37a997c0e 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -67,7 +67,7 @@
* we get zero.
*----------
*/
-#define RIGHTMOST_ONE(x) ((signedbitmapword) (x) & -((signedbitmapword) (x)))
+#define RIGHTMOST_ONE(x) ((bitmapword) (x) & (~((bitmapword) (x)) + 1))
#define HAS_MULTIPLE_ONES(x) ((bitmapword) RIGHTMOST_ONE(x) != (x))
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 9c854e0e5c..0782e84776 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -257,7 +257,11 @@ make_date(PG_FUNCTION_ARGS)
if (tm.tm_year < 0)
{
bc = true;
- tm.tm_year = -tm.tm_year;
+ if (pg_neg_s32_overflow(tm.tm_year, &tm.tm_year))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW),
+ errmsg("date field value out of range: %d-%02d-%02d",
+ tm.tm_year, tm.tm_mon, tm.tm_mday)));
}
dterr = ValidateDate(DTK_DATE_M, false, false, bc, &tm);
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 68069fcfd3..76bb3a79b5 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -77,6 +77,7 @@
#include "catalog/pg_collation.h"
#include "catalog/pg_type.h"
+#include "common/int.h"
#include "common/unicode_case.h"
#include "common/unicode_category.h"
#include "mb/pg_wchar.h"
@@ -3809,7 +3810,12 @@ DCH_from_char(FormatNode *node, const char *in, TmFromChar *out,
ereturn(escontext,,
(errcode(ERRCODE_INVALID_DATETIME_FORMAT),
errmsg("invalid input string for \"Y,YYY\"")));
- years += (millennia * 1000);
+ if (pg_mul_s32_overflow(millennia, 1000, &millennia) ||
+ pg_add_s32_overflow(years, millennia, &years))
+ ereturn(escontext,,
+ (errcode(ERRCODE_INVALID_DATETIME_FORMAT),
+ errmsg("invalid input string for \"Y,YYY\"")));
+
if (!from_char_set_int(&out->year, years, n, escontext))
return;
out->yysz = 4;
@@ -4797,11 +4803,27 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
if (tmfc.bc)
tmfc.cc = -tmfc.cc;
if (tmfc.cc >= 0)
+ {
/* +1 because 21st century started in 2001 */
- tm->tm_year = (tmfc.cc - 1) * 100 + 1;
+ /* tm->tm_year = (tmfc.cc - 1) * 100 + 1 */
+ if (pg_mul_s32_overflow(tmfc.cc - 1, 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("date out of range: \"%s\"",
+ text_to_cstring(date_txt))));
+ }
else
+ {
/* +1 because year == 599 is 600 BC */
- tm->tm_year = tmfc.cc * 100 + 1;
+ /* tm->tm_year = tmfc.cc * 100 + 1 */
+ if (pg_mul_s32_overflow(tmfc.cc, 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("date out of range: \"%s\"",
+ text_to_cstring(date_txt))));
+ }
fmask |= DTK_M(YEAR);
}
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 5d9c62b652..ee2bbe5dc8 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -717,7 +717,7 @@ init_htab(HTAB *hashp, long nelem)
nbuckets <<= 1;
hctl->max_bucket = hctl->low_mask = nbuckets - 1;
- hctl->high_mask = (nbuckets << 1) - 1;
+ hctl->high_mask = ((uint32) nbuckets << 1) - 1;
/*
* Figure number of directory segments needed, round up to a power of 2
@@ -1819,8 +1819,8 @@ next_pow2_long(long num)
static int
next_pow2_int(long num)
{
- if (num > INT_MAX / 2)
- num = INT_MAX / 2;
+ if (num > INT_MAX / 2 + 1)
+ num = INT_MAX / 2 + 1;
return 1 << my_log2(num);
}
diff --git a/src/include/common/int.h b/src/include/common/int.h
index 3b1590d676..6b50aa67b9 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -117,6 +117,22 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
#endif
}
+static inline bool
+pg_neg_s16_overflow(int16 a, int16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT16_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint16
pg_abs_s16(int16 a)
{
@@ -185,6 +201,22 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
#endif
}
+static inline bool
+pg_neg_s32_overflow(int32 a, int32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT32_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint32
pg_abs_s32(int32 a)
{
@@ -300,6 +332,22 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
#endif
}
+static inline bool
+pg_neg_s64_overflow(int64 a, int64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT64_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint64
pg_abs_s64(int64 a)
{
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index f5949f3d17..c8f76c205d 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1532,3 +1532,5 @@ select make_time(10, 55, 100.1);
ERROR: time field value out of range: 10:55:100.1
select make_time(24, 0, 2.1);
ERROR: time field value out of range: 24:00:2.1
+SELECT make_date(-2147483648, 1, 1);
+ERROR: date field value out of range: -2147483648-01-01
diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out
index 241713cc51..df02d268c0 100644
--- a/src/test/regress/expected/horology.out
+++ b/src/test/regress/expected/horology.out
@@ -3448,6 +3448,8 @@ SELECT i, to_timestamp('2018-11-02 12:34:56.123456', 'YYYY-MM-DD HH24:MI:SS.FF'
SELECT i, to_timestamp('2018-11-02 12:34:56.123456789', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
ERROR: date/time field value out of range: "2018-11-02 12:34:56.123456789"
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
+ERROR: invalid input string for "Y,YYY"
SELECT to_date('1 4 1902', 'Q MM YYYY'); -- Q is ignored
to_date
------------
@@ -3778,6 +3780,8 @@ SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
02-01-0001 BC
(1 row)
+SELECT to_date('100000000', 'CC');
+ERROR: date out of range: "100000000"
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
to_char
diff --git a/src/test/regress/expected/union.out b/src/test/regress/expected/union.out
index 0fd0e1c38b..444744848e 100644
--- a/src/test/regress/expected/union.out
+++ b/src/test/regress/expected/union.out
@@ -59,6 +59,49 @@ SELECT 1.1 AS two UNION SELECT 2.2 ORDER BY 1;
2.2
(2 rows)
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1;
+ ?column?
+----------
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+ 1
+(31 rows)
+
-- Mixed types
SELECT 1.1 AS two UNION SELECT 2 ORDER BY 1;
two
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index 1c58ff6966..9a4e5832b9 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -373,3 +373,4 @@ select make_date(2013, 13, 1);
select make_date(2013, 11, -1);
select make_time(10, 55, 100.1);
select make_time(24, 0, 2.1);
+SELECT make_date(-2147483648, 1, 1);
diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql
index e5cf12ff63..db532ee3c0 100644
--- a/src/test/regress/sql/horology.sql
+++ b/src/test/regress/sql/horology.sql
@@ -558,6 +558,7 @@ SELECT i, to_timestamp('2018-11-02 12:34:56.1234', 'YYYY-MM-DD HH24:MI:SS.FF' ||
SELECT i, to_timestamp('2018-11-02 12:34:56.12345', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
SELECT i, to_timestamp('2018-11-02 12:34:56.123456', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
SELECT i, to_timestamp('2018-11-02 12:34:56.123456789', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
SELECT to_date('1 4 1902', 'Q MM YYYY'); -- Q is ignored
SELECT to_date('3 4 21 01', 'W MM CC YY');
@@ -660,6 +661,7 @@ SELECT to_date('2016 365', 'YYYY DDD'); -- ok
SELECT to_date('2016 366', 'YYYY DDD'); -- ok
SELECT to_date('2016 367', 'YYYY DDD');
SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
+SELECT to_date('100000000', 'CC');
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
diff --git a/src/test/regress/sql/union.sql b/src/test/regress/sql/union.sql
index f8826514e4..e1954a92d2 100644
--- a/src/test/regress/sql/union.sql
+++ b/src/test/regress/sql/union.sql
@@ -20,6 +20,15 @@ SELECT 1 AS three UNION SELECT 2 UNION ALL SELECT 2 ORDER BY 1;
SELECT 1.1 AS two UNION SELECT 2.2 ORDER BY 1;
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1 UNION ALL
+SELECT 1 UNION ALL SELECT 1 UNION ALL SELECT 1;
+
-- Mixed types
SELECT 1.1 AS two UNION SELECT 2 ORDER BY 1;
--
2.39.3 (Apple Git-146)
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Remove dependence on integer wrapping
@ 2024-08-21 07:00 Alexander Lakhin <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Alexander Lakhin @ 2024-08-21 07:00 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Joseph Koshakow <[email protected]>; jian he <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
Hello Nathan,
21.08.2024 00:21, Nathan Bossart wrote:
> I've combined all the current proposed changes into one patch. I've also
> introduced signed versions of the negation functions into int.h to avoid
> relying on multiplication.
>
Thank you for taking care of this!
I'd like to add some info to show how big the iceberg is.
Beside other trap-triggered places in date/time conversion functions, I
also discovered:
1)
CREATE TABLE jt(j jsonb); INSERT INTO jt VALUES('[]'::jsonb);
UPDATE jt SET j[0][-2147483648] = '0';
#4Â 0x00007f15ab00d7f3 in __GI_abort () at ./stdlib/abort.c:79
#5Â 0x00005570113b2591 in __addvsi3 ()
#6Â 0x00005570111d55a0 in push_null_elements (ps=0x7fff37385fb8, num=-2147483648) at jsonfuncs.c:1707
#7Â 0x00005570111d5749 in push_path (st=0x7fff37385fb8, level=0, path_elems=0x55701300c880, path_nulls=0x55701300d520,
   path_len=2, newval=0x7fff37386030) at jsonfuncs.c:1770
The "problematic" code:
       while (num-- > 0)
               *ps = 0;
looks innocent to me, but is not for good enough for -ftrapv.
I think there could be other similar places and this raises two questions:
can they be reached with INT_MIN and what to do if so?
By the way, the same can be seen with CC=clang CPPFLAGS="-ftrapv". Please
look at the code produced by both compilers for x86_64:
https://godbolt.org/z/vjszjf4b3
(clang generates ud1, while gcc uses call __addvsi3)
The aside question is: should jsonb subscripting accept negative indexes
when the target array is not initialized yet?
Compare:
CREATE TABLE jt(j jsonb); INSERT INTO jt VALUES('[]'::jsonb);
UPDATE jt SET j[0][-1] = '0';
SELECT * FROM jt;
  j
-------
 [[0]]
with
CREATE TABLE jt(j jsonb); INSERT INTO jt VALUES('[[]]'::jsonb);
UPDATE jt SET j[0][-1] = '0';
ERROR:Â path element at position 2 is out of range: -1
2)
SELECT x, lag(x, -2147483648) OVER (ORDER BY x) FROM (SELECT 1) x;
#4Â 0x00007fa7d00f47f3 in __GI_abort () at ./stdlib/abort.c:79
#5Â 0x00005623a7336851 in __negvsi2 ()
#6Â 0x00005623a726ae35 in leadlag_common (fcinfo=0x7ffd59cca950, forward=false, withoffset=true, withdefault=false)
   at windowfuncs.c:551
#7Â 0x00005623a726af19 in window_lag_with_offset (fcinfo=0x7ffd59cca950) at windowfuncs.c:594
As to 32-bit Debian, I wrote about before, I use gcc (Debian 12.2.0-14).
Please look at the demo code (and it's assembly, produced with
gcc -S -ftrapv t.c) attached:
gcc -Wall -Wextra -fsanitize=signed-integer-overflow -Wstrict-overflow=5 \
 -O0 -ftrapv t.c -o t && ./t
Aborted (core dumped)
#4Â 0xb762226a in __GI_abort () at ./stdlib/abort.c:79
#5Â 0x00495077 in __mulvdi3.cold ()
#6Â 0x00495347 in pg_mul_s64_overflow ()
(It looks like -Wstrict-overflow can't help with the static analysis
desired in such cases.)
Moreover, I got `make check` failed with -ftrapv on aarch64 (using gcc 8.3)
as follows:
#1Â 0x0000007e1edc48e8 in __GI_abort () at abort.c:79
#2Â 0x0000005ee66b71cc in __subvdi3 ()
#3Â 0x0000005ee6560e24 in int8gcd_internal (arg1=-9223372036854775808, arg2=1) at int8.c:623
#4Â 0x0000005ee62f576c in ExecInterpExpr (state=0x5eeaba9d18, econtext=0x5eeaba95f0, isnull=<optimized out>)
   at execExprInterp.c:770
...
#13 0x0000005ee64e5d84 in exec_simple_query (
   query_string=query_string@entry=0x5eeaac7500 "SELECT a, b, gcd(a, b), gcd(a, -b), gcd(b, a), gcd(-b, a)\nFROM
(VALUES (0::int8, 0::int8),\n", ' ' <repeats 13 times>, "(0::int8, 29893644334::int8),\n", ' ' <repeats 13 times>,
"(288484263558::int8, 29893644334::int8),\n", ' ' <repeats 12 times>...) at postgres.c:1284
So I wonder whether enabling -ftrapv can really help us prepare the code
for -fno-wrapv?
Best regards,
Alexander
.file "t.c"
.text
.globl __mulvdi3
.type pg_mul_s64_overflow, @function
pg_mul_s64_overflow:
.LFB0:
.cfi_startproc
pushl %ebp
.cfi_def_cfa_offset 8
.cfi_offset 5, -8
movl %esp, %ebp
.cfi_def_cfa_register 5
pushl %edi
pushl %esi
pushl %ebx
subl $76, %esp
.cfi_offset 7, -12
.cfi_offset 6, -16
.cfi_offset 3, -20
call __x86.get_pc_thunk.ax
addl $_GLOBAL_OFFSET_TABLE_, %eax
movl %eax, -52(%ebp)
movl 8(%ebp), %eax
movl %eax, -32(%ebp)
movl 12(%ebp), %eax
movl %eax, -28(%ebp)
movl 16(%ebp), %eax
movl %eax, -40(%ebp)
movl 20(%ebp), %eax
movl %eax, -36(%ebp)
movl $0, -64(%ebp)
movl $0, -60(%ebp)
movl -32(%ebp), %eax
movl -28(%ebp), %edx
movl %edx, %eax
movl %eax, %edx
sarl $31, %edx
movl %eax, -48(%ebp)
movl %edx, -44(%ebp)
movl -32(%ebp), %eax
movl %eax, %ecx
sarl $31, %ecx
movl -40(%ebp), %eax
movl -36(%ebp), %edx
movl %eax, %esi
movl %edx, %edi
movl %edi, %esi
movl %esi, %edi
sarl $31, %edi
movl -40(%ebp), %eax
sarl $31, %eax
movl -48(%ebp), %ebx
cmpl %ebx, %ecx
jne .L4
cmpl %esi, %eax
jne .L5
movl -40(%ebp), %eax
imull -32(%ebp)
jmp .L2
.L5:
movl -40(%ebp), %eax
movl -36(%ebp), %edx
movl %eax, -72(%ebp)
movl %edx, -68(%ebp)
movl -32(%ebp), %eax
movl %eax, -48(%ebp)
jmp .L6
.L4:
cmpl %esi, %eax
jne .L7
movl -32(%ebp), %eax
movl -28(%ebp), %edx
movl %eax, -72(%ebp)
movl %edx, -68(%ebp)
movl -48(%ebp), %esi
movl -40(%ebp), %eax
movl %eax, -48(%ebp)
.L6:
movl -40(%ebp), %edi
movl %edi, %eax
mull -32(%ebp)
movl %eax, -80(%ebp)
movl %edx, -76(%ebp)
movl -48(%ebp), %edi
movl %edi, %eax
mull %esi
movl %eax, %ecx
movl %edx, %ebx
testl %esi, %esi
jns .L8
movl %edi, %eax
movl $0, %edx
movl %eax, %edx
movl $0, %eax
movl %eax, %esi
movl %edx, %edi
movl %ecx, %eax
movl %ebx, %edx
subl %esi, %eax
sbbl %edi, %edx
movl %eax, %ecx
movl %edx, %ebx
.L8:
cmpl $0, -48(%ebp)
jns .L9
movl %ecx, %eax
movl %ebx, %edx
subl -72(%ebp), %eax
sbbl -68(%ebp), %edx
movl %eax, %ecx
movl %edx, %ebx
.L9:
movl -80(%ebp), %eax
movl -76(%ebp), %edx
movl %edx, %eax
xorl %edx, %edx
addl %ecx, %eax
adcl %ebx, %edx
movl %eax, %ecx
movl %edx, %ebx
movl %ecx, %eax
movl %ebx, %edx
movl %edx, %eax
movl %eax, %edx
sarl $31, %edx
movl %ecx, %esi
sarl $31, %esi
cmpl %eax, %esi
jne .L10
movl %ecx, %ebx
movl $0, %ecx
movl %ecx, %eax
movl %ebx, %edx
movl -80(%ebp), %ecx
movl $0, %ebx
orl %ecx, %eax
orl %ebx, %edx
jmp .L2
.L7:
pushl -36(%ebp)
pushl -40(%ebp)
pushl -28(%ebp)
pushl -32(%ebp)
movl -52(%ebp), %ebx
call __mulvdi3@PLT
addl $16, %esp
movl -48(%ebp), %ebx
leal 1(%ebx), %ecx
cmpl $1, %ecx
ja .L3
leal 1(%esi), %ecx
cmpl $1, %ecx
ja .L3
cmpl %esi, -48(%ebp)
jne .L11
movl $0, %ebx
movl $0, %ecx
cmpl %eax, %ebx
sbbl %edx, %ecx
jge .L3
jmp .L2
.L11:
testl %edx, %edx
jns .L3
jmp .L2
.L10:
pushl -36(%ebp)
pushl -40(%ebp)
pushl -28(%ebp)
pushl -32(%ebp)
movl -52(%ebp), %ebx
call __mulvdi3@PLT
addl $16, %esp
.L3:
movl $1, -64(%ebp)
movl $0, -60(%ebp)
.L2:
movl 24(%ebp), %ecx
movl %eax, (%ecx)
movl %edx, 4(%ecx)
movl -64(%ebp), %eax
movl -60(%ebp), %edx
andl $1, %eax
leal -12(%ebp), %esp
popl %ebx
.cfi_restore 3
popl %esi
.cfi_restore 6
popl %edi
.cfi_restore 7
popl %ebp
.cfi_restore 5
.cfi_def_cfa 4, 4
ret
.cfi_endproc
.LFE0:
.size pg_mul_s64_overflow, .-pg_mul_s64_overflow
.section .rodata
.LC0:
.string "r: %d, c: %lld\n"
.text
.globl main
.type main, @function
main:
.LFB1:
.cfi_startproc
leal 4(%esp), %ecx
.cfi_def_cfa 1, 0
andl $-16, %esp
pushl -4(%ecx)
pushl %ebp
movl %esp, %ebp
.cfi_escape 0x10,0x5,0x2,0x75,0
pushl %ebx
pushl %ecx
.cfi_escape 0xf,0x3,0x75,0x78,0x6
.cfi_escape 0x10,0x3,0x2,0x75,0x7c
subl $32, %esp
call __x86.get_pc_thunk.bx
addl $_GLOBAL_OFFSET_TABLE_, %ebx
movl $-1, -16(%ebp)
movl $2147483647, -12(%ebp)
movl $2, -24(%ebp)
movl $0, -20(%ebp)
movl $0, -40(%ebp)
movl $0, -36(%ebp)
subl $12, %esp
leal -40(%ebp), %eax
pushl %eax
pushl -20(%ebp)
pushl -24(%ebp)
pushl -12(%ebp)
pushl -16(%ebp)
call pg_mul_s64_overflow
addl $32, %esp
movb %al, -25(%ebp)
movl -40(%ebp), %eax
movl -36(%ebp), %edx
movsbl -25(%ebp), %ecx
pushl %edx
pushl %eax
pushl %ecx
leal .LC0@GOTOFF(%ebx), %eax
pushl %eax
call printf@PLT
addl $16, %esp
movl $0, %eax
leal -8(%ebp), %esp
popl %ecx
.cfi_restore 1
.cfi_def_cfa 1, 0
popl %ebx
.cfi_restore 3
popl %ebp
.cfi_restore 5
leal -4(%ecx), %esp
.cfi_def_cfa 4, 4
ret
.cfi_endproc
.LFE1:
.size main, .-main
.section .text.__x86.get_pc_thunk.ax,"axG",@progbits,__x86.get_pc_thunk.ax,comdat
.globl __x86.get_pc_thunk.ax
.hidden __x86.get_pc_thunk.ax
.type __x86.get_pc_thunk.ax, @function
__x86.get_pc_thunk.ax:
.LFB2:
.cfi_startproc
movl (%esp), %eax
ret
.cfi_endproc
.LFE2:
.section .text.__x86.get_pc_thunk.bx,"axG",@progbits,__x86.get_pc_thunk.bx,comdat
.globl __x86.get_pc_thunk.bx
.hidden __x86.get_pc_thunk.bx
.type __x86.get_pc_thunk.bx, @function
__x86.get_pc_thunk.bx:
.LFB3:
.cfi_startproc
movl (%esp), %ebx
ret
.cfi_endproc
.LFE3:
.ident "GCC: (Debian 12.2.0-14) 12.2.0"
.section .note.GNU-stack,"",@progbits
Attachments:
[text/x-csrc] t.c (349B, ../../[email protected]/2-t.c)
download | inline:
#include <stdio.h>
typedef char bool;
typedef long long int int64;
static inline bool
pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
{
return __builtin_mul_overflow(a, b, result);
}
int main()
{
int64 a = 9223372036854775807L;
int64 b = 2L;
int64 c = 0;
bool r;
r = pg_mul_s64_overflow(a, b, &c);
printf("r: %d, c: %lld\n", r, c);
}
[text/plain] t.x86.s (5.0K, ../../[email protected]/3-t.x86.s)
download | inline:
.file "t.c"
.text
.globl __mulvdi3
.type pg_mul_s64_overflow, @function
pg_mul_s64_overflow:
.LFB0:
.cfi_startproc
pushl %ebp
.cfi_def_cfa_offset 8
.cfi_offset 5, -8
movl %esp, %ebp
.cfi_def_cfa_register 5
pushl %edi
pushl %esi
pushl %ebx
subl $76, %esp
.cfi_offset 7, -12
.cfi_offset 6, -16
.cfi_offset 3, -20
call __x86.get_pc_thunk.ax
addl $_GLOBAL_OFFSET_TABLE_, %eax
movl %eax, -52(%ebp)
movl 8(%ebp), %eax
movl %eax, -32(%ebp)
movl 12(%ebp), %eax
movl %eax, -28(%ebp)
movl 16(%ebp), %eax
movl %eax, -40(%ebp)
movl 20(%ebp), %eax
movl %eax, -36(%ebp)
movl $0, -64(%ebp)
movl $0, -60(%ebp)
movl -32(%ebp), %eax
movl -28(%ebp), %edx
movl %edx, %eax
movl %eax, %edx
sarl $31, %edx
movl %eax, -48(%ebp)
movl %edx, -44(%ebp)
movl -32(%ebp), %eax
movl %eax, %ecx
sarl $31, %ecx
movl -40(%ebp), %eax
movl -36(%ebp), %edx
movl %eax, %esi
movl %edx, %edi
movl %edi, %esi
movl %esi, %edi
sarl $31, %edi
movl -40(%ebp), %eax
sarl $31, %eax
movl -48(%ebp), %ebx
cmpl %ebx, %ecx
jne .L4
cmpl %esi, %eax
jne .L5
movl -40(%ebp), %eax
imull -32(%ebp)
jmp .L2
.L5:
movl -40(%ebp), %eax
movl -36(%ebp), %edx
movl %eax, -72(%ebp)
movl %edx, -68(%ebp)
movl -32(%ebp), %eax
movl %eax, -48(%ebp)
jmp .L6
.L4:
cmpl %esi, %eax
jne .L7
movl -32(%ebp), %eax
movl -28(%ebp), %edx
movl %eax, -72(%ebp)
movl %edx, -68(%ebp)
movl -48(%ebp), %esi
movl -40(%ebp), %eax
movl %eax, -48(%ebp)
.L6:
movl -40(%ebp), %edi
movl %edi, %eax
mull -32(%ebp)
movl %eax, -80(%ebp)
movl %edx, -76(%ebp)
movl -48(%ebp), %edi
movl %edi, %eax
mull %esi
movl %eax, %ecx
movl %edx, %ebx
testl %esi, %esi
jns .L8
movl %edi, %eax
movl $0, %edx
movl %eax, %edx
movl $0, %eax
movl %eax, %esi
movl %edx, %edi
movl %ecx, %eax
movl %ebx, %edx
subl %esi, %eax
sbbl %edi, %edx
movl %eax, %ecx
movl %edx, %ebx
.L8:
cmpl $0, -48(%ebp)
jns .L9
movl %ecx, %eax
movl %ebx, %edx
subl -72(%ebp), %eax
sbbl -68(%ebp), %edx
movl %eax, %ecx
movl %edx, %ebx
.L9:
movl -80(%ebp), %eax
movl -76(%ebp), %edx
movl %edx, %eax
xorl %edx, %edx
addl %ecx, %eax
adcl %ebx, %edx
movl %eax, %ecx
movl %edx, %ebx
movl %ecx, %eax
movl %ebx, %edx
movl %edx, %eax
movl %eax, %edx
sarl $31, %edx
movl %ecx, %esi
sarl $31, %esi
cmpl %eax, %esi
jne .L10
movl %ecx, %ebx
movl $0, %ecx
movl %ecx, %eax
movl %ebx, %edx
movl -80(%ebp), %ecx
movl $0, %ebx
orl %ecx, %eax
orl %ebx, %edx
jmp .L2
.L7:
pushl -36(%ebp)
pushl -40(%ebp)
pushl -28(%ebp)
pushl -32(%ebp)
movl -52(%ebp), %ebx
call __mulvdi3@PLT
addl $16, %esp
movl -48(%ebp), %ebx
leal 1(%ebx), %ecx
cmpl $1, %ecx
ja .L3
leal 1(%esi), %ecx
cmpl $1, %ecx
ja .L3
cmpl %esi, -48(%ebp)
jne .L11
movl $0, %ebx
movl $0, %ecx
cmpl %eax, %ebx
sbbl %edx, %ecx
jge .L3
jmp .L2
.L11:
testl %edx, %edx
jns .L3
jmp .L2
.L10:
pushl -36(%ebp)
pushl -40(%ebp)
pushl -28(%ebp)
pushl -32(%ebp)
movl -52(%ebp), %ebx
call __mulvdi3@PLT
addl $16, %esp
.L3:
movl $1, -64(%ebp)
movl $0, -60(%ebp)
.L2:
movl 24(%ebp), %ecx
movl %eax, (%ecx)
movl %edx, 4(%ecx)
movl -64(%ebp), %eax
movl -60(%ebp), %edx
andl $1, %eax
leal -12(%ebp), %esp
popl %ebx
.cfi_restore 3
popl %esi
.cfi_restore 6
popl %edi
.cfi_restore 7
popl %ebp
.cfi_restore 5
.cfi_def_cfa 4, 4
ret
.cfi_endproc
.LFE0:
.size pg_mul_s64_overflow, .-pg_mul_s64_overflow
.section .rodata
.LC0:
.string "r: %d, c: %lld\n"
.text
.globl main
.type main, @function
main:
.LFB1:
.cfi_startproc
leal 4(%esp), %ecx
.cfi_def_cfa 1, 0
andl $-16, %esp
pushl -4(%ecx)
pushl %ebp
movl %esp, %ebp
.cfi_escape 0x10,0x5,0x2,0x75,0
pushl %ebx
pushl %ecx
.cfi_escape 0xf,0x3,0x75,0x78,0x6
.cfi_escape 0x10,0x3,0x2,0x75,0x7c
subl $32, %esp
call __x86.get_pc_thunk.bx
addl $_GLOBAL_OFFSET_TABLE_, %ebx
movl $-1, -16(%ebp)
movl $2147483647, -12(%ebp)
movl $2, -24(%ebp)
movl $0, -20(%ebp)
movl $0, -40(%ebp)
movl $0, -36(%ebp)
subl $12, %esp
leal -40(%ebp), %eax
pushl %eax
pushl -20(%ebp)
pushl -24(%ebp)
pushl -12(%ebp)
pushl -16(%ebp)
call pg_mul_s64_overflow
addl $32, %esp
movb %al, -25(%ebp)
movl -40(%ebp), %eax
movl -36(%ebp), %edx
movsbl -25(%ebp), %ecx
pushl %edx
pushl %eax
pushl %ecx
leal .LC0@GOTOFF(%ebx), %eax
pushl %eax
call printf@PLT
addl $16, %esp
movl $0, %eax
leal -8(%ebp), %esp
popl %ecx
.cfi_restore 1
.cfi_def_cfa 1, 0
popl %ebx
.cfi_restore 3
popl %ebp
.cfi_restore 5
leal -4(%ecx), %esp
.cfi_def_cfa 4, 4
ret
.cfi_endproc
.LFE1:
.size main, .-main
.section .text.__x86.get_pc_thunk.ax,"axG",@progbits,__x86.get_pc_thunk.ax,comdat
.globl __x86.get_pc_thunk.ax
.hidden __x86.get_pc_thunk.ax
.type __x86.get_pc_thunk.ax, @function
__x86.get_pc_thunk.ax:
.LFB2:
.cfi_startproc
movl (%esp), %eax
ret
.cfi_endproc
.LFE2:
.section .text.__x86.get_pc_thunk.bx,"axG",@progbits,__x86.get_pc_thunk.bx,comdat
.globl __x86.get_pc_thunk.bx
.hidden __x86.get_pc_thunk.bx
.type __x86.get_pc_thunk.bx, @function
__x86.get_pc_thunk.bx:
.LFB3:
.cfi_startproc
movl (%esp), %ebx
ret
.cfi_endproc
.LFE3:
.ident "GCC: (Debian 12.2.0-14) 12.2.0"
.section .note.GNU-stack,"",@progbits
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Remove dependence on integer wrapping
@ 2024-08-21 15:37 Nathan Bossart <[email protected]>
parent: Alexander Lakhin <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Nathan Bossart @ 2024-08-21 15:37 UTC (permalink / raw)
To: Alexander Lakhin <[email protected]>; +Cc: Joseph Koshakow <[email protected]>; jian he <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
On Wed, Aug 21, 2024 at 10:00:00AM +0300, Alexander Lakhin wrote:
> I'd like to add some info to show how big the iceberg is.
Hm. It seems pretty clear that removing -fwrapv won't be happening anytime
soon. I don't mind trying to fix a handful of cases from time to time, but
unless there's a live bug, I'm probably not going to treat this stuff as
high priority.
--
nathan
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Remove dependence on integer wrapping
@ 2024-08-24 12:44 Joseph Koshakow <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Joseph Koshakow @ 2024-08-24 12:44 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Alexander Lakhin <[email protected]>; jian he <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
On Wed, Aug 21, 2024 at 11:37 AM Nathan Bossart <[email protected]>
wrote:
>
> Hm. It seems pretty clear that removing -fwrapv won't be happening
anytime
> soon. I don't mind trying to fix a handful of cases from time to time,
but
> unless there's a live bug, I'm probably not going to treat this stuff as
> high priority.
I think I'm also going to take a step back because I'm a bit
fatigued on the overflow work. My goal here wasn't necessarily to
remove -fwrapv, because I think it will always be a useful safeguard.
Instead I wanted to add -ftrapv to builds with asserts enabled to try
and prevent future overflow based bugs. Though, it looks like that
won't happen anytime soon either.
FWIW, Matthew's patch actually does resolve a bug with `to_timestamp`
and `to_date`. It converts the following incorrect queries
test=# SELECT to_timestamp('2147483647,999', 'Y,YYY');
to_timestamp
---------------------------------
0001-01-01 00:00:00-04:56:02 BC
(1 row)
test=# SELECT to_date('-2147483648', 'CC');
to_date
------------
0001-01-01
(1 row)
into errors
test=# SELECT to_timestamp('2147483647,999', 'Y,YYY');
ERROR: invalid input string for "Y,YYY"
test=# SELECT to_date('-2147483648', 'CC');
ERROR: date out of range: "-2147483648"
So, it might be worth committing only his changes before moving on.
Thanks,
Joseph Koshakow
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Remove dependence on integer wrapping
@ 2024-12-05 22:50 Nathan Bossart <[email protected]>
parent: Joseph Koshakow <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Nathan Bossart @ 2024-12-05 22:50 UTC (permalink / raw)
To: Joseph Koshakow <[email protected]>; +Cc: Alexander Lakhin <[email protected]>; jian he <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
On Sat, Aug 24, 2024 at 08:44:40AM -0400, Joseph Koshakow wrote:
> FWIW, Matthew's patch actually does resolve a bug with `to_timestamp`
> and `to_date`. It converts the following incorrect queries
>
> test=# SELECT to_timestamp('2147483647,999', 'Y,YYY');
> to_timestamp
> ---------------------------------
> 0001-01-01 00:00:00-04:56:02 BC
> (1 row)
>
> test=# SELECT to_date('-2147483648', 'CC');
> to_date
> ------------
> 0001-01-01
> (1 row)
>
> into errors
>
> test=# SELECT to_timestamp('2147483647,999', 'Y,YYY');
> ERROR: invalid input string for "Y,YYY"
> test=# SELECT to_date('-2147483648', 'CC');
> ERROR: date out of range: "-2147483648"
>
> So, it might be worth committing only his changes before moving on.
Good point. Here is a v27 patch that extracts the bug fix portions of the
v26 patch. If/when this is committed, I think we should close the
commitfest entry.
--
nathan
From 13e74f283a0994274e656a7d3e430d360884ae13 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Thu, 5 Dec 2024 16:39:00 -0600
Subject: [PATCH v27 1/1] fix date/time overflows
---
src/backend/utils/adt/date.c | 6 +++-
src/backend/utils/adt/formatting.c | 32 +++++++++++++++--
src/include/common/int.h | 48 ++++++++++++++++++++++++++
src/test/regress/expected/date.out | 2 ++
src/test/regress/expected/horology.out | 6 ++++
src/test/regress/sql/date.sql | 1 +
src/test/regress/sql/horology.sql | 4 +++
7 files changed, 95 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 8130f3e8ac..d5ee96aa6c 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -257,7 +257,11 @@ make_date(PG_FUNCTION_ARGS)
if (tm.tm_year < 0)
{
bc = true;
- tm.tm_year = -tm.tm_year;
+ if (pg_neg_s32_overflow(tm.tm_year, &tm.tm_year))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW),
+ errmsg("date field value out of range: %d-%02d-%02d",
+ tm.tm_year, tm.tm_mon, tm.tm_mday)));
}
dterr = ValidateDate(DTK_DATE_M, false, false, bc, &tm);
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 2bcc185708..9eb7d08169 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -77,6 +77,7 @@
#include "catalog/pg_collation.h"
#include "catalog/pg_type.h"
+#include "common/int.h"
#include "common/unicode_case.h"
#include "common/unicode_category.h"
#include "mb/pg_wchar.h"
@@ -3826,7 +3827,12 @@ DCH_from_char(FormatNode *node, const char *in, TmFromChar *out,
ereturn(escontext,,
(errcode(ERRCODE_INVALID_DATETIME_FORMAT),
errmsg("invalid input string for \"Y,YYY\"")));
- years += (millennia * 1000);
+ if (pg_mul_s32_overflow(millennia, 1000, &millennia) ||
+ pg_add_s32_overflow(years, millennia, &years))
+ ereturn(escontext,,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("value for \"Y,YYY\" in source string is out of range")));
+
if (!from_char_set_int(&out->year, years, n, escontext))
return;
out->yysz = 4;
@@ -4814,11 +4820,31 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
if (tmfc.bc)
tmfc.cc = -tmfc.cc;
if (tmfc.cc >= 0)
+ {
/* +1 because 21st century started in 2001 */
- tm->tm_year = (tmfc.cc - 1) * 100 + 1;
+ /* tm->tm_year = (tmfc.cc - 1) * 100 + 1; */
+ if (pg_mul_s32_overflow(tmfc.cc - 1, 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
else
+ {
/* +1 because year == 599 is 600 BC */
- tm->tm_year = tmfc.cc * 100 + 1;
+ /* tm->tm_year = tmfc.cc * 100 + 1; */
+ if (pg_mul_s32_overflow(tmfc.cc, 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
fmask |= DTK_M(YEAR);
}
diff --git a/src/include/common/int.h b/src/include/common/int.h
index 3b1590d676..6b50aa67b9 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -117,6 +117,22 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
#endif
}
+static inline bool
+pg_neg_s16_overflow(int16 a, int16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT16_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint16
pg_abs_s16(int16 a)
{
@@ -185,6 +201,22 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
#endif
}
+static inline bool
+pg_neg_s32_overflow(int32 a, int32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT32_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint32
pg_abs_s32(int32 a)
{
@@ -300,6 +332,22 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
#endif
}
+static inline bool
+pg_neg_s64_overflow(int64 a, int64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT64_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint64
pg_abs_s64(int64 a)
{
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index c9cec70c38..674fcc2456 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1532,3 +1532,5 @@ select make_time(10, 55, 100.1);
ERROR: time field value out of range: 10:55:100.1
select make_time(24, 0, 2.1);
ERROR: time field value out of range: 24:00:2.1
+SELECT make_date(-2147483648, 1, 1);
+ERROR: date field value out of range: -2147483648-01-01
diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out
index 6d7dd5c988..5f1b9ab075 100644
--- a/src/test/regress/expected/horology.out
+++ b/src/test/regress/expected/horology.out
@@ -3453,6 +3453,8 @@ SELECT i, to_timestamp('2018-11-02 12:34:56.123456', 'YYYY-MM-DD HH24:MI:SS.FF'
SELECT i, to_timestamp('2018-11-02 12:34:56.123456789', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
ERROR: date/time field value out of range: "2018-11-02 12:34:56.123456789"
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
+ERROR: value for "Y,YYY" in source string is out of range
SELECT to_date('1 4 1902', 'Q MM YYYY'); -- Q is ignored
to_date
------------
@@ -3783,6 +3785,10 @@ SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
02-01-0001 BC
(1 row)
+SELECT to_date('100000000', 'CC');
+ERROR: date/time field value out of range: "100000000"
+SELECT to_date('-100000000', 'CC');
+ERROR: date/time field value out of range: "-100000000"
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
to_char
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index 1c58ff6966..9a4e5832b9 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -373,3 +373,4 @@ select make_date(2013, 13, 1);
select make_date(2013, 11, -1);
select make_time(10, 55, 100.1);
select make_time(24, 0, 2.1);
+SELECT make_date(-2147483648, 1, 1);
diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql
index 0fe3c783e6..553cf83b15 100644
--- a/src/test/regress/sql/horology.sql
+++ b/src/test/regress/sql/horology.sql
@@ -559,6 +559,8 @@ SELECT i, to_timestamp('2018-11-02 12:34:56.12345', 'YYYY-MM-DD HH24:MI:SS.FF' |
SELECT i, to_timestamp('2018-11-02 12:34:56.123456', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
SELECT i, to_timestamp('2018-11-02 12:34:56.123456789', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
+
SELECT to_date('1 4 1902', 'Q MM YYYY'); -- Q is ignored
SELECT to_date('3 4 21 01', 'W MM CC YY');
SELECT to_date('2458872', 'J');
@@ -660,6 +662,8 @@ SELECT to_date('2016 365', 'YYYY DDD'); -- ok
SELECT to_date('2016 366', 'YYYY DDD'); -- ok
SELECT to_date('2016 367', 'YYYY DDD');
SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
+SELECT to_date('100000000', 'CC');
+SELECT to_date('-100000000', 'CC');
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
--
2.39.5 (Apple Git-154)
Attachments:
[text/plain] v27-0001-fix-date-time-overflows.patch (7.9K, ../../Z1IuOvRk7xQIBGSI@nathan/2-v27-0001-fix-date-time-overflows.patch)
download | inline diff:
From 13e74f283a0994274e656a7d3e430d360884ae13 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Thu, 5 Dec 2024 16:39:00 -0600
Subject: [PATCH v27 1/1] fix date/time overflows
---
src/backend/utils/adt/date.c | 6 +++-
src/backend/utils/adt/formatting.c | 32 +++++++++++++++--
src/include/common/int.h | 48 ++++++++++++++++++++++++++
src/test/regress/expected/date.out | 2 ++
src/test/regress/expected/horology.out | 6 ++++
src/test/regress/sql/date.sql | 1 +
src/test/regress/sql/horology.sql | 4 +++
7 files changed, 95 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 8130f3e8ac..d5ee96aa6c 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -257,7 +257,11 @@ make_date(PG_FUNCTION_ARGS)
if (tm.tm_year < 0)
{
bc = true;
- tm.tm_year = -tm.tm_year;
+ if (pg_neg_s32_overflow(tm.tm_year, &tm.tm_year))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW),
+ errmsg("date field value out of range: %d-%02d-%02d",
+ tm.tm_year, tm.tm_mon, tm.tm_mday)));
}
dterr = ValidateDate(DTK_DATE_M, false, false, bc, &tm);
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 2bcc185708..9eb7d08169 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -77,6 +77,7 @@
#include "catalog/pg_collation.h"
#include "catalog/pg_type.h"
+#include "common/int.h"
#include "common/unicode_case.h"
#include "common/unicode_category.h"
#include "mb/pg_wchar.h"
@@ -3826,7 +3827,12 @@ DCH_from_char(FormatNode *node, const char *in, TmFromChar *out,
ereturn(escontext,,
(errcode(ERRCODE_INVALID_DATETIME_FORMAT),
errmsg("invalid input string for \"Y,YYY\"")));
- years += (millennia * 1000);
+ if (pg_mul_s32_overflow(millennia, 1000, &millennia) ||
+ pg_add_s32_overflow(years, millennia, &years))
+ ereturn(escontext,,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("value for \"Y,YYY\" in source string is out of range")));
+
if (!from_char_set_int(&out->year, years, n, escontext))
return;
out->yysz = 4;
@@ -4814,11 +4820,31 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
if (tmfc.bc)
tmfc.cc = -tmfc.cc;
if (tmfc.cc >= 0)
+ {
/* +1 because 21st century started in 2001 */
- tm->tm_year = (tmfc.cc - 1) * 100 + 1;
+ /* tm->tm_year = (tmfc.cc - 1) * 100 + 1; */
+ if (pg_mul_s32_overflow(tmfc.cc - 1, 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
else
+ {
/* +1 because year == 599 is 600 BC */
- tm->tm_year = tmfc.cc * 100 + 1;
+ /* tm->tm_year = tmfc.cc * 100 + 1; */
+ if (pg_mul_s32_overflow(tmfc.cc, 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
fmask |= DTK_M(YEAR);
}
diff --git a/src/include/common/int.h b/src/include/common/int.h
index 3b1590d676..6b50aa67b9 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -117,6 +117,22 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
#endif
}
+static inline bool
+pg_neg_s16_overflow(int16 a, int16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT16_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint16
pg_abs_s16(int16 a)
{
@@ -185,6 +201,22 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
#endif
}
+static inline bool
+pg_neg_s32_overflow(int32 a, int32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT32_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint32
pg_abs_s32(int32 a)
{
@@ -300,6 +332,22 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
#endif
}
+static inline bool
+pg_neg_s64_overflow(int64 a, int64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT64_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint64
pg_abs_s64(int64 a)
{
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index c9cec70c38..674fcc2456 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1532,3 +1532,5 @@ select make_time(10, 55, 100.1);
ERROR: time field value out of range: 10:55:100.1
select make_time(24, 0, 2.1);
ERROR: time field value out of range: 24:00:2.1
+SELECT make_date(-2147483648, 1, 1);
+ERROR: date field value out of range: -2147483648-01-01
diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out
index 6d7dd5c988..5f1b9ab075 100644
--- a/src/test/regress/expected/horology.out
+++ b/src/test/regress/expected/horology.out
@@ -3453,6 +3453,8 @@ SELECT i, to_timestamp('2018-11-02 12:34:56.123456', 'YYYY-MM-DD HH24:MI:SS.FF'
SELECT i, to_timestamp('2018-11-02 12:34:56.123456789', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
ERROR: date/time field value out of range: "2018-11-02 12:34:56.123456789"
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
+ERROR: value for "Y,YYY" in source string is out of range
SELECT to_date('1 4 1902', 'Q MM YYYY'); -- Q is ignored
to_date
------------
@@ -3783,6 +3785,10 @@ SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
02-01-0001 BC
(1 row)
+SELECT to_date('100000000', 'CC');
+ERROR: date/time field value out of range: "100000000"
+SELECT to_date('-100000000', 'CC');
+ERROR: date/time field value out of range: "-100000000"
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
to_char
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index 1c58ff6966..9a4e5832b9 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -373,3 +373,4 @@ select make_date(2013, 13, 1);
select make_date(2013, 11, -1);
select make_time(10, 55, 100.1);
select make_time(24, 0, 2.1);
+SELECT make_date(-2147483648, 1, 1);
diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql
index 0fe3c783e6..553cf83b15 100644
--- a/src/test/regress/sql/horology.sql
+++ b/src/test/regress/sql/horology.sql
@@ -559,6 +559,8 @@ SELECT i, to_timestamp('2018-11-02 12:34:56.12345', 'YYYY-MM-DD HH24:MI:SS.FF' |
SELECT i, to_timestamp('2018-11-02 12:34:56.123456', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
SELECT i, to_timestamp('2018-11-02 12:34:56.123456789', 'YYYY-MM-DD HH24:MI:SS.FF' || i) FROM generate_series(1, 6) i;
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
+
SELECT to_date('1 4 1902', 'Q MM YYYY'); -- Q is ignored
SELECT to_date('3 4 21 01', 'W MM CC YY');
SELECT to_date('2458872', 'J');
@@ -660,6 +662,8 @@ SELECT to_date('2016 365', 'YYYY DDD'); -- ok
SELECT to_date('2016 366', 'YYYY DDD'); -- ok
SELECT to_date('2016 367', 'YYYY DDD');
SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
+SELECT to_date('100000000', 'CC');
+SELECT to_date('-100000000', 'CC');
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Remove dependence on integer wrapping
@ 2024-12-06 01:00 Joseph Koshakow <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Joseph Koshakow @ 2024-12-06 01:00 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Alexander Lakhin <[email protected]>; jian he <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
On Thu, Dec 5, 2024 at 5:50 PM Nathan Bossart <[email protected]> wrote:
> Good point. Here is a v27 patch that extracts the bug fix portions of the
> v26 patch. If/when this is committed, I think we should close the
> commitfest entry.
I looked through this patch and it looks good to me, assuming cfbot is
happy. Also I agree, we should close the commitfest entry.
Thanks,
Joseph Koshakow
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Remove dependence on integer wrapping
@ 2024-12-06 03:58 Nathan Bossart <[email protected]>
parent: Joseph Koshakow <[email protected]>
0 siblings, 1 reply; 19+ messages in thread
From: Nathan Bossart @ 2024-12-06 03:58 UTC (permalink / raw)
To: Joseph Koshakow <[email protected]>; +Cc: Alexander Lakhin <[email protected]>; jian he <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
On Thu, Dec 05, 2024 at 08:00:12PM -0500, Joseph Koshakow wrote:
> On Thu, Dec 5, 2024 at 5:50 PM Nathan Bossart <[email protected]> wrote:
>> Good point. Here is a v27 patch that extracts the bug fix portions of the
>> v26 patch. If/when this is committed, I think we should close the
>> commitfest entry.
>
> I looked through this patch and it looks good to me, assuming cfbot is
> happy. Also I agree, we should close the commitfest entry.
Thanks for reviewing. In v28, I fixed a silly mistake revealed by cfbot's
Windows run. I also went ahead and tried to fix most of the issues
reported in a nearby thread [0]. The only one I haven't tracked down yet
is the "ISO week" one (case 7).
[0] https://postgr.es/m/18585-db646741dd649abd%40postgresql.org
--
nathan
From c96de41b95f8380ba7ff15c9baef36713be6a43c Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Thu, 5 Dec 2024 21:50:50 -0600
Subject: [PATCH v28 1/1] Fix various overflow hazards in date and timestamp
functions.
Reported-by: Alexander Lakhin
Author: Matthew Kim, Nathan Bossart
Reviewed-by: Joseph Koshakow, Jian He
Discussion: https://postgr.es/m/31ad2cd1-db94-bdb3-f91a-65ffdb4bef95%40gmail.com
Discussion: https://postgr.es/m/18585-db646741dd649abd%40postgresql.org
---
src/backend/utils/adt/date.c | 9 ++-
src/backend/utils/adt/formatting.c | 104 +++++++++++++++++++++++--
src/include/common/int.h | 48 ++++++++++++
src/test/regress/expected/date.out | 2 +
src/test/regress/expected/horology.out | 16 ++++
src/test/regress/sql/date.sql | 1 +
src/test/regress/sql/horology.sql | 8 ++
7 files changed, 179 insertions(+), 9 deletions(-)
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 8130f3e8ac..da61ac0e86 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -256,8 +256,15 @@ make_date(PG_FUNCTION_ARGS)
/* Handle negative years as BC */
if (tm.tm_year < 0)
{
+ int year = tm.tm_year;
+
bc = true;
- tm.tm_year = -tm.tm_year;
+ if (pg_neg_s32_overflow(year, &year))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW),
+ errmsg("date field value out of range: %d-%02d-%02d",
+ tm.tm_year, tm.tm_mon, tm.tm_mday)));
+ tm.tm_year = year;
}
dterr = ValidateDate(DTK_DATE_M, false, false, bc, &tm);
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 2bcc185708..a9daa5c591 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -77,6 +77,7 @@
#include "catalog/pg_collation.h"
#include "catalog/pg_type.h"
+#include "common/int.h"
#include "common/unicode_case.h"
#include "common/unicode_category.h"
#include "mb/pg_wchar.h"
@@ -3826,7 +3827,14 @@ DCH_from_char(FormatNode *node, const char *in, TmFromChar *out,
ereturn(escontext,,
(errcode(ERRCODE_INVALID_DATETIME_FORMAT),
errmsg("invalid input string for \"Y,YYY\"")));
- years += (millennia * 1000);
+
+ /* years += (millennia * 1000); */
+ if (pg_mul_s32_overflow(millennia, 1000, &millennia) ||
+ pg_add_s32_overflow(years, millennia, &years))
+ ereturn(escontext,,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("value for \"Y,YYY\" in source string is out of range")));
+
if (!from_char_set_int(&out->year, years, n, escontext))
return;
out->yysz = 4;
@@ -4785,10 +4793,35 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
tm->tm_year = tmfc.year % 100;
if (tm->tm_year)
{
+ int tmp;
+
if (tmfc.cc >= 0)
- tm->tm_year += (tmfc.cc - 1) * 100;
+ {
+ /* tm->tm_year += (tmfc.cc - 1) * 100; */
+ tmp = tmfc.cc - 1;
+ if (pg_mul_s32_overflow(tmp, 100, &tmp) ||
+ pg_add_s32_overflow(tm->tm_year, tmp, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
else
- tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1;
+ {
+ /* tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1; */
+ tmp = tmfc.cc + 1;
+ if (pg_mul_s32_overflow(tmp, 100, &tmp) ||
+ pg_sub_s32_overflow(tmp, tm->tm_year, &tmp) ||
+ pg_add_s32_overflow(tmp, 1, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
}
else
{
@@ -4814,11 +4847,31 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
if (tmfc.bc)
tmfc.cc = -tmfc.cc;
if (tmfc.cc >= 0)
+ {
/* +1 because 21st century started in 2001 */
- tm->tm_year = (tmfc.cc - 1) * 100 + 1;
+ /* tm->tm_year = (tmfc.cc - 1) * 100 + 1; */
+ if (pg_mul_s32_overflow(tmfc.cc - 1, 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
else
+ {
/* +1 because year == 599 is 600 BC */
- tm->tm_year = tmfc.cc * 100 + 1;
+ /* tm->tm_year = tmfc.cc * 100 + 1; */
+ if (pg_mul_s32_overflow(tmfc.cc, 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
fmask |= DTK_M(YEAR);
}
@@ -4843,11 +4896,35 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
fmask |= DTK_DATE_M;
}
else
- tmfc.ddd = (tmfc.ww - 1) * 7 + 1;
+ {
+ int tmp = 0;
+
+ /* tmfc.ddd = (tmfc.ww - 1) * 7 + 1; */
+ if (pg_sub_s32_overflow(tmfc.ww, 1, &tmp) ||
+ pg_mul_s32_overflow(tmp, 7, &tmp) ||
+ pg_add_s32_overflow(tmp, 1, &tmfc.ddd))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ date_str, "timestamp", escontext);
+ goto fail;
+ }
+ }
}
if (tmfc.w)
- tmfc.dd = (tmfc.w - 1) * 7 + 1;
+ {
+ int tmp = 0;
+
+ /* tmfc.dd = (tmfc.w - 1) * 7 + 1; */
+ if (pg_sub_s32_overflow(tmfc.w, 1, &tmp) ||
+ pg_mul_s32_overflow(tmp, 7, &tmp) ||
+ pg_add_s32_overflow(tmp, 1, &tmfc.dd))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ date_str, "timestamp", escontext);
+ goto fail;
+ }
+ }
if (tmfc.dd)
{
tm->tm_mday = tmfc.dd;
@@ -4912,7 +4989,18 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
}
if (tmfc.ms)
- *fsec += tmfc.ms * 1000;
+ {
+ int tmp = 0;
+
+ /* *fsec += tmfc.ms * 1000; */
+ if (pg_mul_s32_overflow(tmfc.ms, 1000, &tmp) ||
+ pg_add_s32_overflow(*fsec, tmp, fsec))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ date_str, "timestamp", escontext);
+ goto fail;
+ }
+ }
if (tmfc.us)
*fsec += tmfc.us;
if (fprec)
diff --git a/src/include/common/int.h b/src/include/common/int.h
index 3b1590d676..6b50aa67b9 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -117,6 +117,22 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
#endif
}
+static inline bool
+pg_neg_s16_overflow(int16 a, int16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT16_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint16
pg_abs_s16(int16 a)
{
@@ -185,6 +201,22 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
#endif
}
+static inline bool
+pg_neg_s32_overflow(int32 a, int32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT32_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint32
pg_abs_s32(int32 a)
{
@@ -300,6 +332,22 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
#endif
}
+static inline bool
+pg_neg_s64_overflow(int64 a, int64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT64_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint64
pg_abs_s64(int64 a)
{
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index c9cec70c38..dcab9e76f4 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1528,6 +1528,8 @@ select make_date(2013, 13, 1);
ERROR: date field value out of range: 2013-13-01
select make_date(2013, 11, -1);
ERROR: date field value out of range: 2013-11--1
+SELECT make_date(-2147483648, 1, 1);
+ERROR: date field value out of range: -2147483648-01-01
select make_time(10, 55, 100.1);
ERROR: time field value out of range: 10:55:100.1
select make_time(24, 0, 2.1);
diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out
index 6d7dd5c988..b87b88f038 100644
--- a/src/test/regress/expected/horology.out
+++ b/src/test/regress/expected/horology.out
@@ -3743,6 +3743,14 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok
SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS');
ERROR: date/time field value out of range: "2015-02-11 86400"
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
+ERROR: value for "Y,YYY" in source string is out of range
+SELECT to_timestamp('0.-2147483648', 'SS.MS');
+ERROR: date/time field value out of range: "0.-2147483648"
+SELECT to_timestamp('613566758', 'W');
+ERROR: date/time field value out of range: "613566758"
+SELECT to_timestamp('2024 613566758 1', 'YYYY WW D');
+ERROR: date/time field value out of range: "2024 613566758 1"
SELECT to_date('2016-13-10', 'YYYY-MM-DD');
ERROR: date/time field value out of range: "2016-13-10"
SELECT to_date('2016-02-30', 'YYYY-MM-DD');
@@ -3783,6 +3791,14 @@ SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
02-01-0001 BC
(1 row)
+SELECT to_date('100000000', 'CC');
+ERROR: date/time field value out of range: "100000000"
+SELECT to_date('-100000000', 'CC');
+ERROR: date/time field value out of range: "-100000000"
+SELECT to_date('-2147483648 01', 'CC YY');
+ERROR: date/time field value out of range: "-2147483648 01"
+SELECT to_date('2147483647 01', 'CC YY');
+ERROR: date/time field value out of range: "2147483647 01"
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
to_char
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index 1c58ff6966..805aec706c 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -371,5 +371,6 @@ select make_date(0, 7, 15);
select make_date(2013, 2, 30);
select make_date(2013, 13, 1);
select make_date(2013, 11, -1);
+SELECT make_date(-2147483648, 1, 1);
select make_time(10, 55, 100.1);
select make_time(24, 0, 2.1);
diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql
index 0fe3c783e6..808083a6d8 100644
--- a/src/test/regress/sql/horology.sql
+++ b/src/test/regress/sql/horology.sql
@@ -650,6 +650,10 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSS'); -- ok
SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSS');
SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok
SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS');
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
+SELECT to_timestamp('0.-2147483648', 'SS.MS');
+SELECT to_timestamp('613566758', 'W');
+SELECT to_timestamp('2024 613566758 1', 'YYYY WW D');
SELECT to_date('2016-13-10', 'YYYY-MM-DD');
SELECT to_date('2016-02-30', 'YYYY-MM-DD');
SELECT to_date('2016-02-29', 'YYYY-MM-DD'); -- ok
@@ -660,6 +664,10 @@ SELECT to_date('2016 365', 'YYYY DDD'); -- ok
SELECT to_date('2016 366', 'YYYY DDD'); -- ok
SELECT to_date('2016 367', 'YYYY DDD');
SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
+SELECT to_date('100000000', 'CC');
+SELECT to_date('-100000000', 'CC');
+SELECT to_date('-2147483648 01', 'CC YY');
+SELECT to_date('2147483647 01', 'CC YY');
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
--
2.39.5 (Apple Git-154)
Attachments:
[text/plain] v28-0001-Fix-various-overflow-hazards-in-date-and-timesta.patch (11.6K, ../../Z1J2ZVuiY48Ps9nE@nathan/2-v28-0001-Fix-various-overflow-hazards-in-date-and-timesta.patch)
download | inline diff:
From c96de41b95f8380ba7ff15c9baef36713be6a43c Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Thu, 5 Dec 2024 21:50:50 -0600
Subject: [PATCH v28 1/1] Fix various overflow hazards in date and timestamp
functions.
Reported-by: Alexander Lakhin
Author: Matthew Kim, Nathan Bossart
Reviewed-by: Joseph Koshakow, Jian He
Discussion: https://postgr.es/m/31ad2cd1-db94-bdb3-f91a-65ffdb4bef95%40gmail.com
Discussion: https://postgr.es/m/18585-db646741dd649abd%40postgresql.org
---
src/backend/utils/adt/date.c | 9 ++-
src/backend/utils/adt/formatting.c | 104 +++++++++++++++++++++++--
src/include/common/int.h | 48 ++++++++++++
src/test/regress/expected/date.out | 2 +
src/test/regress/expected/horology.out | 16 ++++
src/test/regress/sql/date.sql | 1 +
src/test/regress/sql/horology.sql | 8 ++
7 files changed, 179 insertions(+), 9 deletions(-)
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 8130f3e8ac..da61ac0e86 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -256,8 +256,15 @@ make_date(PG_FUNCTION_ARGS)
/* Handle negative years as BC */
if (tm.tm_year < 0)
{
+ int year = tm.tm_year;
+
bc = true;
- tm.tm_year = -tm.tm_year;
+ if (pg_neg_s32_overflow(year, &year))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW),
+ errmsg("date field value out of range: %d-%02d-%02d",
+ tm.tm_year, tm.tm_mon, tm.tm_mday)));
+ tm.tm_year = year;
}
dterr = ValidateDate(DTK_DATE_M, false, false, bc, &tm);
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 2bcc185708..a9daa5c591 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -77,6 +77,7 @@
#include "catalog/pg_collation.h"
#include "catalog/pg_type.h"
+#include "common/int.h"
#include "common/unicode_case.h"
#include "common/unicode_category.h"
#include "mb/pg_wchar.h"
@@ -3826,7 +3827,14 @@ DCH_from_char(FormatNode *node, const char *in, TmFromChar *out,
ereturn(escontext,,
(errcode(ERRCODE_INVALID_DATETIME_FORMAT),
errmsg("invalid input string for \"Y,YYY\"")));
- years += (millennia * 1000);
+
+ /* years += (millennia * 1000); */
+ if (pg_mul_s32_overflow(millennia, 1000, &millennia) ||
+ pg_add_s32_overflow(years, millennia, &years))
+ ereturn(escontext,,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("value for \"Y,YYY\" in source string is out of range")));
+
if (!from_char_set_int(&out->year, years, n, escontext))
return;
out->yysz = 4;
@@ -4785,10 +4793,35 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
tm->tm_year = tmfc.year % 100;
if (tm->tm_year)
{
+ int tmp;
+
if (tmfc.cc >= 0)
- tm->tm_year += (tmfc.cc - 1) * 100;
+ {
+ /* tm->tm_year += (tmfc.cc - 1) * 100; */
+ tmp = tmfc.cc - 1;
+ if (pg_mul_s32_overflow(tmp, 100, &tmp) ||
+ pg_add_s32_overflow(tm->tm_year, tmp, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
else
- tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1;
+ {
+ /* tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1; */
+ tmp = tmfc.cc + 1;
+ if (pg_mul_s32_overflow(tmp, 100, &tmp) ||
+ pg_sub_s32_overflow(tmp, tm->tm_year, &tmp) ||
+ pg_add_s32_overflow(tmp, 1, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
}
else
{
@@ -4814,11 +4847,31 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
if (tmfc.bc)
tmfc.cc = -tmfc.cc;
if (tmfc.cc >= 0)
+ {
/* +1 because 21st century started in 2001 */
- tm->tm_year = (tmfc.cc - 1) * 100 + 1;
+ /* tm->tm_year = (tmfc.cc - 1) * 100 + 1; */
+ if (pg_mul_s32_overflow(tmfc.cc - 1, 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
else
+ {
/* +1 because year == 599 is 600 BC */
- tm->tm_year = tmfc.cc * 100 + 1;
+ /* tm->tm_year = tmfc.cc * 100 + 1; */
+ if (pg_mul_s32_overflow(tmfc.cc, 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
fmask |= DTK_M(YEAR);
}
@@ -4843,11 +4896,35 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
fmask |= DTK_DATE_M;
}
else
- tmfc.ddd = (tmfc.ww - 1) * 7 + 1;
+ {
+ int tmp = 0;
+
+ /* tmfc.ddd = (tmfc.ww - 1) * 7 + 1; */
+ if (pg_sub_s32_overflow(tmfc.ww, 1, &tmp) ||
+ pg_mul_s32_overflow(tmp, 7, &tmp) ||
+ pg_add_s32_overflow(tmp, 1, &tmfc.ddd))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ date_str, "timestamp", escontext);
+ goto fail;
+ }
+ }
}
if (tmfc.w)
- tmfc.dd = (tmfc.w - 1) * 7 + 1;
+ {
+ int tmp = 0;
+
+ /* tmfc.dd = (tmfc.w - 1) * 7 + 1; */
+ if (pg_sub_s32_overflow(tmfc.w, 1, &tmp) ||
+ pg_mul_s32_overflow(tmp, 7, &tmp) ||
+ pg_add_s32_overflow(tmp, 1, &tmfc.dd))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ date_str, "timestamp", escontext);
+ goto fail;
+ }
+ }
if (tmfc.dd)
{
tm->tm_mday = tmfc.dd;
@@ -4912,7 +4989,18 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
}
if (tmfc.ms)
- *fsec += tmfc.ms * 1000;
+ {
+ int tmp = 0;
+
+ /* *fsec += tmfc.ms * 1000; */
+ if (pg_mul_s32_overflow(tmfc.ms, 1000, &tmp) ||
+ pg_add_s32_overflow(*fsec, tmp, fsec))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ date_str, "timestamp", escontext);
+ goto fail;
+ }
+ }
if (tmfc.us)
*fsec += tmfc.us;
if (fprec)
diff --git a/src/include/common/int.h b/src/include/common/int.h
index 3b1590d676..6b50aa67b9 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -117,6 +117,22 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
#endif
}
+static inline bool
+pg_neg_s16_overflow(int16 a, int16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT16_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint16
pg_abs_s16(int16 a)
{
@@ -185,6 +201,22 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
#endif
}
+static inline bool
+pg_neg_s32_overflow(int32 a, int32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT32_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint32
pg_abs_s32(int32 a)
{
@@ -300,6 +332,22 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
#endif
}
+static inline bool
+pg_neg_s64_overflow(int64 a, int64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT64_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint64
pg_abs_s64(int64 a)
{
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index c9cec70c38..dcab9e76f4 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1528,6 +1528,8 @@ select make_date(2013, 13, 1);
ERROR: date field value out of range: 2013-13-01
select make_date(2013, 11, -1);
ERROR: date field value out of range: 2013-11--1
+SELECT make_date(-2147483648, 1, 1);
+ERROR: date field value out of range: -2147483648-01-01
select make_time(10, 55, 100.1);
ERROR: time field value out of range: 10:55:100.1
select make_time(24, 0, 2.1);
diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out
index 6d7dd5c988..b87b88f038 100644
--- a/src/test/regress/expected/horology.out
+++ b/src/test/regress/expected/horology.out
@@ -3743,6 +3743,14 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok
SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS');
ERROR: date/time field value out of range: "2015-02-11 86400"
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
+ERROR: value for "Y,YYY" in source string is out of range
+SELECT to_timestamp('0.-2147483648', 'SS.MS');
+ERROR: date/time field value out of range: "0.-2147483648"
+SELECT to_timestamp('613566758', 'W');
+ERROR: date/time field value out of range: "613566758"
+SELECT to_timestamp('2024 613566758 1', 'YYYY WW D');
+ERROR: date/time field value out of range: "2024 613566758 1"
SELECT to_date('2016-13-10', 'YYYY-MM-DD');
ERROR: date/time field value out of range: "2016-13-10"
SELECT to_date('2016-02-30', 'YYYY-MM-DD');
@@ -3783,6 +3791,14 @@ SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
02-01-0001 BC
(1 row)
+SELECT to_date('100000000', 'CC');
+ERROR: date/time field value out of range: "100000000"
+SELECT to_date('-100000000', 'CC');
+ERROR: date/time field value out of range: "-100000000"
+SELECT to_date('-2147483648 01', 'CC YY');
+ERROR: date/time field value out of range: "-2147483648 01"
+SELECT to_date('2147483647 01', 'CC YY');
+ERROR: date/time field value out of range: "2147483647 01"
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
to_char
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index 1c58ff6966..805aec706c 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -371,5 +371,6 @@ select make_date(0, 7, 15);
select make_date(2013, 2, 30);
select make_date(2013, 13, 1);
select make_date(2013, 11, -1);
+SELECT make_date(-2147483648, 1, 1);
select make_time(10, 55, 100.1);
select make_time(24, 0, 2.1);
diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql
index 0fe3c783e6..808083a6d8 100644
--- a/src/test/regress/sql/horology.sql
+++ b/src/test/regress/sql/horology.sql
@@ -650,6 +650,10 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSS'); -- ok
SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSS');
SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok
SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS');
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
+SELECT to_timestamp('0.-2147483648', 'SS.MS');
+SELECT to_timestamp('613566758', 'W');
+SELECT to_timestamp('2024 613566758 1', 'YYYY WW D');
SELECT to_date('2016-13-10', 'YYYY-MM-DD');
SELECT to_date('2016-02-30', 'YYYY-MM-DD');
SELECT to_date('2016-02-29', 'YYYY-MM-DD'); -- ok
@@ -660,6 +664,10 @@ SELECT to_date('2016 365', 'YYYY DDD'); -- ok
SELECT to_date('2016 366', 'YYYY DDD'); -- ok
SELECT to_date('2016 367', 'YYYY DDD');
SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
+SELECT to_date('100000000', 'CC');
+SELECT to_date('-100000000', 'CC');
+SELECT to_date('-2147483648 01', 'CC YY');
+SELECT to_date('2147483647 01', 'CC YY');
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 19+ messages in thread
* Re: Remove dependence on integer wrapping
@ 2024-12-06 16:22 Nathan Bossart <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 0 replies; 19+ messages in thread
From: Nathan Bossart @ 2024-12-06 16:22 UTC (permalink / raw)
To: Joseph Koshakow <[email protected]>; +Cc: Alexander Lakhin <[email protected]>; jian he <[email protected]>; Heikki Linnakangas <[email protected]>; Matthew Kim <[email protected]>; Tom Lane <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
On Thu, Dec 05, 2024 at 09:58:29PM -0600, Nathan Bossart wrote:
> Thanks for reviewing. In v28, I fixed a silly mistake revealed by cfbot's
> Windows run. I also went ahead and tried to fix most of the issues
> reported in a nearby thread [0]. The only one I haven't tracked down yet
> is the "ISO week" one (case 7).
The overflow hazard for the ISO week case seems to be in isoweek2j(), which
would require quite a bit of restructuring to work with the soft-error
handling from its callers. I'm not feeling particularly inspired to do
that, so here's a v29 with a comment added above that function.
--
nathan
From 99032579840c75c15acb88e6d9fb9a8a76a39dcd Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Fri, 6 Dec 2024 10:16:51 -0600
Subject: [PATCH v29 1/1] Fix various overflow hazards in date and timestamp
functions.
Reported-by: Alexander Lakhin
Author: Matthew Kim, Nathan Bossart
Reviewed-by: Joseph Koshakow, Jian He
Discussion: https://postgr.es/m/31ad2cd1-db94-bdb3-f91a-65ffdb4bef95%40gmail.com
Discussion: https://postgr.es/m/18585-db646741dd649abd%40postgresql.org
Backpatch-through: 13
---
src/backend/utils/adt/date.c | 9 ++-
src/backend/utils/adt/formatting.c | 104 +++++++++++++++++++++++--
src/backend/utils/adt/timestamp.c | 4 +
src/include/common/int.h | 48 ++++++++++++
src/test/regress/expected/date.out | 2 +
src/test/regress/expected/horology.out | 16 ++++
src/test/regress/sql/date.sql | 1 +
src/test/regress/sql/horology.sql | 8 ++
8 files changed, 183 insertions(+), 9 deletions(-)
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 8130f3e8ac..da61ac0e86 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -256,8 +256,15 @@ make_date(PG_FUNCTION_ARGS)
/* Handle negative years as BC */
if (tm.tm_year < 0)
{
+ int year = tm.tm_year;
+
bc = true;
- tm.tm_year = -tm.tm_year;
+ if (pg_neg_s32_overflow(year, &year))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW),
+ errmsg("date field value out of range: %d-%02d-%02d",
+ tm.tm_year, tm.tm_mon, tm.tm_mday)));
+ tm.tm_year = year;
}
dterr = ValidateDate(DTK_DATE_M, false, false, bc, &tm);
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 2bcc185708..a9daa5c591 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -77,6 +77,7 @@
#include "catalog/pg_collation.h"
#include "catalog/pg_type.h"
+#include "common/int.h"
#include "common/unicode_case.h"
#include "common/unicode_category.h"
#include "mb/pg_wchar.h"
@@ -3826,7 +3827,14 @@ DCH_from_char(FormatNode *node, const char *in, TmFromChar *out,
ereturn(escontext,,
(errcode(ERRCODE_INVALID_DATETIME_FORMAT),
errmsg("invalid input string for \"Y,YYY\"")));
- years += (millennia * 1000);
+
+ /* years += (millennia * 1000); */
+ if (pg_mul_s32_overflow(millennia, 1000, &millennia) ||
+ pg_add_s32_overflow(years, millennia, &years))
+ ereturn(escontext,,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("value for \"Y,YYY\" in source string is out of range")));
+
if (!from_char_set_int(&out->year, years, n, escontext))
return;
out->yysz = 4;
@@ -4785,10 +4793,35 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
tm->tm_year = tmfc.year % 100;
if (tm->tm_year)
{
+ int tmp;
+
if (tmfc.cc >= 0)
- tm->tm_year += (tmfc.cc - 1) * 100;
+ {
+ /* tm->tm_year += (tmfc.cc - 1) * 100; */
+ tmp = tmfc.cc - 1;
+ if (pg_mul_s32_overflow(tmp, 100, &tmp) ||
+ pg_add_s32_overflow(tm->tm_year, tmp, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
else
- tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1;
+ {
+ /* tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1; */
+ tmp = tmfc.cc + 1;
+ if (pg_mul_s32_overflow(tmp, 100, &tmp) ||
+ pg_sub_s32_overflow(tmp, tm->tm_year, &tmp) ||
+ pg_add_s32_overflow(tmp, 1, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
}
else
{
@@ -4814,11 +4847,31 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
if (tmfc.bc)
tmfc.cc = -tmfc.cc;
if (tmfc.cc >= 0)
+ {
/* +1 because 21st century started in 2001 */
- tm->tm_year = (tmfc.cc - 1) * 100 + 1;
+ /* tm->tm_year = (tmfc.cc - 1) * 100 + 1; */
+ if (pg_mul_s32_overflow(tmfc.cc - 1, 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
else
+ {
/* +1 because year == 599 is 600 BC */
- tm->tm_year = tmfc.cc * 100 + 1;
+ /* tm->tm_year = tmfc.cc * 100 + 1; */
+ if (pg_mul_s32_overflow(tmfc.cc, 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
fmask |= DTK_M(YEAR);
}
@@ -4843,11 +4896,35 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
fmask |= DTK_DATE_M;
}
else
- tmfc.ddd = (tmfc.ww - 1) * 7 + 1;
+ {
+ int tmp = 0;
+
+ /* tmfc.ddd = (tmfc.ww - 1) * 7 + 1; */
+ if (pg_sub_s32_overflow(tmfc.ww, 1, &tmp) ||
+ pg_mul_s32_overflow(tmp, 7, &tmp) ||
+ pg_add_s32_overflow(tmp, 1, &tmfc.ddd))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ date_str, "timestamp", escontext);
+ goto fail;
+ }
+ }
}
if (tmfc.w)
- tmfc.dd = (tmfc.w - 1) * 7 + 1;
+ {
+ int tmp = 0;
+
+ /* tmfc.dd = (tmfc.w - 1) * 7 + 1; */
+ if (pg_sub_s32_overflow(tmfc.w, 1, &tmp) ||
+ pg_mul_s32_overflow(tmp, 7, &tmp) ||
+ pg_add_s32_overflow(tmp, 1, &tmfc.dd))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ date_str, "timestamp", escontext);
+ goto fail;
+ }
+ }
if (tmfc.dd)
{
tm->tm_mday = tmfc.dd;
@@ -4912,7 +4989,18 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
}
if (tmfc.ms)
- *fsec += tmfc.ms * 1000;
+ {
+ int tmp = 0;
+
+ /* *fsec += tmfc.ms * 1000; */
+ if (pg_mul_s32_overflow(tmfc.ms, 1000, &tmp) ||
+ pg_add_s32_overflow(*fsec, tmp, fsec))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ date_str, "timestamp", escontext);
+ goto fail;
+ }
+ }
if (tmfc.us)
*fsec += tmfc.us;
if (fprec)
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 57fcfefdaf..18d7d8a108 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -5104,6 +5104,10 @@ interval_trunc(PG_FUNCTION_ARGS)
*
* Return the Julian day which corresponds to the first day (Monday) of the given ISO 8601 year and week.
* Julian days are used to convert between ISO week dates and Gregorian dates.
+ *
+ * XXX: This function has integer overflow hazards, but restructuring it to
+ * work with the soft-error handling that its callers do is likely more
+ * trouble than it's worth.
*/
int
isoweek2j(int year, int week)
diff --git a/src/include/common/int.h b/src/include/common/int.h
index 3b1590d676..6b50aa67b9 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -117,6 +117,22 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
#endif
}
+static inline bool
+pg_neg_s16_overflow(int16 a, int16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT16_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint16
pg_abs_s16(int16 a)
{
@@ -185,6 +201,22 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
#endif
}
+static inline bool
+pg_neg_s32_overflow(int32 a, int32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT32_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint32
pg_abs_s32(int32 a)
{
@@ -300,6 +332,22 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
#endif
}
+static inline bool
+pg_neg_s64_overflow(int64 a, int64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT64_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint64
pg_abs_s64(int64 a)
{
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index c9cec70c38..dcab9e76f4 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1528,6 +1528,8 @@ select make_date(2013, 13, 1);
ERROR: date field value out of range: 2013-13-01
select make_date(2013, 11, -1);
ERROR: date field value out of range: 2013-11--1
+SELECT make_date(-2147483648, 1, 1);
+ERROR: date field value out of range: -2147483648-01-01
select make_time(10, 55, 100.1);
ERROR: time field value out of range: 10:55:100.1
select make_time(24, 0, 2.1);
diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out
index 6d7dd5c988..b87b88f038 100644
--- a/src/test/regress/expected/horology.out
+++ b/src/test/regress/expected/horology.out
@@ -3743,6 +3743,14 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok
SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS');
ERROR: date/time field value out of range: "2015-02-11 86400"
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
+ERROR: value for "Y,YYY" in source string is out of range
+SELECT to_timestamp('0.-2147483648', 'SS.MS');
+ERROR: date/time field value out of range: "0.-2147483648"
+SELECT to_timestamp('613566758', 'W');
+ERROR: date/time field value out of range: "613566758"
+SELECT to_timestamp('2024 613566758 1', 'YYYY WW D');
+ERROR: date/time field value out of range: "2024 613566758 1"
SELECT to_date('2016-13-10', 'YYYY-MM-DD');
ERROR: date/time field value out of range: "2016-13-10"
SELECT to_date('2016-02-30', 'YYYY-MM-DD');
@@ -3783,6 +3791,14 @@ SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
02-01-0001 BC
(1 row)
+SELECT to_date('100000000', 'CC');
+ERROR: date/time field value out of range: "100000000"
+SELECT to_date('-100000000', 'CC');
+ERROR: date/time field value out of range: "-100000000"
+SELECT to_date('-2147483648 01', 'CC YY');
+ERROR: date/time field value out of range: "-2147483648 01"
+SELECT to_date('2147483647 01', 'CC YY');
+ERROR: date/time field value out of range: "2147483647 01"
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
to_char
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index 1c58ff6966..805aec706c 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -371,5 +371,6 @@ select make_date(0, 7, 15);
select make_date(2013, 2, 30);
select make_date(2013, 13, 1);
select make_date(2013, 11, -1);
+SELECT make_date(-2147483648, 1, 1);
select make_time(10, 55, 100.1);
select make_time(24, 0, 2.1);
diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql
index 0fe3c783e6..808083a6d8 100644
--- a/src/test/regress/sql/horology.sql
+++ b/src/test/regress/sql/horology.sql
@@ -650,6 +650,10 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSS'); -- ok
SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSS');
SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok
SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS');
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
+SELECT to_timestamp('0.-2147483648', 'SS.MS');
+SELECT to_timestamp('613566758', 'W');
+SELECT to_timestamp('2024 613566758 1', 'YYYY WW D');
SELECT to_date('2016-13-10', 'YYYY-MM-DD');
SELECT to_date('2016-02-30', 'YYYY-MM-DD');
SELECT to_date('2016-02-29', 'YYYY-MM-DD'); -- ok
@@ -660,6 +664,10 @@ SELECT to_date('2016 365', 'YYYY DDD'); -- ok
SELECT to_date('2016 366', 'YYYY DDD'); -- ok
SELECT to_date('2016 367', 'YYYY DDD');
SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
+SELECT to_date('100000000', 'CC');
+SELECT to_date('-100000000', 'CC');
+SELECT to_date('-2147483648 01', 'CC YY');
+SELECT to_date('2147483647 01', 'CC YY');
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
--
2.39.5 (Apple Git-154)
Attachments:
[text/plain] v29-0001-Fix-various-overflow-hazards-in-date-and-timesta.patch (12.3K, ../../Z1Mk1FZ2beM9ee4q@nathan/2-v29-0001-Fix-various-overflow-hazards-in-date-and-timesta.patch)
download | inline diff:
From 99032579840c75c15acb88e6d9fb9a8a76a39dcd Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Fri, 6 Dec 2024 10:16:51 -0600
Subject: [PATCH v29 1/1] Fix various overflow hazards in date and timestamp
functions.
Reported-by: Alexander Lakhin
Author: Matthew Kim, Nathan Bossart
Reviewed-by: Joseph Koshakow, Jian He
Discussion: https://postgr.es/m/31ad2cd1-db94-bdb3-f91a-65ffdb4bef95%40gmail.com
Discussion: https://postgr.es/m/18585-db646741dd649abd%40postgresql.org
Backpatch-through: 13
---
src/backend/utils/adt/date.c | 9 ++-
src/backend/utils/adt/formatting.c | 104 +++++++++++++++++++++++--
src/backend/utils/adt/timestamp.c | 4 +
src/include/common/int.h | 48 ++++++++++++
src/test/regress/expected/date.out | 2 +
src/test/regress/expected/horology.out | 16 ++++
src/test/regress/sql/date.sql | 1 +
src/test/regress/sql/horology.sql | 8 ++
8 files changed, 183 insertions(+), 9 deletions(-)
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 8130f3e8ac..da61ac0e86 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -256,8 +256,15 @@ make_date(PG_FUNCTION_ARGS)
/* Handle negative years as BC */
if (tm.tm_year < 0)
{
+ int year = tm.tm_year;
+
bc = true;
- tm.tm_year = -tm.tm_year;
+ if (pg_neg_s32_overflow(year, &year))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW),
+ errmsg("date field value out of range: %d-%02d-%02d",
+ tm.tm_year, tm.tm_mon, tm.tm_mday)));
+ tm.tm_year = year;
}
dterr = ValidateDate(DTK_DATE_M, false, false, bc, &tm);
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 2bcc185708..a9daa5c591 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -77,6 +77,7 @@
#include "catalog/pg_collation.h"
#include "catalog/pg_type.h"
+#include "common/int.h"
#include "common/unicode_case.h"
#include "common/unicode_category.h"
#include "mb/pg_wchar.h"
@@ -3826,7 +3827,14 @@ DCH_from_char(FormatNode *node, const char *in, TmFromChar *out,
ereturn(escontext,,
(errcode(ERRCODE_INVALID_DATETIME_FORMAT),
errmsg("invalid input string for \"Y,YYY\"")));
- years += (millennia * 1000);
+
+ /* years += (millennia * 1000); */
+ if (pg_mul_s32_overflow(millennia, 1000, &millennia) ||
+ pg_add_s32_overflow(years, millennia, &years))
+ ereturn(escontext,,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("value for \"Y,YYY\" in source string is out of range")));
+
if (!from_char_set_int(&out->year, years, n, escontext))
return;
out->yysz = 4;
@@ -4785,10 +4793,35 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
tm->tm_year = tmfc.year % 100;
if (tm->tm_year)
{
+ int tmp;
+
if (tmfc.cc >= 0)
- tm->tm_year += (tmfc.cc - 1) * 100;
+ {
+ /* tm->tm_year += (tmfc.cc - 1) * 100; */
+ tmp = tmfc.cc - 1;
+ if (pg_mul_s32_overflow(tmp, 100, &tmp) ||
+ pg_add_s32_overflow(tm->tm_year, tmp, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
else
- tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1;
+ {
+ /* tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1; */
+ tmp = tmfc.cc + 1;
+ if (pg_mul_s32_overflow(tmp, 100, &tmp) ||
+ pg_sub_s32_overflow(tmp, tm->tm_year, &tmp) ||
+ pg_add_s32_overflow(tmp, 1, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
}
else
{
@@ -4814,11 +4847,31 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
if (tmfc.bc)
tmfc.cc = -tmfc.cc;
if (tmfc.cc >= 0)
+ {
/* +1 because 21st century started in 2001 */
- tm->tm_year = (tmfc.cc - 1) * 100 + 1;
+ /* tm->tm_year = (tmfc.cc - 1) * 100 + 1; */
+ if (pg_mul_s32_overflow(tmfc.cc - 1, 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
else
+ {
/* +1 because year == 599 is 600 BC */
- tm->tm_year = tmfc.cc * 100 + 1;
+ /* tm->tm_year = tmfc.cc * 100 + 1; */
+ if (pg_mul_s32_overflow(tmfc.cc, 100, &tm->tm_year) ||
+ pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ text_to_cstring(date_txt), "timestamp",
+ escontext);
+ goto fail;
+ }
+ }
fmask |= DTK_M(YEAR);
}
@@ -4843,11 +4896,35 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
fmask |= DTK_DATE_M;
}
else
- tmfc.ddd = (tmfc.ww - 1) * 7 + 1;
+ {
+ int tmp = 0;
+
+ /* tmfc.ddd = (tmfc.ww - 1) * 7 + 1; */
+ if (pg_sub_s32_overflow(tmfc.ww, 1, &tmp) ||
+ pg_mul_s32_overflow(tmp, 7, &tmp) ||
+ pg_add_s32_overflow(tmp, 1, &tmfc.ddd))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ date_str, "timestamp", escontext);
+ goto fail;
+ }
+ }
}
if (tmfc.w)
- tmfc.dd = (tmfc.w - 1) * 7 + 1;
+ {
+ int tmp = 0;
+
+ /* tmfc.dd = (tmfc.w - 1) * 7 + 1; */
+ if (pg_sub_s32_overflow(tmfc.w, 1, &tmp) ||
+ pg_mul_s32_overflow(tmp, 7, &tmp) ||
+ pg_add_s32_overflow(tmp, 1, &tmfc.dd))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ date_str, "timestamp", escontext);
+ goto fail;
+ }
+ }
if (tmfc.dd)
{
tm->tm_mday = tmfc.dd;
@@ -4912,7 +4989,18 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std,
}
if (tmfc.ms)
- *fsec += tmfc.ms * 1000;
+ {
+ int tmp = 0;
+
+ /* *fsec += tmfc.ms * 1000; */
+ if (pg_mul_s32_overflow(tmfc.ms, 1000, &tmp) ||
+ pg_add_s32_overflow(*fsec, tmp, fsec))
+ {
+ DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL,
+ date_str, "timestamp", escontext);
+ goto fail;
+ }
+ }
if (tmfc.us)
*fsec += tmfc.us;
if (fprec)
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 57fcfefdaf..18d7d8a108 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -5104,6 +5104,10 @@ interval_trunc(PG_FUNCTION_ARGS)
*
* Return the Julian day which corresponds to the first day (Monday) of the given ISO 8601 year and week.
* Julian days are used to convert between ISO week dates and Gregorian dates.
+ *
+ * XXX: This function has integer overflow hazards, but restructuring it to
+ * work with the soft-error handling that its callers do is likely more
+ * trouble than it's worth.
*/
int
isoweek2j(int year, int week)
diff --git a/src/include/common/int.h b/src/include/common/int.h
index 3b1590d676..6b50aa67b9 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -117,6 +117,22 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result)
#endif
}
+static inline bool
+pg_neg_s16_overflow(int16 a, int16 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT16_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint16
pg_abs_s16(int16 a)
{
@@ -185,6 +201,22 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
#endif
}
+static inline bool
+pg_neg_s32_overflow(int32 a, int32 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT32_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint32
pg_abs_s32(int32 a)
{
@@ -300,6 +332,22 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
#endif
}
+static inline bool
+pg_neg_s64_overflow(int64 a, int64 *result)
+{
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
+ return __builtin_sub_overflow(0, a, result);
+#else
+ if (unlikely(a == PG_INT64_MIN))
+ {
+ *result = 0x5EED; /* to avoid spurious warnings */
+ return true;
+ }
+ *result = -a;
+ return false;
+#endif
+}
+
static inline uint64
pg_abs_s64(int64 a)
{
diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out
index c9cec70c38..dcab9e76f4 100644
--- a/src/test/regress/expected/date.out
+++ b/src/test/regress/expected/date.out
@@ -1528,6 +1528,8 @@ select make_date(2013, 13, 1);
ERROR: date field value out of range: 2013-13-01
select make_date(2013, 11, -1);
ERROR: date field value out of range: 2013-11--1
+SELECT make_date(-2147483648, 1, 1);
+ERROR: date field value out of range: -2147483648-01-01
select make_time(10, 55, 100.1);
ERROR: time field value out of range: 10:55:100.1
select make_time(24, 0, 2.1);
diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out
index 6d7dd5c988..b87b88f038 100644
--- a/src/test/regress/expected/horology.out
+++ b/src/test/regress/expected/horology.out
@@ -3743,6 +3743,14 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok
SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS');
ERROR: date/time field value out of range: "2015-02-11 86400"
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
+ERROR: value for "Y,YYY" in source string is out of range
+SELECT to_timestamp('0.-2147483648', 'SS.MS');
+ERROR: date/time field value out of range: "0.-2147483648"
+SELECT to_timestamp('613566758', 'W');
+ERROR: date/time field value out of range: "613566758"
+SELECT to_timestamp('2024 613566758 1', 'YYYY WW D');
+ERROR: date/time field value out of range: "2024 613566758 1"
SELECT to_date('2016-13-10', 'YYYY-MM-DD');
ERROR: date/time field value out of range: "2016-13-10"
SELECT to_date('2016-02-30', 'YYYY-MM-DD');
@@ -3783,6 +3791,14 @@ SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
02-01-0001 BC
(1 row)
+SELECT to_date('100000000', 'CC');
+ERROR: date/time field value out of range: "100000000"
+SELECT to_date('-100000000', 'CC');
+ERROR: date/time field value out of range: "-100000000"
+SELECT to_date('-2147483648 01', 'CC YY');
+ERROR: date/time field value out of range: "-2147483648 01"
+SELECT to_date('2147483647 01', 'CC YY');
+ERROR: date/time field value out of range: "2147483647 01"
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
to_char
diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql
index 1c58ff6966..805aec706c 100644
--- a/src/test/regress/sql/date.sql
+++ b/src/test/regress/sql/date.sql
@@ -371,5 +371,6 @@ select make_date(0, 7, 15);
select make_date(2013, 2, 30);
select make_date(2013, 13, 1);
select make_date(2013, 11, -1);
+SELECT make_date(-2147483648, 1, 1);
select make_time(10, 55, 100.1);
select make_time(24, 0, 2.1);
diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql
index 0fe3c783e6..808083a6d8 100644
--- a/src/test/regress/sql/horology.sql
+++ b/src/test/regress/sql/horology.sql
@@ -650,6 +650,10 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSS'); -- ok
SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSS');
SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok
SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS');
+SELECT to_timestamp('1000000000,999', 'Y,YYY');
+SELECT to_timestamp('0.-2147483648', 'SS.MS');
+SELECT to_timestamp('613566758', 'W');
+SELECT to_timestamp('2024 613566758 1', 'YYYY WW D');
SELECT to_date('2016-13-10', 'YYYY-MM-DD');
SELECT to_date('2016-02-30', 'YYYY-MM-DD');
SELECT to_date('2016-02-29', 'YYYY-MM-DD'); -- ok
@@ -660,6 +664,10 @@ SELECT to_date('2016 365', 'YYYY DDD'); -- ok
SELECT to_date('2016 366', 'YYYY DDD'); -- ok
SELECT to_date('2016 367', 'YYYY DDD');
SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be
+SELECT to_date('100000000', 'CC');
+SELECT to_date('-100000000', 'CC');
+SELECT to_date('-2147483648 01', 'CC YY');
+SELECT to_date('2147483647 01', 'CC YY');
-- to_char's TZ format code produces zone abbrev if known
SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ');
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 19+ messages in thread
end of thread, other threads:[~2024-12-06 16:22 UTC | newest]
Thread overview: 19+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-02-19 16:14 [PATCH v6] pg_rewind: options to use restore_command from command line or cluster config Alexey Kondratov <[email protected]>
2019-12-20 01:25 [PATCH v37 11/11] Add documentations about Incremental View Maintenance Yugo Nagata <[email protected]>
2024-08-16 16:52 Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
2024-08-16 18:00 ` Re: Remove dependence on integer wrapping Alexander Lakhin <[email protected]>
2024-08-16 18:35 ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
2024-08-16 18:56 ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
2024-08-16 20:11 ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
2024-08-17 19:16 ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
2024-08-17 21:52 ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
2024-08-18 12:00 ` Re: Remove dependence on integer wrapping Alexander Lakhin <[email protected]>
2024-08-20 21:21 ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
2024-08-21 07:00 ` Re: Remove dependence on integer wrapping Alexander Lakhin <[email protected]>
2024-08-21 15:37 ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
2024-08-24 12:44 ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
2024-12-05 22:50 ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
2024-12-06 01:00 ` Re: Remove dependence on integer wrapping Joseph Koshakow <[email protected]>
2024-12-06 03:58 ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
2024-12-06 16:22 ` Re: Remove dependence on integer wrapping Nathan Bossart <[email protected]>
2024-08-18 03:00 ` Re: Remove dependence on integer wrapping Alexander Lakhin <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox