Valgrind warnings with LCARS OSD

  Hi all,

I played a little with Valgrind:

valgrind --vgdb=yes --vgdb-error=0 ./vdr ...
and in gdb,
(gdb) target remote |vgdb

This seems to give me a false alarm for cRecording::cRecording():

Program received signal SIGTRAP, Trace/breakpoint trap.
0x0811951a in cRecording::cRecording (this=0xbee4d958, FileName=0x0)
     at recording.c:801
801	  if (*(fileName + strlen(fileName) - 1) == '/')

The machine code is doing some magic after the strdup() on the previous 
line. I suspect that it is gcc that is performing strlen() with some 
black magic that trips this warning:

==3212== Invalid read of size 4
==3212==    at 0x811951A: cRecording::cRecording(char const*) (recording.c:801)
==3212==    by 0x80CEF77: cDvbPlayer::cDvbPlayer(char const*, bool) (dvbplayer.c:268)
==3212==  Address 0x4454004 is 76 bytes inside a block of size 78 alloc'd
==3212==    at 0x4028308: malloc (vg_replace_malloc.c:263)
==3212==    by 0x4315E1F: strdup (strdup.c:43)
==3212==    by 0x81194FE: cRecording::cRecording(char const*) (recording.c:800)

Then I got and fixed many warnings in Softdevice, which forgot to 
initialize some class members in constructors. Finally, I got a real 
warning for VDR:

==3212== Conditional jump or move depends on uninitialised value(s)
==3212==    at 0x4029654: __GI_strcmp (mc_replace_strmem.c:712)
==3212==    by 0x813ACD8: cSkinLCARSDisplayReplay::DrawTrack() (skinlcars.c:1786)
==3212==    by 0x813AE9F: cSkinLCARSDisplayReplay::Flush() (skinlcars.c:1861)
==3212==    by 0x80FCA55: cReplayControl::ShowProgress(bool) (menu.c:4670)
==3212==    by 0x80FCBF7: cReplayControl::ShowTimed(int) (menu.c:4583)
==3212==    by 0x80FCDEF: cReplayControl::cReplayControl(bool) (menu.c:4510)
==3212==    by 0x80AC745: main (vdr.c:1307)

"(gdb) monitor get_vbits" tells me that all of lastTrackId is 
uninitialized:

(gdb) up
#1  0x0813acd9 in cSkinLCARSDisplayReplay::DrawTrack (
     this=this@entry=0x457c3a0) at skinlcars.c:1786
1786	  if (!Track && *lastTrackId.description || Track && 
strcmp(lastTrackId.description, Track->description)) {
(gdb) p lastTrackId
$27 = {id = 0, language = "\000\000\000\000\000\000\000", description = 
'\000' <repeats 31 times>}
(gdb) p &lastTrackId
$28 = (tTrackId *) 0x457c434
(gdb) p sizeof lastTrackId
$29 = 42
(gdb) monitor get_vbits 0x457c434 42
ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff ffffffff
ffffffff ffffffff ffff

BTW, !Track && x || Track && y
should IMO be simpler written Track ? y : x.

It looks like a memset() is missing from the cSkinLCARSDisplayReplay 
constructor. cSkinLCARSDisplayChannel::cSkinLCARSDisplayChannel() is 
doing the right thing:

   memset(&lastTrackId, 0, sizeof(lastTrackId));

Adding the memset() made this message go away. (Patch attached.)

The next problem is this one, which I get every time by pressing Play, 
Pause, Menu, Recordings after startup:

==3601== Conditional jump or move depends on uninitialised value(s)
==3601==    at 0x810C0DB: cRect::Intersected(cRect const&) const (osd.h:411)
==3601==    by 0x810E3D1: cPixmapMemory::DrawRectangle(cRect const&, 
unsigned int) (osd.c:1333)
==3601==    by 0x810AA0B: cOsd::DrawRectangle(int, int, int, int, 
unsigned int) (osd.c:1922)
==3601==    by 0x8130482: cSkinLCARSDisplayMenu::Clear() (skinlcars.c:1463)
==3601==    by 0x810651F: cOsdMenu::Display() (osdbase.c:223)
==3601==    by 0x80FA3D1: cMenuMain::Set() (menu.c:3432)
==3601==    by 0x80FA9FD: cMenuMain::cMenuMain(eOSState, bool) (menu.c:3376)
==3601==    by 0x80AC21B: main (vdr.c:1078)

According to "monitor get_vbits", the cRect is totally uninitialized.

Program received signal SIGTRAP, Trace/breakpoint trap.
0x0810c0db in IsEmpty (this=0xbeba08a0) at osd.h:411
411	  bool IsEmpty(void) const { return Width() <= 0 || Height() <= 0; }
(gdb) up
#1  cRect::Intersected (this=this@entry=0xbeba08a0, Rect=...) at osd.c:912
912	  if (!IsEmpty() && !Rect.IsEmpty()) {
(gdb) up
#2  0x0810e3d2 in cPixmapMemory::DrawRectangle (this=0x6d3fe78, 
Rect=..., Color=2566914048) at osd.c:1333
1333	  cRect r = Rect.Intersected(DrawPort().Size());

As far as I can tell, the entirely uninitialized cRect is being passed 
as the Rect parameter to cPixmapMemory::DrawRectangle(). Unfortunately, 
gdb cannot show me the stack above that. It would seem to me that 
cSkinLCARSDisplayMenu::Clear() is passing uninitialized bounds to 
cOsd::DrawRectangle(), which will lead to funny values like this:

(gdb) p *this
$31 = {point = {x = 1418239204, y = 0}, size = {width = -1379480940, 
height = 201}, static Null = {point = {x = 0, y = 0}, size = {width = 0, 
height = 0}, static Null = <same as static member of an already seen 
type>}}

As a workaround, I guess that I will be switching away from the LCARS 
skin for now. I got into this exercise because vdr sometimes crashed 
when I pressed the Recordings or Menu button when using the LCARS skin.

I am also attaching my patch against softdevice CVS, in case someone 
finds it useful. I was unable to figure out how to clear the garbage at 
the bottom of the OSD screen. It goes away if the dfb:mgatv video 
display area is high enough (4:3 video instead of 16:9).

Best regards,

	Marko

Valgrind warnings with LCARS OSD

Commit Message

Comments

Patch