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

uninitialized read in Item_cond::fix_fields leads to crash: select .. where .. in ( select ... )

Details

    • Bug
    • Status: Closed (View Workflow)
    • Major
    • Resolution: Fixed
    • 10.0.0, 5.5.28a
    • 10.0.2, 5.5.29
    • None
    • None
    • windows, linux

    Description

      How to repeat:
      ----------------
      #Run mysqld in valgrind. Execute:

      drop table if exists `t1`;
      		 
      create table `t1`(`a` char(1) character set utf8)engine=innodb;
      		 
      select 1 from `t1` where `a` in (select group_concat(`a`) from t1);

      Windows call stack:
      ---------------------

      Version: '10.0.0-MariaDB-log'  socket: ''  port: 3306  Source distribution
      		 
      121228  7:46:33 [ERROR] mysqld got exception 0xc0000005 ;
      		 
      mysqld.exe!setup_jtbm_semi_joins()[opt_subselect.cc:5224]
      		 
      mysqld.exe!JOIN::optimize_inner()[sql_select.cc:1123]
      		 
      mysqld.exe!JOIN::optimize()[sql_select.cc:992]
      		 
      mysqld.exe!mysql_select()[sql_select.cc:3176]
      		 
      mysqld.exe!handle_select()[sql_select.cc:362]
      		 
      mysqld.exe!execute_sqlcom_select()[sql_parse.cc:4937]
      		 
      mysqld.exe!mysql_execute_command()[sql_parse.cc:2421]
      		 
      mysqld.exe!mysql_parse()[sql_parse.cc:6061]
      		 
      mysqld.exe!dispatch_command()[sql_parse.cc:1219]
      		 
      mysqld.exe!do_command()[sql_parse.cc:951]
      		 
      mysqld.exe!threadpool_process_request()[threadpool_common.cc:225]
      		 
      mysqld.exe!io_completion_callback()[threadpool_win.cc:568]

      See attached file for full valgrind outputs.

      Attachments

        Activity

          Ascending order - Click to sort in descending order
          elenst Elena Stepanova added a comment -

          On a debug version, causes assertion `0' failure in bool subselect_hash_sj_engine::init(List<Item>*, uint).
          The assertion failure is also reproducible with a non-empty MyISAM table:

          SET optimizer_switch = 'materialization=on';

          create table `t1`(`a` char(1) character set utf8) engine=myisam;
          insert into t1 values ('a'),('b');

          select 1 from `t1` where `a` in (select group_concat(a) from t1);

          Minimal optimizer_switch: materialization=on.

          elenst Elena Stepanova added a comment - On a debug version, causes assertion `0' failure in bool subselect_hash_sj_engine::init(List<Item>*, uint). The assertion failure is also reproducible with a non-empty MyISAM table: SET optimizer_switch = 'materialization=on'; create table `t1`(`a` char(1) character set utf8) engine=myisam; insert into t1 values ('a'),('b'); select 1 from `t1` where `a` in (select group_concat(a) from t1); Minimal optimizer_switch: materialization=on.
          timour Timour Katchaounov (Inactive) added a comment -

          Analysis:
          The following patch:
          revno: 2502.494.2
          committer: Sergey Petrunya <psergey@askmonty.org>
          branch nick: 5.3-look41
          timestamp: Wed 2011-12-07 19:21:51 +0400
          message:
          BUG#868908: Crash in check_simple_equality() with semijoin + materialization + prepared statement

          • Part2: safety and code cleanup

          Adds an assert that makes sure that if non-SJ materialization was chosen, then the temporary table for the materialized subquery was created successfully. This assumes that subquery_types_allow_materialization() is complete, and it would reject all cases when a unique index cannot be created over the materialized subquery.

          However, if the subquery to be materialized selects group_concat(), and the collation is utf-8, the test in subquery_types_allow_materialization() is not consistent with the corresponding logic in Item_func_group_concat::make_string_field(). The latter function would create a field of type BLOB for the temporary table if

          (max_length / collation.collation->mbminlen > CONVERT_IF_BIGGER_TO_BLOB)

          This field is not unique-indexable, so the temporary table for the subquery cannot be used to perform lookups and to compute the IN predicate.

          At the same time subquery_types_allow_materialization() tests for

          (inner->max_length / inner->collation.collation->mbmaxlen > CONVERT_IF_BIGGER_TO_BLOB)

          and thus it decides that materialization is possible.

          timour Timour Katchaounov (Inactive) added a comment - Analysis: The following patch: revno: 2502.494.2 committer: Sergey Petrunya <psergey@askmonty.org> branch nick: 5.3-look41 timestamp: Wed 2011-12-07 19:21:51 +0400 message: BUG#868908: Crash in check_simple_equality() with semijoin + materialization + prepared statement Part2: safety and code cleanup Adds an assert that makes sure that if non-SJ materialization was chosen, then the temporary table for the materialized subquery was created successfully. This assumes that subquery_types_allow_materialization() is complete, and it would reject all cases when a unique index cannot be created over the materialized subquery. However, if the subquery to be materialized selects group_concat(), and the collation is utf-8, the test in subquery_types_allow_materialization() is not consistent with the corresponding logic in Item_func_group_concat::make_string_field(). The latter function would create a field of type BLOB for the temporary table if (max_length / collation.collation->mbminlen > CONVERT_IF_BIGGER_TO_BLOB) This field is not unique-indexable, so the temporary table for the subquery cannot be used to perform lookups and to compute the IN predicate. At the same time subquery_types_allow_materialization() tests for (inner->max_length / inner->collation.collation->mbmaxlen > CONVERT_IF_BIGGER_TO_BLOB) and thus it decides that materialization is possible.
          timour Timour Katchaounov (Inactive) added a comment -

          The fix is to correct Item_func_group_concat::make_string_field() to use collation.collation->mbmaxlen instead of collation.collation->mbminlen.

          After a discussion with Serg, he said that he created a patch that that removes almost all comparisons with CONVERT_IF_BIGGER_TO_BLOB, and fixes all such inconsistencies.
          The test case for this bug will be pushed after Serg pushes his patch.

          timour Timour Katchaounov (Inactive) added a comment - The fix is to correct Item_func_group_concat::make_string_field() to use collation.collation->mbmaxlen instead of collation.collation->mbminlen. After a discussion with Serg, he said that he created a patch that that removes almost all comparisons with CONVERT_IF_BIGGER_TO_BLOB, and fixes all such inconsistencies. The test case for this bug will be pushed after Serg pushes his patch.
          pomyk Patryk Pomykalski added a comment -

          A quick and dirty solution:
          — sql/item_sum.cc 2012-11-22 09:19:31 +0000
          +++ sql/item_sum.cc 2013-01-07 16:40:56 +0000
          @@ -3213,7 +3213,7 @@
          max_characters * CS->mbmaxlen.
          */
          const uint32 max_characters= max_length / collation.collation->mbminlen;

          • if (max_characters > CONVERT_IF_BIGGER_TO_BLOB)
            + if (max_characters > collation.collation->mbmaxlen*CONVERT_IF_BIGGER_TO_BLOB)
            field= new Field_blob(max_characters * collation.collation->mbmaxlen,
            maybe_null, name, collation.collation, TRUE);
            else

          As a side effect some tests output changes from text to varchar(1024).

          pomyk Patryk Pomykalski added a comment - A quick and dirty solution: — sql/item_sum.cc 2012-11-22 09:19:31 +0000 +++ sql/item_sum.cc 2013-01-07 16:40:56 +0000 @@ -3213,7 +3213,7 @@ max_characters * CS->mbmaxlen. */ const uint32 max_characters= max_length / collation.collation->mbminlen; if (max_characters > CONVERT_IF_BIGGER_TO_BLOB) + if (max_characters > collation.collation->mbmaxlen*CONVERT_IF_BIGGER_TO_BLOB) field= new Field_blob(max_characters * collation.collation->mbmaxlen, maybe_null, name, collation.collation, TRUE); else As a side effect some tests output changes from text to varchar(1024).
          timour Timour Katchaounov (Inactive) added a comment -

          I approve serg's patch:
          http://bazaar.launchpad.net/~maria-captains/maria/5.5-serg/revision/3609

          timour Timour Katchaounov (Inactive) added a comment - I approve serg's patch: http://bazaar.launchpad.net/~maria-captains/maria/5.5-serg/revision/3609

          People

            serg Sergei Golubchik
            sbester1 sbester1
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Git Integration

                Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.