MDEV-37530 fixes

* MDEV-38410: Use array, not `std::initializer_list`
  Some environments appear not to retain the backing array of a
  static `std::initializer_list` in the MDEV-37530 release candidate,
  and eventually crash when reading overwritten data.
  This commit resolves the stealth issue by reverting to conventional
  arrays, while maintaining convenience through deductive overloads.
* Compile problems
  * Some of our platforms (namely SUSE 15, which uses GCC 7.5) support
    C++17 syntaxes, but not all libraries, `<charconv>`` among those.
* Update to the current `main` branch

Co-authored-by: Sergei Golubchik <serg@mariadb.org>
Co-authored-by: Brandon Nesterenko <brandon.nesterenko@mariadb.com>
This commit is contained in:
ParadoxV5 2026-01-28 21:33:25 -07:00 committed by Sergei Golubchik
commit 0cc1eb46ea
9 changed files with 105 additions and 119 deletions

View file

@ -2,11 +2,12 @@
# Shared between rpl.rpl_heartbeat and binlog_in_engine.rpl_heartbeat.
#
# Including:
# - user interface, grammar, checking the range and warnings about
# unreasonable values for the heartbeat period;
# - no rotation of relay log if heartbeat is less that slave_net_timeout
# - SHOW STATUS like 'Slave_received_heartbeats' action
# - SHOW STATUS like 'Slave_heartbeat_period' report
#
# `rpl.rpl_heartbeat_basic` tests the user interface, grammar, range,
# and warnings about unreasonable values for the heartbeat period.
connection slave;
-- source include/stop_slave.inc

View file

@ -8,7 +8,7 @@ master_ssl_ca_path,
master_ssl_cert,
master_ssl_cipher,
master_ssl_key,
`master_ssl_verify_server_cert`, # MDEV-38194
master_ssl_verify_server_cert,
master_ssl_crl,
master_ssl_crlpath,
using_gtid,

View file

@ -20,7 +20,7 @@ SELECT connection_name,
master_ssl_cert,
master_ssl_cipher,
master_ssl_key,
`master_ssl_verify_server_cert`, # MDEV-38194
master_ssl_verify_server_cert,
master_ssl_crl,
master_ssl_crlpath,
using_gtid,

View file

@ -8,41 +8,6 @@ reset master;
connection slave;
set @restore_slave_net_timeout= @@global.slave_net_timeout;
set @@global.slave_net_timeout= 10;
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root';
show status like 'Slave_heartbeat_period';;
Variable_name Slave_heartbeat_period
Value 5.000
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 4294968;
ERROR HY000: The requested value for the heartbeat period is either negative or exceeds the maximum allowed (4294967 seconds)
show status like 'Slave_heartbeat_period';;
Variable_name Slave_heartbeat_period
Value 5.000
connection slave;
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 0.0009999;
Warnings:
Warning 1703 The requested value for the heartbeat period is less than 1 millisecond. The value is reset to 0, meaning that heartbeating will effectively be disabled
show status like 'Slave_heartbeat_period';;
Variable_name Slave_heartbeat_period
Value 0.000
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 4294967;
Warnings:
Warning 1704 The requested value for the heartbeat period exceeds the value of `slave_net_timeout' seconds. A sensible value for the period should be less than the timeout
show status like 'Slave_heartbeat_period';;
Variable_name Slave_heartbeat_period
Value 4294967.000
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 0.001;
show status like 'Slave_heartbeat_period';;
Variable_name Slave_heartbeat_period
Value 0.001
reset slave;
set @@global.slave_net_timeout= 5;
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 5.001;
Warnings:
Warning 1704 The requested value for the heartbeat period exceeds the value of `slave_net_timeout' seconds. A sensible value for the period should be less than the timeout
show status like 'Slave_heartbeat_period';;
Variable_name Slave_heartbeat_period
Value 5.001
reset slave;
set @@global.slave_net_timeout= 5;
change master to master_host='127.0.0.1',master_port=MASTER_PORT, master_user='root', master_heartbeat_period= 4;
show status like 'Slave_heartbeat_period';;

View file

@ -162,7 +162,7 @@ SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period';
Variable_name Value
Slave_heartbeat_period 4294967.295
RESET SLAVE;
CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=4294968;
CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=4294967.296;
ERROR HY000: The requested value for the heartbeat period is either negative or exceeds the maximum allowed (4294967.295 seconds)
RESET SLAVE;
CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_PORT, MASTER_USER='root', MASTER_CONNECT_RETRY=20, MASTER_HEARTBEAT_PERIOD=8589935;

View file

@ -210,7 +210,7 @@ SHOW GLOBAL STATUS LIKE 'slave_heartbeat_period';
RESET SLAVE;
--replace_result $MASTER_MYPORT MASTER_PORT
--error ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE
eval CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=$MASTER_MYPORT, MASTER_USER='root', MASTER_CONNECT_RETRY=$connect_retry, MASTER_HEARTBEAT_PERIOD=4294968;
eval CHANGE MASTER TO MASTER_HOST='127.0.0.1', MASTER_PORT=$MASTER_MYPORT, MASTER_USER='root', MASTER_CONNECT_RETRY=$connect_retry, MASTER_HEARTBEAT_PERIOD=4294967.296;
RESET SLAVE;
# Check double size of max allowed value for master_heartbeat_period
--replace_result $MASTER_MYPORT MASTER_PORT

View file

@ -19,7 +19,6 @@
#define RPL_INFO_FILE_H
#include <cstdint> // uintN_t
#include <charconv> // std::from/to_chars and other helpers
#include <functional> // superclass of Info_file::Mem_fn
#include <my_sys.h> // IO_CACHE, FN_REFLEN, ...
@ -36,7 +35,6 @@ namespace Int_IO_CACHE
*/
template<typename I> static constexpr size_t BUF_SIZE=
std::numeric_limits<I>::digits10 + 1 + std::numeric_limits<I>::is_signed;
static constexpr auto ERRC_OK= std::errc();
/**
@ref IO_CACHE (reading one line with the `\n`) version of std::from_chars()
@ -45,19 +43,38 @@ namespace Int_IO_CACHE
*/
template<typename I> static bool from_chars(IO_CACHE *file, I &value)
{
int error;
/**
+2 for the terminating `\n\0` (They are ignored by
std::from_chars(), but my_b_gets() includes them.)
+2 for the terminating `\n\0`
(They are ignored, but my_b_gets() includes them.)
*/
char buf[BUF_SIZE<I> + 2];
/// includes the `\n` but excludes the `\0`
size_t length= my_b_gets(file, buf, sizeof(buf));
if (!length) // EOF
return true;
// SFINAE if `I` is not a numeric type
std::from_chars_result result= std::from_chars(buf, &(buf[length]), value);
// Return `true` if the conversion failed or if the number ended early
return result.ec != ERRC_OK || *(result.ptr) != '\n';
char *end= &(buf[length]);
longlong val= my_strtoll10(buf, &end, &error);
switch (error) {
case -1:
if (!std::numeric_limits<I>::is_signed)
return true;
[[fallthrough]];
case 0:
/*TODO
This upper range check is not needed when using type-
specific variants of a safe string-to-integer converter
(e.g., std::from_chars() when all platforms support it).
*/
if (*end == '\n' && value <= std::numeric_limits<I>::max())
{
value= static_cast<I>(val);
return false;
}
[[fallthrough]];
default:
return true;
}
}
/**
Convenience overload of from_chars(IO_CACHE *, I &) for `operator=` types
@ -79,14 +96,19 @@ namespace Int_IO_CACHE
*/
template<typename I> static void to_chars(IO_CACHE *file, I value)
{
/**
my_b_printf() uses a buffer too,
so we might as well save on format parsing and buffer resizing
*/
char buf[BUF_SIZE<I>];
std::to_chars_result result= std::to_chars(buf, &buf[sizeof(buf)], value);
DBUG_ASSERT(result.ec == ERRC_OK);
my_b_write(file, reinterpret_cast<const uchar *>(buf), result.ptr - buf);
/*TODO:
* my_b_printf() needs updates and so doesn't
support `long long`s at the moment.
* We can avoid format parsing by expanding
int10_to_str() if not supporting std::to_chars().
*/
int len= std::numeric_limits<I>::is_signed ?
snprintf(buf, BUF_SIZE<I>, "%lld", static_cast<long long>(value)) :
snprintf(buf, BUF_SIZE<I>, "%llu", static_cast<unsigned long long>(value))
;
DBUG_ASSERT(len > 0);
my_b_write(file, reinterpret_cast<const uchar *>(buf), len);
}
};
@ -209,7 +231,6 @@ protected:
*/
struct Mem_fn: std::function<Persistent &(Info_file *self)>
{
using List= const std::initializer_list<Mem_fn>;
/// Null Constructor
Mem_fn(std::nullptr_t null= nullptr):
std::function<Persistent &(Info_file *)>(null) {}
@ -242,36 +263,55 @@ protected:
(either contains a `.` or is entirely empty) rather than an integer.
@return `false` if the file has parsed successfully or `true` if error
*/
bool load_from_file(Mem_fn::List value_list, size_t default_line_count)
template<size_t size> bool load_from_file(
const Mem_fn (&value_list)[size],
size_t default_line_count= 0
) { return load_from_file(value_list, size, default_line_count); }
/**
Flush the MySQL line-based section to the @ref file
@param value_list List of wrapped member pointers to values.
@param total_line_count
The number of lines to describe the file as on the first line of the file.
If this is larger than `value_list.size()`, suffix the file with empty
lines until the line count (including the line count line) is this many.
This reservation provides compatibility with MySQL,
who has added more old-style lines while MariaDB innovated.
*/
template<size_t size> void save_to_file(
const Mem_fn (&value_list)[size],
size_t total_line_count= size + /* line count line */ 1
) { return save_to_file(value_list, size, total_line_count); }
private:
bool
load_from_file(const Mem_fn *values, size_t size, size_t default_line_count)
{
long val;
/**
The first row is temporarily stored in the first value. If it is a line
count and not a log name (new format), the second row will overwrite it.
*/
auto &line1= dynamic_cast<String_value<> &>((*(value_list.begin()))(this));
auto &line1= dynamic_cast<String_value<> &>(values[0](this));
if (line1.load_from(&file))
return true;
size_t line_count;
std::from_chars_result result= std::from_chars(
line1.buf, &line1.buf[sizeof(line1.buf)], line_count);
char *end= str2int(line1.buf, 10, 0, INT32_MAX, &val);
/**
If this first line was not a number - the line count,
then it was the first value for real,
so the for loop should then skip over it, the index 0 of the list.
*/
size_t i= result.ec != Int_IO_CACHE::ERRC_OK || *(result.ptr) != '\0';
size_t i= !end || *end != '\0';
/*
Set the default after parsing: While std::from_chars() does not replace
the output if it failed, it does replace if the line is not fully spent.
*/
if (i)
line_count= default_line_count;
size_t line_count= i ? default_line_count: static_cast<size_t>(val);
for (; i < line_count; ++i)
{
int c;
if (i < value_list.size()) // line known in the `value_list`
if (i < size) // line known in the `value_list`
{
const Mem_fn &pm= value_list.begin()[i];
const Mem_fn &pm= values[i];
if (pm)
{
if (pm(this).load_from(&file))
@ -294,19 +334,9 @@ protected:
return false;
}
/**
Flush the MySQL line-based section to the @ref file
@param value_list List of wrapped member pointers to values.
@param total_line_count
The number of lines to describe the file as on the first line of the file.
If this is larger than `value_list.size()`, suffix the file with empty
lines until the line count (including the line count line) is this many.
This reservation provides compatibility with MySQL,
who has added more old-style lines while MariaDB innovated.
*/
void save_to_file(Mem_fn::List value_list, size_t total_line_count)
void save_to_file(const Mem_fn *values, size_t size, size_t total_line_count)
{
DBUG_ASSERT(total_line_count > value_list.size());
DBUG_ASSERT(total_line_count > size);
my_b_seek(&file, 0);
/*
If the new contents take less space than the previous file contents,
@ -316,8 +346,9 @@ protected:
*/
Int_IO_CACHE::to_chars(&file, total_line_count);
my_b_write_byte(&file, '\n');
for (const Mem_fn &pm: value_list)
for (size_t i= 0; i < size; ++i)
{
const Mem_fn &pm= values[i];
if (pm)
pm(this).save_to(&file);
my_b_write_byte(&file, '\n');
@ -327,7 +358,7 @@ protected:
(1 for the line count line + line count) inclusive -> max line inclusive
= line count exclusive <- max line inclusive
*/
for (; total_line_count > value_list.size(); --total_line_count)
for (; total_line_count > size; --total_line_count)
my_b_write_byte(&file, '\n');
}

View file

@ -244,7 +244,7 @@ struct Master_info_file: Info_file
bool load_from(IO_CACHE *file) override
{
uint32_t count;
long count;
size_t i;
/// +1 for the terminating delimiter
char buf[Int_IO_CACHE::BUF_SIZE<uint32_t> + 1];
@ -257,22 +257,18 @@ struct Master_info_file: Info_file
if (c == /* End of Line */ '\n' || c == /* End of Count */ ' ')
break;
}
/*
* std::from_chars() fails if `count` will overflow in any way.
* exclusive end index of the string = size
*/
std::from_chars_result result= std::from_chars(buf, &buf[i], count);
char *end= str2int(buf, 10, 1, INT32_MAX, &count);
// Reserve enough elements ahead of time.
if (result.ec != Int_IO_CACHE::ERRC_OK || allocate_dynamic(&array, count))
if (!end || allocate_dynamic(&array, count))
return true;
while (count--)
{
uint32_t value;
long value;
/*
Check that the previous number ended with a ` `,
not `\n` or anything else.
*/
if (*(result.ptr) != ' ')
if (*end != ' ')
return true;
for (i= 0; i < sizeof(buf); ++i)
{
@ -287,8 +283,8 @@ struct Master_info_file: Info_file
if (c == /* End of Count */ ' ' || c == /* End of Line */ '\n')
break;
}
result= std::from_chars(buf, &buf[i], value);
if (result.ec != Int_IO_CACHE::ERRC_OK)
end= str2int(buf, 10, 1, INT32_MAX, &value);
if (!end)
return true;
ulong id= value;
bool oom= insert_dynamic(&array, (uchar *)&id);
@ -301,7 +297,7 @@ struct Master_info_file: Info_file
return true;
}
// Check that the last number ended with a `\n`, not ` ` or anything else.
if (*(result.ptr) != '\n')
if (*end != '\n')
return true;
sort_dynamic(&array, change_master_id_cmp); // to be safe
return false;
@ -417,8 +413,8 @@ struct Master_info_file: Info_file
}
void save_to(IO_CACHE *file) override
{
my_b_write_byte(file,
'0' + static_cast<unsigned char>(operator enum_master_use_gtid()));
my_b_write_byte(file, static_cast<uchar>(
'0' + static_cast<unsigned char>(operator enum_master_use_gtid())));
}
}
/// Using_Gtid
@ -473,25 +469,28 @@ struct Master_info_file: Info_file
}
};
/*
The ideal use would work with `double`s, including the `*1000` step,
but disappointingly, the double interfaces of @ref decimal_t are
implemented by printing into a string and parsing that char array.
The ideal implementation would work with native `double`s as they are
sufficiently precise in this case; but decimal2double() is currently
implemented by printing into a string and parsing that char array,
which is an even larger overhead than `my_decimal` multiplication.
*/
static const auto MAX_PERIOD= Decimal_from_str(STRING_WITH_LEN(MAX)),
THOUSAND = Decimal_from_str(STRING_WITH_LEN("1000"));
ulonglong decimal_out;
[[maybe_unused]] ulonglong decimal_out;
if (decimal.sign || decimal_cmp(&MAX_PERIOD, &decimal) < 0)
return true; // ER_SLAVE_HEARTBEAT_VALUE_OUT_OF_RANGE
overprecise= decimal.frac > 3;
// decomposed from my_decimal2int() to reduce a bit of computations
auto rounded= my_decimal(), product= my_decimal();
int unexpected_error=
decimal_round(&decimal, &rounded, 3, HALF_UP) |
decimal_mul(&rounded, &THOUSAND, &product) |
bool unexpected_error=
decimal_round(&decimal, &rounded, 3, HALF_UP) ||
decimal_mul(&rounded, &THOUSAND, &product) ||
decimal2ulonglong(&product, &decimal_out);
DBUG_ASSERT(!unexpected_error);
if (unexpected_error)
return true;
result= static_cast<uint32_t>(decimal_out);
return unexpected_error;
return false;
}
/** Load from a '\0'-terminated string
@param expected_end This function also checks that the exclusive end
@ -532,18 +531,8 @@ struct Master_info_file: Info_file
full advantage of the non-negative `DECIMAL(10,3)` format.
*/
void save_to(IO_CACHE *file) override {
char buf[Int_IO_CACHE::BUF_SIZE<uint32_t> - /* decimal part */ 3];
auto[integer_part, decimal_part]= div(operator uint32_t(), 1000);
std::to_chars_result result=
std::to_chars(buf, &buf[sizeof(buf)], integer_part);
DBUG_ASSERT(result.ec == Int_IO_CACHE::ERRC_OK);
my_b_write(file, reinterpret_cast<const uchar *>(buf), result.ptr - buf);
my_b_write_byte(file, '.');
result= std::to_chars(buf, &buf[sizeof(buf)], decimal_part);
DBUG_ASSERT(result.ec == Int_IO_CACHE::ERRC_OK);
for (ptrdiff_t digits= result.ptr - buf; digits < 3; ++digits)
my_b_write_byte(file, '0');
my_b_write(file, reinterpret_cast<const uchar *>(buf), result.ptr - buf);
my_b_printf(file, "%u.%03u", integer_part, decimal_part);
}
}
/// `Slave_heartbeat_period` of SHOW ALL SLAVES STATUS
@ -552,7 +541,7 @@ struct Master_info_file: Info_file
/// }@
inline static Mem_fn::List VALUE_LIST= {
inline static const Mem_fn VALUE_LIST[] {
&Master_info_file::master_log_file,
&Master_info_file::master_log_pos,
&Master_info_file::master_host,
@ -587,7 +576,8 @@ struct Master_info_file: Info_file
keys should match the corresponding old property name in @ref Master_info.
*/
// C++ default allocator to match that `mysql_execute_command()` uses `new`
inline static const std::unordered_map<std::string_view, Mem_fn> VALUE_MAP= {
inline static
const std::unordered_map<std::string_view, const Mem_fn> VALUE_MAP= {
/*
These are here to annotate whether they are `DEFAULT`.
They are repeated from @ref VALUE_LIST to enable bidirectional

View file

@ -36,7 +36,7 @@ struct Relay_log_info_file: Info_file
Int_value<uint32_t> sql_delay;
/// }@
inline static const Mem_fn::List VALUE_LIST= {
inline static const Mem_fn VALUE_LIST[] {
&Relay_log_info_file::relay_log_file,
&Relay_log_info_file::relay_log_pos,
&Relay_log_info_file::read_master_log_file,
@ -50,8 +50,7 @@ struct Relay_log_info_file: Info_file
}
void save_to_file() override
{
return Info_file::save_to_file(VALUE_LIST,
VALUE_LIST.size() + /* line count line */ 1);
return Info_file::save_to_file(VALUE_LIST);
}
};