MDEV-38747: ASAN errors in Optimizer_hint_parser::Identifier::to_ide…

[DaveGosselin-MariaDB]

…nt_cli

Summary:
A trigger specifying a hint where the hint has a query block name will cause
an ASAN failure because hint resolution occurs after query parsing, not
during query parsing. The trigger execution logic uses a stack-local
string to hold the query and hint text during parsing. During trigger
execution, query parsing and query execution happen in different function
contexts, so the query string used during parsing goes out of scope, freeing
its memory. But as hint resolution occurs after parsing is complete (and
hints merely point into the query string, they don't copy from it), the hints
refer into a deallocated query string upon hint resolution.

Details:
Prior to the commit introducing this bug, hint resolution was done via a call
to LEX::resolve_optimizer_hints_in_last_select when parsing the
query_specification: grammar rule. This meant that any string containing
the query (and hints) was in scope for the entire lifetime of query parsing
and hint resolution.

In the patch introducing this bug, resolve_optimizer_hints_in_last_select
was replaced with handle_parsed_optimizer_hints_in_last_select, changing
the parsing such that it merely cached hints for resolution during query
execution. Later, after parsing ends and upon query execution,
mysql_execute_command calls LEX::resolve_optimizer_hints to resolve hints.
When executing a typical SQL command trigger, sp_lex_instr::parse_expr
reparses the query associated with the trigger and does so using a stack-local
String variable to hold the query text. sp_lex_instr::parse_expr returns after
query parsing completes but before hint resolution begins. Since
the string holding the query was stack-local in sp_lex_instr::parse_expr and
destroyed when the method returned, the query string (and hints with it) were
deallocated, leading to the ASAN failure on hint resolution. When executing
the trigger, sp_instr_stmt::exec_core calls mysql_execute_command which
calls LEX::resolve_optimizer_hints to complete hint resolution but the query
string that the hints depends on no longer exists at this point.

As noted, the stack-local query_string variable in sp_lex_inst::parse_expr
goes out-of-scope and is freed when the sp_lex_instr::parse_expr returns. In
contrast, in the general case, when a COM_QUERY is processed during
dispatch_command, the query string lives on the THD for the lifetime of
the query independent of some particular function's scope.

For triggers, the necessary lifetime of that query string needs to be as long
as sp_lex_keeper::validate_lex_and_exec_core which covers both the query
string parsing via sp_lex_instr::parse_expr and the procedure's execution
during reset_lex_and_exec_core. Consequently, this patch lifts the
query_string buffer up out of parse_expr and onto the sp_lex_instr itself
which guarantees that its lifetime is as long as the instruction, which also
guarantees the query string's lifetime extends across parsing and execution,
including hint resolution. This also covers any cases where the trigger is
successfully executed consecutive times but not reparsed between those
executions.

QB_NAME is not the only affected hint kind; hints with some query block
identifier text for the query block, like

NO_MERGE(`@select#1`)

will also cause the crash while NO_MERGE() will not.

[spetrunia]

What I do not like about this fix.

So after the fix we have:

int sp_lex_keeper::validate_lex_and_exec_core(THD *thd, uint *nextp,
                                              bool open_tables,
                                              sp_lex_instr* instr)
{
...                                              
  while (true)
  {
    String sql_query;

    if (instr->is_invalid() || m_lex->needs_reprepare)
    {
      ...
      LEX *lex= instr->parse_expr(thd, thd->spcont->m_sp, m_lex, &sql_query);
      ...
    }

    bool rc= reset_lex_and_exec_core(thd, nextp, open_tables, instr,
                                     rerun_the_same_instr);
    if (!rc)
      break;
    ...

    instr->invalidate();
  }
}

Consider this siutation:

We enter this function and instr->is_invalid()==true...
We create sql_query string.
We reach intr->parse_expr(...) call which puts content into sql_query and makes the statement have pointers into the sql_query.
We make reset_lex_and_exec_core(...) call, it returns 0 (no error).
We reach if (!rc) break and sql_query goes out of scope so the string is deallocated.

Note that the parsed statement remains available for further execution.

If/when we enter sp_lex_keeper::validate_lex_and_exec_core() again, we will have instr->is_invalid()==false.
Let's assume we will have m_lex->needs_reprepare=false (this is normal state)
Then, we will proceed to reset_lex_and_exec_core() without re-parsing.

We'll have statement execute while hints data structures refer to free'd memory.
How bad this is?
One could say that hint resolution was done on the first execution and is not required.
But what if the statement prints its hints somewhere (EXPLAIN output? SHOW ANALYZE? ).

[spetrunia]

Looks good now.