Details

      Description

      MemorySanitizer is a compile-time instrumentation layer in clang and GCC. Together with AddressSanitizer mostly makes the run-time instrumentation of Valgrind redundant. It is a little more tricky to set up, because running with uninstrumented libraries will lead into false positives.

      Creating MSAN-instrumented libc++

      cd /mariadb
      sudo apt source libc++-8-dev
      cd llvm-toolchain-8-8.0.1
      mkdir libc++msan; cd libc++msan
      cmake ../libcxx -DCMAKE_BUILD_TYPE=Release -DLLVM_USE_SANITIZER=Memory -DCMAKE_C_COMPILER=clang-8 -DCMAKE_CXX_COMPILER=clang++-8
      

      Introduce an option WITH_MSAN

      patch -p1<<'EOF'
      diff --git a/CMakeLists.txt b/CMakeLists.txt
      index 33b69a9a1e3..ed61853bbc5 100644
      --- a/CMakeLists.txt
      +++ b/CMakeLists.txt
      @@ -236,6 +236,11 @@ IF (WITH_UBSAN)
         MY_CHECK_AND_SET_COMPILER_FLAG("-fsanitize=undefined -fno-sanitize=alignment -U_FORTIFY_SOURCE" DEBUG RELWITHDEBINFO)
       ENDIF()
       
      +OPTION(WITH_MSAN "Enable memory sanitizer" OFF)
      +IF (WITH_MSAN)
      +  MY_CHECK_AND_SET_COMPILER_FLAG("-fsanitize=memory -U_FORTIFY_SOURCE" DEBUG RELWITHDEBINFO)
      +ENDIF()
      +
       IF(NOT WITH_TSAN)
         # enable security hardening features, like most distributions do
         # in our benchmarks that costs about ~1% of performance, depending on the load
      diff --git a/cmake/plugin.cmake b/cmake/plugin.cmake
      index 89dfdbb306b..3582f8ac005 100644
      --- a/cmake/plugin.cmake
      +++ b/cmake/plugin.cmake
      @@ -223,7 +223,7 @@ MACRO(MYSQL_ADD_PLUGIN)
             ELSEIF(NOT CMAKE_SYSTEM_NAME STREQUAL "Linux")
               TARGET_LINK_LIBRARIES (${target} mysqld)
             ENDIF()
      -    ELSEIF(CMAKE_SYSTEM_NAME STREQUAL "Linux" AND NOT WITH_ASAN AND NOT WITH_TSAN AND NOT WITH_UBSAN)
      +    ELSEIF(CMAKE_SYSTEM_NAME STREQUAL "Linux" AND NOT WITH_ASAN AND NOT WITH_TSAN AND NOT WITH_UBSAN AND NOT WITH_MSAN)
             TARGET_LINK_LIBRARIES (${target} "-Wl,--no-undefined")
           ENDIF()
       
      diff --git a/libmariadb/libmariadb/CMakeLists.txt b/libmariadb/libmariadb/CMakeLists.txt
      index 9581461..8ba9c32 100644
      --- a/libmariadb/libmariadb/CMakeLists.txt
      +++ b/libmariadb/libmariadb/CMakeLists.txt
      @@ -412,7 +412,7 @@ ENDIF()
       
       IF(CMAKE_SYSTEM_NAME MATCHES "Linux" OR
          CMAKE_SYSTEM_NAME MATCHES "GNU")
      -  IF (NOT WITH_ASAN AND NOT WITH_TSAN)
      +  IF (NOT WITH_ASAN AND NOT WITH_TSAN AND NOT WITH_MSAN)
           TARGET_LINK_LIBRARIES (libmariadb "-Wl,--no-undefined")
         ENDIF()
         SET_TARGET_PROPERTIES(libmariadb PROPERTIES LINK_FLAGS "${CC_BINARY_DIR}/libmariadb/mariadbclient.def")
      EOF
      

      Compile with libc++ (instead of libstdc++) and bundled libraries

      mkdir build; cd build
      cmake -DWITH_MSAN=ON -DWITH_SSL=bundled -DWITH_ZLIB=bundled -DCMAKE_CXX_FLAGS='-stdlib=libc++' ..
      make -j$(nproc)
      

      Run tests with the instrumented libc++

      cd mysql-test
      LD_LIBRARY_PATH=/mariadb/llvm-toolchain-8-8.0.1/libc++msan/lib ./mtr main.1st
      LD_LIBRARY_PATH=/mariadb/llvm-toolchain-8-8.0.1/libc++msan/lib MSAN_OPTIONS=abort_on_error=1 ./mtr --big-test --parallel=auto --force --retry=0
      

      Problems found so far

      For some reason, getservbyname() claims that the buffer is uninitialized. Maybe we need an instrumented library? Workaround: comment out the calls:

      diff --git a/sql/mysqld.cc b/sql/mysqld.cc
      index 695616f9269..f1df81c9095 100644
      --- a/sql/mysqld.cc
      +++ b/sql/mysqld.cc
      @@ -2171,7 +2171,7 @@ static void set_ports()
             line options.
           */
       
      -#if MYSQL_PORT_DEFAULT == 0
      +#if 0 // MYSQL_PORT_DEFAULT == 0
           struct  servent *serv_ptr;
           if ((serv_ptr= getservbyname("mysql", "tcp")))
             SYSVAR_AUTOSIZE(mysqld_port, ntohs((u_short) serv_ptr->s_port));
      diff --git a/libmariadb/mariadb_lib.c b/libmariadb/mariadb_lib.c
      index d43b68c..fb6236f 100644
      --- a/libmariadb/mariadb_lib.c
      +++ b/libmariadb/mariadb_lib.c
      @@ -3539,12 +3539,16 @@ static void mysql_once_init()
         }
         if (!mysql_port)
         {
      +#if 0
           struct servent *serv_ptr;
      +#endif
           char *env;
       
           mysql_port = MARIADB_PORT;
      +#if 0
           if ((serv_ptr = getservbyname("mysql", "tcp")))
             mysql_port = (uint)ntohs((ushort)serv_ptr->s_port);
      +#endif
           if ((env = getenv("MYSQL_TCP_PORT")))
             mysql_port =(uint)atoi(env);
         }
      

      Inline assembler code leads to bogus claims about uninitialized memory. Compiler built-ins or intrinsic functions seem to work correctly. Alas, we will have to add some compile-time options to allow these instructions to be emitted. I used -march=native as a quick hack:

      diff --git a/extra/wolfssl/wolfssl/wolfcrypt/src/random.c b/extra/wolfssl/wolfssl/wolfcrypt/src/random.c
      index 6b0d5dafc..817619d16 100644
      --- a/extra/wolfssl/wolfssl/wolfcrypt/src/random.c
      +++ b/extra/wolfssl/wolfssl/wolfcrypt/src/random.c
      @@ -173,7 +173,7 @@ int wc_RNG_GenerateByte(WC_RNG* rng, byte* b)
           static int wc_GenerateRand_IntelRD(OS_Seed* os, byte* output, word32 sz);
           #endif
       
      -#ifdef USE_WINDOWS_API
      +#if 1 /*def USE_WINDOWS_API */
           #include <immintrin.h>
       #endif /* USE_WINDOWS_API */
       #endif
      @@ -1282,7 +1282,7 @@ int wc_FreeNetRandom(void)
       
       #ifdef HAVE_INTEL_RDSEED
       
      -#ifndef USE_WINDOWS_API
      +#if 0/*ndef USE_WINDOWS_API*/
       
           /* return 0 on success */
           static WC_INLINE int IntelRDseed64(word64* seed)
      @@ -1302,7 +1302,7 @@ int wc_FreeNetRandom(void)
           {
               int ok;
       
      -        ok = _rdseed64_step(seed);
      +        ok = _rdseed64_step((unsigned long long*) seed);
               return (ok) ? 0 : -1;
           }
       
      diff --git a/storage/innobase/ut/ut0crc32.cc b/storage/innobase/ut/ut0crc32.cc
      index 4a6447c1dcf..58273e9058e 100644
      --- a/storage/innobase/ut/ut0crc32.cc
      +++ b/storage/innobase/ut/ut0crc32.cc
      @@ -219,6 +219,8 @@ ut_crc32_8_hw(
       {
       #ifdef _MSC_VER
       	*crc = _mm_crc32_u8(*crc, (*data)[0]);
      +#elif 1
      +	*crc = __builtin_ia32_crc32qi(*crc, (*data)[0]);
       #else
       	asm("crc32b %1, %0"
       	    /* output operands */
      @@ -251,6 +253,8 @@ ut_crc32_64_low_hw(
       #else
       #error Not Supported processors type.
       #endif
      +#elif 1
      +	crc_64bit = __builtin_ia32_crc32di(crc_64bit, data);
       #else
       	asm("crc32q %1, %0"
       	    /* output operands */
      

      Errors in mysqltest:

      diff --git a/mysql-test/lib/My/SafeProcess/safe_process.cc b/mysql-test/lib/My/SafeProcess/safe_process.cc
      index 9b544a25178..84003288dab 100644
      --- a/mysql-test/lib/My/SafeProcess/safe_process.cc
      +++ b/mysql-test/lib/My/SafeProcess/safe_process.cc
      @@ -219,6 +219,7 @@ int main(int argc, char* const argv[] )
         sigemptyset(&sa.sa_mask);
       
         sa_abort.sa_handler= handle_abort;
      +  sa_abort.sa_flags= 0;
         sigemptyset(&sa_abort.sa_mask);
         /* Install signal handlers */
         sigaction(SIGTERM, &sa,NULL);
      diff --git a/client/mysqltest.cc b/client/mysqltest.cc
      index 5f28bf422e1..0ba9cdc13c4 100644
      --- a/client/mysqltest.cc
      +++ b/client/mysqltest.cc
      @@ -1687,6 +1687,7 @@ void abort_not_supported_test(const char *fmt, ...)
                 cur_file->file_name, cur_file->lineno);
       
         char buff[DIE_BUFF_SIZE];
      +  buff[0] = '\0';
         print_file_stack(buff, buff + sizeof(buff));
         fprintf(stderr, "%s", buff);
       
      @@ -10363,6 +10364,7 @@ void free_replace_regex()
         string - the string to perform substitutions in
         icase - flag, if set to 1 the match is case insensitive
       */
      +__attribute__((no_sanitize("memory")))
       int reg_replace(char** buf_p, int* buf_len_p, char *pattern,
                       char *replace, char *string, int icase)
       {
      

      Note: the possible bug in reg_replace() should be investigated deeper, and not simply suppressed.

      SSL-related problem

      diff --git a/mysys_ssl/my_crypt.cc b/mysys_ssl/my_crypt.cc
      index 02770644259..cb9e60e4dc7 100644
      --- a/mysys_ssl/my_crypt.cc
      +++ b/mysys_ssl/my_crypt.cc
      @@ -94,6 +94,8 @@ class MyCTX
         }
       };
       
      +#include <sanitizer/msan_interface.h>
      +
       class MyCTX_nopad : public MyCTX
       {
       public:
      @@ -143,6 +145,7 @@ class MyCTX_nopad : public MyCTX
               of this class too.
             */
             uchar mask[MY_AES_BLOCK_SIZE];
      +      __msan_unpoison(mask, sizeof mask);
             uint mlen;
       
             int rc= my_aes_crypt(MY_AES_ECB, ENCRYPTION_FLAG_ENCRYPT | ENCRYPTION_FLAG_NOPAD,
      

      The above is only a work-around. Without the above, the last 4 payload bytes in an encrypted InnoDB redo log block (at offset 512-8) would be claimed to uninitialized on pwrite64(). This needs to be investigated properly.

      Some remaining problems (blocking further tests):

      innodb.innodb-page_compression_lz4 alleges uninitialized value in buf_page_encrypt() when computing ut_crc32_8_hw(); maybe we simply need an instrumented liblz4, or should avoid linking with the uninstrumented library?

      This one occurs at least in innodb.innodb-wl5980-alter and innodb.innodb-alter:

      10.5 da53fb6d7de906fd8bd73d5f244bac4d77b687aa

      ==16796==WARNING: MemorySanitizer: use-of-uninitialized-value
          #0 0x4b9edc in var_get(char const*, char const**, char, char) /mariadb/10.5/client/mysqltest.cc:2498:12
          #1 0x51b995 in append_replace_regex(char*, char*, st_replace_regex*, char**) /mariadb/10.5/client/mysqltest.cc:10205:17
          #2 0x5139f2 in init_replace_regex(char*) /mariadb/10.5/client/mysqltest.cc:10163:3
          #3 0x5139f2 in do_get_replace_regex(st_command*) /mariadb/10.5/client/mysqltest.cc:10324
          #4 0x50f060 in main /mariadb/10.5/client/mysqltest.cc:9608:9
          #5 0x7f8847a7409a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
          #6 0x438b19 in _start (/dev/shm/10.5/client/mysqltest+0x438b19)
       
        Uninitialized value was created by an allocation of 'v_end' in the stack frame of function '_Z20append_replace_regexPcS_P16st_replace_regexPS_'
          #0 0x51b630 in append_replace_regex(char*, char*, st_replace_regex*, char**) /mariadb/10.5/client/mysqltest.cc:10176
      

      10.5 da53fb6d7de906fd8bd73d5f244bac4d77b687aa

      CURRENT_TEST: innodb.innodb-replace-debug
      mysqltest: At line 13: query 'replace into t1 values (14, 25, 34)' failed: 2013: Lost connection to MySQL server during query
      ha_commit_trans: info: is_real_trans: 1  rw_trans:  1  rw_ha_count: 1
      MDL_context::acquire_lock: enter: ==20608==WARNING: MemorySanitizer: use-of-uninitialized-value
          #0 0x397a8df in dtoa /mariadb/10.5/strings/dtoa.c:2227:7
          #1 0x3970d61 in my_fcvt /mariadb/10.5/strings/dtoa.c:96:8
          #2 0x398f8bb in process_dbl_arg /mariadb/10.5/strings/my_vsnprintf.c:246:10
          #3 0x398f8bb in my_vsnprintf_ex /mariadb/10.5/strings/my_vsnprintf.c:638
          #4 0x385db9f in DbugVfprintf /mariadb/10.5/dbug/dbug.c:1329:10
          #5 0x385db9f in _db_doprnt_ /mariadb/10.5/dbug/dbug.c:1309
          #6 0x120342f in MDL_context::acquire_lock(MDL_request*, double) /mariadb/10.5/sql/mdl.cc:2242:3
          #7 0x17d8de3 in ha_commit_trans(THD*, bool) /mariadb/10.5/sql/handler.cc:1470:24
          #8 0x1228d0a in trans_commit_stmt(THD*) /mariadb/10.5/sql/transaction.cc:436:10
          #9 0xc28e14 in mysql_execute_command(THD*) /mariadb/10.5/sql/sql_parse.cc:6134:7
          #10 0xbfcce2 in mysql_parse(THD*, char*, unsigned int, Parser_state*, bool, bool) /mariadb/10.5/sql/sql_parse.cc:7884:18
          #11 0xbed4e5 in dispatch_command(enum_server_command, THD*, char*, unsigned int, bool, bool) /mariadb/10.5/sql/sql_parse.cc:1842:7
          #12 0xbfee52 in do_command(THD*) /mariadb/10.5/sql/sql_parse.cc:1359:17
          #13 0x11cf88f in do_handle_one_connection(CONNECT*, bool) /mariadb/10.5/sql/sql_connect.cc:1414:11
          #14 0x11ce99c in handle_one_connection /mariadb/10.5/sql/sql_connect.cc:1309:5
          #15 0x2533fd8 in pfs_spawn_thread /mariadb/10.5/storage/perfschema/pfs.cc:1862:3
          #16 0x7ff07e856fa2 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x7fa2)
          #17 0x7ff07de3e4ce in clone (/lib/x86_64-linux-gnu/libc.so.6+0xf94ce)
       
        Uninitialized value was created by an allocation of 'cvtbuf.i' in the stack frame of function '_db_doprnt_'
          #0 0x385d4d0 in _db_doprnt_ /mariadb/10.5/dbug/dbug.c:1288
      

      This one was reported by Valgrind on some (not all) platforms. Here it is for innodb.doublewrite:

      10.5 da53fb6d7de906fd8bd73d5f244bac4d77b687aa

      ==21866==WARNING: MemorySanitizer: use-of-uninitialized-value
          #0 0x3093dcc in buf_page_is_corrupted(bool, unsigned char const*, unsigned long) /mariadb/10.5/storage/innobase/buf/buf0buf.cc:1037:14
          #1 0x338b7cb in Datafile::find_space_id() /mariadb/10.5/storage/innobase/fsp/fsp0file.cc:711:22
          #2 0x3389af5 in Datafile::validate_for_recovery() /mariadb/10.5/storage/innobase/fsp/fsp0file.cc:461:9
          #3 0x33123ed in fil_ibd_load(unsigned long, char const*, fil_space_t*&) /mariadb/10.5/storage/innobase/fil/fil0fil.cc:3746:15
      

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                marko Marko Mäkelä
                Reporter:
                marko Marko Mäkelä
              • Votes:
                1 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated: