Commit d26a6f4
committed
MDEV-38747: ASAN errors in Optimizer_hint_parser::Identifier::to_ident_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.1 parent 75c4adf commit d26a6f4
4 files changed
Lines changed: 35 additions & 7 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8867 | 8867 | | |
8868 | 8868 | | |
8869 | 8869 | | |
| 8870 | + | |
| 8871 | + | |
| 8872 | + | |
| 8873 | + | |
| 8874 | + | |
| 8875 | + | |
| 8876 | + | |
| 8877 | + | |
| 8878 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9618 | 9618 | | |
9619 | 9619 | | |
9620 | 9620 | | |
| 9621 | + | |
| 9622 | + | |
| 9623 | + | |
| 9624 | + | |
| 9625 | + | |
| 9626 | + | |
| 9627 | + | |
| 9628 | + | |
| 9629 | + | |
| 9630 | + | |
| 9631 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
899 | 899 | | |
900 | 900 | | |
901 | 901 | | |
902 | | - | |
| 902 | + | |
| 903 | + | |
903 | 904 | | |
904 | | - | |
905 | | - | |
906 | | - | |
| 905 | + | |
907 | 906 | | |
908 | 907 | | |
909 | 908 | | |
| |||
953 | 952 | | |
954 | 953 | | |
955 | 954 | | |
956 | | - | |
| 955 | + | |
957 | 956 | | |
958 | 957 | | |
959 | 958 | | |
| |||
969 | 968 | | |
970 | 969 | | |
971 | 970 | | |
972 | | - | |
| 971 | + | |
973 | 972 | | |
974 | 973 | | |
975 | 974 | | |
| |||
1068 | 1067 | | |
1069 | 1068 | | |
1070 | 1069 | | |
1071 | | - | |
| 1070 | + | |
1072 | 1071 | | |
1073 | 1072 | | |
1074 | 1073 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
503 | 503 | | |
504 | 504 | | |
505 | 505 | | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
506 | 515 | | |
507 | 516 | | |
508 | 517 | | |
| |||
0 commit comments