From 24faa5de16c980fa5575cfc4a89618e48e9a3305 Mon Sep 17 00:00:00 2001 From: Yuchen Pei Date: Wed, 28 Jun 2023 12:22:56 +1000 Subject: [PATCH 1/6] MDEV-30542 Fixing spider/bugfix.self_reference_multi The server needs to have a unique name --- .../spider/bugfix/r/self_reference_multi.result | 8 ++++---- .../spider/bugfix/t/self_reference_multi.test | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/storage/spider/mysql-test/spider/bugfix/r/self_reference_multi.result b/storage/spider/mysql-test/spider/bugfix/r/self_reference_multi.result index c4399ddf9d2..50db034c8cd 100644 --- a/storage/spider/mysql-test/spider/bugfix/r/self_reference_multi.result +++ b/storage/spider/mysql-test/spider/bugfix/r/self_reference_multi.result @@ -4,11 +4,11 @@ for child3 MDEV-6268 SPIDER table with no COMMENT clause causes queries to wait forever -CREATE SERVER srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET "$MASTER_1_MYSOCK", DATABASE 'test',user 'root'); +CREATE SERVER $srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET "$MASTER_1_MYSOCK", DATABASE 'test',user 'root'); create table t2 (c int); -create table t1 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv",TABLE "t2"'; -create table t0 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv",TABLE "t1"'; -alter table t2 ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv",TABLE "t0"'; +create table t1 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv_self_reference_multi",TABLE "t2"'; +create table t0 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv_self_reference_multi",TABLE "t1"'; +alter table t2 ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv_self_reference_multi",TABLE "t0"'; select * from t0; ERROR HY000: An infinite loop is detected when opening table test.t0 select * from t1; diff --git a/storage/spider/mysql-test/spider/bugfix/t/self_reference_multi.test b/storage/spider/mysql-test/spider/bugfix/t/self_reference_multi.test index 8b6f070d167..a3d561f3fae 100644 --- a/storage/spider/mysql-test/spider/bugfix/t/self_reference_multi.test +++ b/storage/spider/mysql-test/spider/bugfix/t/self_reference_multi.test @@ -8,12 +8,12 @@ --echo MDEV-6268 SPIDER table with no COMMENT clause causes queries to wait forever --echo ---replace_regex /SOCKET ".*"/SOCKET "$MASTER_1_MYSOCK"/ -eval CREATE SERVER srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET "$MASTER_1_MYSOCK", DATABASE 'test',user 'root'); +--let $srv=srv_self_reference_multi +evalp CREATE SERVER $srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET "$MASTER_1_MYSOCK", DATABASE 'test',user 'root'); create table t2 (c int); -create table t1 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv",TABLE "t2"'; -create table t0 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv",TABLE "t1"'; -alter table t2 ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv",TABLE "t0"'; +eval create table t1 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "$srv",TABLE "t2"'; +eval create table t0 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "$srv",TABLE "t1"'; +eval alter table t2 ENGINE=Spider COMMENT='WRAPPER "mysql", srv "$srv",TABLE "t0"'; --error 12719 select * from t0; --error 12719 From 33877cfeae6e36ac177e3102dd98ac19f520ef74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 28 Jun 2023 17:07:00 +0300 Subject: [PATCH 2/6] Fix WITH_UBSAN GCC -Wconversion --- storage/innobase/rem/rem0rec.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/storage/innobase/rem/rem0rec.cc b/storage/innobase/rem/rem0rec.cc index d202afa9e20..0adea369037 100644 --- a/storage/innobase/rem/rem0rec.cc +++ b/storage/innobase/rem/rem0rec.cc @@ -242,9 +242,11 @@ enum rec_leaf_format { REC_LEAF_INSTANT }; -#if defined __GNUC__ && !defined __clang__ && __GNUC__ < 12 +#if defined __GNUC__ && !defined __clang__ # pragma GCC diagnostic push -# pragma GCC diagnostic ignored "-Wconversion" /* GCC 5 to 11 need this */ +# if __GNUC__ < 12 || defined WITH_UBSAN +# pragma GCC diagnostic ignored "-Wconversion" +# endif #endif /** Determine the offset to each field in a leaf-page record in ROW_FORMAT=COMPACT,DYNAMIC,COMPRESSED. @@ -1707,7 +1709,7 @@ rec_convert_dtuple_to_rec_new( REC_INFO_BITS_MASK, REC_INFO_BITS_SHIFT); return buf; } -#if defined __GNUC__ && !defined __clang__ && __GNUC__ < 11 +#if defined __GNUC__ && !defined __clang__ # pragma GCC diagnostic pop /* ignored "-Wconversion" */ #endif From 0d3720c12a6ff0a2bb104c852e6c204c887b3dd6 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Mon, 15 May 2023 12:41:31 +0400 Subject: [PATCH 3/6] MDEV-30680 Warning: Memory not freed: 280 on mangled query, LeakSanitizer: detected memory leaks The parser works as follows: The rule expr_lex returns a pointer to a newly created sp_expr_lex instance which is not linked to any MariaDB structures yet - it is pointed only from a Bison stack variable. The sp_expr_lex instance gets linked to other structures (such as sp_instr_jump_if_not) later, after scanning some following grammar. Problem before the fix: If a parse error happened immediately after expr_lex (before it got linked), the created sp_expr_lex value got lost causing a memory leak. Fix: - Using Bison's "destructor" directive to free the results of expr_lex on parse/oom errors. - Moving the call for LEX::cleanup_lex_after_parse_error() from MYSQL_YYABORT and yyerror inside parse_sql(). This is needed because Bison calls destructors after yyerror(), while it's important to delete the sp_expr_lex instance before LEX::cleanup_lex_after_parse_error(). The latter frees the memory root containing the sp_expr_lex instance. After this change the code block are executed in the following order: - yyerror() -- now only raises the error to DA (no cleanup done any more) - %destructor { delete $$; } -- destructs the sp_expr_lex instance - LEX::cleanup_lex_after_parse_error() -- frees the memory root containing the sp_expr_lex instance - Removing the "delete sublex" related code from restore_lex(): - restore_lex() is called in most cases on success, when delete is not needed. - There is one place when restore_lex() is called on error: In sp_create_assignment_instr(). But in this case LEX::sp_lex_in_use is true anyway. The patch adds a new DBUG_ASSERT(lex->sp_lex_in_use) to guard this. --- mysql-test/main/sp-memory-leak.result | 21 +++++++++++++++++++ mysql-test/main/sp-memory-leak.test | 29 +++++++++++++++++++++++++++ sql/sp_head.h | 24 +++++++++++++--------- sql/sql_lex.cc | 11 ++++++---- sql/sql_parse.cc | 11 ++++++++++ sql/sql_yacc.yy | 24 ++++++++++++---------- 6 files changed, 95 insertions(+), 25 deletions(-) create mode 100644 mysql-test/main/sp-memory-leak.result create mode 100644 mysql-test/main/sp-memory-leak.test diff --git a/mysql-test/main/sp-memory-leak.result b/mysql-test/main/sp-memory-leak.result new file mode 100644 index 00000000000..aea278801d8 --- /dev/null +++ b/mysql-test/main/sp-memory-leak.result @@ -0,0 +1,21 @@ +# +# MDEV-30680 Warning: Memory not freed: 280 on mangled query, LeakSanitizer: detected memory leaks +# +BEGIN NOT ATOMIC +IF SCALAR() expected_THEN_here; +END +$$ +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'expected_THEN_here; +END' at line 2 +BEGIN NOT ATOMIC +WHILE SCALAR() expected_DO_here; +END +$$ +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'expected_DO_here; +END' at line 2 +BEGIN NOT ATOMIC +REPEAT SELECT 1; UNTIL SCALAR() expected_END_here; +END +$$ +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'expected_END_here; +END' at line 2 diff --git a/mysql-test/main/sp-memory-leak.test b/mysql-test/main/sp-memory-leak.test new file mode 100644 index 00000000000..5b346aa8b10 --- /dev/null +++ b/mysql-test/main/sp-memory-leak.test @@ -0,0 +1,29 @@ +--echo # +--echo # MDEV-30680 Warning: Memory not freed: 280 on mangled query, LeakSanitizer: detected memory leaks +--echo # + +DELIMITER $$; +--error ER_PARSE_ERROR +BEGIN NOT ATOMIC + IF SCALAR() expected_THEN_here; +END +$$ +DELIMITER ;$$ + + +DELIMITER $$; +--error ER_PARSE_ERROR +BEGIN NOT ATOMIC + WHILE SCALAR() expected_DO_here; +END +$$ +DELIMITER ;$$ + + +DELIMITER $$; +--error ER_PARSE_ERROR +BEGIN NOT ATOMIC + REPEAT SELECT 1; UNTIL SCALAR() expected_END_here; +END +$$ +DELIMITER ;$$ diff --git a/sql/sp_head.h b/sql/sp_head.h index 12db9c485cf..700485ffd21 100644 --- a/sql/sp_head.h +++ b/sql/sp_head.h @@ -608,20 +608,24 @@ public: restore_lex(THD *thd) { DBUG_ENTER("sp_head::restore_lex"); + /* + There is no a need to free the current thd->lex here. + - In the majority of the cases restore_lex() is called + on success and thd->lex does not need to be deleted. + - In cases when restore_lex() is called on error, + e.g. from sp_create_assignment_instr(), thd->lex is + already linked to some sp_instr_xxx (using sp_lex_keeper). + + Note, we don't get to here in case of a syntax error + when the current thd->lex is not yet completely + initialized and linked. It gets automatically deleted + by the Bison %destructor in sql_yacc.yy. + */ LEX *oldlex= (LEX *) m_lex.pop(); if (!oldlex) DBUG_RETURN(false); // Nothing to restore - LEX *sublex= thd->lex; // This restores thd->lex and thd->stmt_lex - if (thd->restore_from_local_lex_to_old_lex(oldlex)) - DBUG_RETURN(true); - if (!sublex->sp_lex_in_use) - { - sublex->sphead= NULL; - lex_end(sublex); - delete sublex; - } - DBUG_RETURN(false); + DBUG_RETURN(thd->restore_from_local_lex_to_old_lex(oldlex)); } /** diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 257acebe9e0..7e0ca6d868d 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -485,11 +485,15 @@ bool sp_create_assignment_instr(THD *thd, bool no_lookahead, be deleted by the destructor ~sp_instr_xxx(). So we should remove "lex" from the stack sp_head::m_lex, to avoid double free. - Note, in case "lex" is not owned by any sp_instr_xxx, - it's also safe to remove it from the stack right now. - So we can remove it unconditionally, without testing lex->sp_lex_in_use. */ lex->sphead->restore_lex(thd); + /* + No needs for "delete lex" here: "lex" is already linked + to the sp_instr_stmt (using sp_lex_keeper) instance created by + the call for new_sp_instr_stmt() above. It will be freed + by ~sp_head/~sp_instr/~sp_lex_keeper during THD::end_statement(). + */ + DBUG_ASSERT(lex->sp_lex_in_use); // used by sp_instr_stmt return true; } enum_var_type inner_option_type= lex->option_type; @@ -6773,7 +6777,6 @@ bool LEX::sp_for_loop_implicit_cursor_statement(THD *thd, if (unlikely(!(bounds->m_index= new (thd->mem_root) sp_assignment_lex(thd, this)))) return true; - bounds->m_index->sp_lex_in_use= true; sphead->reset_lex(thd, bounds->m_index); DBUG_ASSERT(thd->lex != this); /* diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 59f21247445..1e8ea5c5c80 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -10454,6 +10454,17 @@ bool parse_sql(THD *thd, Parser_state *parser_state, ((thd->variables.sql_mode & MODE_ORACLE) ? ORAparse(thd) : MYSQLparse(thd)) != 0; + + if (mysql_parse_status) + { + /* + Restore the original LEX if it was replaced when parsing + a stored procedure. We must ensure that a parsing error + does not leave any side effects in the THD. + */ + LEX::cleanup_lex_after_parse_error(thd); + } + DBUG_ASSERT(opt_bootstrap || mysql_parse_status || thd->lex->select_stack_top == 0); thd->lex->current_select= thd->lex->first_select_lex(); diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index a5a39228776..601ff119fc3 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -98,7 +98,6 @@ int yylex(void *yylval, void *yythd); #define MYSQL_YYABORT \ do \ { \ - LEX::cleanup_lex_after_parse_error(thd); \ YYABORT; \ } while (0) @@ -149,13 +148,6 @@ static Item* escape(THD *thd) static void yyerror(THD *thd, const char *s) { - /* - Restore the original LEX if it was replaced when parsing - a stored procedure. We must ensure that a parsing error - does not leave any side effects in the THD. - */ - LEX::cleanup_lex_after_parse_error(thd); - /* "parse error" changed into "syntax error" between bison 1.75 and 1.875 */ if (strcmp(s,"parse error") == 0 || strcmp(s,"syntax error") == 0) s= ER_THD(thd, ER_SYNTAX_ERROR); @@ -1521,6 +1513,19 @@ bool my_yyoverflow(short **a, YYSTYPE **b, size_t *yystacksize); %type expr_lex +%destructor +{ + /* + In case of a syntax/oom error let's free the sp_expr_lex + instance, but only if it has not been linked to any structures + such as sp_instr_jump_if_not::m_lex_keeper yet, e.g.: + IF f1() THEN1 + i.e. THEN1 came instead of the expected THEN causing a syntax error. + */ + if (!$$->sp_lex_in_use) + delete $$; +} + %type assignment_source_lex assignment_source_expr @@ -3806,7 +3811,6 @@ expr_lex: expr { $$= $1; - $$->sp_lex_in_use= true; $$->set_item($2); Lex->pop_select(); //min select if (Lex->check_cte_dependencies_and_resolve_references()) @@ -3838,7 +3842,6 @@ assignment_source_expr: { DBUG_ASSERT($1 == thd->lex); $$= $1; - $$->sp_lex_in_use= true; $$->set_item_and_free_list($3, thd->free_list); thd->free_list= NULL; Lex->pop_select(); //min select @@ -3859,7 +3862,6 @@ for_loop_bound_expr: { DBUG_ASSERT($1 == thd->lex); $$= $1; - $$->sp_lex_in_use= true; $$->set_item_and_free_list($3, NULL); Lex->pop_select(); //main select if (unlikely($$->sphead->restore_lex(thd))) From fdab2c4c648cd3f2db524ef7534843a9d29c4f4f Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Thu, 29 Jun 2023 17:30:02 +0400 Subject: [PATCH 4/6] MDEV-31578 DECLARE CURSOR: "Memory not freed: 280 bytes lost" on syntax error When CURSOR parameters get parsed, their sp_assignment_lex instances (one instance per parameter) get collected to List. These instances get linked to sphead only in the end of the list. If a syntax error happened in the middle of the parameter list, these instances were not deleted, which caused memory leaks. Fix: using a Bison %destructor to free rules of the type (on syntax errors). Afte the fix these sp_assignment_lex instances from CURSOR parameters deleted as follows: - If the CURSOR statement was fully parsed, then these instances get properly linked to sp_head structures, so they are deleted during ~sp_head (this did not change) - If the CURSOR statement failed on a syntax error, then by Bison's %destructor (this is being added in the current patch). --- mysql-test/main/sp-memory-leak.result | 21 +++++++++++++++++++++ mysql-test/main/sp-memory-leak.test | 25 +++++++++++++++++++++++++ sql/sql_lex.h | 6 +++++- sql/sql_yacc.yy | 15 +++++++++++++++ 4 files changed, 66 insertions(+), 1 deletion(-) diff --git a/mysql-test/main/sp-memory-leak.result b/mysql-test/main/sp-memory-leak.result index aea278801d8..37a0f119341 100644 --- a/mysql-test/main/sp-memory-leak.result +++ b/mysql-test/main/sp-memory-leak.result @@ -19,3 +19,24 @@ END $$ ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'expected_END_here; END' at line 2 +# +# MDEV-31578 DECLARE CURSOR: "Memory not freed: 280 bytes lost" on syntax error +# +BEGIN NOT ATOMIC +DECLARE cur CURSOR (a INT) FOR SELECT a+1; +OPEN cur(sp_followed_by_syntax_error(); +CLOSE cur; +END; +$$ +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '; +CLOSE cur; +END' at line 3 +BEGIN NOT ATOMIC +DECLARE cur CURSOR (a INT) FOR SELECT a+1; +OPEN cur(1,sp_followed_by_syntax_error(); +CLOSE cur; +END; +$$ +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '; +CLOSE cur; +END' at line 3 diff --git a/mysql-test/main/sp-memory-leak.test b/mysql-test/main/sp-memory-leak.test index 5b346aa8b10..0035044209a 100644 --- a/mysql-test/main/sp-memory-leak.test +++ b/mysql-test/main/sp-memory-leak.test @@ -27,3 +27,28 @@ BEGIN NOT ATOMIC END $$ DELIMITER ;$$ + + +--echo # +--echo # MDEV-31578 DECLARE CURSOR: "Memory not freed: 280 bytes lost" on syntax error +--echo # + +DELIMITER $$; +--error ER_PARSE_ERROR +BEGIN NOT ATOMIC + DECLARE cur CURSOR (a INT) FOR SELECT a+1; + OPEN cur(sp_followed_by_syntax_error(); + CLOSE cur; +END; +$$ +DELIMITER ;$$ + +DELIMITER $$; +--error ER_PARSE_ERROR +BEGIN NOT ATOMIC + DECLARE cur CURSOR (a INT) FOR SELECT a+1; + OPEN cur(1,sp_followed_by_syntax_error(); + CLOSE cur; +END; +$$ +DELIMITER ;$$ diff --git a/sql/sql_lex.h b/sql/sql_lex.h index 2e3ce7d7503..5538fdd04e6 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -3429,7 +3429,11 @@ public: sp_head *sphead; sp_name *spname; bool sp_lex_in_use; // Keep track on lex usage in SPs for error handling - + void delete_if_not_sp_lex_in_use() + { + if (!sp_lex_in_use) + delete this; + } sp_pcontext *spcont; st_sp_chistics sp_chistics; diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 601ff119fc3..524da7be4c3 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -1535,6 +1535,21 @@ bool my_yyoverflow(short **a, YYSTYPE **b, size_t *yystacksize); cursor_actual_parameters opt_parenthesized_cursor_actual_parameters +%destructor +{ + if ($$) + { + sp_assignment_lex *elem; + List_iterator li(*$$); + while ((elem= li++)) + { + if (!elem->sp_lex_in_use) + delete elem; + } + } +} + + %type option_type opt_var_type opt_var_ident_type From 5f2a77cef1cced322d3a6e6a48f4f4e5480283dc Mon Sep 17 00:00:00 2001 From: Daniel Black Date: Sat, 10 Jun 2023 08:35:58 +1000 Subject: [PATCH 5/6] MDEV-31268 Fedora MariaDB-shared replaces mariadb-connector-c file /usr/lib64/libmariadb.so.3 from install of MariaDB-shared-10.11.3-1.fc38.x86_64 conflicts with file from package mariadb-connector-c-3.3.5-1.fc38.x86_64 --- cmake/cpack_rpm.cmake | 1 + 1 file changed, 1 insertion(+) diff --git a/cmake/cpack_rpm.cmake b/cmake/cpack_rpm.cmake index 7b1f0b0ff87..3cc24375c06 100644 --- a/cmake/cpack_rpm.cmake +++ b/cmake/cpack_rpm.cmake @@ -291,6 +291,7 @@ ENDIF() # MDEV-24629, we need it outside of ELSIFs IF(RPM MATCHES "fedora") ALTERNATIVE_NAME("common" "mariadb-connector-c-config" ${MARIADB_CONNECTOR_C_VERSION}-1) + ALTERNATIVE_NAME("shared" "mariadb-connector-c" ${MARIADB_CONNECTOR_C_VERSION}-1) ENDIF() SET(PYTHON_SHEBANG "/usr/bin/python3" CACHE STRING "python shebang") From bd7908e6ace1edce3530a0aa663ec3527e53c8e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 5 Jul 2023 15:15:04 +0300 Subject: [PATCH 6/6] MDEV-31568 InnoDB protection against dual processes accessing data insufficient fil_node_open_file_low(): Always acquire an advisory lock on the system tablespace. Originally, we already did this in SysTablespace::open_file(), but SysTablespace::open_or_create() would release those locks when it is closing the file handles. This is a 10.5+ specific follow up to commit 0ee1082bd2e7e7049c4f0e686bad53cf7ba053ab (MDEV-28495). Thanks to Daniel Black for verifying this bug. --- storage/innobase/fil/fil0fil.cc | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc index c75144413ac..725345957ee 100644 --- a/storage/innobase/fil/fil0fil.cc +++ b/storage/innobase/fil/fil0fil.cc @@ -391,8 +391,16 @@ static bool fil_node_open_file_low(fil_node_t *node) : OS_FILE_OPEN | OS_FILE_ON_ERROR_NO_EXIT, OS_FILE_AIO, type, srv_read_only_mode, &success); - if (success) + if (node->is_open()) + { + ut_ad(success); +#ifndef _WIN32 + if (!node->space->id && !srv_read_only_mode && my_disable_locking && + os_file_lock(node->handle, node->name)) + goto fail; +#endif break; + } /* The following call prints an error message */ if (os_file_get_last_error(true) == EMFILE + 100 && @@ -406,6 +414,9 @@ static bool fil_node_open_file_low(fil_node_t *node) if (node->size); else if (!node->read_page0() || !fil_comp_algo_validate(node->space)) { +#ifndef _WIN32 + fail: +#endif os_file_close(node->handle); node->handle= OS_FILE_CLOSED; return false;