https://bugs.dragonflybsd.org/https://bugs.dragonflybsd.org/favicon.ico?16293952082008-05-08T02:07:00ZDragonFlyBSD bugtrackerDragonFlyBSD - Bug #1003: Qt 4.4 QtConcurrent and libthread_xuhttps://bugs.dragonflybsd.org/issues/1003?journal_id=45432008-05-08T02:07:00Zcorecode
<ul></ul><p>Well, somebody seems to pass a null pointer here.</p>
<p>could you compile glib with -g and also include their sources?</p>
<p>cheers<br /> simon</p> DragonFlyBSD - Bug #1003: Qt 4.4 QtConcurrent and libthread_xuhttps://bugs.dragonflybsd.org/issues/1003?journal_id=45442008-05-08T02:24:01Zdillon
<ul></ul><p>Something in that chain of calls is passing a NULL to<br /> __pthread_mutex_trylock().</p>
<pre><code>In libc_r we have this:</code></pre>
<p>int<br />_pthread_mutex_trylock(pthread_mutex_t * mutex)
{<br /> struct pthread *curthread = _get_curthread();<br /> int ret = 0;</p>
<pre><code>if (mutex == NULL)<br /> ret = EINVAL;<br /> ...<br />}</code></pre>
<pre><code>In libthread_xu it assumes non-NULL and will crash.</code></pre>
<pre><code>Try this patch. It will do the same check that libc_r does. I'm<br /> not convinced that Qt isn't broken, though, Qt shouldn't be passing<br /> NULL to the mutex functions, it should be passing the address of<br /> a pthread_mutex_t which itself can be NULL, but it should be passing<br /> NULL.</code></pre>
<pre><code>-Matt</code></pre>
<p>Index: thread/thr_mutex.c
===================================================================<br />RCS file: /cvs/src/lib/libthread_xu/thread/thr_mutex.c,v<br />retrieving revision 1.14<br />diff <del>u -p -r1.14 thr_mutex.c<br />--</del> thread/thr_mutex.c 13 Apr 2006 11:53:39 -0000 1.14<br />+++ thread/thr_mutex.c 7 May 2008 19:18:04 -0000<br /><code>@ -285,6 +285,8 </code>@ {<br /> struct pthread *curthread = tls_get_curthread();<br /> int ret;</p>
<p>+ if (__predict_false(m == NULL))<br />+ return(EINVAL);<br /> /*
* If the mutex is statically initialized, perform the dynamic
* initialization:<br /><code>@ -372,12 +374,14 </code>@ int ret;</p>
<pre><code>_thr_check_init();</code></pre>
<p>- curthread = tls_get_curthread();<br />+ if (__predict_false(m == NULL))<br />+ return(EINVAL);</p>
<pre><code>/*
* If the mutex is statically initialized, perform the dynamic
* initialization:<br /> */<br />+ curthread = tls_get_curthread();<br /> if (_<em>predict_false(*m == NULL)) {<br /> ret = init_static(curthread, m);<br /> if (</em>_predict_false(ret))<br /><code>@ -394,12 +398,14 </code>@ int ret;</code></pre>
<pre><code>_thr_check_init();</code></pre>
<p>- curthread = tls_get_curthread();<br />+ if (__predict_false(m == NULL))<br />+ return(EINVAL);</p>
<pre><code>/*
* If the mutex is statically initialized, perform the dynamic
* initialization marking it private (delete safe):<br /> */<br />+ curthread = tls_get_curthread();<br /> if (_<em>predict_false(*m == NULL)) {<br /> ret = init_static_private(curthread, m);<br /> if (</em>_predict_false(ret))<br /><code>@ -417,12 +423,14 </code>@ int ret;</code></pre>
<pre><code>_thr_check_init();</code></pre>
<p>- curthread = tls_get_curthread();<br />+ if (__predict_false(m == NULL))<br />+ return(EINVAL);</p>
<pre><code>/*
* If the mutex is statically initialized, perform the dynamic
* initialization:<br /> */<br />+ curthread = tls_get_curthread();<br /> if (_<em>predict_false(*m == NULL)) {<br /> ret = init_static(curthread, m);<br /> if (</em>_predict_false(ret))<br /><code>@ -440,6 +448,9 </code>@ int ret;</code></pre>
<pre><code>_thr_check_init();</code></pre>
<p>+ if (__predict_false(m == NULL))<br />+ return(EINVAL);<br />+<br /> curthread = tls_get_curthread();</p>
<pre><code>/*<br /><code>@ -457,6 +468,8 </code>@ <br /> int<br /> _pthread_mutex_unlock(pthread_mutex_t *m)
{<br />+ if (__predict_false(m NULL))<br />+ return(EINVAL);<br /> return (mutex_unlock_common(m));<br /> }</code></pre>
<p><code>@ -556,7 +569,6 </code>@ struct pthread_mutex *m;</p>
<pre><code>if (_<em>predict_false((m = *mutex) NULL))<br /> return (EINVAL);<br /><del><br /> if (</em>_predict_false(m</del>>m_owner != curthread))<br /> return (EPERM);</code></pre>
<p><code>@ -600,9 +612,10 </code>@ {<br /> struct pthread *curthread = tls_get_curthread();<br /> struct pthread_mutex *m;</p>
<p>- if (_<em>predict_false((m = *mutex)== NULL))<br />+ if (</em>_predict_false(mutex NULL))<br />+ return (EINVAL);<br />+ if (_<em>predict_false((m = *mutex) NULL))<br /> return (EINVAL);<br /><del><br /> if (</em>_predict_false(m</del>>m_owner != curthread))<br /> return (EPERM);</p> DragonFlyBSD - Bug #1003: Qt 4.4 QtConcurrent and libthread_xuhttps://bugs.dragonflybsd.org/issues/1003?journal_id=45452008-05-08T03:04:00Zhasso
<ul></ul><p>Compiled glib2 (2.14.6 in 2008Q1) distfile from source:</p>
<p>Program terminated with signal 11, Segmentation fault.<br />#0 _<em>pthread_mutex_trylock (m=0x0) <br />at /home/hasso/dragonfly-src/lib/libthread_xu/thread/thr_mutex.c:292<br />292 if (</em>_predict_false(*m == NULL)) {<br />(gdb) bt<br />#0 _<em>pthread_mutex_trylock (m=0x0) <br />at /home/hasso/dragonfly-src/lib/libthread_xu/thread/thr_mutex.c:292<br /><a class="issue tracker-1 status-5 priority-3 priority-lowest closed" title="Bug: lib/libcr/sys/ cleanup (Closed)" href="https://bugs.dragonflybsd.org/issues/1">#1</a> 0x081415f2 in g_mutex_trylock_posix_impl (mutex=0x0) at <br />gthread-posix.c:187<br /><a class="issue tracker-1 status-5 priority-3 priority-lowest closed" title="Bug: K&R -> ANSI cleanup status (Closed)" href="https://bugs.dragonflybsd.org/issues/2">#2</a> 0x08151b6b in IA</em>_g_slice_alloc (mem_size=12) at gslice.c:395<br /><a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: freebsds pipe-reverse test fails on dfly (Closed)" href="https://bugs.dragonflybsd.org/issues/3">#3</a> 0x0815cd76 in IA__g_ptr_array_sized_new (reserved_size=0) at <br />garray.c:372<br /><a class="issue tracker-1 status-5 priority-5 priority-high3 closed" title="Bug: Rework of nrelease (Closed)" href="https://bugs.dragonflybsd.org/issues/4">#4</a> 0x0815cdb1 in IA__g_ptr_array_new () at garray.c:366<br /><a class="issue tracker-1 status-5 priority-3 priority-lowest closed" title="Bug: sys/dev cleanup (Closed)" href="https://bugs.dragonflybsd.org/issues/5">#5</a> 0x0814b65c in IA__g_main_context_new () at gmain.c:757<br /><a class="issue tracker-1 status-5 priority-3 priority-lowest closed" title="Bug: sys/emulation cleanup (Closed)" href="https://bugs.dragonflybsd.org/issues/6">#6</a> 0x080fd298 in QEventDispatcherGlibPrivate (this=0x281fb400, <br />context=0x0)<br /> at kernel/qeventdispatcher_glib.cpp:241<br /><a class="issue tracker-1 status-5 priority-3 priority-lowest closed" title="Bug: /sys/boot cleanup (Closed)" href="https://bugs.dragonflybsd.org/issues/7">#7</a> 0x080fd4e7 in QEventDispatcherGlib (this=0x281f9260, parent=0x0) at <br />kernel/qeventdispatcher_glib.cpp:268<br /><a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: make upgrade broken (Closed)" href="https://bugs.dragonflybsd.org/issues/8">#8</a> 0x08057e1a in QThreadPrivate::createEventDispatcher (data=0x281fa740) <br />at thread/qthread_unix.cpp:161<br /><a class="issue tracker-1 status-5 priority-5 priority-high3 closed" title="Bug: panic with HEAD (Closed)" href="https://bugs.dragonflybsd.org/issues/9">#9</a> 0x08057f0c in QThreadPrivate::start (arg=0x281f91b0) at <br />thread/qthread_unix.cpp:185<br /><a class="issue tracker-1 status-5 priority-5 priority-high3 closed" title="Bug: make buildworld broken (Closed)" href="https://bugs.dragonflybsd.org/issues/10">#10</a> 0x08145087 in thread_start (arg=0x281f6200)<br /> at /home/hasso/dragonfly-src/lib/libthread_xu/thread/thr_create.c:239<br /><a class="issue tracker-1 status-5 priority-3 priority-lowest closed" title="Bug: libstand cleanup (Closed)" href="https://bugs.dragonflybsd.org/issues/11">#11</a> 0x00000000 in ?? ()</p> DragonFlyBSD - Bug #1003: Qt 4.4 QtConcurrent and libthread_xuhttps://bugs.dragonflybsd.org/issues/1003?journal_id=45462008-05-08T03:23:00Zcorecode
<ul></ul><p>From libc_r:</p>
<p>int<br />_pthread_mutex_trylock(pthread_mutex_t * mutex)
{<br /> struct pthread *curthread = _get_curthread();<br /> int ret = 0;</p>
<pre><code>if (mutex == NULL)<br /> ret = EINVAL;</code></pre>
<pre><code>/*
* If the mutex is statically initialized, perform the dynamic
* initialization:<br /> <strong>/<br /> else if (*mutex != NULL || (ret = init_static(mutex)) == 0) {<br /> /</strong></code></pre>
<p>from libthread_xu:</p>
<p>int<br />__pthread_mutex_trylock(pthread_mutex_t *m)
{<br /> struct pthread *curthread = tls_get_curthread();<br /> int ret;</p>
<pre><code>/*
* If the mutex is statically initialized, perform the dynamic
* initialization:<br /> */<br /> if (__predict_false(*m == NULL)) {</code></pre>
<p>Not sure if we are required to catch NULL pointers? They shouldn't pass <br />them in the first place...</p>
<p>cheers<br /> simon</p> DragonFlyBSD - Bug #1003: Qt 4.4 QtConcurrent and libthread_xuhttps://bugs.dragonflybsd.org/issues/1003?journal_id=45472008-05-08T03:35:00Zaoiko
<ul></ul><p>On Wednesday 07 May 2008, Matthew Dillon wrote:<br />[...]</p>
<blockquote>
<p>In libthread_xu it assumes non-NULL and will crash.</p>
<p>Try this patch. It will do the same check that libc_r does. I'm<br />not convinced that Qt isn't broken, though, Qt shouldn't be passing<br />NULL to the mutex functions, it should be passing the address of<br />a pthread_mutex_t which itself can be NULL, but it should be passing<br />NULL.</p>
</blockquote>
<p>Still, it's bad form for a library function to crash just because somebody<br />called it with an obviously invalid argument.</p>
<p>Aggelos</p> DragonFlyBSD - Bug #1003: Qt 4.4 QtConcurrent and libthread_xuhttps://bugs.dragonflybsd.org/issues/1003?journal_id=45482008-05-08T03:53:00ZJohannes.Hofmann
<ul></ul><p>Not sure about that. There are many illeagal pointers that can't be catched<br />so why treat NULL differently? <br />And a crash with an obvious stack trace shows that there is a problem that<br />should be fixed.</p>
<pre><code>Johannes</code></pre> DragonFlyBSD - Bug #1003: Qt 4.4 QtConcurrent and libthread_xuhttps://bugs.dragonflybsd.org/issues/1003?journal_id=45492008-05-08T04:36:10Zaoiko
<ul></ul><p>Because NULL is guaranteed not to point to an object.</p>
<p>While an error of EINVAL is hard to debug?</p>
<p>Aggelos</p> DragonFlyBSD - Bug #1003: Qt 4.4 QtConcurrent and libthread_xuhttps://bugs.dragonflybsd.org/issues/1003?journal_id=45502008-05-08T05:09:01Zcorecode
<ul></ul><p>This is true. Fair enough, we can add a check. I still wonder why they <br />are passing NULL in the first place.</p>
<p>Yes, most people seem to ignore error messages returned by mutex functions <br />anyways, or so I've heard :)</p>
<p>cheers<br /> simon</p> DragonFlyBSD - Bug #1003: Qt 4.4 QtConcurrent and libthread_xuhttps://bugs.dragonflybsd.org/issues/1003?journal_id=45512008-05-08T06:00:03Zdillon
<ul></ul><p>Ultimately it comes down to what does the standard intend? </p>
<pre><code>In my years of programming I've found that it is generally better to<br /> enforce the stated intention by crashing the program (or allowing it<br /> to crash) rather then returning an error code. It results in far better<br /> code quality from the people using the API. Unless the standard<br /> specifically says that NULL is ok, then the problem is with the Qt<br /> library and not with the threads library.</code></pre>
<pre><code>That said, from an administrative point of view I want to have maximum<br /> compatibility between libthread_xu and libpthread, because we aren't<br /> really in the business of policing other people's work.</code></pre>
<pre><code>So I am willing to commit the change, even if the standard doesn't allow<br /> NULL.</code></pre>
<pre><code>-Matt</code></pre> DragonFlyBSD - Bug #1003: Qt 4.4 QtConcurrent and libthread_xuhttps://bugs.dragonflybsd.org/issues/1003?journal_id=45522008-05-08T06:29:02Zaoiko
<ul></ul><p>[...]</p>
<blockquote>
<p>So I am willing to commit the change, even if the standard doesn't allow<br />NULL.</p>
</blockquote>
<p>Well, SUSv3 says</p>
<p>"The pthread_mutex_lock(), pthread_mutex_trylock(), and pthread_mutex_unlock()<br />functions may fail if:<br />[EINVAL]<br />The value specified by mutex does not refer to an initialized mutex object."</p>
<p>So it definitely allows NULL and seems to encourage extra mtx->magic checks.</p>
<p>Aggelos</p> DragonFlyBSD - Bug #1003: Qt 4.4 QtConcurrent and libthread_xuhttps://bugs.dragonflybsd.org/issues/1003?journal_id=45532008-05-08T06:30:01Zcorecode
<ul></ul><p>The standard says:</p>
<p>int pthread_mutex_lock(pthread_mutex_t *mutex);<br />int pthread_mutex_trylock(pthread_mutex_t *mutex);<br />int pthread_mutex_unlock(pthread_mutex_t *mutex);</p>
<p>Note that none of the parameters has the modifier "restricted". That <br />means that they may be NULL.</p>
<p>The pthread_mutex_lock(), pthread_mutex_trylock(), and <br />pthread_mutex_unlock() functions may fail if:</p>
<p>[EINVAL]<br /> The value specified by mutex does not refer to an initialized mutex <br />object.</p>
<p>Now, of course we cannot detect every single value that might not refer to <br />an initialized mutex object (unless we caught exceptions, etc.), but <br />returning EINVAL on a NULL pointer for sure isn't the wrong thing to do.</p>
<p>cheers<br /> simon</p> DragonFlyBSD - Bug #1003: Qt 4.4 QtConcurrent and libthread_xuhttps://bugs.dragonflybsd.org/issues/1003?journal_id=45542008-05-08T13:40:03Zhasso
<ul></ul><p>Yep, but ...</p>
<p>It just makes backtraces huge. Obviously passing NULL to <br />__pthread_mutex_trylock() isn't the problem, but probably just <br />consequence.</p>
<p>Qt doesn't pass NULL, it's glib. It doesn't mean that problem may not be <br />in Qt though. Just to be sure I tested these examples in some other <br />platforms and all these examples work fine in Linux, OpenBSD and FreeBSD.</p>
<p>I updated static binaries tarball to include debug info from glib as well.</p>
<p><a class="external" href="http://leaf.dragonflybsd.org/%7Ehasso/qt44-threaded-tests.tar.bz2">http://leaf.dragonflybsd.org/%7Ehasso/qt44-threaded-tests.tar.bz2</a></p> DragonFlyBSD - Bug #1003: Qt 4.4 QtConcurrent and libthread_xuhttps://bugs.dragonflybsd.org/issues/1003?journal_id=45552008-05-08T14:26:01Zhasso
<ul></ul><p>BTW, seems that static binaries don't give enough info though. With <br />dynamic binaries I almost always get the error:</p>
<p>GThread-ERROR **: GThread system may only be initialized once.</p>
<p>And I also managed to deadlock the system again. Unfortunately I couldn't <br />get any info from system. Something very fishy is going on.</p> DragonFlyBSD - Bug #1003: Qt 4.4 QtConcurrent and libthread_xuhttps://bugs.dragonflybsd.org/issues/1003?journal_id=45562008-05-08T15:08:01Zcorecode
<ul></ul><p>Both don't fail/crash here.</p>
<p>cheers<br /> simon</p> DragonFlyBSD - Bug #1003: Qt 4.4 QtConcurrent and libthread_xuhttps://bugs.dragonflybsd.org/issues/1003?journal_id=45632008-05-09T16:21:09Zcorecode
<ul></ul><p>checking with linux (ubuntu hardy), passing a NULL pointer to <br />pthread_mutex_trylock() crashes, so I don't think we have to do the checks <br />as well. The root of the problem seems to be somewhere else.</p>
<p>cheers<br /> simon</p> DragonFlyBSD - Bug #1003: Qt 4.4 QtConcurrent and libthread_xuhttps://bugs.dragonflybsd.org/issues/1003?journal_id=45652008-05-09T23:06:03Zdillon
<ul></ul><p>I'm going to go ahead and commit the NULL checks for the mutexes anyway,<br /> just to get those off my table.</p>
<pre><code>-Matt<br /> Matthew Dillon <br /> &lt;<a class="email" href="mailto:dillon@backplane.com">dillon@backplane.com</a>&gt;</code></pre> DragonFlyBSD - Bug #1003: Qt 4.4 QtConcurrent and libthread_xuhttps://bugs.dragonflybsd.org/issues/1003?journal_id=47292008-05-29T16:37:10Zhasso
<ul></ul><p>Original issue seems to be in Qt though. My guess is that it's the <br /><a class="external" href="http://trolltech.com/developer/task-tracker/index_html?method=entry&id=210246">http://trolltech.com/developer/task-tracker/index_html?method=entry&id=210246</a> .<br />It's not confirmed yet by Trolltech, but sounds like it is. Compiling Qt-4.4.0 <br />without glib support fixes things as well. libc_r isn't probably affected due <br />to nature of its' threads.</p>
<p>I'm closing bug for now.</p> DragonFlyBSD - Bug #1003: Qt 4.4 QtConcurrent and libthread_xuhttps://bugs.dragonflybsd.org/issues/1003?journal_id=113392013-03-09T21:11:36Ztuxillo
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/11339/diff?detail_id=861">diff</a>)</li><li><strong>Status</strong> changed from <i>New</i> to <i>Closed</i></li><li><strong>Assignee</strong> deleted (<del><i>0</i></del>)</li></ul><p>Effectively closing the ticket, as the caller meant to do so.</p>