From cb7e39b75bfc18813f818404c83a4fb6dc519320 Mon Sep 17 00:00:00 2001 From: Nikita Malyavin Date: Tue, 8 Apr 2025 21:57:11 +0200 Subject: [PATCH] MDEV-36181 Field pointer may be uninitialized in fill_record Newer gcc reports: error: 'rfield' may be used uninitialized [-Werror=maybe-uninitialized] 9041 | unwind_stored_field_offsets(fields, rfield); After investigation, it turned to be an impossible case: 1. The only way it could be broken is if if (!(field= fld->field_for_view_update())) line case would succeed from the first time. 2. Consequent checks initialize rfield. fld may return NULL in field_for_view_update() only for views. 3. Before fill_record, UPDATE first calls check_fields, where field_for_view_update() result is already checked. INSERT calls check_view_insertability that checks that all view fields are updateable. It all means that field_for_view_update() cannot be NULL in fill_record, so the if can be converted to DBUG_ASSERT. This essentially shifts the responsibility on preliminary field_for_view_update() check to the caller. In this patch: 1. convert field_for_view_update() check to DBUG_ASSERT 2. harden unwind_stored_field_offsets function so that it can be used even if field_for_view_update() is NULL 3. As a consequence, `field` is passed instead of `rfield` as a terminator. 4. Initialize `field` to NULL to bypass a false-positive warning! --- sql/sql_base.cc | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 09dac838c3f..ac4bce1dce0 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -8485,14 +8485,15 @@ err_no_arena: } -static void unwind_stored_field_offsets(const List &fields, Field *end) +static void unwind_stored_field_offsets(const List &fields, Item_field *end) { - for (Item &item_field: fields) + for (Item &item: fields) { - Field *f= item_field.field_for_view_update()->field; - if (f == end) + Item_field *item_field= item.field_for_view_update(); + if (item_field == end) break; + Field *f= item_field->field; if (f->stored_in_db()) { TABLE *table= f->table; @@ -8537,7 +8538,7 @@ fill_record(THD *thd, TABLE *table_arg, List &fields, List &values, { List_iterator_fast f(fields),v(values); Item *value, *fld; - Item_field *field; + Item_field *field= NULL; Field *rfield; TABLE *table; bool only_unvers_fields= update && table_arg->versioned(); @@ -8555,11 +8556,8 @@ fill_record(THD *thd, TABLE *table_arg, List &fields, List &values, while ((fld= f++)) { - if (!(field= fld->field_for_view_update())) - { - my_error(ER_NONUPDATEABLE_COLUMN, MYF(0), fld->name.str); - goto err_unwind_fields; - } + field= fld->field_for_view_update(); + DBUG_ASSERT(field); // ensured by check_fields or check_view_insertability. value=v++; DBUG_ASSERT(value); rfield= field->field; @@ -8621,7 +8619,7 @@ fill_record(THD *thd, TABLE *table_arg, List &fields, List &values, DBUG_RETURN(thd->is_error()); err_unwind_fields: if (update && thd->variables.sql_mode & MODE_SIMULTANEOUS_ASSIGNMENT) - unwind_stored_field_offsets(fields, rfield); + unwind_stored_field_offsets(fields, field); err: DBUG_PRINT("error",("got error")); thd->abort_on_warning= save_abort_on_warning;