<html>
<head>
<base href="https://bugs.freedesktop.org/">
</head>
<body><table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>Bug ID</th>
<td><a class="bz_bug_link
bz_status_NEW "
title="NEW - SIGSEGV on wl_list_insert"
href="https://bugs.freedesktop.org/show_bug.cgi?id=100878">100878</a>
</td>
</tr>
<tr>
<th>Summary</th>
<td>SIGSEGV on wl_list_insert
</td>
</tr>
<tr>
<th>Product</th>
<td>Wayland
</td>
</tr>
<tr>
<th>Version</th>
<td>unspecified
</td>
</tr>
<tr>
<th>Hardware</th>
<td>All
</td>
</tr>
<tr>
<th>OS</th>
<td>All
</td>
</tr>
<tr>
<th>Status</th>
<td>NEW
</td>
</tr>
<tr>
<th>Severity</th>
<td>normal
</td>
</tr>
<tr>
<th>Priority</th>
<td>medium
</td>
</tr>
<tr>
<th>Component</th>
<td>weston
</td>
</tr>
<tr>
<th>Assignee</th>
<td>wayland-bugs@lists.freedesktop.org
</td>
</tr>
<tr>
<th>Reporter</th>
<td>worknesday@gmail.com
</td>
</tr></table>
<p>
<div>
<pre>As seen on SHA 9ad4de1f7ad411fcb5a62eb85e17cf96ae076a0f; no modification
At the bottom of the description, you can find stacktrace.
Analysis:
When I saw this issue, I did some digging around on wl_list type and I suspect
the seg fault arose from the design of the circular-doubly-linked lists
themselves.
If we look at wayland-util.h (where these list functions are declared), you can
see something odd and (potentially) dangerous:
void
wl_list_insert(struct wl_list *list, struct wl_list *elm);
void
wl_list_remove(struct wl_list *elm); //
The problem here is that the *identity* of the list is subject to change as you
add/remove nodes -- especially wl_list_remove. If, say, you have a list
struct wl_list *my_list; // initialized to some list with 3 elements
then you do
wl_list_remove(my_list)
then you will lose the other elements and my_list is no longer valid nor
circular because wl_list_remove sets prev and next to NULL. Indeed,
wl_list_remove says so explicitly:
/**
* Removes an element from the list.
*
* \note This operation leaves \p elm in an invalid state.
*
* \param elm Link of the containing struct to remove from the list
*
* \memberof wl_list
*/
I think a better (robust) design is to wrap and hide the actual node pointers
so that
1. we preserve the identity (pointer) of the list itself
2. we can call wl_list_remove multiple times
I envision something like
struct wl_list_node {
/** Previous list element */
struct wl_list_node *prev;
/** Next list element */
struct wl_list_node *next;
};
and wl_list will be
struct wl_list {
struct wl_list_node *node;
};
Then, removing a node would be something like
// assert that node is an element of list
void
wl_list_remove(struct wl_list *list, struct wl_list_node *node);
So, you can snip node out of list. If list->node == node, then you can perform
a fixup (e.g. nudge list->node to the "next" element).
Thus, you ensure the integrity of the list.
I think this is the best option given the circumstance and I hope that I'm not
stepping on peoples' toes.
Stack trace:
#0 0x00007ffff79a0313 in wl_list_insert () from
/usr/lib64/libwayland-server.so.0
#1 0x00007fffeeae16cf in wl_signal_add (signal=<optimized out>,
listener=<optimized out>) at /usr/include/wayland-server-core.h:320
#2 focus_state_set_focus (state=<optimized out>, surface=<optimized out>)
at desktop-shell/shell.c:735
#3 0x00007fffeeae7435 in activate (shell=0x7820e0, view=<optimized out>,
seat=0x622100, flags=3) at desktop-shell/shell.c:3686
#4 0x00007ffff7bc5d5d in weston_compositor_run_button_binding
(compositor=compositor@entry=0x614c60, pointer=pointer@entry=0x76c900,
time=time@entry=3071980015, button=button@entry=272,
state=state@entry=WL_POINTER_BUTTON_STATE_PRESSED) at libweston/bindings.c:368
#5 0x00007ffff7bc13cd in notify_button (seat=seat@entry=0x622100,
time=3071980015, button=button@entry=272,
state=WL_POINTER_BUTTON_STATE_PRESSED) at libweston/input.c:1673
#6 0x00007ffff531065b in x11_backend_deliver_button_event (event=0x7bc180,
b=0x622070) at libweston/compositor-x11.c:1230
#7 x11_backend_handle_event (fd=<optimized out>, mask=<optimized out>,
data=0x622070) at libweston/compositor-x11.c:1422
#8 0x00007ffff799dd12 in wl_event_loop_dispatch () from
/usr/lib64/libwayland-server.so.0
#9 0x00007ffff799c1b5 in wl_display_run () from
/usr/lib64/libwayland-server.so.0
#10 0x0000000000405719 in main (argc=1, argv=<optimized out>) at
compositor/main.c:1969
surface data:
(gdb) p *es
$6 = {resource = 0x7b2fb0, destroy_signal = {listener_list = {prev =
0x622190, next = 0x793a90}}, ...
(gdb) p *es->destroy_signal.listener_list.prev
$10 = {prev = 0x0, next = 0x7e1348}</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>