From aab7ab7dee66430b0448e392b505b95ed22f2a2f Mon Sep 17 00:00:00 2001 From: Timofey Turenko Date: Tue, 29 Jan 2019 12:06:33 +0200 Subject: [PATCH 1/9] add core dump configiration line to maxscale.service --- etc/maxscale.service.in | 1 + 1 file changed, 1 insertion(+) diff --git a/etc/maxscale.service.in b/etc/maxscale.service.in index bc2c94838..f9a9b9d11 100644 --- a/etc/maxscale.service.in +++ b/etc/maxscale.service.in @@ -5,6 +5,7 @@ After=network.target [Service] Type=forking Restart=on-abort +LimitCORE=infinity # Make sure /var/run/maxscale exists PermissionsStartOnly=true From da6d358f9fee7cb7302f082f179c45f581e681d3 Mon Sep 17 00:00:00 2001 From: Timofey Turenko Date: Tue, 29 Jan 2019 14:14:46 +0200 Subject: [PATCH 2/9] add systemd-dev package to the build env --- BUILD/install_build_deps.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/BUILD/install_build_deps.sh b/BUILD/install_build_deps.sh index df67add78..e64fa50b6 100755 --- a/BUILD/install_build_deps.sh +++ b/BUILD/install_build_deps.sh @@ -18,6 +18,11 @@ then uuid-dev libsqlite3-dev liblzma-dev libpam0g-dev pkg-config \ libedit-dev + # One of these will work, older systems use libsystemd-daemon-dev + sudo apt-get install -y libsystemd-dev || \ + sudo apt-get install -y libsystemd-daemon-dev + + ## separatelibgnutls installation process for Ubuntu Trusty cat /etc/*release | grep -E "Trusty|wheezy" if [ $? == 0 ] @@ -68,9 +73,14 @@ else sqlite sqlite-devel pkgconfig lua lua-devel rpm-build createrepo yum-utils \ gnutls-devel libgcrypt-devel pam-devel + # Attempt to install libasan, it'll only work on CentOS 7 sudo yum install -y --nogpgcheck libasan + + # Attempt to install systemd-devel, doesn't work on CentOS 6 + sudo yum install -y systemd-devel + cat /etc/redhat-release | grep "release 5" if [ $? == 0 ] then From 45f85a2740a1d2fd234c9285a97b5c861d871246 Mon Sep 17 00:00:00 2001 From: Timofey Turenko Date: Wed, 30 Jan 2019 18:45:24 +0200 Subject: [PATCH 3/9] add prlimit command to service start --- etc/maxscale.service.in | 1 + 1 file changed, 1 insertion(+) diff --git a/etc/maxscale.service.in b/etc/maxscale.service.in index f9a9b9d11..daa8bb81e 100644 --- a/etc/maxscale.service.in +++ b/etc/maxscale.service.in @@ -10,6 +10,7 @@ LimitCORE=infinity # Make sure /var/run/maxscale exists PermissionsStartOnly=true ExecStartPre=/usr/bin/install -d @MAXSCALE_VARDIR@/run/maxscale -o maxscale -g maxscale +ExecStartPost=prlimit -p $(pidof maxscale) --core=unlimited PIDFile=@MAXSCALE_VARDIR@/run/maxscale/maxscale.pid From f0abe9cbdc149c42cf17beec93c756e74c3b119b Mon Sep 17 00:00:00 2001 From: Timofey Turenko Date: Wed, 30 Jan 2019 23:31:45 +0200 Subject: [PATCH 4/9] add /bin/sh before prlimit command in service start --- etc/maxscale.service.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/etc/maxscale.service.in b/etc/maxscale.service.in index daa8bb81e..068bb342e 100644 --- a/etc/maxscale.service.in +++ b/etc/maxscale.service.in @@ -10,7 +10,7 @@ LimitCORE=infinity # Make sure /var/run/maxscale exists PermissionsStartOnly=true ExecStartPre=/usr/bin/install -d @MAXSCALE_VARDIR@/run/maxscale -o maxscale -g maxscale -ExecStartPost=prlimit -p $(pidof maxscale) --core=unlimited +ExecStartPost=/bin/sh -c 'prlimit -p $(pidof maxscale) --core=unlimited' PIDFile=@MAXSCALE_VARDIR@/run/maxscale/maxscale.pid From abaa22898779eb7c36a673cd8ab45f6be73aecdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 31 Jan 2019 12:56:17 +0200 Subject: [PATCH 5/9] Add extra definitions to debug build service files The unlimited core dumps are now only enabled for debug builds. --- cmake/init_scripts.cmake | 9 +++++++++ etc/maxscale.service.in | 3 +-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/cmake/init_scripts.cmake b/cmake/init_scripts.cmake index 83f8dd138..1ab095eba 100644 --- a/cmake/init_scripts.cmake +++ b/cmake/init_scripts.cmake @@ -25,6 +25,15 @@ else() endif() configure_file(${CMAKE_SOURCE_DIR}/etc/maxscale.conf.in ${CMAKE_BINARY_DIR}/maxscale.conf @ONLY) + +# The systemd service file +if (CMAKE_BUILD_TYPE MATCHES "(D|d)(E|e)(B|b)(U|u)(G|g)") + # Options enabled only in debug builds (a literal multi-line string) + set(SERVICE_FILE_DEBUG_OPTIONS + "LimitCORE=infinity +ExecStartPost=/bin/sh -c 'prlimit -p $(pidof maxscale) --core=unlimited'") +endif() + configure_file(${CMAKE_SOURCE_DIR}/etc/maxscale.service.in ${CMAKE_BINARY_DIR}/maxscale.service @ONLY) if(PACKAGE) diff --git a/etc/maxscale.service.in b/etc/maxscale.service.in index 068bb342e..7c087e6f2 100644 --- a/etc/maxscale.service.in +++ b/etc/maxscale.service.in @@ -5,12 +5,11 @@ After=network.target [Service] Type=forking Restart=on-abort -LimitCORE=infinity +@SERVICE_FILE_DEBUG_OPTIONS@ # Make sure /var/run/maxscale exists PermissionsStartOnly=true ExecStartPre=/usr/bin/install -d @MAXSCALE_VARDIR@/run/maxscale -o maxscale -g maxscale -ExecStartPost=/bin/sh -c 'prlimit -p $(pidof maxscale) --core=unlimited' PIDFile=@MAXSCALE_VARDIR@/run/maxscale/maxscale.pid From a9f6955fc89bdf6b2ae752a891e5b306e2a32e6a Mon Sep 17 00:00:00 2001 From: Timofey Turenko Date: Thu, 31 Jan 2019 14:07:52 +0200 Subject: [PATCH 6/9] Revert "add systemd-dev package to the build env" This reverts commit da6d358f9fee7cb7302f082f179c45f581e681d3. --- BUILD/install_build_deps.sh | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/BUILD/install_build_deps.sh b/BUILD/install_build_deps.sh index e64fa50b6..df67add78 100755 --- a/BUILD/install_build_deps.sh +++ b/BUILD/install_build_deps.sh @@ -18,11 +18,6 @@ then uuid-dev libsqlite3-dev liblzma-dev libpam0g-dev pkg-config \ libedit-dev - # One of these will work, older systems use libsystemd-daemon-dev - sudo apt-get install -y libsystemd-dev || \ - sudo apt-get install -y libsystemd-daemon-dev - - ## separatelibgnutls installation process for Ubuntu Trusty cat /etc/*release | grep -E "Trusty|wheezy" if [ $? == 0 ] @@ -73,14 +68,9 @@ else sqlite sqlite-devel pkgconfig lua lua-devel rpm-build createrepo yum-utils \ gnutls-devel libgcrypt-devel pam-devel - # Attempt to install libasan, it'll only work on CentOS 7 sudo yum install -y --nogpgcheck libasan - - # Attempt to install systemd-devel, doesn't work on CentOS 6 - sudo yum install -y systemd-devel - cat /etc/redhat-release | grep "release 5" if [ $? == 0 ] then From cc6665c732114248aa39343c41fd7d826384459e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 6 Feb 2019 13:03:06 +0200 Subject: [PATCH 7/9] Add solo mode to compare In this mode the compare program does a sanity check where it compares the output of a classifier when a statement is classified multiple times. The main use-case for this is to get the verbose output generated when the -v3 option is added, not the sanity check itself. --- query_classifier/test/CMakeLists.txt | 24 +++++----- query_classifier/test/compare.cc | 67 ++++++++++++++++++---------- 2 files changed, 56 insertions(+), 35 deletions(-) diff --git a/query_classifier/test/CMakeLists.txt b/query_classifier/test/CMakeLists.txt index 5310121b4..0f2b69609 100644 --- a/query_classifier/test/CMakeLists.txt +++ b/query_classifier/test/CMakeLists.txt @@ -17,24 +17,26 @@ if (BUILD_QC_MYSQLEMBEDDED) file(COPY ${ERRMSG} DESTINATION ${CMAKE_CURRENT_BINARY_DIR}) endif() endif() +endif() - add_executable(classify classify.c) - target_link_libraries(classify maxscale-common) +add_executable(classify classify.c) +target_link_libraries(classify maxscale-common) - add_executable(compare compare.cc testreader.cc) - target_link_libraries(compare maxscale-common) +add_executable(compare compare.cc testreader.cc) +target_link_libraries(compare maxscale-common) - add_executable(qc_cache qc_cache.cc) - target_link_libraries(qc_cache maxscale-common) +add_executable(qc_cache qc_cache.cc) +target_link_libraries(qc_cache maxscale-common) - add_executable(version_sensitivity version_sensitivity.cc) - target_link_libraries(version_sensitivity maxscale-common) +add_executable(version_sensitivity version_sensitivity.cc) +target_link_libraries(version_sensitivity maxscale-common) - add_executable(crash_qc_sqlite crash_qc_sqlite.cc) - target_link_libraries(crash_qc_sqlite maxscale-common) +add_executable(crash_qc_sqlite crash_qc_sqlite.cc) +target_link_libraries(crash_qc_sqlite maxscale-common) - add_test(TestQC_Crash_qcsqlite crash_qc_sqlite) +add_test(TestQC_Crash_qcsqlite crash_qc_sqlite) +if (BUILD_QC_MYSQLEMBEDDED) # TestQC_MySQLEmbedded excluded, classify is now solely used for verifying the # functionality of qc_sqlite. #add_test(TestQC_MySQLEmbedded classify qc_mysqlembedded ${CMAKE_CURRENT_SOURCE_DIR}/input.sql ${CMAKE_CURRENT_SOURCE_DIR}/expected.sql) diff --git a/query_classifier/test/compare.cc b/query_classifier/test/compare.cc index 32ca53b4c..a90e077b7 100644 --- a/query_classifier/test/compare.cc +++ b/query_classifier/test/compare.cc @@ -20,7 +20,6 @@ #include #include #include -#include #include #include #include @@ -48,10 +47,11 @@ namespace { char USAGE[] = - "usage: compare [-r count] [-d] [-1 classfier1] [-2 classifier2] " + "usage: compare [-r count] [-d] [-0 classifier] [-1 classfier1] [-2 classifier2] " "[-A args] [-B args] [-C args] [-m [default|oracle]] [-v [0..2]] [-s statement]|[file]]\n\n" "-r redo the test the specified number of times; 0 means forever, default is 1\n" "-d don't stop after first failed query\n" + "-0 sanity check mode, compares the statement twice with the same classifier\n" "-1 the first classifier, default 'qc_mysqlembedded'\n" "-2 the second classifier, default 'qc_sqlite'\n" "-A arguments for the first classifier\n" @@ -172,17 +172,22 @@ QUERY_CLASSIFIER* load_classifier(const char* name) QUERY_CLASSIFIER* get_classifier(const char* zName, qc_sql_mode_t sql_mode, const char* zArgs) { - QUERY_CLASSIFIER* pClassifier = load_classifier(zName); + QUERY_CLASSIFIER* pClassifier = nullptr; - if (pClassifier) + if (zName) { - if (pClassifier->qc_setup(sql_mode, zArgs) != QC_RESULT_OK - || pClassifier->qc_process_init() != QC_RESULT_OK - || pClassifier->qc_thread_init() != QC_RESULT_OK) + pClassifier = load_classifier(zName); + + if (pClassifier) { - cerr << "error: Could not setup or init classifier " << zName << "." << endl; - qc_unload(pClassifier); - pClassifier = 0; + if (pClassifier->qc_setup(sql_mode, zArgs) != QC_RESULT_OK + || pClassifier->qc_process_init() != QC_RESULT_OK + || pClassifier->qc_thread_init() != QC_RESULT_OK) + { + cerr << "error: Could not setup or init classifier " << zName << "." << endl; + qc_unload(pClassifier); + pClassifier = 0; + } } } @@ -209,21 +214,18 @@ bool get_classifiers(qc_sql_mode_t sql_mode, bool rc = false; QUERY_CLASSIFIER* pClassifier1 = get_classifier(zName1, sql_mode, zArgs1); + QUERY_CLASSIFIER* pClassifier2 = get_classifier(zName2, sql_mode, zArgs2); - if (pClassifier1) + if ((!zName1 || pClassifier1) && (!zName2 || pClassifier2)) { - QUERY_CLASSIFIER* pClassifier2 = get_classifier(zName2, sql_mode, zArgs2); - - if (pClassifier2) - { - *ppClassifier1 = pClassifier1; - *ppClassifier2 = pClassifier2; - rc = true; - } - else - { - put_classifier(pClassifier1); - } + *ppClassifier1 = pClassifier1; + *ppClassifier2 = pClassifier2; + rc = true; + } + else + { + put_classifier(pClassifier1); + put_classifier(pClassifier2); } return rc; @@ -1494,11 +1496,12 @@ int main(int argc, char* argv[]) #endif const char* zStatement = NULL; qc_sql_mode_t sql_mode = QC_SQL_MODE_DEFAULT; + bool solo = false; size_t rounds = 1; int v = VERBOSITY_NORMAL; int c; - while ((c = getopt(argc, argv, "r:d1:2:v:A:B:C:m:s:SR")) != -1) + while ((c = getopt(argc, argv, "r:d0:1:2:v:A:B:C:m:s:SR")) != -1) { switch (c) { @@ -1510,6 +1513,12 @@ int main(int argc, char* argv[]) v = atoi(optarg); break; + case '0': + zClassifier1 = optarg; + zClassifier2 = nullptr; + solo = true; + break; + case '1': zClassifier1 = optarg; break; @@ -1601,6 +1610,11 @@ int main(int argc, char* argv[]) size_t round = 0; bool terminate = false; + if (solo) + { + pClassifier2 = pClassifier1; + } + pClassifier1->qc_set_server_version(version); pClassifier2->qc_set_server_version(version); @@ -1649,6 +1663,11 @@ int main(int argc, char* argv[]) } while (!terminate && ((rounds == 0) || (round < rounds))); + if (solo) + { + pClassifier2 = nullptr; + } + put_classifiers(pClassifier1, pClassifier2); cout << "\n"; From 08a05d3ab95a630e35cbdd16a2a7cef066ca6e92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 6 Feb 2019 14:10:19 +0200 Subject: [PATCH 8/9] Fix use of uninitialized variables Building with optimization in debug mode revealed code that could in theory result in undefined behavior. --- include/maxscale/router.hh | 2 +- server/modules/filter/cache/test/testkeygeneration.cc | 2 +- server/modules/filter/hintfilter/hintparser.cc | 4 +++- server/modules/filter/tpmfilter/tpmfilter.cc | 2 +- .../protocol/MySQL/mariadbclient/setsqlmodeparser.hh | 2 +- server/modules/routing/avrorouter/avro_schema.cc | 3 ++- server/modules/routing/binlogrouter/blr_master.cc | 6 +++--- 7 files changed, 12 insertions(+), 9 deletions(-) diff --git a/include/maxscale/router.hh b/include/maxscale/router.hh index 278cfc40f..b4eb1a5e8 100644 --- a/include/maxscale/router.hh +++ b/include/maxscale/router.hh @@ -149,7 +149,7 @@ public: static MXS_ROUTER_SESSION* newSession(MXS_ROUTER* pInstance, MXS_SESSION* pSession) { RouterType* pRouter = static_cast(pInstance); - RouterSessionType* pRouter_session; + RouterSessionType* pRouter_session = nullptr; MXS_EXCEPTION_GUARD(pRouter_session = pRouter->newSession(pSession)); diff --git a/server/modules/filter/cache/test/testkeygeneration.cc b/server/modules/filter/cache/test/testkeygeneration.cc index f823e5c6e..b9ab5bec9 100644 --- a/server/modules/filter/cache/test/testkeygeneration.cc +++ b/server/modules/filter/cache/test/testkeygeneration.cc @@ -123,7 +123,7 @@ int test(StorageFactory& factory, istream& in) int main(int argc, char* argv[]) { int rv = EXIT_FAILURE; - StorageFactory* pFactory; + StorageFactory* pFactory = nullptr; if ((argc == 2) || (argc == 3)) { char* libdir = MXS_STRDUP("../../../../../query_classifier/qc_sqlite/"); diff --git a/server/modules/filter/hintfilter/hintparser.cc b/server/modules/filter/hintfilter/hintparser.cc index e302eaa02..09a06f14d 100644 --- a/server/modules/filter/hintfilter/hintparser.cc +++ b/server/modules/filter/hintfilter/hintparser.cc @@ -131,7 +131,9 @@ HINT* hint_parser(HINT_SESSION* session, GWBUF* request) int len, residual, state; int found, escape, quoted, squoted; HINT* rval = NULL; - char* pname, * lvalue, * hintname = NULL; + char* pname = nullptr; + char* lvalue = nullptr; + char* hintname = nullptr; GWBUF* buf; HINT_TOKEN* tok; HINT_MODE mode = HM_EXECUTE; diff --git a/server/modules/filter/tpmfilter/tpmfilter.cc b/server/modules/filter/tpmfilter/tpmfilter.cc index 711e81779..7ec410382 100644 --- a/server/modules/filter/tpmfilter/tpmfilter.cc +++ b/server/modules/filter/tpmfilter/tpmfilter.cc @@ -740,7 +740,7 @@ static void destroyInstance(MXS_FILTER* instance) static void checkNamedPipe(TPM_INSTANCE* inst) { - int ret; + int ret = 0; char buffer[2]; char buf[4096]; char* named_pipe = inst->named_pipe; diff --git a/server/modules/protocol/MySQL/mariadbclient/setsqlmodeparser.hh b/server/modules/protocol/MySQL/mariadbclient/setsqlmodeparser.hh index a5a86412f..635469358 100644 --- a/server/modules/protocol/MySQL/mariadbclient/setsqlmodeparser.hh +++ b/server/modules/protocol/MySQL/mariadbclient/setsqlmodeparser.hh @@ -487,7 +487,7 @@ private: { result_t rv = IS_SET_SQL_MODE; - char c; + char c = *pSql_mode; do { diff --git a/server/modules/routing/avrorouter/avro_schema.cc b/server/modules/routing/avrorouter/avro_schema.cc index e0e95eff9..54eab9e67 100644 --- a/server/modules/routing/avrorouter/avro_schema.cc +++ b/server/modules/routing/avrorouter/avro_schema.cc @@ -58,7 +58,8 @@ bool json_extract_field_names(const char* filename, std::vector& columns bool rval = false; json_error_t err; err.text[0] = '\0'; - json_t* obj, * arr; + json_t* obj; + json_t* arr = nullptr; if ((obj = json_load_file(filename, 0, &err)) && (arr = json_object_get(obj, "fields"))) { diff --git a/server/modules/routing/binlogrouter/blr_master.cc b/server/modules/routing/binlogrouter/blr_master.cc index d06ca8948..cb081de60 100644 --- a/server/modules/routing/binlogrouter/blr_master.cc +++ b/server/modules/routing/binlogrouter/blr_master.cc @@ -3053,17 +3053,17 @@ void blr_handle_fake_gtid_list(ROUTER_INSTANCE* router, static bool blr_handle_missing_files(ROUTER_INSTANCE* router, char* new_file) { - char* fptr; + char* fptr = strrchr(new_file, '.'); uint32_t new_fseqno; uint32_t curr_fseqno; char buf[BLRM_BINLOG_NAME_STR_LEN]; char bigbuf[PATH_MAX + 1]; - if (*new_file - && (fptr = strrchr(new_file, '.')) == NULL) + if (fptr == NULL) { return false; } + if (router->fileroot) { MXS_FREE(router->fileroot); From 994bfcd2858eadf8317c883971c6ef1042887c33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 6 Feb 2019 22:36:12 +0200 Subject: [PATCH 9/9] MXS-2268: Fix undefined behavior Using a void return value as an integer results in undefined behavior. apparently in this case it doesn't translate into a crash and instead only manifests itself when all the planets align. --- query_classifier/qc_sqlite/qc_sqlite.cc | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/query_classifier/qc_sqlite/qc_sqlite.cc b/query_classifier/qc_sqlite/qc_sqlite.cc index 55755d515..b993c8361 100644 --- a/query_classifier/qc_sqlite/qc_sqlite.cc +++ b/query_classifier/qc_sqlite/qc_sqlite.cc @@ -3282,7 +3282,7 @@ extern void maxscale_update_function_info(const char* name, const Expr* pExpr); // 'unsigned int' and not 'uint32_t' because 'uint32_t' is unknown in sqlite3 context. extern void maxscale_set_type_mask(unsigned int type_mask); -extern void maxscaleComment(); +extern int maxscaleComment(); extern int maxscaleKeyword(int token); extern int maxscaleTranslateKeyword(int token); @@ -4080,14 +4080,18 @@ void maxscaleCreateSequence(Parse* pParse, Token* pDatabase, Token* pTable) QC_EXCEPTION_GUARD(pInfo->maxscaleCreateSequence(pParse, pDatabase, pTable)); } -void maxscaleComment() +int maxscaleComment() { QC_TRACE(); QcSqliteInfo* pInfo = this_thread.pInfo; ss_dassert(pInfo); - QC_EXCEPTION_GUARD(pInfo->maxscaleComment()); + int rc = 0; + + QC_EXCEPTION_GUARD(rc = pInfo->maxscaleComment()); + + return rc; } void maxscaleDeclare(Parse* pParse)