MySQL Bugs: #44796: valgrind: too many my_longlong10_to_str_8bit warnings after uncompressed

Bug #44796	valgrind: too many my_longlong10_to_str_8bit warnings after uncompressed_length
Submitted:	11 May 2009 17:28	Modified:	26 Jun 2009 2:32
Reporter:	Shane Bester (Platinum Quality Contributor)	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: DML	Severity:	S3 (Non-critical)
Version:	5.0.82, 5.1.30, 5.1.34, 5.1.35	OS:	Linux (32-bit fc8)
Assigned to:	Alexey Kopytov	CPU Architecture:	Any
Tags:	uncompressed_length, valgrind

Description:
when running the testcase, which uses uncompressed_length and avg functions together, 5.1.35 saw many of these warnings:

Version: '5.1.35-debug'  socket: '/tmp/mysql.sock'  port: 3306  yes
 Thread 10:
 Conditional jump or move depends on uninitialised value(s)
: my_longlong10_to_str_8bit (ctype-simple.c:907)
: String::set_int(long long, bool, charset_info_st*) (sql_string.cc:102)
: Item_int_func::val_str(String*) (item_func.cc:632)
: Item_copy_string::copy() (item.cc:3276)
: copy_fields(TMP_TABLE_PARAM*) (sql_select.cc:15027)
: end_send_group(JOIN*, st_join_table*, bool) (sql_select.cc:12133)
: do_select (sql_select.cc:10854)
: JOIN::exec (sql_select.cc:2199)
: mysql_select (sql_select.cc:2378)
: handle_select(THD*, st_lex*, select_result*, unsigned long) (sql_select.cc:268)
: execute_sqlcom_select(THD*, TABLE_LIST*) (sql_parse.cc:5009)
: mysql_execute_command(THD*) (sql_parse.cc:2211)
 
full output uploaded in file.
6.0.10 didn't have valgrind errors, but gave a different result to 5.x

How to repeat:
#run mysqld under valgrind, then run the sql:

drop table if exists `t1`;
create table `t1` (`c1` int) engine=myisam;
insert into `t1` values (1);
select avg(`c1`),uncompressed_length(`c1`) from `t1` limit 1;

all errors from 5.1.35 valgrind output

Attachment: bug44796_5.1.5_valgrind_output.txt (text/plain), 21.28 KiB.

the following testcase has warnings in strdup_root also, which is more serious i think.

drop table if exists `t1`;
create table `t1`(id int)engine=myisam;
insert into `t1` values (1),(2),(3),(4),(5),(6);
explain extended select *
from
(
        select
                uncompressed_length(`id`)
        from
                t1
        limit 1
) as `s`
limit 1;

Conditional jump or move depends on uninitialised value(s)
at 0x84F5A93: strdup_root (my_alloc.c:398)
by 0x8292D52: MYSQL_ERROR::set_msg(THD*, char const*) (sql_error.cc:56)
by 0x8292FFD: push_warning (sql_error.h:31)
by 0x8204F2B: execute_sqlcom_select(THD*, TABLE_LIST*) (sql_parse.cc:4996)
by 0x820EB04: mysql_execute_command(THD*) (sql_parse.cc:2211)
by 0x820FE04: mysql_parse (sql_parse.cc:5929)
by 0x8211123: dispatch_command (sql_parse.cc:1216)
by 0x82121F0: do_command(THD*) (sql_parse.cc:857)
by 0x8200889: handle_one_connection (sql_connect.cc:1115)
by 0x4893DA: start_thread (in /lib/libpthread-2.5.so)
by 0x3D606D: clone (in /lib/libc-2.5.so)

Cannot reproduce it on my 32-bit Ubuntu machine. 

Analyzing the reported valgrind backtraces does not help either. Here's the relevant code from ctype-simple.c in 5.1.35:

    907   if (uval == 0)
    908   {
    909     *--p= '0';
    910     len= 1;
    911     goto cnv;
    912   }

uval is unconditionally initialized at the very start of my_longlong10_to_str_8bit(), so there is no way it could be used uninitialized there.

Running the second test case in a debugger also does not show any traces of uninitialized variables usage.

My compiler and valgrind versions:

gcc (Ubuntu 4.3.2-1ubuntu12) 4.3.2
valgrind-3.3.1-Debian

Some compiler overoptimization or a valgrind bug in FC8?

Alexey, the exact server startup command was used:

valgrind --tool=memcheck --leak-check=full -v --show-reachable=yes  ./bin/mysqld  --no-defaults --basedir=./ --datadir=./data  --skip-grant-tables --skip-name-resolve --server-id=2 --port=3310 --socket=sock.sock

I get the valgrind warnings in standard mysql-5.1.34-linux-i686-glibc23.tar.gz version, as well as self compiled debug versions (CC=gcc CFLAGS=" -O2 -g -march=pentiumpro" CXX=gcc CXXFLAGS="-O1 -g -march=pentiumpro -felide-constructors -fno-exceptions -fno-rtti" )

Tried two versions:

valgrind-2.4.0
valgrind-3.2.1

it's complaining about uval, but i see it's indeed initialized. I'll try find out what's the problem after I decompile this binary :)

Reproduced it on mysql-5.1.34-linux-i686-glibc23.tar.gz.

The problem is actually in Item_func_uncompressed_length::val_int() rather than my_longlong10_to_str_8bit().

Item_func_uncompressed_length::val_int() assumes that the argument to uncompressed_length() is always at least 4 bytes long. This is where valgrind should have thrown an error about reading uninitialized memory. Instead this value is passed further to my_longlong10_to_str_8bit() where we get the error.

Why we get the error only on some builds is another question.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/74546

2748 Alexey Kopytov	2009-05-20
      Bug #44796:  valgrind: too many my_longlong10_to_str_8bit 
                   warnings after uncompressed_length 
       
      UNCOMPRESSED_LENGTH() did not validate its argument. In 
      particular, if the argument length was less than 4 bytes, 
      an uninitialized memory value was returned as a result. 
       
      Since the result of COMPRESS() is either an empty string or 
      a 4-byte length prefix followed by compressed data, the bug was 
      fixed by ensuring that the argument of UNCOMPRESSED_LENGTH() is 
      either an empty string or contains at least 5 bytes (as done in 
      UNCOMPRESS()). This is the best we can do to validate input 
      without decompressing. 
      modified:
        mysql-test/r/func_compress.result
        mysql-test/t/func_compress.test
        sql/item_strfunc.cc

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/74709

2749 Alexey Kopytov	2009-05-21
      Fixed a PB failure introduced by the patch for bug #44796.
      
      Set max_allowed_packet to get a consistent error message.
      modified:
        mysql-test/r/func_compress.result
        mysql-test/t/func_compress.test

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/74729

2750 Alexey Kopytov	2009-05-21
      Attempt #2 to fix PB failures introduced by the patch for bug #44796.
      
      Since max_allowed_packet is a read-only variable in 5.1 and up,
      disable warnings to avoid unnecessary test case complication.
      modified:
        mysql-test/r/func_compress.result
        mysql-test/t/func_compress.test

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/74772

3315 Alexey Kopytov	2009-05-22
      Fixed the incorrect merge to 6.0 in the patch for bug #44796:
      
      Since we can issue errors from SELECT in 6.0, the warning level
      in Item_func_uncompressed_length::val_int() was lowered to
      MYSQL_ERROR::WARN_LEVEL_WARN. 
      modified:
        sql/item_strfunc.cc

Pushed into 5.0.83 (revid:joro@sun.com-20090528073529-q9b8s60vlpu28fny) (version source revid:patrick.crews@sun.com-20090522153852-xlmvn3eg3zeak6yq) (merge vers: 5.0.83) (pib:6)

Pushed into 5.1.36 (revid:joro@sun.com-20090528073639-yohsb4q1jzg7ycws) (version source revid:alexey.kopytov@sun.com-20090521175148-zzoi1tg5yu0wp2t2) (merge vers: 5.1.36) (pib:6)

Noted in 5.0.83, 5.1.36 changelogs.

Several Valgrind warnings were silenced.

Setting report to NDI pending push into 6.0.x.

Corrected changelog entry:

UNCOMPRESSED_LENGTH() returned a garbage result when passed a string
shorter than 5 bytes. Now UNCOMPRESSED_LENGTH() returns NULL and
generates a warning.

Pushed into 5.4.4-alpha (revid:alik@sun.com-20090616183122-chjzbaa30qopdra9) (version source revid:alexey.kopytov@sun.com-20090522085950-ax7v97h395a4g2yy) (merge vers: 6.0.12-alpha) (pib:11)

Noted in 5.4.4 changelog.

Noted in 5.4.2 changelog because next 5.4 version will be 5.4.2 and not 5.4.4.

Ignore previous comment about 5.4.2.

Pushed into 5.1.37-ndb-7.0.8 (revid:jonas@mysql.com-20090826132541-yablppc59e3yb54l) (version source revid:jonas@mysql.com-20090826132541-yablppc59e3yb54l) (merge vers: 5.1.37-ndb-7.0.8) (pib:11)

Pushed into 5.1.37-ndb-6.3.27 (revid:jonas@mysql.com-20090826105955-bkj027t47gfbamnc) (version source revid:jonas@mysql.com-20090826105955-bkj027t47gfbamnc) (merge vers: 5.1.37-ndb-6.3.27) (pib:11)

Pushed into 5.1.37-ndb-6.2.19 (revid:jonas@mysql.com-20090825194404-37rtosk049t9koc4) (version source revid:jonas@mysql.com-20090825194404-37rtosk049t9koc4) (merge vers: 5.1.37-ndb-6.2.19) (pib:11)

Pushed into 5.1.35-ndb-7.1.0 (revid:magnus.blaudd@sun.com-20090827163030-6o3kk6r2oua159hr) (version source revid:jonas@mysql.com-20090826132541-yablppc59e3yb54l) (merge vers: 5.1.37-ndb-7.0.8) (pib:11)

The 5.4 fix has been pushed to 5.4.2.