Commit 811614d4 authored by Hugo Wen's avatar Hugo Wen Committed by Andrew Hutchings

MDEV-34625 Fix undefined behavior of using uninitialized member variables

Commit a8a75ba2 causes the MariaDB server to crash, usually with signal
11, at random code locations due to invalid pointer values during any
table operation. This issue occurs when the server is built with -O3 and
other customized compiler flags.

For example, the command `use db1;` causes server to crash in the
`check_table_access` function at line sql_parse.cc:7080 because
`tables->correspondent_table` is an invalid pointer value of 0x1.

The crashes are due to undefined behavior from using uninitialized
variables. The problematic commit a8a75ba2 introduces code that
allocates memory and sets it to 0 using thd->calloc before initializing
it with a placement new operation.
This process depends on setting memory to 0 to initialize member
variables not explicitly set in the constructor. However, the compiler
can optimize out the memset/bfill, leading to uninitialized values and
unpredictable issues.

Once a constructor function initializes an object, any uninitialized
variables within that object are subject to undefined behavior. The
state of memory before the constructor runs, whether it involves
memset or was used for other purposes, is irrelevant after the
placement new operation.

This behavior can be demonstrated with this
[test](https://gcc.godbolt.org/z/5n87z1raG) I wrote to examine the
assembly code. The code in MariaDB can be abstracted to the following,
though it has many layers wrapped around it and more complex logic,
causing slight differences in optimization in the MariaDB build.
To summarize, on x86, the memset in the following code is optimized out
with both -O2 and -O3 in GCC 13, and is only preserved in the much older
GCC 4.9.

    struct S {
      int i;     // uninitialized in consturctor
      S() {};
    };
    int bar() {
      void *buf = malloc(sizeof(S));
      memset(buf, 0, sizeof(S));       // optimized out
      S* s = new(buf) S;
      return s->i;
    }

With GCC13 -O3:

    bar():
          sub     rsp, 8
          mov     edi, 4
          call    malloc
          mov     eax, DWORD PTR [rax]
          add     rsp, 8
          ret

With GCC4.9 -O3

    bar():
          sub     rsp, 8
          mov     edi, 4
          call    malloc
          mov     DWORD PTR [rax], 0
          xor     eax, eax
          add     rsp, 8
          ret

Now we ensure the constructor initializes variables correctly by running
the reset() function in the constructor to perform the memset/bfill(0)
operation. After applying the fix, the crash is gone.

All new code of the whole pull request, including one or several files
that are either new files or modified ones, are contributed under the
BSD-new license. I am contributing on behalf of my employer Amazon Web
Services.
parent ee5f7692
......@@ -8375,7 +8375,7 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd,
}
bool has_alias_ptr= alias != nullptr;
void *memregion= thd->calloc(sizeof(TABLE_LIST));
void *memregion= thd->alloc(sizeof(TABLE_LIST));
TABLE_LIST *ptr= new (memregion) TABLE_LIST(thd, db, fqtn, alias_str,
has_alias_ptr, table, lock_type,
mdl_type, table_options,
......
......@@ -5865,6 +5865,7 @@ TABLE_LIST::TABLE_LIST(THD *thd,
List<Index_hint> *index_hints_ptr,
LEX_STRING *option_ptr)
{
reset();
db= db_str;
is_fqtn= fqtn;
alias= alias_str;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment