Bug #43884 SP + filename param as TEXT + LOAD_FILE inside, causes valgrind warnings in PB
Submitted: 26 Mar 2009 14:15 Modified: 1 Aug 2010 22:26
Reporter: Luis Soares Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Stored Routines Severity:S3 (Non-critical)
Version:5.1-bugteam, 6.0-bugteam, 5.0, 5.1, 6.0 bzr OS:Any
Assigned to: Tatiana Azundris Nuernberg CPU Architecture:Any
Tags: load_file, PB, stored procedure, text data type, valgrind

[26 Mar 2009 14:15] Luis Soares
Description:
Description
===========

Creating a stored procedure that takes a filename as a parameter with data type TEXT causes valgrind to produce several warnings, eg:

==6224== Conditional jump or move depends on uninitialised value(s)
==6224==    at 0x5C1ABC: Item_load_file::val_str(String*) (sql_string.h:102)
==6224==    by 0x56C3DC: Item::save_in_field(Field*, bool) (item.cc:4889)
==6224==    by 0x6882AE: fill_record(THD*, Field**, List<Item>&, bool) (sql_base.cc:8195)
==6224==    by 0x688E23: fill_record_n_invoke_before_triggers(THD*, Field**, List<Item>&, bool, Table_triggers_list*, trg_event_type) (sql_base.cc:8238)
==6224==    by 0x6D451D: mysql_insert(THD*, TABLE_LIST*, List<Item>&, List<List<Item> >&, List<Item>&, List<Item>&, enum_duplicates, bool) (sql_insert.cc:778)
==6224==    by 0x64BBEC: mysql_execute_command(THD*) (sql_parse.cc:3088)
==6224==    by 0x7D7B8B: sp_instr_stmt::exec_core(THD*, unsigned*) (sp_head.cc:2899)
==6224==    by 0x7DB589: sp_lex_keeper::reset_lex_and_exec_core(THD*, unsigned*, bool, sp_instr*) (sp_head.cc:2728)
==6224==    by 0x7DE868: sp_instr_stmt::execute(THD*, unsigned*) (sp_head.cc:2842)
==6224==    by 0x7DCD56: sp_head::execute(THD*) (sp_head.cc:1248)
==6224==    by 0x7DFC0F: sp_head::execute_procedure(THD*, List<Item>*) (sp_head.cc:1977)
==6224==    by 0x64BA0B: mysql_execute_command(THD*) (sql_parse.cc:4286)
==6224==    by 0x64F6BA: mysql_parse(THD*, char const*, unsigned, char const**) (sql_parse.cc:5865)
==6224==    by 0x64FCC2: dispatch_command(enum_server_command, THD*, char*, unsigned) (sql_parse.cc:1216)
==6224==    by 0x651089: do_command(THD*) (sql_parse.cc:857)
==6224==    by 0x64057B: handle_one_connection (sql_connect.cc:1115)

NOTES:

 * The above stack trace has been observed in pushbuild. However, the warning is also reproducible locally. In my local environment the warnings are slightly different from Pushbuild, but seems the same issue.

 * Replacing TEXT with VARCHAR for the type in the SP makes the warnings go away.

Details for pushbuild failure(s)
================================

https://intranet.mysql.com/secure/pushbuild/showpush.pl?dir=bzr_mysql-5.1-bugteam&order=63... 

'pb-valgrind-*' Valgrind rpl_binlog_row

http://tinyurl.com/dc3l5w

How to repeat:
Build MySQL with valgrind support
=================================

For example, using the build script:

./BUILD/compile-pentium-valgrind-max

Create the following test:
==========================

let file= ../../std_data/words2.dat;

CREATE TABLE t1 (t TEXT);
DELIMITER |;
CREATE PROCEDURE p(file TEXT)
BEGIN
  INSERT INTO t1 VALUES (LOAD_FILE(file));
END|
DELIMITER ;|
--replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
--eval CALL p('$file')

DROP TABLE t1; 
DROP PROCEDURE p;

exit;

Run the test using MTR:
=======================

/mtr --valgrind "--valgrind-options=--gen-suppressions=all --show-reachable=yes" <test_name>

Check var/log/mysqld.1.err for the warnings.
[26 Mar 2009 14:33] Luis Soares
Fix for the how to repeat test:

In the test posted, replace:

  let file= ../../std_data/words2.dat;

with

  let $file= $MYSQLTEST_VARDIR/std_data/words2.dat;
[26 Mar 2009 20:48] Sveta Smirnova
Thank you for the report.

Verified as described.
[27 Mar 2009 11:00] Bugs System
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/70653

2847 Georgi Kodinov	2009-03-27
      Worked around the problem described in bug #43884.
[27 Mar 2009 14:56] Bugs System
Pushed into 5.1.34 (revid:joro@sun.com-20090327143448-wuuuycetc562ty6o) (version source revid:joro@sun.com-20090327105931-ehads47i23q0gj25) (merge vers: 5.1.34) (pib:6)
[7 Apr 2009 9:18] Tatiana Azundris Nuernberg
==21204== Conditional jump or move depends on uninitialised value(s)
==21204==    at 0x5F4377: String::c_ptr() (sql_string.h:102)
==21204==    by 0x6368B2: Item_load_file::val_str(String*) (item_strfunc.cc:2943)
==21204==    by 0x5E3579: Item::save_in_field(Field*, bool) (item.cc:4892)
==21204==    by 0x707A94: fill_record(THD*, Field**, List<Item>&, bool) (sql_base.cc:8203)
==21204==    by 0x70A7D7: fill_record_n_invoke_before_triggers(THD*, Field**, List<Item>&, bool, Table_triggers_list*, trg_event_type) (sql_base.cc:8246)
==21204==    by 0x761CC6: mysql_insert(THD*, TABLE_LIST*, List<Item>&, List<List<Item> >&, List<Item>&, List<Item>&, enum_duplicates, bool) (sql_insert.cc:778)
==21204==    by 0x6C69E0: mysql_execute_command(THD*) (sql_parse.cc:3144)
==21204==    by 0x88248E: sp_instr_stmt::exec_core(THD*, unsigned*) (sp_head.cc:2904)
==21204==    by 0x8826C6: sp_lex_keeper::reset_lex_and_exec_core(THD*, unsigned*, bool, sp_instr*) (sp_head.cc:2732)
==21204==    by 0x888814: sp_instr_stmt::execute(THD*, unsigned*) (sp_head.cc:2846)
==21204==    by 0x8849BB: sp_head::execute(THD*) (sp_head.cc:1252)
==21204==    by 0x885740: sp_head::execute_procedure(THD*, List<Item>*) (sp_head.cc:1981)
==21204==    by 0x6CAA6C: mysql_execute_command(THD*) (sql_parse.cc:4350)
==21204==    by 0x6CCB83: mysql_parse(THD*, char const*, unsigned, char const**) (sql_parse.cc:5929)
==21204==    by 0x6CD979: dispatch_command(enum_server_command, THD*, char*, unsigned) (sql_parse.cc:1216)
==21204==    by 0x6CECBF: do_command(THD*) (sql_parse.cc:857)

#0  Item_load_file::val_str (this=0x18ee180, str=0x18ee190) at item_strfunc.cc:2943
#1  0x00000000005e357a in Item::save_in_field (this=0x18ee180, field=0x18fbe90, no_conversions=false) at item.cc:4892
#2  0x0000000000707a95 in fill_record (thd=0x18d27a0, ptr=0x18fbe68, values=@0x18edf40, ignore_errors=false) at sql_base.cc:8203
#3  0x000000000070a7d8 in fill_record_n_invoke_before_triggers (thd=0x18d27a0, ptr=0x18fbe60, values=@0x18edf40, ignore_errors=false, triggers=0x0, event=TRG_EVENT_INSERT) at sql_base.cc:8246
#4  0x0000000000761cc7 in mysql_insert (thd=0x18d27a0, table_list=0x18edbf0, fields=@0x18f6a10, values_list=@0x18f6a70, update_fields=@0x18f6a50, update_values=@0x18f6a30, duplic=DUP_ERROR, ignore=false) at sql_insert.cc:778
#5  0x00000000006c69e1 in mysql_execute_command (thd=0x18d27a0) at sql_parse.cc:3144
#6  0x000000000088248f in sp_instr_stmt::exec_core (this=0x18ee2d0, thd=0x18d27a0, nextp=0x7f2f07a4e1d8) at sp_head.cc:2904
#7  0x00000000008826c7 in sp_lex_keeper::reset_lex_and_exec_core (this=0x18ee318, thd=0x18d27a0, nextp=0x7f2f07a4e1d8, open_tables=false, instr=0x18ee2d0) at sp_head.cc:2732
#8  0x0000000000888815 in sp_instr_stmt::execute (this=0x18ee2d0, thd=0x18d27a0, nextp=0x7f2f07a4e1d8) at sp_head.cc:2846
#9  0x00000000008849bc in sp_head::execute (this=0x18e8190, thd=0x18d27a0) at sp_head.cc:1252
#10 0x0000000000885741 in sp_head::execute_procedure (this=0x18e8190, thd=0x18d27a0, args=0x18d4b98) at sp_head.cc:1981
#11 0x00000000006caa6d in mysql_execute_command (thd=0x18d27a0) at sql_parse.cc:4350
#12 0x00000000006ccb84 in mysql_parse (thd=0x18d27a0, inBuf=0x18e9220 "CALL p('/misc/mysql/forest/43884/51-43884/mysql-test/var/std_data/words2.dat')", length=78, found_semicolon=0x7f2f07a4ff00) at sql_parse.cc:5929
#13 0x00000000006cd97a in dispatch_command (command=COM_QUERY, thd=0x18d27a0, packet=0x18e3aa1 "CALL p('/misc/mysql/forest/43884/51-43884/mysql-test/var/std_data/words2.dat')", packet_length=78) at sql_parse.cc:1216
#14 0x00000000006cecc0 in do_command (thd=0x18d27a0) at sql_parse.cc:857
#15 0x00000000006bb941 in handle_one_connection (arg=0x18d27a0) at sql_connect.cc:1115

String *Item_load_file::val_str(String *str)
{
  String *file_name;
...
  if (!(file_name= args[0]->val_str(str))
...
  (void) fn_format(path, file_name->c_ptr(), mysql_real_data_home, "",
		   MY_RELATIVE_PATH | MY_UNPACK_FILENAME);

print *str
$2 = {Ptr = 0x7fc3fd6147e0 "", str_length = 766, Alloced_length = 766, alloced = false, str_charset = 0x111f880}

  if (!(file_name= args[0]->val_str(str))

args[0]: (this is our TEXT parameter)
print *this
$3 = {<Item> = {_vptr.Item = 0xb8ab90, rsize = 0, str_value = {Ptr = 0x0, str_length = 0, Alloced_length = 0, alloced = false, str_charset = 0x111f880}, name = 0x18e4880 "file", orig_name = 0x0, next = 0x0, max_length = 65535, name_length = 4, marker = 0 '\0', decimals = 31 '\037', maybe_null = 1 '\001', null_value = 0 '\0', unsigned_flag = 0 '\0', with_sum_func = 0 '\0', fixed = 1 '\001', is_autogenerated_name = 1 '\001', collation = {collation = 0x1120d40, derivation = DERIVATION_IMPLICIT, repertoire = 3}, with_subselect = 0 '\0', cmp_context = 4294967295}, m_thd = 0x18ce960, m_name = {str = 0x18e4790 "file", length = 4}, m_sp = 0x18e3e60}

String *Item_sp_variable::val_str(String *sp)
{
  DBUG_ASSERT(fixed);
  Item *it= this_item();

print *it (This is the argument for the above parameter)
$4 = {_vptr.Item = 0xb89c90, rsize = 0, str_value = {Ptr = 0x18f6a70 "/misc/mysql/forest/43884/51-43884/mysql-test/var/std_data/words2.dat", str_length = 68, Alloced_length = 0, alloced = false, str_charset = 0x1120d40}, name = 0x18ea890 "file", orig_name = 0x0, next = 0x18e4f50, max_length = 65535, name_length = 0, marker = 0 '\0', decimals = 31 '\037', maybe_null = 1 '\001', null_value = 0 '\0', unsigned_flag = 0 '\0', with_sum_func = 0 '\0', fixed = 1 '\001', is_autogenerated_name = 1 '\001', collation = {collation = 0x1120d40, derivation = DERIVATION_IMPLICIT, repertoire = 3}, with_subselect = 0 '\0', cmp_context = 4294967295}

  String *res= it->val_str(sp);

print *res (note that Ptr is identical with above it->Ptr!)
$9 = {Ptr = 0x18f6a70 "/misc/mysql/forest/43884/51-43884/mysql-test/var/std_data/words2.dat", str_length = 68, Alloced_length = 0, alloced = false, str_charset = 0x1120d40}

print str_value (note that Ptr is identical with above it->Ptr!)
$11 = {Ptr = 0x18f6a70 "/misc/mysql/forest/43884/51-43884/mysql-test/var/std_data/words2.dat", str_length = 68, Alloced_length = 0, alloced = false, str_charset = 0x1120d40}

Note also that 68 is the length of the payload only, no trailing \0!

print *file_name
$4 = {Ptr = 0x18e7ee0 "/misc/mysql/forest/43884/51-43884/mysql-test/var/std_data/words2.dat", str_length = 68, Alloced_length = 0, alloced = false, str_charset = 0x1120d40}

  inline char *c_ptr()
  {
    if (!Ptr || Ptr[str_length])		/* Should be safe */
      (void) realloc(str_length);
    return Ptr;
  }

^- let's make this c_ptr_safe(), shall we?
[7 Apr 2009 11:45] Bugs System
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/71530

2857 Tatiana A. Nurnberg	2009-04-07
      Bug#43884: SP + filename param as TEXT + LOAD_FILE inside, causes valgrind warnings in PB
      
      We were trying to \0 terminate a string literal.
      valgrind rightfully didn't like that.
      
      We now reallocate as needed before appending
      characters.
     @ mysql-test/suite/rpl/r/rpl_loadfile.result
        Revert hotfix for this test
        now that the source has been
        amended.
     @ mysql-test/suite/rpl/t/rpl_loadfile.test
        Revert hotfix for this test
        now that the source has been
        amended.
     @ sql/item_strfunc.cc
        Re-alloc as needed when appending a \0.
[13 Apr 2009 9:20] Bugs System
Pushed into 6.0.11-alpha (revid:alik@sun.com-20090413084402-snnrocwzktcl88ny) (version source revid:joro@sun.com-20090327120330-0jovz2hiatezpr7m) (merge vers: 6.0.11-alpha) (pib:6)
[9 May 2009 16:39] Bugs System
Pushed into 5.1.34-ndb-6.2.18 (revid:jonas@mysql.com-20090508185236-p9b3as7qyauybefl) (version source revid:jonas@mysql.com-20090508185236-p9b3as7qyauybefl) (merge vers: 5.1.34-ndb-6.2.18) (pib:6)
[9 May 2009 17:36] Bugs System
Pushed into 5.1.34-ndb-6.3.25 (revid:jonas@mysql.com-20090509063138-1u3q3v09wnn2txyt) (version source revid:jonas@mysql.com-20090509063138-1u3q3v09wnn2txyt) (merge vers: 5.1.34-ndb-6.3.25) (pib:6)
[9 May 2009 18:33] Bugs System
Pushed into 5.1.34-ndb-7.0.6 (revid:jonas@mysql.com-20090509154927-im9a7g846c6u1hzc) (version source revid:jonas@mysql.com-20090509154927-im9a7g846c6u1hzc) (merge vers: 5.1.34-ndb-7.0.6) (pib:6)
[8 Oct 2009 0:07] Paul DuBois
Noted in 6.0.11 changelog.

Incorrect string termination caused a Valgrind warning.

Setting report to NDI pending push into 5.1.x, 5.4.x.