[PATCH] Fix 2 issues found by valgrind

[PATCH] Fix 2 issues found by valgrind

[Jakub Jelinek]

Hi!

1) in internal_fnmatch we have is_seqval is uninitialized
   after we goto to normal_bracket, as we jump into the
   middle of is_seqval's scope, don't set it to anything
   and later on use it
2) tst-mktime2.c did not initialize tm.tm_isdst, yet mktime
   uses this

Will keep looking at other problems.

2004-11-29  Jakub Jelinek  <jakub@redhat.com>

	* posix/fnmatch_loop.c (internal_fnmatch): Clear is_seqval after
	normal_bracket label.

	* time/tst-mktime2.c (bigtime_test): Initialize tm.tm_isdst to -1.

--- libc/posix/fnmatch_loop.c.jj	2004-09-04 09:16:57.000000000 +0200
+++ libc/posix/fnmatch_loop.c	2004-11-29 17:32:06.284085221 +0100
@@ -600,6 +600,9 @@ FCT (pattern, string, string_end, no_lea
 			if (!is_range && c == fn)
 			  goto matched;
 
+			/* This is needed if we goto normal_bracket; from
+			   outside of is_seqval's scope.  */
+			is_seqval = 0;
 			cold = c;
 			c = *p++;
 		      }
--- libc/time/tst-mktime2.c.jj	2004-11-01 01:21:23.000000000 +0100
+++ libc/time/tst-mktime2.c	2004-11-29 15:01:24.509816991 +0100
@@ -78,6 +78,7 @@ bigtime_test (int j)
   struct tm tm;
   time_t now;
   tm.tm_year = tm.tm_mon = tm.tm_mday = tm.tm_hour = tm.tm_min = tm.tm_sec = j;
+  tm.tm_isdst = -1;
   now = mktime (&tm);
   if (now != (time_t) -1)
     {

	Jakub

Re: [PATCH] Fix 2 issues found by valgrind

[Roland McGrath]

I applied those fixes.   Can you make publically visible somewhere the
error reports you are looking into?  If you have already classified some as
false-positive, that is helpful for all to see as well.


Thanks,
Roland

Re: [PATCH] Fix 2 issues found by valgrind

[Jakub Jelinek]

[-- Attachment #1: Type: text/plain, Size: 2942 bytes --]

On Wed, Dec 01, 2004 at 11:56:45AM -0800, Roland McGrath wrote:
> I applied those fixes.   Can you make publically visible somewhere the
> error reports you are looking into?  If you have already classified some as
> false-positive, that is helpful for all to see as well.

The false positives are e.g. in tst-fmemopen and several others (see
attached valgrind-bug2.c for simplified testcase):

Another one is:
==12068== Conditional jump or move depends on uninitialised value(s)
==12068==    at 0x1B9A2A0B: re_compile_fastmap_iter (regcomp.c:356)
==12068==    by 0x1B9A2E9B: re_compile_fastmap (regcomp.c:301)
==12068==    by 0x1B9AC671: regcomp (regcomp.c:515)
==12068==    by 0x8048C73: test (tst-rxspencer.c:214)
(see attached valgrind-bug1.c).

Anything with -lpthread apparently can't be trusted with valgrind,
as valgrind's -lpthread is very incomplete and not compatible
with NPTL libc.so.

Then there are things like:
==15623== Syscall param sigqueueinfo(uinfo) contains uninitialised or unaddressable byte(s)
==15623==    at 0x1B90BFE7: __aio_sigqueue (aio_sigqueue.c:51)
==15623==    by 0x1B90C0F5: __aio_notify_only (aio_notify.c:108)
==15623==    by 0x1B90C209: __aio_notify (aio_notify.c:160)
==15623==    by 0x1B90AD89: handle_fildes_io (aio_misc.c:595)
==15623==  Address 0x1BF87D37 is on thread 2's stack

==15702== Syscall param timer_create(evp) contains uninitialised or unaddressable byte(s)
==15702==    at 0x1B90CCA4: timer_create (timer_create.c:91)
==15702==    by 0x8048BBF: do_timer_create (tst-timer3.c:22)
==15702==    by 0x1B922925: thread_wrapper (vg_libpthread.c:867)
==15702==    by 0xB000F14F: do__quit (vg_scheduler.c:1872)
==15702==  Address 0x52BFDC44 is on thread 1's stack

==15866== Syscall param socketcall.sendto(msg) contains uninitialised or unaddressable byte(s)
==15866==    at 0x1B9D3731: sendto (socket.S:65)
==15866==    by 0x1B9EEAA2: if_nameindex (if_index.c:192)
==15866==    by 0x80485F4: main (test_ifindex.c:31)
==15866==  Address 0x52BFDC0D is on thread 1's stack
==15866==
==15866== Syscall param ioctl(SIOCGIFNAME) contains uninitialised or unaddressable byte(s)
==15866==    at 0x1B9CA959: ioctl (in /usr/src/libc/obj32/libc.so)
==15866==    by 0x8048663: main (test_ifindex.c:52)
==15866==  Address 0x1 is not stack'd, malloc'd or (recently) free'd

==15881== Syscall param socketcall.sendto(msg) contains uninitialised or unaddressable byte(s)
==15881==    at 0x1B9D3731: sendto (socket.S:65)
==15881==    by 0x1B9EF35F: getifaddrs (ifaddrs.c:292)
==15881==    by 0x8048559: main (test-ifaddrs.c:70)
==15881==  Address 0x52BFDADD is on thread 1's stack
==15881==
==15881== Syscall param socketcall.sendto(msg) contains uninitialised or unaddressable byte(s)
==15881==    at 0x1B9D3731: sendto (socket.S:65)
==15881==    by 0x1B9EF397: getifaddrs (ifaddrs.c:310)
==15881==    by 0x8048559: main (test-ifaddrs.c:70)
==15881==  Address 0x52BFDADD is on thread 1's stack

etc.

	Jakub

[-- Attachment #2: valgrind-bug1.c --]
[-- Type: text/plain, Size: 510 bytes --]

/* gcc34 -m32 -O2 -march=i386 -mtune=pentium4 */
#include <stdlib.h>

struct S
{
  unsigned int s1 : 8, s2 : 10;
  unsigned int s3 : 1, s4 : 1, s5 : 1, s6 : 1;
};

int test (struct S *s, int n)
{
  int i, ret;
  i = 0, ret = 0;
  while (++i < n && s[i].s1 == 1 && s[i].s5)
    ++ret;
  return ret;
}

int main (void)
{
  struct S *s = malloc (4 * sizeof (struct S));
  int i;
  for (i = 0; i < 4; ++i)
    {
      s[i].s1 = (i + 2) / 2;
      if (i < 2)
        s[i].s5 = 1;
    }
  return test (s, 4) != 1;
}

[-- Attachment #3: valgrind-bug2.c --]
[-- Type: text/plain, Size: 302 bytes --]

/* gcc34 -m32 -O2 -march=i686 */
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int
main (void)
{
  const char blah[] = "BLAH";
  FILE *fp;

  if ((fp = fopen ("test-1", "w+")) == NULL)
    exit (1);
  if (fwrite (blah, 1, strlen (blah), fp) != strlen (blah))
    exit (2);

  return 0;
}

Re: [PATCH] Fix 2 issues found by valgrind

[Roland McGrath]

> ==15623== Syscall param sigqueueinfo(uinfo) contains uninitialised or unaddressable byte(s)

Is there a valgrind FAQ that mentions this?  These are all true
false-positive cases.  i.e., the kernel does not examine the uninitialized
areas.