Skip to content

MDEV-40147 MSAN: use-of-uninitialized-value in my_time_compare#5281

Open
grooverdan wants to merge 1 commit into
MariaDB:10.11from
grooverdan:MDEV-36344
Open

MDEV-40147 MSAN: use-of-uninitialized-value in my_time_compare#5281
grooverdan wants to merge 1 commit into
MariaDB:10.11from
grooverdan:MDEV-36344

Conversation

@grooverdan

Copy link
Copy Markdown
Member

Field::get_date and Item::get_date can return true and not have their ltime pointer structure initialized.

This was the case with Field_vers_trx_id::get_date.

Type_handler_time_common::stored_field_cmp_to_item rather than doing a my_time_compare on uninitialized values can return a non-equality (!=0) value for when the fetching of Field or Item fails and avoid the call to my_time_compare.

Added checks for other invalid results before being passed to my_time_compare.

Add nonnull as an attribute on my_time_compare and the unpack_time function it used. Also pack_time since related and it obviously requires a non-null argument.

Field::get_date and Item::get_date can return true and not
have their ltime pointer structure initialized.

This was the case with Field_vers_trx_id::get_date.

Type_handler_time_common::stored_field_cmp_to_item rather
than doing a my_time_compare on uninitialized values can
return a non-equality (!=0) value for when the fetching of
Field or Item fails and avoid the call to my_time_compare.

Added checks for other invalid results before being
passed to my_time_compare.

Add nonnull as an attribute on my_time_compare and the
unpack_time function it used. Also pack_time since related
and it obviously requires a non-null argument.
@grooverdan grooverdan requested a review from abarkov June 26, 2026 03:14
@grooverdan grooverdan added the MariaDB Foundation Pull requests created by MariaDB Foundation label Jun 26, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses an MSAN uninitialized value issue in my_time_compare (MDEV-40147) by adding __attribute__((nonnull)) to temporal functions, checking return values of get_date before comparisons, and adding a regression test. The review feedback points out an inconsistency in Type_handler_temporal_with_date::stored_field_cmp_to_item where returning 1 when field->get_date fails is incorrect and asymmetric, suggesting it should return -1 instead to match similar comparison handlers.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread sql/sql_type.cc
Comment on lines +9110 to 9114
if (field->get_date(&field_time, Datetime::Options(TIME_INVALID_DATES, thd))
|| item->get_date(thd, &item_time, Datetime::Options(TIME_INVALID_DATES, thd))
|| (item_time.time_type == MYSQL_TIMESTAMP_TIME &&
time_to_datetime(thd, &item_time, item_time_cmp= &item_time2)))
return 1;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In Type_handler_temporal_with_date::stored_field_cmp_to_item, if field->get_date fails, returning 1 (which indicates field > item) is inconsistent and incorrect when item is valid. To be consistent with Type_handler_time_common::stored_field_cmp_to_item (lines 9124-9127), we should return -1 when field->get_date fails, and 1 when item->get_date (or its conversion) fails. This ensures symmetric and correct comparison behavior when one of the operands is invalid.

  if (field->get_date(&field_time, Datetime::Options(TIME_INVALID_DATES, thd)))
    return -1;
  if (item->get_date(thd, &item_time, Datetime::Options(TIME_INVALID_DATES, thd)) ||
      (item_time.time_type == MYSQL_TIMESTAMP_TIME &&
       time_to_datetime(thd, &item_time, item_time_cmp= &item_time2)))
    return 1;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

MariaDB Foundation Pull requests created by MariaDB Foundation

Development

Successfully merging this pull request may close these issues.

2 participants