Re: [vdr] Valgrind warnings with LCARS OSD
On 15.11.2013 18:17, Marko Mäkelä wrote:
Hi Klaus,
On Fri, Nov 15, 2013 at 04:47:10PM +0100, Klaus Schmidinger wrote:
#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 =
}}
The constructor of cRect makes sure that all members are initialized to zero.
I'm afraid I can't think of a way there could be an uninitialized cRect.
Sorry, I used a bit sloppy language. cRect appears to be initialized, but with
uninitialized values. You know, Valgrind does not complain when you copy
uninitialized data around. It only complains when you are comparing
uninitialized data or passing uninitialized data to a system call. The Valgrind
V-bits are tracking which bits are uninitialized.
The cRect constructor is not at fault. I tried this twice, but both times gdb
would only show me the 3 topmost stack frames, claiming that the rest of the
stack is corrupted. Valgrind did show more (quoting from my previous message):
==3601==by 0x810AA0B: cOsd::DrawRectangle(int, int, int, int,
unsigned int) (osd.c:1922)
==3601==by 0x8130482: cSkinLCARSDisplayMenu::Clear() (skinlcars.c:1463)
I could obviously not verify this (due to gdb claiming that the stack is
corrupted), but I suspect that the parameters that are being passed are
uninitialized:
void cSkinLCARSDisplayMenu::Clear(void)
{
textScroller.Reset();
osd->DrawRectangle(xi00, yi00, xi03 - 1, yi01 - 1,
Theme.Color(clrBackground));
}
AFAICT, it is invoking this code in cOsd::DrawRectangle():
pixmaps[0]->DrawRectangle(cRect(x1, y1, x2 - x1 + 1, y2 - y1 + 1), Color);
This in turn should be invoking this constructor:
cRect(int X, int Y, int Width, int Height): point(X, Y), size(Width, Height)
{}
cSkinLCARSDisplayMenu::cSkinLCARSDisplayMenu() is not initializing any of the
members xi00, yi00, xi03, yi01.
Is there a reproducible set of actions that causes this to happen?
Yes. Hit Play (to start playing the last played recording), Pause, Menu,
Recordings while using the LCARS skin. The first 2 or 3 keypresses ought to be
optional. I had to stop the video playback with the 2 first keypresses, because
the softdevice framerate is measured in seconds per frame when
running under Valgrind :)
I have verified that cSkinLCARSDisplayMenu::Clear() actually gets called
*before*
cSkinLCARSDisplayMenu::SetMenuCategory(), where the variables in question are
initialized.
And in that call to Clear() the values are actually totally bogus.
Thanks for debugging this!
Please try whether this fixes it:
--- skinlcars.c
+++ skinlcars.c
@@ -900,6 +900,15 @@
ys03 = ys04 - Gap;
ys05 = yb15;
+ // The item area (just to have them initialized, actual setting will be done
in SetMenuCategory():
+
+ xi00 = 0;
+ xi01 = 0;
+ xi02 = 0;
+ xi03 = 1;
+ yi00 = 0;
+ yi01 = 1;
+
// The color buttons in submenus:
xb00 = xa06;
xb15 = xa07;
Klaus
___
vdr mailing list
vdr@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
[vdr] [PATCH] spelling in usage text
Hi, "--vfat for backwards compatibility (same as\n" " --dirnames=250,40,1\n" Either use a comma before "same" or a closing bracket in the second line. Whatever you prefer, I would take the comma. :) Patches attached for both variants. Lars. diff --git a/vdr.c b/vdr.c index 083d838..ad2469c 100644 --- a/vdr.c +++ b/vdr.c @@ -540,7 +540,7 @@ int main(int argc, char *argv[]) " -v DIR, --video=DIRuse DIR as video directory (default: %s)\n" " -V, --version print version information and exit\n" "--vfat for backwards compatibility (same as\n" - " --dirnames=250,40,1\n" + " --dirnames=250,40,1)\n" " -w SEC, --watchdog=SEC activate the watchdog timer with a timeout of SEC\n" " seconds (default: %d); '0' disables the watchdog\n" "\n", diff --git a/vdr.c b/vdr.c index 083d838..31c1928 100644 --- a/vdr.c +++ b/vdr.c @@ -539,7 +539,7 @@ int main(int argc, char *argv[]) "--userdump allow coredumps if -u is given (debugging)\n" " -v DIR, --video=DIRuse DIR as video directory (default: %s)\n" " -V, --version print version information and exit\n" - "--vfat for backwards compatibility (same as\n" + "--vfat for backwards compatibility, same as\n" " --dirnames=250,40,1\n" " -w SEC, --watchdog=SEC activate the watchdog timer with a timeout of SEC\n" " seconds (default: %d); '0' disables the watchdog\n" ___ vdr mailing list vdr@linuxtv.org http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
Re: [vdr] Valgrind warnings with LCARS OSD
Hi Klaus,
Thanks for debugging this!
Please try whether this fixes it:
--- skinlcars.c
+++ skinlcars.c
@@ -900,6 +900,15 @@
ys03 = ys04 - Gap;
ys05 = yb15;
+ // The item area (just to have them initialized, actual setting will be done
in SetMenuCategory():
+
+ xi00 = 0;
+ xi01 = 0;
+ xi02 = 0;
+ xi03 = 1;
+ yi00 = 0;
+ yi01 = 1;
+
// The color buttons in submenus:
xb00 = xa06;
xb15 = xa07;
After applying this fix, the only Valgrind warnings I got from VDR were
bogus-looking ones: the cRecording::cRecording (based on my reading of
the disassembly, gcc's built-in version of strcmp() doing a 32-bit
access that goes 2 bytes beyond the end of the string), and a few
bogus-looking warnings on ioctl(). 3 of them were complaining about
"invalid pointer" 0x0 or 0x1 for DirectFB calls, and the 4th one was
this:
==7120== Syscall param ioctl(arg) contains uninitialised byte(s)
==7120==at 0x436AA99: ioctl (syscall-template.S:82)
==7120==by 0x80CB31A: cDvbDevice::SetPid(cDevice::cPidHandle*, int,
bool) (dvbdevice.c:1394)
==7120==by 0x80C22B4: cDevice::DelPid(int, cDevice::ePidType)
(device.c:544)
==7120==by 0x80C4047: cDevice::Detach(cReceiver*) (device.c:1690)
==7120==by 0x8116605: cReceiver::Detach() (receiver.c:89)
==7120==by 0x42B3E45: (below main) (libc-start.c:228)
CHECK(ioctl(Handle->handle, DMX_STOP));
#define CHECK(s) { if ((s) < 0) LOG_ERROR; } // used for 'ioctl()' calls
Given that Handle->handle is a signed integer (file handle), the
Valgrind warning should be for DMX_STOP, which does not look like a
pointer to me. Maybe the constant happens to coincide with the address
of some memory block (not too hard with only 32 bits of address space).
Or perhaps Valgrind is misinterpreting the number of arguments to
ioctl().
I switched back to using LCARS, and did not experience any crashes yet
after applying the patch, no matter how much I played with the Menu and
Recordings buttons. So, the patch seems to work. Thank you!
Marko
___
vdr mailing list
vdr@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
