From 4faef6e2405f7a42f2f80ee9bfd9b96a5dcc74d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 25 Apr 2022 09:40:40 +0300 Subject: [PATCH] Cleanup: Remove IF_VALGRIND The purpose of the compress() wrapper my_compress_buffer() was twofold: silence Valgrind warnings about uninitialized memory access before zlib 1.2.4, and have PERFORMANCE_SCHEMA instrumentation of some zlib related memory allocation. Because of PERFORMANCE_SCHEMA, we cannot trivially replace my_compress_buffer() with compress(). az_open(): Remove a crc32() call. Any CRC of the empty string is 0. --- include/my_valgrind.h | 6 -- mysql-test/valgrind.supp | 166 +-------------------------------------- mysys/my_compress.c | 27 +------ sql/mysqld.cc | 8 +- storage/archive/azio.c | 2 +- 5 files changed, 10 insertions(+), 199 deletions(-) diff --git a/include/my_valgrind.h b/include/my_valgrind.h index a24ad597d36..dfe2c3db7b3 100644 --- a/include/my_valgrind.h +++ b/include/my_valgrind.h @@ -83,12 +83,6 @@ https://github.com/google/sanitizers/wiki/AddressSanitizerManualPoisoning */ # define MSAN_STAT_WORKAROUND(st) ((void) 0) #endif /* __has_feature(memory_sanitizer) */ -#ifdef HAVE_valgrind -#define IF_VALGRIND(A,B) A -#else -#define IF_VALGRIND(A,B) B -#endif - #ifdef TRASH_FREED_MEMORY /* _TRASH_FILL() has to call MEM_MAKE_ADDRESSABLE() to cancel any effect of diff --git a/mysql-test/valgrind.supp b/mysql-test/valgrind.supp index 1f2aa47820b..53c340ee6ec 100644 --- a/mysql-test/valgrind.supp +++ b/mysql-test/valgrind.supp @@ -1,5 +1,5 @@ # Copyright (c) 2005, 2015, Oracle and/or its affiliates. -# Copyright (c) 2008, 2019, MariaDB +# Copyright (c) 2008, 2022, MariaDB # # This program is free software; you can redistribute it and/or # modify it under the terms of the GNU Library General Public @@ -100,137 +100,6 @@ fun:_dl_start } -# -# Warnings in libz becasue it works with aligned memory(?) -# - -{ - libz tr_flush_block - Memcheck:Cond - fun:_tr_flush_block - fun:deflate_slow - fun:deflate - fun:do_flush - fun:gzclose -} - -{ - libz tr_flush_block2 - Memcheck:Cond - fun:_tr_flush_block - fun:deflate_slow - fun:deflate - fun:compress2 -} - -{ - libz longest_match - Memcheck:Cond - fun:longest_match - fun:deflate_slow - fun:deflate - fun:do_flush -} - -{ - libz longest_match called from btr_store_big_rec_extern_fields - Memcheck:Cond - fun:longest_match - fun:deflate_slow - fun:deflate - fun:btr_store_big_rec_extern_fields -} - -{ - libz longest_match called from page_zip_compress - Memcheck:Cond - fun:longest_match - fun:deflate_slow - fun:deflate - fun:page_zip_compress -} - -{ - libz longest_match2 - Memcheck:Cond - fun:longest_match - fun:deflate_slow - fun:deflate - fun:compress2 -} - -{ - libz longest_match 3 - Memcheck:Cond - fun:longest_match - fun:deflate_slow - fun:deflate - fun:gzclose -} - -{ - libz longest_match 4 - Memcheck:Cond - fun:longest_match - fun:deflate_slow - fun:deflate - fun:gzflush -} - -{ - libz longest_match3 - Memcheck:Cond - fun:longest_match - fun:deflate_slow - fun:deflate - fun:azflush -} - -{ - libz longest_match3 - Memcheck:Cond - fun:longest_match - fun:deflate_slow - fun:deflate - fun:azclose -} - -{ - libz deflate - Memcheck:Cond - obj:*/libz.so.* - obj:*/libz.so.* - fun:deflate - fun:compress2 -} - -{ - libz deflate2 - Memcheck:Cond - obj:*/libz.so.* - obj:*/libz.so.* - fun:deflate - obj:*/libz.so.* - fun:gzflush -} - -{ - libz deflate3 - Memcheck:Cond - obj:*/libz.so.* - obj:*/libz.so.* - fun:deflate - fun:do_flush -} - -{ - libz inflatereset2 - Memcheck:Cond - fun:inflateReset2 - fun:inflateInit2_ - fun:uncompress -} - # # Warning from my_thread_init becasue mysqld dies before kill thread exists @@ -705,39 +574,6 @@ fun:buf_buddy_relocate } -{ - Bug 59874 Valgrind warning in InnoDB compression code - Memcheck:Cond - fun:* - fun:* - fun:deflate - fun:btr_store_big_rec_extern_fields_func - fun:row_ins_index_entry_low - fun:row_ins_index_entry - fun:row_ins_index_entry_step - fun:row_ins - fun:row_ins_step - fun:row_insert_for_mysql -} - -{ - In page0zip.c we have already checked that the memory is initialized before calling deflate() - Memcheck:Cond - fun:* - fun:* - fun:deflate - fun:page_zip_compress -} - -{ - In page0zip.c we have already checked that the memory is initialized before calling deflate() - Memcheck:Cond - fun:* - fun:* - fun:deflate - fun:page_zip_compress_deflate -} - { Bug 59875 Valgrind warning in buf0buddy.c Memcheck:Addr1 diff --git a/mysys/my_compress.c b/mysys/my_compress.c index 11603e7ba24..e30d42c2dbf 100644 --- a/mysys/my_compress.c +++ b/mysys/my_compress.c @@ -1,4 +1,5 @@ /* Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights reserved. + Copyright (c) 2022, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -57,35 +58,11 @@ my_bool my_compress(uchar *packet, size_t *len, size_t *complen) } -/* - Valgrind normally gives false alarms for zlib operations, in the form of - "conditional jump depends on uninitialised values" etc. The reason is - explained in the zlib FAQ (http://www.zlib.net/zlib_faq.html#faq36): - - "That is intentional for performance reasons, and the output of deflate - is not affected." - - Also discussed on a blog - (http://www.sirena.org.uk/log/2006/02/19/zlib-generating-valgrind-warnings/): - - "...loop unrolling in the zlib library causes the mentioned - “Conditional jump or move depends on uninitialised value(s)” - warnings. These are safe since the results of the comparison are - subsequently ignored..." - - "the results of the calculations are discarded by bounds checking done - after the loop exits" - - Fix by initializing the memory allocated by zlib when running under Valgrind. - - This fix is safe, since such memory is only used internally by zlib, so we - will not hide any bugs in mysql this way. -*/ void *my_az_allocator(void *dummy __attribute__((unused)), unsigned int items, unsigned int size) { return my_malloc(key_memory_my_compress_alloc, (size_t)items*(size_t)size, - IF_VALGRIND(MY_ZEROFILL, MYF(0))); + MYF(0)); } void my_az_free(void *dummy __attribute__((unused)), void *address) diff --git a/sql/mysqld.cc b/sql/mysqld.cc index a3b417c8d07..574cce897a1 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -8706,12 +8706,16 @@ void set_server_version(char *buf, size_t size) { bool is_log= opt_log || global_system_variables.sql_log_slow || opt_bin_log; bool is_debug= IF_DBUG(!strstr(MYSQL_SERVER_SUFFIX_STR, "-debug"), 0); - bool is_valgrind= IF_VALGRIND(!strstr(MYSQL_SERVER_SUFFIX_STR, "-valgrind"), 0); + const char *is_valgrind= +#ifdef HAVE_VALGRIND + !strstr(MYSQL_SERVER_SUFFIX_STR, "-valgrind") ? "-valgrind" : +#endif + ""; strxnmov(buf, size - 1, MYSQL_SERVER_VERSION, MYSQL_SERVER_SUFFIX_STR, IF_EMBEDDED("-embedded", ""), - is_valgrind ? "-valgrind" : "", + is_valgrind, is_debug ? "-debug" : "", is_log ? "-log" : "", NullS); diff --git a/storage/archive/azio.c b/storage/archive/azio.c index 3529d875f72..d4aae2094a7 100644 --- a/storage/archive/azio.c +++ b/storage/archive/azio.c @@ -71,7 +71,7 @@ int az_open (azio_stream *s, const char *path, int Flags, File fd) s->in = 0; s->out = 0; s->back = EOF; - s->crc = crc32(0L, Z_NULL, 0); + s->crc = 0; s->transparent = 0; s->mode = 'r'; s->version = (unsigned char)az_magic[1]; /* this needs to be a define to version */