On Wednesday 23 January 2013 12:24:30 Julian Seward wrote:
> > ==9070== Lock at 0xD81F6F8 was first observed
> > ==9070==    at 0x4C3077F: QMutex::QMutex(QMutex::RecursionMode)
> > (hg_intercepts.c:2186) ==9070==    by 0x4C307A4:
> > QMutex::QMutex(QMutex::RecursionMode) (hg_intercepts.c:2192) ==9070==   
> > by 0x585A9CE: QPostEventList::QPostEventList() (qthread_p.h:110) [...]
> > 
> > Should I just duplicate the code of the wrappers instead?
> > Or is there a more clever solution I'm missing?
> 
> One alternative approach -- which doesn't really solve the problem, but
> which you might want to look at -- is to put the real code in a worker
> function and call that from all entry points.  For example, see how
> sem_wait_WRK is used in hg_intercepts.c.  That doesn't get rid of the
> second stack frame, but it does at least avoid the impression that
> there's some kind of strange recursion going on.  Personally I quite
> like this scheme, in that it doesn't duplicate code.

OK.

> If you turned the __attribute__((noinline)) into
> __attribute__((always_inline)) then I think you'd get rid of the extra
> frame without having to duplicate the code by hand.

I tried, and it works, but it makes gcc warn:
hg_intercepts.c:2048:13: warning: always_inline function might not be 
inlinable [-Wattributes]

> > What I don't know is whether some implementations might should differ from
> > the Qt4 implementation...
> 
> Right.  That's the real problem.  I don't think there's an easy way to
> figure this out, short of comparing the Qt4 and Qt5 implementations of
> the relevant functions.

Yeah. But in fact, the implementation of QMutex itself having changed, doesn't 
matter for helgrind's intercepts. The API is the same, so the expected 
behavior is the same, so helgrind can react the same. So I think we're fine for 
now.

> Once you have a patch you're satisfied with, I'd be happy to commit it.

Please find the patch attached.

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5
Index: hg_intercepts.c
===================================================================
--- hg_intercepts.c	(revision 13107)
+++ hg_intercepts.c	(working copy)
@@ -2033,9 +2033,15 @@
    ret_ty I_WRAP_SONAME_FNNAME_ZU(libQtCoreZdsoZa,f)(args); \
    ret_ty I_WRAP_SONAME_FNNAME_ZU(libQtCoreZdsoZa,f)(args)
 
+// soname is libQt5Core.so.4 ; match against libQt5Core.so*
+#define QT5_FUNC(ret_ty, f, args...) \
+   ret_ty I_WRAP_SONAME_FNNAME_ZU(libQt5CoreZdsoZa,f)(args); \
+   ret_ty I_WRAP_SONAME_FNNAME_ZU(libQt5CoreZdsoZa,f)(args)
+
 //-----------------------------------------------------------
 // QMutex::lock()
-QT4_FUNC(void, _ZN6QMutex4lockEv, void* self)
+//__attribute__((always_inline)) // works, but makes gcc warn...
+static void QMutex_lock_WRK(void* self)
 {
    OrigFn fn;
    VALGRIND_GET_ORIG_FN(fn);
@@ -2056,9 +2062,16 @@
    }
 }
 
+QT4_FUNC(void, _ZN6QMutex4lockEv, void* self) {
+    QMutex_lock_WRK(self);
+}
+QT5_FUNC(void, _ZN6QMutex4lockEv, void* self) {
+    QMutex_lock_WRK(self);
+}
+
 //-----------------------------------------------------------
 // QMutex::unlock()
-QT4_FUNC(void, _ZN6QMutex6unlockEv, void* self)
+static void QMutex_unlock_WRK(void* self)
 {
    OrigFn fn;
    VALGRIND_GET_ORIG_FN(fn);
@@ -2080,10 +2093,17 @@
    }
 }
 
+QT4_FUNC(void, _ZN6QMutex6unlockEv, void* self) {
+    QMutex_unlock_WRK(self);
+}
+QT5_FUNC(void, _ZN6QMutex6unlockEv, void* self) {
+    QMutex_unlock_WRK(self);
+}
+
 //-----------------------------------------------------------
 // bool QMutex::tryLock()
 // using 'long' to mimic C++ 'bool'
-QT4_FUNC(long, _ZN6QMutex7tryLockEv, void* self)
+static long QMutex_tryLock_WRK(void* self)
 {
    OrigFn fn;
    long   ret;
@@ -2110,10 +2130,17 @@
    return ret;
 }
 
+QT4_FUNC(long, _ZN6QMutex7tryLockEv, void* self) {
+    return QMutex_tryLock_WRK(self);
+}
+QT5_FUNC(long, _ZN6QMutex7tryLockEv, void* self) {
+    return QMutex_tryLock_WRK(self);
+}
+
 //-----------------------------------------------------------
 // bool QMutex::tryLock(int)
 // using 'long' to mimic C++ 'bool'
-QT4_FUNC(long, _ZN6QMutex7tryLockEi, void* self, long arg2)
+static long QMutex_tryLock_int_WRK(void* self, long arg2)
 {
    OrigFn fn;
    long   ret;
@@ -2141,6 +2168,12 @@
    return ret;
 }
 
+QT4_FUNC(long, _ZN6QMutex7tryLockEi, void* self, long arg2) {
+    return QMutex_tryLock_int_WRK(self, arg2);
+}
+QT5_FUNC(long, _ZN6QMutex7tryLockEi, void* self, long arg2) {
+    return QMutex_tryLock_int_WRK(self, arg2);
+}
 
 //-----------------------------------------------------------
 // It's not really very clear what the args are here.  But from
@@ -2151,9 +2184,7 @@
 // is that of the mutex and the second is either zero or one,
 // probably being the recursion mode, therefore.
 // QMutex::QMutex(QMutex::RecursionMode)  ("C1ENS" variant)
-QT4_FUNC(void*, _ZN6QMutexC1ENS_13RecursionModeE,
-         void* mutex,
-         long  recmode)
+static void* QMutex_constructor_WRK(void* mutex, long recmode)
 {
    OrigFn fn;
    long   ret;
@@ -2165,9 +2196,16 @@
    return (void*)ret;
 }
 
+QT4_FUNC(void*, _ZN6QMutexC1ENS_13RecursionModeE, void* self, long recmode) {
+    return QMutex_constructor_WRK(self, recmode);
+}
+QT5_FUNC(void*, _ZN6QMutexC1ENS_13RecursionModeE, void* self, long recmode) {
+    return QMutex_constructor_WRK(self, recmode);
+}
+
 //-----------------------------------------------------------
 // QMutex::~QMutex()  ("D1Ev" variant)
-QT4_FUNC(void*, _ZN6QMutexD1Ev, void* mutex)
+static void* QMutex_destructor_WRK(void* mutex)
 {
    OrigFn fn;
    long   ret;
@@ -2178,6 +2216,12 @@
    return (void*)ret;
 }
 
+QT4_FUNC(void*, _ZN6QMutexD1Ev, void* self) {
+    return QMutex_destructor_WRK(self);
+}
+QT5_FUNC(void*, _ZN6QMutexD1Ev, void* self) {
+    return QMutex_destructor_WRK(self);
+}
 
 //-----------------------------------------------------------
 // QMutex::QMutex(QMutex::RecursionMode)  ("C2ENS" variant)
@@ -2192,6 +2236,12 @@
    return NULL;
 }
 
+QT5_FUNC(void*, _ZN6QMutexC2ENS_13RecursionModeE, void* self, long recmode)
+{
+   assert(0);
+   /*NOTREACHED*/
+   return NULL;
+}
 
 //-----------------------------------------------------------
 // QMutex::~QMutex()  ("D2Ev" variant)
@@ -2203,6 +2253,12 @@
    return NULL;
 }
 
+QT5_FUNC(void*, _ZN6QMutexD2Ev, void* self)
+{
+   assert(0);
+   /*NOTREACHED*/
+   return NULL;
+}
 
 // QReadWriteLock is not intercepted directly.  See comments
 // above.
------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnnow-d2d
_______________________________________________
Valgrind-users mailing list
Valgrind-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/valgrind-users

Reply via email to