valgrind errors in Bitcoin-Qt #1776

Running valgrind on Bitcoin-Qt, I get these memory errors reported. They look like problems in Qt and libminiupnp, but it'd be nice to verify that it's not our use of those libraries that is causing the errors.

==69076== Memcheck, a memory error detector
==69076== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==69076== Using Valgrind-3.6.1 and LibVEX; rerun with -h for copyright info
==69076== Command: Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt
==69076== 
--69076-- run: /usr/bin/dsymutil "Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt"
warning: no debug symbols in executable (-arch i386)
==69076== Conditional jump or move depends on uninitialised value(s)
==69076==    at 0xAB0D47: _DPSNextEvent (in /System/Library/Frameworks/AppKit.framework/Versions/C/AppKit)
==69076==    by 0xAAFDD5: -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (in /System/Library/Frameworks/AppKit.framework/Versions/C/AppKit)
==69076==    by 0xA721F2: -[NSApplication run] (in /System/Library/Frameworks/AppKit.framework/Versions/C/AppKit)
==69076==    by 0x1CD99EA: QEventDispatcherMacPrivate::ensureNSAppInitialized() (in /opt/local/Library/Frameworks/QtGui.framework/Versions/4/QtGui)
==69076==    by 0x1CDB761: QEventDispatcherMac::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (in /opt/local/Library/Frameworks/QtGui.framework/Versions/4/QtGui)
==69076==    by 0x2A159DD: QCoreApplication::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (in /opt/local/Library/Frameworks/QtCore.framework/Versions/4/QtCore)
==69076==    by 0x1D7D905: QWidgetPrivate::show_helper() (in /opt/local/Library/Frameworks/QtGui.framework/Versions/4/QtGui)
==69076==    by 0x1D7F422: QWidget::setVisible(bool) (in /opt/local/Library/Frameworks/QtGui.framework/Versions/4/QtGui)
==69076==    by 0x1E76D: main (in Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt)
==69076== 
==69076== Conditional jump or move depends on uninitialised value(s)
==69076==    at 0xAB0D4C: _DPSNextEvent (in /System/Library/Frameworks/AppKit.framework/Versions/C/AppKit)
==69076==    by 0xAAFDD5: -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (in /System/Library/Frameworks/AppKit.framework/Versions/C/AppKit)
==69076==    by 0xA721F2: -[NSApplication run] (in /System/Library/Frameworks/AppKit.framework/Versions/C/AppKit)
==69076==    by 0x1CD99EA: QEventDispatcherMacPrivate::ensureNSAppInitialized() (in /opt/local/Library/Frameworks/QtGui.framework/Versions/4/QtGui)
==69076==    by 0x1CDB761: QEventDispatcherMac::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (in /opt/local/Library/Frameworks/QtGui.framework/Versions/4/QtGui)
==69076==    by 0x2A159DD: QCoreApplication::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) (in /opt/local/Library/Frameworks/QtCore.framework/Versions/4/QtCore)
==69076==    by 0x1D7D905: QWidgetPrivate::show_helper() (in /opt/local/Library/Frameworks/QtGui.framework/Versions/4/QtGui)
==69076==    by 0x1D7F422: QWidget::setVisible(bool) (in /opt/local/Library/Frameworks/QtGui.framework/Versions/4/QtGui)
==69076==    by 0x1E76D: main (in Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt)
==69076== 
==69076== Thread 9:
==69076== Conditional jump or move depends on uninitialised value(s)
==69076==    at 0x6DF09C: getHTTPResponse (in /opt/local/lib/libminiupnpc.8.dylib)
==69076==    by 0x6E12AF: simpleUPnPcommand2 (in /opt/local/lib/libminiupnpc.8.dylib)
==69076==    by 0x6E1327: simpleUPnPcommand (in /opt/local/lib/libminiupnpc.8.dylib)
==69076==    by 0x6E21CC: UPNP_AddPortMapping (in /opt/local/lib/libminiupnpc.8.dylib)
==69076==    by 0x106293: ThreadMapPort2(void*) (in Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt)
==69076==    by 0x10655B: ThreadMapPort(void*) (in Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt)
==69076==    by 0x2D0C258: _pthread_start (in /usr/lib/libSystem.B.dylib)
==69076==    by 0x2D0C0DD: thread_start (in /usr/lib/libSystem.B.dylib)
==69076== 
==69076== Conditional jump or move depends on uninitialised value(s)
==69076==    at 0x6DF0A0: getHTTPResponse (in /opt/local/lib/libminiupnpc.8.dylib)
==69076==    by 0x6E12AF: simpleUPnPcommand2 (in /opt/local/lib/libminiupnpc.8.dylib)
==69076==    by 0x6E1327: simpleUPnPcommand (in /opt/local/lib/libminiupnpc.8.dylib)
==69076==    by 0x6E21CC: UPNP_AddPortMapping (in /opt/local/lib/libminiupnpc.8.dylib)
==69076==    by 0x106293: ThreadMapPort2(void*) (in Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt)
==69076==    by 0x10655B: ThreadMapPort(void*) (in Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt)
==69076==    by 0x2D0C258: _pthread_start (in /usr/lib/libSystem.B.dylib)
==69076==    by 0x2D0C0DD: thread_start (in /usr/lib/libSystem.B.dylib)
==69076== 

I find it really hard to interpret these.

• There is no direct call to setVisible from main(). It's likely inlined somewhere. hide and show are called various times, but I don't see any place where it could conditionally depend on an uninitialized value. Main is a pretty large function so it'd help to know where in the function it happens.

• UPNP errors are about the call:

  r = UPNP_AddPortMapping(urls.controlURL, data.first.servicetype,
                      port, port, lanaddr, strDesc.c_str(), "TCP", 0, "0");
  

urls, data and lanaddr are output arguments of UPNP_GetValidIGD, whose return value is checked to be 1 (success?). port is a char[6] that comes from sprintf(port, "%d", GetListenPort()); (which, I agree, is a bit dangerous as the buffer is only 6 chars wide and %d can represent a full 32 bit integer. But I doubt that is the specific problem here. Fixed this in #1777 anyway) strDesc is simply built from "Bitcoin " + FormatFullVersion() (yes, which is still in scope) ... and the other arguments are literal values.

I could try running valgrind on Linux (maybe with debug info) and see if the same errors appear.

Unable to reproduce any of these on linux.

Perhaps @gavinandresen could re-run his tests with current master. And I'm asking myself if setVisible() could contain bugs because of the "old" Qt version we use for compiling Bitcoin-Qt?

@gavinandresen Can you comment again please, such orphans need more love ;).

Closing this, we can always run valgrind again.