Fix libthr segfault.

[mmokhi]

Summary

Using pthread_workqueue and libthr, locking/unlockig mutex results in segfault (null-deref.), due adding node to NULL tailq.
This fixes it

[cem]

This seems wrong to me. TAILQ_INSERT_TAIL should do the right thing on empty lists. If it doesn't already, that's where the fix should go, I think.

[kib]

In D6087#129590, @cem wrote:

This seems wrong to me. TAILQ_INSERT_TAIL should do the right thing on empty lists. If it doesn't already, that's where the fix should go, I think.

TAILQ_INSERT_TAIL handles empty queues just fine.

If the fault is real, the debugging data, i.e. at least the backtrace (from libthr compiled with -g) and dump of the curthread content are due.

[mmokhi]

In D6087#129590, @cem wrote:

This seems wrong to me. TAILQ_INSERT_TAIL should do the right thing on empty lists.

I personally expected that from TAILQ_INSERT_TAIL, but as far as i followed it, it's doing *(head)->tqh_last = (elm);
which assumes tqh_last != NULL without no clear reason.

Maybe we should do something for TAILQ_INSERT_TAIL, but till then this seems fixes it.

[mmokhi]

In D6087#129593, @kib wrote:

In D6087#129590, @cem wrote:

This seems wrong to me. TAILQ_INSERT_TAIL should do the right thing on empty lists. If it doesn't already, that's where the fix should go, I think.

TAILQ_INSERT_TAIL handles empty queues just fine.

If the fault is real, the debugging data, i.e. at least the backtrace (from libthr compiled with -g) and dump of the curthread content are due.

I should attach dumps here ?
Or in bugzilla ?

[mmokhi]

In D6087#129593, @kib wrote:

In D6087#129590, @cem wrote:

This seems wrong to me. TAILQ_INSERT_TAIL should do the right thing on empty lists. If it doesn't already, that's where the fix should go, I think.

TAILQ_INSERT_TAIL handles empty queues just fine.

If the fault is real, the debugging data, i.e. at least the backtrace (from libthr compiled with -g) and dump of the curthread content are due.

bugzilla issue : https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209036

[kib]

In D6087#129603, @mokhi64_gmail.com wrote:

In D6087#129590, @cem wrote:

This seems wrong to me. TAILQ_INSERT_TAIL should do the right thing on empty lists.

I personally expected that from TAILQ_INSERT_TAIL, but as far as i followed it, it's doing *(head)->tqh_last = (elm);
which assumes tqh_last != NULL without no clear reason.

tqh_last is not NULL for initialized tailq.

Maybe we should do something for TAILQ_INSERT_TAIL, but till then this seems fixes it.

[mmokhi]

In D6087#129620, @kib wrote:

In D6087#129603, @mokhi64_gmail.com wrote:

In D6087#129590, @cem wrote:

This seems wrong to me. TAILQ_INSERT_TAIL should do the right thing on empty lists.

I personally expected that from TAILQ_INSERT_TAIL, but as far as i followed it, it's doing *(head)->tqh_last = (elm);
which assumes tqh_last != NULL without no clear reason.

tqh_last is not NULL for initialized tailq.

Maybe we should do something for TAILQ_INSERT_TAIL, but till then this seems fixes it.

Yes :) you are right.
I think we can delete this review.
Though i think it's a good feature for TAILQ_INSERT_TAIL to check for empty conditions too ;)