Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-26637

ASAN: main.metadata and user_variables.basic MTR failures after MDEV-26572

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed (View Workflow)
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 10.7
    • Fix Version/s: 10.7.1
    • Component/s: OTHER
    • Labels:
      None

      Description

      After MDEV-26572 two tests fail with an ASAN build:

      READ of size 4 at 0x6290000e6f60 thread T5
          #0 0x558dcc06be73 in my_strcoll_ascii_4bytes_found /home/buildbot/buildbot/build/mariadb-10.7.0/strings/ctype-ascii.h:111
          #1 0x558dcc06d45a in my_strnncoll_utf8mb3_general_ci /home/buildbot/buildbot/build/mariadb-10.7.0/strings/strcoll.ic:226
          #2 0x558dcbf06683 in hashcmp /home/buildbot/buildbot/build/mariadb-10.7.0/mysys/hash.c:373
          #3 0x558dcbf05ef6 in my_hash_first_from_hash_value /home/buildbot/buildbot/build/mariadb-10.7.0/mysys/hash.c:288
          #4 0x558dcbf05d11 in my_hash_first /home/buildbot/buildbot/build/mariadb-10.7.0/mysys/hash.c:262
          #5 0x558dcbf05a04 in my_hash_search /home/buildbot/buildbot/build/mariadb-10.7.0/mysys/hash.c:235
          #6 0x558dcac84412 in get_variable(st_hash*, st_mysql_const_lex_string*, bool) /home/buildbot/buildbot/build/mariadb-10.7.0/sql/item_func.cc:4634
          #7 0x558dcac84924 in Item_func_set_user_var::set_entry(THD*, bool) /home/buildbot/buildbot/build/mariadb-10.7.0/sql/item_func.cc:4687
      

      This is a minimum reproducible script:

      SET NAMES utf8;
      set @arg00=1;
      SELECT @:=1e0;
      

      The problem was not really introduced by MDEV-26572, it only revealed the fact that hashcmp() in mysys/hash.c asubes strnncoll():

        NOTES:
          If length is 0, comparison is done using the length of the
          record being compared against.
      

      This quick patch fixes the problem:

      diff --git a/mysys/hash.c b/mysys/hash.c
      index abc11b42500..c0c60a8117d 100644
      --- a/mysys/hash.c
      +++ b/mysys/hash.c
      @@ -369,6 +369,22 @@ static int hashcmp(const HASH *hash, HASH_LINK *pos, const uchar *key,
       {
         size_t rec_keylength;
         uchar *rec_key= (uchar*) my_hash_key(hash, pos->data, &rec_keylength, 1);
      +  if (hash->charset != &my_charset_bin)
      +  {
      +    /*
      +      The "length == rec_keylength" is needed for backward compatibility,
      +      to avoid things like partition names and plugin names to be
      +      accent insensitive. We should eventually:
      +      - Add a new utf8 accent sensitive case insensitive collation
      +      - Change SQL object names (plugins, partitions, etc) to use
      +        this new collation.
      +      - Remove the length test from here
      +    */
      +    return length != rec_keylength ||
      +           my_strnncoll(hash->charset,
      +                        (uchar*) rec_key, rec_keylength,
      +                        (uchar*) key, length);
      +  }
         return ((length && length != rec_keylength) ||
                my_strnncoll(hash->charset, (uchar*) rec_key, rec_keylength,
                             (uchar*) key, rec_keylength));
      

      This patch assumes that the length "0" is passed only on binary data, while in case of text string data a correct length value is passed.

      All MTR tests passed with the patch. So this assumption seems to be true.

      But the patch slows down a very hot function by adding a new if testing hash->charset . It should not be pushed AS IS. It's only a demo showing where the real problem is.

      The real patch should fix all my_hash_search() calls not to pass 0 as length.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              sanja Oleksandr Byelkin
              Reporter:
              bar Alexander Barkov
              Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Git Integration