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)
{
