Skip to content

Commit 6fd57f4

Browse files
committed
MDEV-36542 Remove UNINIT_VAR(x)=x under UBSAN
Clang processes the "int x=x" code from UNINIT_VAR literally resulting in an uninitialized read and write. This is something we want to avoid. Gcc does the same without emitting warnings. As the UNINIT_VAR was around avoiding compiler false detection, and clang doesn't false detect, is default action is a noop. Static analysers (examined Infer and SonarQube) are clang based and have the same detection. Using a __clang__ instead of WITH_UBSAN would acheived a better result, however reviewer wanted to keep WITH_UBSAN only. LINT_INIT_STRUCT is no longer required, even a gcc-4.8.5 doesn't warn with this construct removed which matches the comment that it was fixed in gcc ~4.7. mysql.cc - all paths in com_go populate buff before use. json: Item_func_json_merge::val_str LINT_INIT(js2) unneeded as usage in the previous statements it is explicitly initialized to NULL. Item_func_json_contains_path::val_bool n_found is guarded by an uninitialized read by mode_one and from gcc-13.3.0 in Ubuntu 24.04 this is detected. As the only remaining use of LINIT_INIT this usage has been applied with the expanded macro with the unused _lint define removed. The LINT_INIT macro is removed. _ma_ck_delete - org_key only valid under share->now_transactional likewise with _ma_ck_write_btree_with_log connect engine never used anything that FORCE_INIT_OF_VARS would change. Reviewer: Monty
1 parent 2147951 commit 6fd57f4

File tree

7 files changed

+16
-27
lines changed

7 files changed

+16
-27
lines changed

client/mysql.cc

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3499,8 +3499,6 @@ static int com_go(String *buffer, char *)
34993499
old_buffer.copy();
35003500
}
35013501

3502-
/* Remove garbage for nicer messages */
3503-
LINT_INIT_STRUCT(buff[0]);
35043502
remove_cntrl(*buffer);
35053503

35063504
if (buffer->is_empty())

include/m_string.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,6 @@ extern const char _dig_vec_lower[];
7979

8080
extern char *strmov_overlapp(char *dest, const char *src);
8181

82-
#if defined(_lint) || defined(FORCE_INIT_OF_VARS)
83-
#define LINT_INIT_STRUCT(var) bzero(&var, sizeof(var)) /* No uninitialize-warning */
84-
#else
85-
#define LINT_INIT_STRUCT(var)
86-
#endif
87-
8882
/* Prototypes for string functions */
8983

9084
extern void bmove_upp(uchar *dst,const uchar *src,size_t len);

include/my_global.h

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -448,22 +448,19 @@ extern "C" int madvise(void *addr, size_t len, int behav);
448448
/*
449449
Suppress uninitialized variable warning without generating code.
450450
*/
451-
#if defined(__GNUC__)
452-
/* GCC specific self-initialization which inhibits the warning. */
451+
#if defined(__GNUC__) && !defined(WITH_UBSAN)
452+
/*
453+
GCC specific self-initialization which inhibits the warning.
454+
clang and static analysis will complain loudly about this
455+
so compile those under WITH_UBSAN.
456+
*/
453457
#define UNINIT_VAR(x) x= x
454-
#elif defined(_lint) || defined(FORCE_INIT_OF_VARS)
458+
#elif defined(FORCE_INIT_OF_VARS)
455459
#define UNINIT_VAR(x) x= 0
456460
#else
457461
#define UNINIT_VAR(x) x
458462
#endif
459463

460-
/* This is only to be used when resetting variables in a class constructor */
461-
#if defined(_lint) || defined(FORCE_INIT_OF_VARS)
462-
#define LINT_INIT(x) x= 0
463-
#else
464-
#define LINT_INIT(x)
465-
#endif
466-
467464
#if !defined(HAVE_UINT)
468465
#undef HAVE_UINT
469466
#define HAVE_UINT

sql/item_jsonfunc.cc

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1728,9 +1728,16 @@ bool Item_func_json_contains_path::val_bool()
17281728
longlong result;
17291729
json_path_t p;
17301730
int n_found;
1731-
LINT_INIT(n_found);
17321731
int array_sizes[JSON_DEPTH_LIMIT];
17331732
uint has_negative_path= 0;
1733+
#if defined(FORCE_INIT_OF_VARS)
1734+
/*
1735+
Initialization force not required after gcc 13.3
1736+
where it correctly sees that an uninitialized read
1737+
of n_found doesn't occur with mode_one being true.
1738+
*/
1739+
n_found= 0;
1740+
#endif
17341741

17351742
if ((null_value= args[0]->null_value))
17361743
return 0;
@@ -1770,8 +1777,6 @@ bool Item_func_json_contains_path::val_bool()
17701777
bzero(p_found, (arg_count-2) * sizeof(bool));
17711778
n_found= arg_count - 2;
17721779
}
1773-
else
1774-
n_found= 0; /* Just to prevent 'uninitialized value' warnings */
17751780

17761781
result= 0;
17771782
while (json_get_path_next(&je, &p) == 0)
@@ -2644,7 +2649,6 @@ String *Item_func_json_merge::val_str(String *str)
26442649
String *js1= args[0]->val_json(&tmp_js1), *js2=NULL;
26452650
uint n_arg;
26462651
THD *thd= current_thd;
2647-
LINT_INIT(js2);
26482652

26492653
JSON_DO_PAUSE_EXECUTION(thd, 0.0002);
26502654

storage/connect/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ user_connect.h valblk.h value.h xindex.h xobject.h xtable.h)
4646
#
4747
# Definitions that are shared for all OSes
4848
#
49-
add_definitions( -DMARIADB -DFORCE_INIT_OF_VARS -Dconnect_EXPORTS)
49+
add_definitions( -DMARIADB -Dconnect_EXPORTS)
5050
add_definitions( -DHUGE_SUPPORT -DGZ_SUPPORT )
5151

5252
macro(DISABLE_WARNING W)

storage/maria/ma_delete.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,6 @@ my_bool _ma_ck_delete(MARIA_HA *info, MARIA_KEY *key)
165165
MARIA_KEY org_key;
166166
DBUG_ENTER("_ma_ck_delete");
167167

168-
LINT_INIT_STRUCT(org_key);
169-
170168
alloc_on_stack(*info->stack_end_ptr, key_buff, buff_alloced,
171169
key->keyinfo->max_store_length);
172170
if (!key_buff)

storage/maria/ma_write.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -483,8 +483,6 @@ static my_bool _ma_ck_write_btree_with_log(MARIA_HA *info, MARIA_KEY *key,
483483
my_bool transactional= share->now_transactional;
484484
DBUG_ENTER("_ma_ck_write_btree_with_log");
485485

486-
LINT_INIT_STRUCT(org_key);
487-
488486
if (transactional)
489487
{
490488
/* Save original value as the key may change */

0 commit comments

Comments
 (0)