Re: [Valgrind-users] valgrind sprintf arguments

2014-07-30 Thread Matthias Apitz
El día Tuesday, July 29, 2014 a las 08:35:51PM +0200, Philippe Waroquiers 
escribió:

 On Tue, 2014-07-29 at 08:15 +0200, Matthias Apitz wrote:
  El día Monday, July 28, 2014 a las 07:11:02AM -0700, John Reiser escribió:
  
==17454== Conditional jump or move depends on uninitialised value(s)
==17454==at 0x5921F10: strchrnul (in /lib/libc-2.11.3.so)
==17454==by 0x58E55D6: vfprintf (in /lib/libc-2.11.3.so)
 ...
  All was fine. Why is valgrind complaining?
 
 Here is an hypothesis:
 
 Looking at (in the SVN version) shared/vg_replace_strmem.c
 strchrnul should have been replaced by the implementation
 in vg_replace_strmem.c.
 
 From the stacktrace above, it looks like strchrnul was not replaced.

This (your hypothesis) matches with the fact that I was not able to
reproduce the same problem with a small C-pgm doing exactly the same
sprintf() call:

#include stdio.h
#define MAX_SEL_LEN 1024

static int select_record();

main()
{

select_record(SELECT rowid, sisisinst.* from sisisinst);

}

static int select_record (char *sel_anw)
{
  char  select_anw[MAX_SEL_LEN];
  char *name = sisisinst;

  sprintf (select_anw, sel_anw, name, name);

}

in this case the valgrind does not complain with the sprintf() ...
strchrnul() stack trace; and in addition, the output shows the
replacement of strchrnul() to some valgrind' function implementation; I
have the output here:

the big C-server (where strchrnul() is not replaced):

http://www.unixarea.de/valg1.txt

the small C-code aboce (where strchrnul() is replaced):

http://www.unixarea.de/valg2.txt

I'm not a valgrind expert and can not say what is causing the
difference; let me know if you want me to file a bug report with the data.

 Often, the glibc implementations of the str* functions
 are highly optimised, and causes false positive
 (e.g. by assuming they can read a few more bytes than
 the end of the string).
 
 That might be the case for you.
 
 What you could do is to redo your GDB session, but using
 the valgrind gdbserver monitor command to check the definedness
 of the printf args at various momenets and
 just before the print call.
 
 Note that using --track-origins=yes should indicate where
 this unitialised byte is coming from.
 
 Would be nice to understand why strchrnul was not replaced
 (using e.g. -v -v -v --trace-redir=yes).

yes, would be nice; let me know if you need more data to understand this
problem of not replacing strchrnul()

Thx

matthias

-- 
Matthias Apitz   |  /\   ASCII Ribbon Campaign:
E-mail: g...@unixarea.de |  \ /   - No HTML/RTF in E-mail
WWW: http://www.unixarea.de/ |   X- No proprietary attachments
phone: +49-170-4527211   |  / \   - Respect for open standards
 | en.wikipedia.org/wiki/ASCII_Ribbon_Campaign

--
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls. 
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071iu=/4140/ostg.clktrk
___
Valgrind-users mailing list
Valgrind-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/valgrind-users


Re: [Valgrind-users] valgrind sprintf arguments

2014-07-30 Thread Matthias Apitz
El día Wednesday, July 30, 2014 a las 08:17:11AM +0200, Matthias Apitz escribió:
 
 the big C-server (where strchrnul() is not replaced):
 
 http://www.unixarea.de/valg1.txt
 
 the small C-code above (where strchrnul() is replaced):
 
 http://www.unixarea.de/valg2.txt
 
 I'm not a valgrind expert and can not say what is causing the
 difference; let me know if you want me to file a bug report with the data.

I found the reason by reading and comparing both files line by line; the
problem is visible in valg1.txt:

ERROR: ld.so: object '/usr/local/lib/valgrind/vgpreload_core-x86-linux.so' from 
LD_PRELOAD
cannot be preloaded: ignored.

this was caused by a Linux problem; someone did an error and updated the
SuSE Linux to x86_64 while it should stay 32bit; valgrind was
was compiled on it; later the box was resetted to 32bit x86
but valgrind was not re-compiled again; I did this now and now all the
C-lib functions are redirected correctly.

Thread closed :-)

Thanks

matthias
-- 
Matthias Apitz   |  /\   ASCII Ribbon Campaign:
E-mail: g...@unixarea.de |  \ /   - No HTML/RTF in E-mail
WWW: http://www.unixarea.de/ |   X- No proprietary attachments
phone: +49-170-4527211   |  / \   - Respect for open standards
 | en.wikipedia.org/wiki/ASCII_Ribbon_Campaign

--
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls. 
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071iu=/4140/ostg.clktrk
___
Valgrind-users mailing list
Valgrind-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/valgrind-users


Re: [Valgrind-users] valgrind sprintf arguments

2014-07-29 Thread Matthias Apitz
El día Monday, July 28, 2014 a las 07:11:02AM -0700, John Reiser escribió:

  ==17454== Conditional jump or move depends on uninitialised value(s)
  ==17454==at 0x5921F10: strchrnul (in /lib/libc-2.11.3.so)
  ==17454==by 0x58E55D6: vfprintf (in /lib/libc-2.11.3.so)
 
  the involved fuctions are shown below; the statement in question (see below)
  is
  
sprintf (select_anw, sel_anw, name, name);  * 
  sisisinst.c:1397
  
  I have checked carefully the code and the 4 args to sprintf() are
  all correct defined on the stack; when I change the code to:
  
  
select_anw[0] = '\0';
sprintf (select_anw, sel_anw, name, name); 
  
  then is valgrind happy, i.e, does not raise the messages any more;
 
 You say that all 4 args are on the stack.  What are their actual addresses?
 Run with --db-attach=yes, say 'y' when asked, and use gdb to look around.
 
 One possibility is that sel_anw (the format string) has been overwritten
 because the string being built into select_anw (the buffer) has overflowed.
 
 Try changing the code to use
   snprintf(select_anw, LEN_SELECT, sel_anw, name, name);
 which is much safer.

Thanks for your hints. Before I will change the code (yes, your proposal is
much safer), I will try to understand why valgrind is complaining;

I grabbed the gdb and debugged through the code:

(gdb) where
#0  DB_rdir (tabmodul=0xf6a68170 sisisinst, key=2, scroll=1, lock=0, 
p_daten=0xc860) at dbcall.c:1834
#1  0xf6a4cc21 in DB_ChkVer () at dbcall.c:604
#2  0xf6a4d099 in DB_opdbP (mode=1) at dbcall.c:955
#3  0xf6a4cd3a in DB_opdb () at dbcall.c:654
#4  0x0804bf6a in InitVDaemon () at ZFLVDaemon.c:715
#5  0x0804baad in main (argc=1, argv=0xce14) at ZFLVDaemon.c:413
(gdb) p sel_anw
$3 = (char (*)[1000]) 0xc3c0

sel_anw is an automatic char[1000] area and will now be initialized from
some static string 'SELECT1':

1885strcpy(sel_anw, SELECT1);
(gdb) 
1887  strcpy(where_anw, WHERE1);
(gdb) 

'sel_anw' and 'where_anw' both are set correctly:

(gdb) p sel_anw
$4 = SELECT rowid, %s.* from %s, '\000' repeats 46 times ...
(gdb) p where_anw
$5 = %s = :v1, '\000' repeats 24 times ...

(gdb) p sel_anw
$6 = (char (*)[1000]) 0xc3c0

(gdb) p where_anw
$7 = (char (*)[5000]) 0xb030

the pointers are passed correctly to sisisinst() function:

(gdb) s
sisisinst (zugriff=1, scroll=1, lock=0, key=2, sto=-2, p_daten=0xc860, 
sel_anw=0xc3c0 SELECT rowid, %s.* from %s, where_anw=0xb030 %s = 
:v1, p_btw_daten=0x0, 
order_by=0x0, auf_ab=0x0, group_by=0x0, having=0x0, into_temp=0x0, 
count=0xb02c) at sisisinst.c:799

933 case  RDIR  :   db_ret = select_record(scroll, lock, key,
(gdb) s

and passed further to select_record() function:

Breakpoint 2, select_record (scroll=1, lock=0, key=2, sel_anw=0xc3c0 
SELECT rowid, %s.* from %s, 
where_anw=0xb030 %s = :v1, p_daten=0xf6ae04a0 hrec_sisisinst, 
i_between=0, p_oben=0xaf30, 
order_by=0x0, auf_ab=0x0, group_by=0x0, having=0x0, into_temp=0x0, 
count=0xb02c) at sisisinst.c:1353

(gdb) p sel_anw
$8 = 0xc3c0 SELECT rowid, %s.* from %s
(gdb) p where_anw
$9 = 0xb030 %s = :v1

(gdb) 
1396  char *name = TAB_SISISINST;
(gdb) 

this is now the call to sprintf() which was identified by valgrind:

1397  sprintf (select_anw, sel_anw, name, name);
(gdb) p name
$10 = 0xf6ac8f3e sisisinst
(gdb) p sel_anw
$11 = 0xc3c0 SELECT rowid, %s.* from %s
(gdb) p select_anw
$12 = (char (*)[5000]) 0x9ac0

now executing the sprintf() ...

(gdb) n
1401  switch (key)

the result is fine and the target buffer of sprintf(), the 'select_anw'
is corretcly filled:

(gdb) p select_anw
$13 = SELECT rowid, sisisinst.* from sisisinst, '\000' repeats 536 times, 
ALTER SESSION SET NLS_LANGUAGE= 'GERMAN' NLS_TERRITORY= 'GERMANY' 
NLS_CURRENCY= '??' NLS_ISO_CURRENCY= 'GERMANY' NLS_NUMERIC_CHARACTERS= ',.' 
NLS_CALEN...
(gdb) p select_anw
$14 = (char (*)[5000]) 0x9ac0

All was fine. Why is valgrind complaining?

Thanks

matthias

-- 
Matthias Apitz   |  /\   ASCII Ribbon Campaign:
E-mail: g...@unixarea.de |  \ /   - No HTML/RTF in E-mail
WWW: http://www.unixarea.de/ |   X- No proprietary attachments
phone: +49-170-4527211   |  / \   - Respect for open standards
 | en.wikipedia.org/wiki/ASCII_Ribbon_Campaign

--
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls. 
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071iu=/4140/ostg.clktrk
___
Valgrind-users mailing list
Valgrind-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/valgrind-users


Re: [Valgrind-users] valgrind sprintf arguments

2014-07-29 Thread Philippe Waroquiers
On Tue, 2014-07-29 at 08:15 +0200, Matthias Apitz wrote:
 El día Monday, July 28, 2014 a las 07:11:02AM -0700, John Reiser escribió:
 
   ==17454== Conditional jump or move depends on uninitialised value(s)
   ==17454==at 0x5921F10: strchrnul (in /lib/libc-2.11.3.so)
   ==17454==by 0x58E55D6: vfprintf (in /lib/libc-2.11.3.so)
...
 All was fine. Why is valgrind complaining?

Here is an hypothesis:

Looking at (in the SVN version) shared/vg_replace_strmem.c
strchrnul should have been replaced by the implementation
in vg_replace_strmem.c.

From the stacktrace above, it looks like strchrnul was not replaced.

Often, the glibc implementations of the str* functions
are highly optimised, and causes false positive
(e.g. by assuming they can read a few more bytes than
the end of the string).

That might be the case for you.

What you could do is to redo your GDB session, but using
the valgrind gdbserver monitor command to check the definedness
of the printf args at various momenets and
just before the print call.

Note that using --track-origins=yes should indicate where
this unitialised byte is coming from.

Would be nice to understand why strchrnul was not replaced
(using e.g. -v -v -v --trace-redir=yes).

Philippe



--
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls. 
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071iu=/4140/ostg.clktrk
___
Valgrind-users mailing list
Valgrind-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/valgrind-users


Re: [Valgrind-users] valgrind sprintf arguments

2014-07-28 Thread John Reiser
 ==17454== Conditional jump or move depends on uninitialised value(s)
 ==17454==at 0x5921F10: strchrnul (in /lib/libc-2.11.3.so)
 ==17454==by 0x58E55D6: vfprintf (in /lib/libc-2.11.3.so)

 the involved fuctions are shown below; the statement in question (see below)
 is
 
   sprintf (select_anw, sel_anw, name, name);  * sisisinst.c:1397
 
 I have checked carefully the code and the 4 args to sprintf() are
 all correct defined on the stack; when I change the code to:
 
 
   select_anw[0] = '\0';
   sprintf (select_anw, sel_anw, name, name); 
   
 then is valgrind happy, i.e, does not raise the messages any more;

You say that all 4 args are on the stack.  What are their actual addresses?
Run with --db-attach=yes, say 'y' when asked, and use gdb to look around.

One possibility is that sel_anw (the format string) has been overwritten
because the string being built into select_anw (the buffer) has overflowed.

Try changing the code to use
snprintf(select_anw, LEN_SELECT, sel_anw, name, name);
which is much safer.


--
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls. 
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071iu=/4140/ostg.clktrk
___
Valgrind-users mailing list
Valgrind-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/valgrind-users


Re: [Valgrind-users] valgrind sprintf arguments

2014-07-28 Thread Philippe Waroquiers
On Mon, 2014-07-28 at 07:11 -0700, John Reiser wrote:
  ==17454== Conditional jump or move depends on uninitialised value(s)
  ==17454==at 0x5921F10: strchrnul (in /lib/libc-2.11.3.so)
  ==17454==by 0x58E55D6: vfprintf (in /lib/libc-2.11.3.so)
 
  the involved fuctions are shown below; the statement in question (see below)
  is
  
sprintf (select_anw, sel_anw, name, name);  * 
  sisisinst.c:1397
  
  I have checked carefully the code and the 4 args to sprintf() are
  all correct defined on the stack; when I change the code to:
  
  
select_anw[0] = '\0';
sprintf (select_anw, sel_anw, name, name); 
  
  then is valgrind happy, i.e, does not raise the messages any more;
 
 You say that all 4 args are on the stack.  What are their actual addresses?
 Run with --db-attach=yes, say 'y' when asked, and use gdb to look around.
--db-attach=yes should be considered as (is?) obsolete.

You could instead use --vgdb-error=1 (to just attach when the error is
reported) or better use --vgdb-error=0, put breakpoints
and verify the (un-)definedness of the relevant variables at various
points between their declaration and their usage.

Philippe


--
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls. 
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071iu=/4140/ostg.clktrk
___
Valgrind-users mailing list
Valgrind-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/valgrind-users


Re: [Valgrind-users] valgrind sprintf arguments

2014-07-28 Thread John Reiser
 --db-attach=yes should be considered as (is?) obsolete.
 
 You could instead use --vgdb-error=1 (to just attach when the error is
 reported) or better use --vgdb-error=0, put breakpoints
 and verify the (un-)definedness of the relevant variables at various
 points between their declaration and their usage.

https://bugs.kde.org/show_bug.cgi?id=337871
   deprecate --db-attach=yes in favor of --vgdb-debug=1

--
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls. 
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071iu=/4140/ostg.clktrk
___
Valgrind-users mailing list
Valgrind-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/valgrind-users