Skip to content

Commit cc125be

Browse files
montywivuvova
authored andcommitted
Fix all warnings given by UBSAN
The 'special' cases where we disable, suppress or circumvent UBSAN are: - ref10 source (as here we intentionally do some shifts that UBSAN complains about. - x86 version of optimized int#korr() methods. UBSAN do not like unaligned memory access of integers. Fixed by using byte_order_generic.h when compiling with UBSAN - We use smaller thread stack with ASAN and UBSAN, which forced me to disable a few tests that prints the thread stack size. - Verifying class types does not work for shared libraries. I added suppression in mysql-test-run.pl for this case. - Added '#ifdef WITH_UBSAN' when using integer arithmetic where it is safe to have overflows (two cases, in item_func.cc). Things fixed: - Don't left shift signed values (byte_order_generic.h, mysqltest.c, item_sum.cc and many more) - Don't assign not non existing values to enum variables. - Ensure that bool and enum values are properly initialized in constructors. This was needed as UBSAN checks that these types has correct values when one copies an object. (gcalc_tools.h, ha_partition.cc, item_sum.cc, partition_element.h ...) - Ensure we do not called handler functions on unallocated objects or deleted objects. (events.cc, sql_acl.cc). - Fixed bugs in Item_sp::Item_sp() where we did not call constructor on Query_arena object. - Fixed several cast of objects to an incompatible class! (Item.cc, Item_buff.cc, item_timefunc.cc, opt_subselect.cc, sql_acl.cc, sql_select.cc ...) - Ensure we do not do integer arithmetic that causes over or underflows. This includes also ++ and -- of integers. (Item_func.cc, Item_strfunc.cc, item_timefunc.cc, sql_base.cc ...) - Added JSON_VALUE_UNITIALIZED to json_value_types and ensure that value_type is initialized to this instead of to -1, which is not a valid enum value for json_value_types. - Ensure we do not call memcpy() when second argument could be null. Other things: - Changed struct st_position to an OBJECT and added an initialization function to it to ensure that we do not copy or use uninitialized members. The change to a class was also motived that we used "struct st_position" and POSITION randomly trough the code which was confusing. - Notably big rewrite in sql_acl.cc to avoid using deleted objects. - Changed in sql_partition to use '^' instead of '-'. This is safe as the operator is either 0 or 0x8000000000000000ULL. - Added check for select_nr < INT_MAX in JOIN::build_explain() to avoid bug when get_select() could return NULL. - Reordered elements in POSITION for better alignment. - Changed sql_test.cc::print_plan() to use pointers instead of objects. - Fixed bug in find_set() where could could execute '1 << -1'. - Added variable have_sanitizer, used by mtr. (This variable was before only in 10.5 and up). It can now have one of two values: ASAN or UBSAN. - Moved ~Archive_share() from ha_archive.cc to ha_archive.h and marked it virtual. This was an effort to get UBSAN to work with loaded storage engines. I kept the change as the new place is better. - Added in CONNECT engine COLBLK::SetName(), to get around a wrong cast in tabutil.cpp. Changes that should not be needed but had to be done to suppress warnings from UBSAN: - Added static_cast<<uint16_t>> around shift to get rid of a LOT of compiler warnings when using UBSAN. - Had to change some '/' of 2 base integers to shift to get rid of some compile time warnings. Fixes: MDEV-25505 Assertion `old_flags == ((my_flags & 0x10000U) ? 1 : 0) fixed (was caused by an old version if this commit). Reviewed by: - Json changes: Alexey Botchkov - Charset changes in ctype-uca.c: Alexander Barkov - InnoDB changes: Marko Mäkelä - sql_acl.cc changes: Vicențiu Ciorbaru - build_explain() changes: Sergey Petrunia Temporary commit to log changes for UBSAN
1 parent b332ffc commit cc125be

File tree

13 files changed

+218
-157
lines changed

13 files changed

+218
-157
lines changed

BUILD/compile-pentium64-ubsan

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,6 @@ path=`dirname $0`
2424
. "$path/SETUP.sh"
2525

2626
extra_flags="$pentium64_cflags $debug_cflags -fsanitize=undefined -DWITH_UBSAN -Wno-conversion -Wno-uninitialized"
27-
extra_configs="$pentium_configs $debug_configs -DWITH_UBSAN=ON -DMYSQL_MAINTAINER_MODE=NO --without-spider --without-tokudb"
27+
extra_configs="$pentium_configs $debug_configs -DWITH_UBSAN=ON -DMYSQL_MAINTAINER_MODE=NO --without-spider"
2828

2929
. "$path/FINISH.sh"

mysql-test/suite/maria/alter.result

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,3 +184,12 @@ SET GLOBAL aria_max_sort_file_size= @max_size.save;
184184
#
185185
# End of 10.4 test
186186
#
187+
#
188+
# MDEV-25505 Assertion `old_flags == ((my_flags & 0x10000U) ? 1 : 0)'
189+
# failed in my_realloc
190+
#
191+
CREATE TABLE t1 (pk int, c text, primary key (pk), key(c(32))) ENGINE=Aria ROW_FORMAT=DYNAMIC;
192+
ALTER TABLE t1 DISABLE KEYS;
193+
INSERT INTO t1 VALUES (1, 'Nine chars or more');
194+
ALTER TABLE t1 ENABLE KEYS;
195+
DROP TABLE t1;

mysql-test/suite/maria/alter.test

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,3 +192,14 @@ SET GLOBAL aria_max_sort_file_size= @max_size.save;
192192
--echo #
193193
--echo # End of 10.4 test
194194
--echo #
195+
196+
--echo #
197+
--echo # MDEV-25505 Assertion `old_flags == ((my_flags & 0x10000U) ? 1 : 0)'
198+
--echo # failed in my_realloc
199+
--echo #
200+
201+
CREATE TABLE t1 (pk int, c text, primary key (pk), key(c(32))) ENGINE=Aria ROW_FORMAT=DYNAMIC;
202+
ALTER TABLE t1 DISABLE KEYS;
203+
INSERT INTO t1 VALUES (1, 'Nine chars or more');
204+
ALTER TABLE t1 ENABLE KEYS;
205+
DROP TABLE t1;

mysql-test/suite/plugins/t/multiauth.test

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
--source include/not_ubsan.inc
2+
23
let $REGEX_VERSION_ID=/$mysql_get_server_version/VERSION_ID/;
34
let $REGEX_PASSWORD_LAST_CHANGED=/password_last_changed": [0-9]*/password_last_changed": #/;
45
let $REGEX_GLOBAL_PRIV=$REGEX_PASSWORD_LAST_CHANGED $REGEX_VERSION_ID;

sql/item_strfunc.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3867,6 +3867,7 @@ String *Item_load_file::val_str(String *str)
38673867
File file;
38683868
MY_STAT stat_info;
38693869
char path[FN_REFLEN];
3870+
size_t file_size;
38703871
DBUG_ENTER("load_file");
38713872

38723873
if (!(file_name= args[0]->val_str(str))
@@ -3891,10 +3892,11 @@ String *Item_load_file::val_str(String *str)
38913892
/* my_error(ER_TEXTFILE_NOT_READABLE, MYF(0), file_name->c_ptr()); */
38923893
goto err;
38933894
}
3895+
file_size= stat_info.st_size;
38943896

38953897
{
38963898
THD *thd= current_thd;
3897-
if (stat_info.st_size > (long) thd->variables.max_allowed_packet)
3899+
if (file_size >= (size_t) thd->variables.max_allowed_packet)
38983900
{
38993901
push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
39003902
ER_WARN_ALLOWED_PACKET_OVERFLOWED,
@@ -3903,7 +3905,7 @@ String *Item_load_file::val_str(String *str)
39033905
goto err;
39043906
}
39053907
}
3906-
if (tmp_value.alloc((size_t)stat_info.st_size))
3908+
if (tmp_value.alloc(file_size))
39073909
goto err;
39083910
if ((file= mysql_file_open(key_file_loadfile,
39093911
file_name->ptr(), O_RDONLY, MYF(0))) < 0)

sql/sql_acl.cc

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6955,7 +6955,7 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table_list,
69556955
bool revoke_grant)
69566956
{
69576957
privilege_t column_priv(NO_ACL);
6958-
int result;
6958+
int result, res;
69596959
List_iterator <LEX_USER> str_list (user_list);
69606960
LEX_USER *Str, *tmp_Str;
69616961
bool create_new_users=0;
@@ -7156,10 +7156,10 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table_list,
71567156
revoke_grant))
71577157
result= TRUE;
71587158
}
7159-
if (int res= replace_table_table(thd, grant_table,
7160-
tables.tables_priv_table().table(),
7161-
*Str, db_name, table_name,
7162-
rights, column_priv, revoke_grant))
7159+
if ((res= replace_table_table(thd, grant_table,
7160+
tables.tables_priv_table().table(),
7161+
*Str, db_name, table_name,
7162+
rights, column_priv, revoke_grant)))
71637163
{
71647164
if (res > 0)
71657165
{
@@ -11277,7 +11277,7 @@ mysql_revoke_sp_privs(THD *thd, Grant_tables *tables, const Sp_handler *sph,
1127711277
bool mysql_revoke_all(THD *thd, List <LEX_USER> &list)
1127811278
{
1127911279
uint counter, revoked;
11280-
int result;
11280+
int result, res;
1128111281
ACL_DB *acl_db;
1128211282
DBUG_ENTER("mysql_revoke_all");
1128311283

@@ -11361,30 +11361,33 @@ bool mysql_revoke_all(THD *thd, List <LEX_USER> &list)
1136111361
{
1136211362
for (counter= 0, revoked= 0 ; counter < column_priv_hash.records ; )
1136311363
{
11364-
const char *user,*host;
11365-
GRANT_TABLE *grant_table=
11366-
(GRANT_TABLE*) my_hash_element(&column_priv_hash, counter);
11364+
const char *user,*host;
11365+
GRANT_TABLE *grant_table= ((GRANT_TABLE*)
11366+
my_hash_element(&column_priv_hash, counter));
11367+
1136711368
user= grant_table->user;
1136811369
host= safe_str(grant_table->host.hostname);
1136911370

1137011371
if (!strcmp(lex_user->user.str,user) &&
1137111372
!strcmp(lex_user->host.str, host))
11372-
{
11373-
List<LEX_COLUMN> columns;
11374-
/* TODO(cvicentiu) refactor to use
11373+
{
11374+
List<LEX_COLUMN> columns;
11375+
/* TODO(cvicentiu) refactor replace_db_table to use
11376+
Db_table instead of TABLE directly. */
11377+
if (replace_column_table(grant_table,
11378+
tables.columns_priv_table().table(),
11379+
*lex_user, columns, grant_table->db,
11380+
grant_table->tname, ALL_KNOWN_ACL, 1))
11381+
result= -1;
11382+
11383+
/* TODO(cvicentiu) refactor replace_db_table to use
1137511384
Db_table instead of TABLE directly. */
11376-
if (replace_column_table(grant_table,
11377-
tables.columns_priv_table().table(),
11378-
*lex_user, columns,
11379-
grant_table->db, grant_table->tname,
11380-
ALL_KNOWN_ACL, 1))
11381-
result= -1;
11382-
if (int res= replace_table_table(thd, grant_table,
11383-
tables.tables_priv_table().table(),
11384-
*lex_user,
11385-
grant_table->db, grant_table->tname,
11386-
ALL_KNOWN_ACL, NO_ACL, 1))
11387-
{
11385+
if ((res= replace_table_table(thd, grant_table,
11386+
tables.tables_priv_table().table(),
11387+
*lex_user, grant_table->db,
11388+
grant_table->tname, ALL_KNOWN_ACL,
11389+
NO_ACL, 1)))
11390+
{
1138811391
if (res > 0)
1138911392
result= -1;
1139011393
else
@@ -11397,8 +11400,8 @@ bool mysql_revoke_all(THD *thd, List <LEX_USER> &list)
1139711400
continue;
1139811401
}
1139911402
}
11400-
}
11401-
counter++;
11403+
}
11404+
counter++;
1140211405
}
1140311406
} while (revoked);
1140411407

sql/sql_select.cc

Lines changed: 95 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,6 @@ bool dbug_user_var_equals_int(THD *thd, const char *name, int value)
366366
}
367367
#endif /* DBUG_OFF */
368368

369-
370369
/*
371370
Intialize POSITION structure.
372371
*/
@@ -391,6 +390,100 @@ POSITION::POSITION()
391390
}
392391

393392

393+
void JOIN::init(THD *thd_arg, List<Item> &fields_arg,
394+
ulonglong select_options_arg, select_result *result_arg)
395+
{
396+
join_tab= 0;
397+
table= 0;
398+
table_count= 0;
399+
top_join_tab_count= 0;
400+
const_tables= 0;
401+
const_table_map= found_const_table_map= 0;
402+
aggr_tables= 0;
403+
eliminated_tables= 0;
404+
join_list= 0;
405+
implicit_grouping= FALSE;
406+
sort_and_group= 0;
407+
first_record= 0;
408+
do_send_rows= 1;
409+
duplicate_rows= send_records= 0;
410+
found_records= accepted_rows= 0;
411+
fetch_limit= HA_POS_ERROR;
412+
thd= thd_arg;
413+
sum_funcs= sum_funcs2= 0;
414+
procedure= 0;
415+
having= tmp_having= having_history= 0;
416+
having_is_correlated= false;
417+
group_list_for_estimates= 0;
418+
select_options= select_options_arg;
419+
result= result_arg;
420+
lock= thd_arg->lock;
421+
select_lex= 0; //for safety
422+
select_distinct= MY_TEST(select_options & SELECT_DISTINCT);
423+
no_order= 0;
424+
simple_order= 0;
425+
simple_group= 0;
426+
rand_table_in_field_list= 0;
427+
ordered_index_usage= ordered_index_void;
428+
need_distinct= 0;
429+
skip_sort_order= 0;
430+
with_two_phase_optimization= 0;
431+
save_qep= 0;
432+
spl_opt_info= 0;
433+
ext_keyuses_for_splitting= 0;
434+
spl_opt_info= 0;
435+
need_tmp= 0;
436+
hidden_group_fields= 0; /*safety*/
437+
error= 0;
438+
select= 0;
439+
return_tab= 0;
440+
ref_ptrs.reset();
441+
items0.reset();
442+
items1.reset();
443+
items2.reset();
444+
items3.reset();
445+
zero_result_cause= 0;
446+
optimization_state= JOIN::NOT_OPTIMIZED;
447+
have_query_plan= QEP_NOT_PRESENT_YET;
448+
initialized= 0;
449+
cleaned= 0;
450+
cond_equal= 0;
451+
having_equal= 0;
452+
exec_const_cond= 0;
453+
group_optimized_away= 0;
454+
no_rows_in_result_called= 0;
455+
positions= best_positions= 0;
456+
pushdown_query= 0;
457+
original_join_tab= 0;
458+
explain= NULL;
459+
tmp_table_keep_current_rowid= 0;
460+
461+
all_fields= fields_arg;
462+
if (&fields_list != &fields_arg) /* Avoid valgrind-warning */
463+
fields_list= fields_arg;
464+
non_agg_fields.empty();
465+
bzero((char*) &keyuse,sizeof(keyuse));
466+
having_value= Item::COND_UNDEF;
467+
tmp_table_param.init();
468+
tmp_table_param.end_write_records= HA_POS_ERROR;
469+
rollup.state= ROLLUP::STATE_NONE;
470+
471+
no_const_tables= FALSE;
472+
first_select= sub_select;
473+
set_group_rpa= false;
474+
group_sent= 0;
475+
476+
outer_ref_cond= pseudo_bits_cond= NULL;
477+
in_to_exists_where= NULL;
478+
in_to_exists_having= NULL;
479+
emb_sjm_nest= NULL;
480+
sjm_lookup_tables= 0;
481+
sjm_scan_tables= 0;
482+
is_orig_degenerated= false;
483+
with_ties_order_count= 0;
484+
};
485+
486+
394487
static void trace_table_dependencies(THD *thd,
395488
JOIN_TAB *join_tabs, uint table_count)
396489
{
@@ -29888,6 +29981,7 @@ bool JOIN::optimize_upper_rownum_func()
2988829981
return process_direct_rownum_comparison(thd, unit, outer_select->where);
2988929982
}
2989029983

29984+
2989129985
/**
2989229986
Test if the predicate compares rownum() with a constant
2989329987

sql/sql_select.h

Lines changed: 1 addition & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -1546,98 +1546,7 @@ class JOIN :public Sql_alloc
15461546
}
15471547

15481548
void init(THD *thd_arg, List<Item> &fields_arg, ulonglong select_options_arg,
1549-
select_result *result_arg)
1550-
{
1551-
join_tab= 0;
1552-
table= 0;
1553-
table_count= 0;
1554-
top_join_tab_count= 0;
1555-
const_tables= 0;
1556-
const_table_map= found_const_table_map= 0;
1557-
aggr_tables= 0;
1558-
eliminated_tables= 0;
1559-
join_list= 0;
1560-
implicit_grouping= FALSE;
1561-
sort_and_group= 0;
1562-
first_record= 0;
1563-
do_send_rows= 1;
1564-
duplicate_rows= send_records= 0;
1565-
found_records= accepted_rows= 0;
1566-
fetch_limit= HA_POS_ERROR;
1567-
thd= thd_arg;
1568-
sum_funcs= sum_funcs2= 0;
1569-
procedure= 0;
1570-
having= tmp_having= having_history= 0;
1571-
having_is_correlated= false;
1572-
group_list_for_estimates= 0;
1573-
select_options= select_options_arg;
1574-
result= result_arg;
1575-
lock= thd_arg->lock;
1576-
select_lex= 0; //for safety
1577-
select_distinct= MY_TEST(select_options & SELECT_DISTINCT);
1578-
no_order= 0;
1579-
simple_order= 0;
1580-
simple_group= 0;
1581-
rand_table_in_field_list= 0;
1582-
ordered_index_usage= ordered_index_void;
1583-
need_distinct= 0;
1584-
skip_sort_order= 0;
1585-
with_two_phase_optimization= 0;
1586-
save_qep= 0;
1587-
spl_opt_info= 0;
1588-
ext_keyuses_for_splitting= 0;
1589-
spl_opt_info= 0;
1590-
need_tmp= 0;
1591-
hidden_group_fields= 0; /*safety*/
1592-
error= 0;
1593-
select= 0;
1594-
return_tab= 0;
1595-
ref_ptrs.reset();
1596-
items0.reset();
1597-
items1.reset();
1598-
items2.reset();
1599-
items3.reset();
1600-
zero_result_cause= 0;
1601-
optimization_state= JOIN::NOT_OPTIMIZED;
1602-
have_query_plan= QEP_NOT_PRESENT_YET;
1603-
initialized= 0;
1604-
cleaned= 0;
1605-
cond_equal= 0;
1606-
having_equal= 0;
1607-
exec_const_cond= 0;
1608-
group_optimized_away= 0;
1609-
no_rows_in_result_called= 0;
1610-
positions= best_positions= 0;
1611-
pushdown_query= 0;
1612-
original_join_tab= 0;
1613-
explain= NULL;
1614-
tmp_table_keep_current_rowid= 0;
1615-
1616-
all_fields= fields_arg;
1617-
if (&fields_list != &fields_arg) /* Avoid valgrind-warning */
1618-
fields_list= fields_arg;
1619-
non_agg_fields.empty();
1620-
bzero((char*) &keyuse,sizeof(keyuse));
1621-
having_value= Item::COND_UNDEF;
1622-
tmp_table_param.init();
1623-
tmp_table_param.end_write_records= HA_POS_ERROR;
1624-
rollup.state= ROLLUP::STATE_NONE;
1625-
1626-
no_const_tables= FALSE;
1627-
first_select= sub_select;
1628-
set_group_rpa= false;
1629-
group_sent= 0;
1630-
1631-
outer_ref_cond= pseudo_bits_cond= NULL;
1632-
in_to_exists_where= NULL;
1633-
in_to_exists_having= NULL;
1634-
emb_sjm_nest= NULL;
1635-
sjm_lookup_tables= 0;
1636-
sjm_scan_tables= 0;
1637-
is_orig_degenerated= false;
1638-
1639-
with_ties_order_count= 0;
1640-
}
1549+
select_result *result_arg);
16411550

16421551
/* True if the plan guarantees that it will be returned zero or one row */
16431552
bool only_const_tables() { return const_tables == table_count; }

0 commit comments

Comments
 (0)