Starting with clang-16, MemorySanitizer appears to check that
uninitialized values not be passed by value nor returned.
Previously, it was allowed to copy uninitialized data in such cases.
get_foreign_key_info(): Remove a local variable that was passed
uninitialized to a function.
DsMrr_impl: Initialize key_buffer, because DsMrr_impl::dsmrr_init()
is reading it.
test_bind_result_ext1(): MYSQL_TYPE_LONG is 32 bits, hence we must
use a 32-bit type, such as int. sizeof(long) differs between
LP64 and LLP64 targets.
Follow-up to fix issue with access to probably not-initialized mutex/cond_var
Constructor of the class st_debug_sync_globals was changed to initialize
the data members dsp_hits, dsp_executed, dsp_max_active with zero.
Formerly, these data members were filled with zeroes by C-runtime since
the variable debug_sync_global was declared as static and according with C rules
the static variable initialized with zero bytes.
By the same reason, the data members
debug_sync_global->ds_mutex
debug_sync_global->ds_cond
were initialized by zeros before the patch for MDEV-31871. After this patch
the memory for the synch primitives debug_sync_global->ds_mutex
and debug_sync_global->ds_cond are initialized explicitly by calling
the functions mysql_mutex_init/mysql_cond_init so access to these synch
primitives should be done only after such initialization be completed.
Guarded access to these synch primitives has been added to the function
debug_sync_end_thread() that is called on clean up since that was single
problem place detected by MSAN. Theoretically problem places located in the
function debug_sync_execute were not protected with similar check since
it is not obvious that the variables debug_sync_global->ds_mutex
and debug_sync_global->ds_cond could be not initilialized for use cases where
the function debug_sync_execute() is called. It is required additional study
to conclude whether it does need or not.
mariadb-install crashes on start when the static variable debug_sync_global
of the class st_debug_sync_globals is initialized by constructor.
Definition of the class st_debug_sync_globals has a data member of the type
Hash_set whose implementation depends on thread-specific data associated
with the key THD_KEY_mysys. This dependency results from constructor of
the class Hash_set that runs my_hash_init2() which in turn invokes my_malloc.
The thread-specific data value associated with the key THD_KEY_mysys is used
by the function sf_malloc to get id of the current thread. The key
THD_KEY_mysys is defined as static variable at my_thr_init.c initialized
with the value -1. Proper initialization of the key THD_KEY_mysys is done
with the library call pthread_key_create but it happens at the my_init()
that called much later after the first time the THD_KEY_mysys() has been
invoked. In according with Single Unix Specification, the effect of calling
pthread_setspecific() or pthread_getspecific() with a key value not obtained
from pthread_key_create() is undefined. That is the reason why mariadb-install
crashes on MacOS.
To fix the issue, the static variable debug_sync_global is converted to
a pointer to the class st_debug_sync_globals and its instantiation is done
explicitly at the function debug_sync_init() that is called at right time.
This is the follow-up patch to the commits
8885225de6f6ecadfee8
where was introduced a statically instantiated object debug_sync_global of
the structure st_debug_sync_globals and the key THR_KEY_mysys for this
thread-specific data was initialized with the value -1.
This is allowed:
STRING_WITH_LEN("string literal")
This is not:
char *str = "pointer to string";
... STRING_WITH_LEN(str) ..
In C++ this is also allowed:
const char str[] = "string literal";
... STRING_WITH_LEN(str) ...
When running with DBUG trace enabled, print all current active signals.
The output looks like this with the signals present in curr:
T@6 ... debug_sync_exec: wait for 'nothing' at: 'now', curr: 'something,from_function,from_myvar'
The patch is inspired from MySQL. Instead of using a single String to
hold the current active debug_sync signal, use a Hash_set to store
LEX_STRINGS. This patch ensures that a signal can not be lost, by being
overwritten by another thread via set DEBUG_SYNC = '... SIGNAL ...';
All signals are kepts "alive" until they are consumed by a wait event.
This requires updating test cases that assume the GLOBAL signal is never
consumed.
Follow-up work needed:
Port the additional syntax that allows one to set multiple signals
and also conditionally deactivate signals when waiting.
In commit 28325b0863
a compile-time option was introduced to disable the macros
DBUG_ENTER and DBUG_RETURN or DBUG_VOID_RETURN.
The parameter name WITH_DBUG_TRACE would hint that it also
covers DBUG_PRINT statements. Let us do that: WITH_DBUG_TRACE=OFF
shall disable DBUG_PRINT() as well.
A few InnoDB recovery tests used to check that some output from
DBUG_PRINT("ib_log", ...) is present. We can live without those checks.
Reviewed by: Vladislav Vaintroub
The issue was that we sent two different signals to different threads
after each other. The DEBUG_SYNC functionality cannot handle this (as
the signal is stored in a global variable) and the first one can get
lost.
Fixed by using the same signal for both threads.
- Major rewrite of ddl_log.cc and ddl_log.h
- ddl_log.cc described in the beginning how the recovery works.
- ddl_log.log has unique signature and is dynamic. It's easy to
add more information to the header and other ddl blocks while still
being able to execute old ddl entries.
- IO_SIZE for ddl blocks is now dynamic. Can be changed without affecting
recovery of old logs.
- Code is more modular and is now usable outside of partition handling.
- Renamed log file to dll_recovery.log and added option --log-ddl-recovery
to allow one to specify the path & filename.
- Added ddl_log_entry_phase[], number of phases for each DDL action,
which allowed me to greatly simply set_global_from_ddl_log_entry()
- Changed how strings are stored in log entries, which allows us to
store much more information in a log entry.
- ddl log is now always created at start and deleted on normal shutdown.
This simplices things notable.
- Added probes debug_crash_here() and debug_simulate_error() to simply
crash testing and allow crash after a given number of times a probe
is executed. See comments in debug_sync.cc and rename_table.test for
how this can be used.
- Reverting failed table and view renames is done trough the ddl log.
This ensures that the ddl log is tested also outside of recovery.
- Added helper function 'handler::needs_lower_case_filenames()'
- Extend binary log with Q_XID events. ddl log handling is using this
to check if a ddl log entry was logged to the binary log (if yes,
it will be deleted from the log during ddl_log_close_binlogged_events()
- If a DDL entry fails 3 time, disable it. This is to ensure that if
we have a crash in ddl recovery code the server will not get stuck
in a forever crash-restart-crash loop.
mysqltest.cc changes:
- --die will now replace $variables with their values
- $error will contain the error of the last failed statement
storage engine changes:
- maria_rename() was changed to be more robust against crashes during
rename.
Passing a null pointer to a nonnull argument is not only undefined
behaviour, but it also grants the compiler the permission to optimize
away further checks whether the pointer is null. GCC -O2 at least
starting with version 8 may do that, potentially causing SIGSEGV.
These problems were caught in a WITH_UBSAN=ON build with the
Bug#7024 test in main.view.
It now lives from THD constructor to THD destructor. Reset before THD is
released to a cache. Change user doesn't reset debug_sync_control anymore.
Needed to be able to make use of DEBUG_SYNC() at later stages like
ha_close_connection().
There were two newly enabled warnings:
1. cast for a function pointers. Affected sql_analyse.h, mi_write.c
and ma_write.cc, mf_iocache-t.cc, mysqlbinlog.cc, encryption.cc, etc
2. memcpy/memset of nontrivial structures. Fixed as:
* the warning disabled for InnoDB
* TABLE, TABLE_SHARE, and TABLE_LIST got a new method reset() which
does the bzero(), which is safe for these classes, but any other
bzero() will still cause a warning
* Table_scope_and_contents_source_st uses `TABLE_LIST *` (trivial)
instead of `SQL_I_List<TABLE_LIST>` (not trivial) so it's safe to
bzero now.
* added casts in debug_sync.cc and sql_select.cc (for JOIN)
* move assignment method for MDL_request instead of memcpy()
* PARTIAL_INDEX_INTERSECT_INFO::init() instead of bzero()
* remove constructor from READ_RECORD() to make it trivial
* replace some memcpy() with c++ copy assignments
Handle string length as size_t, consistently (almost always:))
Change function prototypes to accept size_t, where in the past
ulong or uint were used. change local/member variables to size_t
when appropriate.
This fix excludes rocksdb, spider,spider, sphinx and connect for now.
- Fix win64 pointer truncation warnings
(usually coming from misusing 0x%lx and long cast in DBUG)
- Also fix printf-format warnings
Make the above mentioned warnings fatal.
- fix pthread_join on Windows to set return value.
Before this patch running full mtr generated some 70 cores (at least
on systemd). Now no cores should be generated.
- Changed DBUG_ABORT()'s used by mysql-test-run to DBUG_SUICIDE()
- Changed DBUG_ABORT() used to crash server with core to DBUG_ASSERT(0)
- DBUG_ASSERT now flushes DBUG files
- Added sql/mariadb.h file that should be included first by files in sql
directory, if sql_plugin.h is not used (sql_plugin.h adds SHOW variables
that must be done before my_global.h is included)
- Removed a lot of include my_global.h from include files
- Removed include's of some files that my_global.h automatically includes
- Removed duplicated include's of my_sys.h
- Replaced include my_config.h with my_global.h
