memory reference
There are two issues present here.
1) There is a possibility that we test a byte beyond the
allocated buffer
2) We compare a byte that might never have been
initalized to see if it's 0.
The first issue is not triggered by existing code, but an
ASSERT has been added to safe-guard against introducing
new code that triggers it.
The second issue is what triggers the Valgrind warnings
reported in the bug report. A buffer is allocated in
class String to hold the value. This buffer is populated
by the character data constituting the string, but is not
zero-terminated in most cases. Testing if it is indeed
zero-terminated means that we check a byte that has never
been explicitly set, thus causing Valgrind to trigger.
Note that issue 2 is not a serious problem. The variable
is read, and if it's not zero, we will set it to zero.
There are no further consequences.
Note that this patch does not fix the underlying problems
with issue 1, as it is deemed too risky to fix at this
point (as noted in the bug report). As discussed in
the report, the c_ptr() method should probably be
replaced, but this requires a thorough analysis of the
~200 calls to the method.
sql/set_var.cc:
These two cases have been reported to fail
with Valgrind.
With --mem if fails with
+UNEXPECTED ERROR NUMBER: 1290
In var/log/mysqld.2.err we have:
[ERROR] LOAD DATA INFILE in the slave SQL Thread can only read from --slave-load-tmpdir. Please, report a bug.
[ERROR] Slave SQL: Error 'The MySQL server is running with the --slave-load-tmpdir option so it cannot execute this statement' on query. Default database: 'test'. Query: 'LOAD DATA INFILE '../../tmp/SQL_LOAD-2-1-1.data' INTO TABLE `t1` FIELDS TERMINATED BY '\t' ENCLOSED BY '' ESCAPED BY '\\' LINES TERMINATED BY '\n' (`a`, `b`)', Error_code: 1290
getcwd() in the server yields something like: /dev/shm/var_auto_iv5Q/mysqld.2/data
Removes SHOW NEW MASTER statement and all related code.
Also removes the unused function update_slave_list from repl_failsafe.cc.
mysql-test/r/signal_code.result:
Updated result file.
The output of SHOW PROCEDURE CODE has changed, because the numerical values
of some SQLCOM_ commands have changes. This is not a problem, because
SHOW PROCEDURE CODE only exists in debug builds and the numerical values of
SQLCOM_ constants are not exposed elsewhere.
mysql-test/r/sp-code.result:
Updated result file.
The output of SHOW PROCEDURE CODE has changed, because the numerical values
of some SQLCOM_ commands have changes. This is not a problem, because
SHOW PROCEDURE CODE only exists in debug builds and the numerical values of
SQLCOM_ constants are not exposed elsewhere.
sql/mysqld.cc:
Remove SQLCOM_SHOW_NEW_MASTER.
sql/repl_failsafe.cc:
Remove show_new_master, which was only used by the removed SHOW NEW MASTER statement.
Remove translate_master, which was only used by show_new_master.
Remove find_slave_event, which was only used by translate_master.
Remove find_target_pos, which was only used by translate_master.
Remove cmp_master_pos, which was only used by translate_master.
Remove update_slave_list, which was not used at all.
sql/repl_failsafe.h:
Remove declarations of functions removed by this patch:
update_slave_list, show_new_master, translate_master, update_slave_list
Remove declarations of functions that did not exist before this patch:
handle_failsafe_rpl, load_master_data
Remove declaration of function that is static in slave.cc:
connect_to_master
sql/sp_head.cc:
Removed SQLCOM_SHOW_NEW_MASTER
sql/sql_lex.h:
Removed SQLCOM_SHOW_NEW_MASTER
sql/sql_parse.cc:
Removed SQLCOM_SHOW_NEW_MASTER
sql/sql_repl.cc:
Removed cmp_master_pos(char*,ulonglong,char*,ulonglong), which was
only used by cmp_master_pos*Slave_log_event* sev, LEX_MASTER_INFO* mi) in repl_failsafe.cc,
which has been removed.
sql/sql_repl.h:
removed cmp_master_pos
sql/sql_yacc.yy:
removed syntax SHOW NEW MASTER.
post-push fix: make it work in out-of-source builds.
mysql-test/mysql-test-run.pl:
Set MYSQL_BINDIR, to be used by tests.
mysql-test/t/file_contents.test:
Use MYSQL_BINDIR, which is different from MYSQL_LIBDIR for out-of-source builds.
attempt to create spatial index on char > 31 bytes".
Attempt to create spatial index on char field with length
greater than 31 byte led to assertion failure on server
compiled with safemutex support.
The problem occurred in mi_create() function which was called
to create a new version of table being altered. This function
failed since it detected an attempt to create a spatial key
on non-binary column and tried to return an error.
On its error path it tried to unlock THR_LOCK_myisam mutex
which has not been not locked at this point. Indeed such an
incorrect behavior was caught by safemutex wrapper and caused
assertion failure.
This patch fixes the problem by ensuring that mi_create()
doesn't releases THR_LOCK_myisam mutex on error path if it was
not acquired.
mysql-test/r/gis.result:
Added test for bug @59888 "debug assertion when attempt to
create spatial index on char > 31 bytes".
mysql-test/t/gis.test:
Added test for bug @59888 "debug assertion when attempt to
create spatial index on char > 31 bytes".
storage/myisam/mi_create.c:
Changed mi_create() not to release THR_LOCK_myisam mutex on
error path if it was not acquired.
rw_lock_create_func(): Initialize lock->writer_thread, so that Valgrind
will not complain even when Valgrind instrumentation is not enabled.
Flag lock->writer_thread uninitialized, so that Valgrind can complain
when it is used uninitialized.
rw_lock_set_writer_id_and_recursion_flag(): Revert the bogus Valgrind
instrumentation that was pushed in the first attempt to fix this bug.
Assert in Diagnostics_area::set_ok_status() for XA COMMIT
This assert was triggered if XA COMMIT was issued when an XA transaction
already had encountered an error (e.g. a deadlock) which required
the XA transaction to be rolled back.
In general, the assert is triggered if a statement tries to send OK to
the client when an error has already been reported. It was triggered
in this case because the trans_xa_commit() function first reported an
error, then rolled back the transaction and finally returned FALSE,
indicating success. Since trans_xa_commit() reported success,
mysql_execute_command() tried to report OK, triggering the assert.
This patch fixes the problem by fixing trans_xa_commit() to return TRUE
if it encounters an error that requires rollback, even if the rollback
itself is successful.
Test case added to xa.test.
The problem was that Item_sum_hybrid::val_xxx() did not propagate null values
up the expression tree.
mysql-test/r/func_time.result:
New test case.
mysql-test/t/func_time.test:
New test case.
sql/item_sum.cc:
Check for null_value when evaluating sub-items in sub-trees in Item_sum_hybrid::val_xxx()
With this change, there will be new files "INFO_SRC"
and "INFO_BIN", which describe the source and the
binaries.
They will be contained in all packages:
- in "tar.gz" and derived packages, in "docs/",
- in RPMs, in "/usr/share/doc/packages/MySQL-server".
"INFO_SRC" is also part of a source tarball.
It gives the version as exact as possible, preferably
by calling "bzr version-info" on the source tree.
If that is not possible, it just contains the three
level version number.
"INFO_BIN" contains some info when and where the
binaries were built, the options given to the compiler,
and the flags controlling the included features.
The tests (test "mysql" in the main suite) are extended
to verify the existence of both "INFO_SRC" and "INFO_BIN",
as well as some of the expected contents.
CMakeLists.txt:
For the new files describing the source and the build
("INFO_SRC" and "INFO_BIN"), we need a new file
"cmake/info_macros.cmake.in" with the build rules.
1) This file must be configured with the current variables.
2) "INFO_SRC" can be created during the cmake phase,
but this should be repeated with each "make" to
protect against a developer doing only "make" after
a "bzr pull" (or "bzr commit").
So have it both as a cmake rule and as a custom target.
3) "INFO_BIN" must be created during the make phase
only, because it contains information from files
which will be written at the end of the cmake phase only.
Therefore, it must be a custom target which is included
in all "make" targets.
4) The resulting "INFO_*" files must be included in packages.
cmake/info_bin.cmake:
This is the file to create "INFO_BIN",
by calling the "CREATE_INFO_BIN()" macro.
It must be a separate file, so that the macro
definitions can be included in other cmake scripts
without that file inclusion causing a side effect,
the macro call.
That call would modify the source tree which should
be trated read-only.
cmake/info_macros.cmake.in:
This new file contains the macros to create the
"INFO_*" files during various steps of the build,
the calls will be at other places.
1) For source: If running from a BZR tree, always create
(update) "INFO_SRC" by running "bzr version-info".
Outside a BZR tree, try to take it from exported
sources, and create it only if missing, in that
case put the three level version number into it.
2) "INFO_BIN" contains
- date/time and host name of the build host,
- information about the platform,
- information about the C and CXX compiler
and the options given to them (Unix only),
- the feature flags as reported by "cmake -L".
cmake/info_src.cmake:
This is the file to create "INFO_SRC",
by calling the "CREATE_INFO_SRC()" macro.
It must be a separate file, so that the macro
definitions can be included in other cmake scripts
without that file inclusion causing a side effect,
the macro call.
That call would modify the source tree which should
be trated read-only.
cmake/make_dist.cmake.in:
Create a "VERSION_src" file during "make dist".
In case it already exists from a preceding "cmake" run
or tree export (which is quite likely), a new
"make dist" must not modify it.
mysql-test/r/file_contents.result:
Result of test for bug#42969.
mysql-test/t/file_contents.test:
Perl test scriptlet for bug#42969.
support-files/mysql.spec.sh:
Add "INFO_SRC" and "INFO_BIN" to the RPM contents.
"set optimizer_switch to e or d causes invalid memory writes/valgrind warnings":
due to prefix support, the argument "e" was overwritten with its full value
"engine_condition_pushdown", which caused a buffer overrun.
This was wrong usage of find_type(); other wrong usages are fixed here too.
Please start reading with the comment of typelib.c.
client/mysqldump.c:
A bug: find_type() expects a bitmap as 3rd argument
(each bit is a flag controlling a behaviour of the function);
here it was instead passed the length of the string to search!
That could give random behaviour of find_type()
depending on the string.
We rather need to pass a correct flag to find_type().
The correct flag is FIND_TYPE_BASIC (0).
Flag 8 is not needed as buff cannot have a comma (see how buff is filled).
Flag 1 looks like a superfluous restriction.
Flag 4 is not user-friendly (why use
--compatible=2 rather than --compatible=mysql40 ?, and
we probably not commit to "2" always meaning "mysql40"
until the end of times).
include/mysql.h.pp:
This isn't a problematic API change as we go from char* to const char*:
existing code will run unchanged.
include/typelib.h:
named constants. Not an enum to not significantly change
the declaration of find_type() which would be an API change
(typelib.h is included in mysql.h).
mysql-test/r/mysqldump.result:
correct result (see the two requested modes in SQL_MODE)
mysql-test/suite/sys_vars/t/optimizer_switch_basic.test:
test for BUG#59894. The second SET used to crash.
mysql-test/t/mysqldump.test:
we had no test for multiple modes in --compatible, which is
supported according to --help
mysys/typelib.c:
Fix for BUG#59894. parse_name() is asked to match "e" with a row
of the TYPELIB (the TYPELIB lists permitted flags of optimizer_switch;
and comes from optimizer_switch_names[] of sys_vars.cc).
find_type() is capable of supporting prefixes, but if it is not
passed flag 2 in third argument, it will overwrite its first
argument (the string to search for) with the complete name,
here overwriting "e" with "engine_condition_pushdown". But
as this "e" was a buffer allocated in an Item, it was not big
enough to host the longer name, thus the crash.
We don't need to know the complete flag's name; the output used
from find_type() is just the flag's number (== function's return
code). So we can pass flag 2 to find_type() in parse_name().
After doing this fix and the other fixes in this patch, all usages
of find_type() were using flag 2; in most usages the string to search for,
is not guaranteed to be long enough to host the complete name
(it is either directly from argv, or from alloc_root/my_malloc
done in an earlier call).
Thus, flag 2 is here made implicit: callers need not pass it anymore,
it is always automatically turned on.
This allows to eliminate an oddity: parse_name() took a const char**,
and then removed "const" before calling find_type(), which could
theoretically modify the pointed data, thus lying on constness.
Last, constants for find_type() are now named.
sql-common/client.c:
Two bugs:
1) The enum was not in sync with the array (due to a bad porting of WL 1054;
the extra OPT_ values are about options present in 5.1 and deleted in 5.5);
added a compile_time_assert() to make sure this doesn't happen again
2) find_type() was writing past the end of opt_arg; as opt_arg was allocated
with alloc_root() with no extra space, this was an overrun; it could be seen
when
** building with -DWITH_VALGRIND -DHAVE_purify -DEXTRA_DEBUG
** making execution go through the faulty code; this faulty
code is executed only if the client asks to read a configuration
file like this:
mysql_options(mysql, MYSQL_READ_DEFAULT_FILE, "/tmp/cnf.cnf");
so by adding such line to the start of mysql_client_test.c::client_connect(),
we could see the valgrind warning:
==30548== Invalid write of size 1
==30548== at 0x4C2624C: strcpy (mc_replace_strmem.c:303)
==30548== by 0x48DC29: find_type (typelib.c:120)
==30548== by 0x465686: mysql_read_default_options (client.c:1344)
==30548== by 0x46830F: mysql_real_connect (client.c:2971)
==30548== by 0x409339: client_connect (mysql_client_test.c:331)
==30548== by 0x463A7F: main (mysql_client_test.c:19902)
==30548== Address 0x61875ad is 0 bytes after a block of size 29 alloc'd
==30548== at 0x4C25153: malloc (vg_replace_malloc.c:195)
==30548== by 0x49BFF1: my_malloc (my_malloc.c:38)
==30548== by 0x49C65C: alloc_root (my_alloc.c:166)
==30548== by 0x48EF97: handle_default_option (default.c:381)
==30548== by 0x49068C: search_default_file_with_ext (default.c:992)
==30548== by 0x48F929: search_default_file (default.c:670)
==30548== by 0x48EDC4: my_search_option_files (default.c:312)
==30548== by 0x48F4B1: my_load_defaults (default.c:576)
==30548== by 0x46517A: mysql_read_default_options (client.c:1207)
==30548== by 0x46830F: mysql_real_connect (client.c:2971)
==30548== by 0x409339: client_connect (mysql_client_test.c:331)
==30548== by 0x463A7F: main (mysql_client_test.c:19902)
This is fixed by having find_type() not overwrite anymore.
sql/sql_help.cc:
cast not needed anymore.
sql/table.cc:
cast not needed anymore.
The crash happens because Item_cache which is result
holder for Item_subselect can't correctly convert
a DATETIME value from string to int representation.
The fix is to disable constant item convertion for
subselect(partial rollback of bug52157 fix).
mysql-test/r/type_date.result:
test case
mysql-test/t/type_date.test:
test case
sql/item_cmpfunc.cc:
disable constant item convertion for subselects.
Test failed on a certain Linux platform in automated environment. It turns out that this platform has an old version of Perl modules DBI and DBD::mysql installed, as well as the OS itself being relatively old.
Allowing error code 11 to be returned from mysqlhotcopy on expected error seems harmless and will make the test pass also with older libraries.
