The previous rewrite of the wayland-egl ABI checker introduced checks
for removed symbols as well as added symbols, but broke some failure
conditions. Add an explict return-code variable set in failure paths,
rather than chaining or conditions.
If we cannot find the binary or nm, we regard this as an error
condition, rather than test failure.
v2: Don't test if we can execute $NM.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reported-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Fixes: 21b1f22eb0 ("wayland-egl: enhance the symbol test")
Cc: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
A previous patch used $NM as an environment variable, but this was only
set as a make variable. Make sure it is passed through from make to the
environment we use to run tests.
v2: Quote argument when passing to shell.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reported-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Fixes: 6903e4d539 ("wayland-egl: use correct `nm` path when cross-compiling")
Cc: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
Seems like I was overoptimistic with my earlier assumption, namely:
"... 17.3.x should be the last version that ships the library."
Mesa 18.0.0 and its wayland-egl is about to be released any time soon,
so bump the number since it must no be smaller. As soon as we get
a wayland release I'll drop the Mesa copy but for now.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
The current test had a few fall-outs:
- it was checking only for T (.text) symbols
- did not consider symbol removal
Fix that by fetching all the symbols and doing a bidirectional check -
for added and removed symbols. Error out with informative message for
each case.
v2: Rebase on top of $NM patch.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Based on a similar patch (in Mesa) by Eric Engestrom.
v2: Rebase on top of $NM patch
v3: Rebase
Reviewed-by: Eric Engestrom <eric@engestrom.ch> (v1)
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Earlier commit changed to passing the binary name as env. variable
introducing a typo.
That went unnoticed, since we do not check if the file is present or
not.
Cc: Pukka Paalanen <ppaalanen@gmail.com>
Cc: Daniel Stone <daniels@collabora.com>
Fixes: 85cb5ed64a ("wayland-egl-symbols-check: pass the DSO name via
the build system")
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
One should always be using the shared libraries.
Spotted while going through the Debian packaing.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
SEGV and ABRT by default cause a core dump, which may create a file,
launch crash handlers, and so on. sanity-test has 21 processes that
are expected to crash like this. Disable core dumps on them all.
I counted 21 entries in coredumpctl list, while only 16 functions needed
patching. After this patch no entries appear in coredumpctl list.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
previous commit, a9187853d4 added
a trailing { on a line it shouldn't have, and I pushed without
building first.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
commit 239ba39331 which was intended
to stop leaking fds in events for zombie objects didn't notice that
passing 0 to wl_connection_close_fds_in() would still close fds.
Test the fd count before calling.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Commit e501230d1d "scanner: Fix broken
private-code generation" changed the scanner output without updating the
reference output for scanner tests.
Update the reference data. This fixes 'make check'.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Inspired by Heiko Becker and Eric's work in libdrm and Mesa
respectively.
Cc: Eric Engestrom <eric.engestrom@imgtec.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
First one is deprecated in favour of the second option.
The latter is newly introduced and annotates the generated symbols
accordingly.
v2: Don't introduce small-public-code.c - reuse small-code.c (Pekka)
Cc: Pekka Paalanen <ppaalanen@gmail.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
A more generic way to evaluating various attributes, __has_attribute is
available with gcc, clang, even the Oracle/Sun compiler.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
The core wayland interfaces are public, via the libwayland-server and
libwayland-client DSOs. Hence use "public-code" cmdline option, instead
of the deprecated code".
As the host wayland-scanner may not know about the new option, use the
legacy "code".
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
The options are used to indicate how the code will be used - will it be
public, as part of a DSO or private.
In nearly every instance, people want to use the latter. One noticeable
exception is the wayland libraries. They provide the base marshalling
protocol that everyone uses.
The option "code" was deprecated in favour of "public-code" with a
warning message produced to guide people.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Like the similar wl_log() message further into this function that was
fixed in commit 2fc248dc2c this should
be printing the sender_id saved earlier instead of *p.
Since p is incremented during the loop it would not only print an
incorrect object id, it could read past the end of the array.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
private is a reserved keyword in C++. Thus if one is to have a backend
driver written in said language build will fail as below:
.../wayland-egl-backend.h:56:8: expected unqualified-id before ‘private’
Rename it to driver_private and update the test.
NOTE: version bump is not required since:
- this is a pure API change, ABI is identical
- hardware drivers already require [minor] changes to move to the
upstream wayland-egl-backend.h
Cc: Arnaud Vrac <rawoul@gmail.com>
Cc: Miguel A . Vico <mvicomoya@nvidia.com>
Suggested-by: Arnaud Vrac <rawoul@gmail.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Miguel A Vico Moya <mvicomoya@nvidia.com>
Now we have all the wayland-egl bits in a single place.
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Arnaud Vrac <avrac@freebox.fr>
The location of the file is build system specific so, keep it there.
Cc: Daniel Stone <daniels@collabora.com>
Suggested-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Arnaud Vrac <avrac@freebox.fr>
Wire-up the imported sources, test and pkg-config files.
v2:
- Don't mangle with existing EXTRA_DIST list
- Add the symbols check script to the `make check' target
- Rename wayland-egl-{priv,backend}.h
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Arnaud Vrac <avrac@freebox.fr>
File will be installed alongside the backend header.
This way vendor implementations have enough information about the
interface and they can build their backend/driver library accordingly.
Cc: Miguel A. Vico <mvicomoya@nvidia.com>
Cc: James Jones <jajones@nvidia.com>
Cc: Daniel Stone <daniels@collabora.com>
Cc: duncan-roe <duncan_roe@optusnet.com.au>
Cc: Takanari Hayama <taki@igel.co.jp>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Miguel A Vico Moya <mvicomoya@nvidia.com>
Reviewed-by: Arnaud Vrac <avrac@freebox.fr>
Drop the "Mesa" part from the commit message and change the version to
17.4.0. The number bears references from its Mesa heritage.
Currently Mesa provides 17.2.x while 17.3.x should be the last version
that ships the library.
Some version numbers of wayland-egl.pc providers and users.
The latter is taken from Ubuntu 17.04.
Providers:
- Mali: 7.10
- Mesa: 17.2.2
Users:
- retroarch: NA
- qtwayland5: NA
- mpv, libmpv1: 9.0
- mesa-utils-extra: NA
- weston, libweston-1-0: NA
- libwaffle-1-0: 9.1
- libsdl2-2.0-0: NA
- libgstreamer-plugins-bad1.0-0: autotools 9.0, meson 1.0
- libglfw3-wayland: NA
- kwin-wayland, kwin-wayland-backend-wayland: NA?
- glmark2-wayland, glmark2-es2-wayland: NA
- libwebkit2gtk-4.0-37, libwebkit2gtk-4.0-37-gtk2, (webkit overall): NA
- libgtk-3-0 (gtk overall): NA
- libcogl20 (cogl overall): 1.0.0
v2: Bump the version to 17.4.0
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Arnaud Vrac <avrac@freebox.fr>
There's little point if redefining new one ourselves. Just reuse the one
that's already available.
Cc: Daniel Stone <daniels@collabora.com>
Suggested-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Arnaud Vrac <avrac@freebox.fr>
Currently the client-facing libwayland-egl API is defined by a header
file shipped by Wayland, but the implementation is left to each vendor.
This can cause collisions when multiple implementations are installed on
the same system. Importing the implementation into Wayland with a stable
and versioned driver-facing ABI allows multiple drivers to coexist on
the same system.
Pull the sample implementation from Mesa commit 677edff5cfd
("wayland-egl: rework and simplify wl_egl_window initialization")
It has been used by the Mesa open source drivers, NVIDIA and others[1].
v2: Reword commit message, rebase on top of newer Mesa.
[1] https://github.com/thayama/wayland-egl
Cc: Miguel A. Vico <mvicomoya@nvidia.com>
Cc: James Jones <jajones@nvidia.com>
Cc: Daniel Stone <daniels@collabora.com>
Cc: duncan-roe <duncan_roe@optusnet.com.au>
Cc: Takanari Hayama <taki@igel.co.jp>
Suggested-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Miguel A Vico Moya <mvicomoya@nvidia.com>
Reviewed-by: Arnaud Vrac <avrac@freebox.fr>
When an mmap() fails, a WL_SHM_ERROR_INVALID_FD is raised and the client
is killed.
However, there is no indication of the actual system error that caused
mmap() to fail, which makes such error harder to investigate.
Provide the actual error message that caused mmap() to fail.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Jonas Ådahl <jadahl@gmail.com>
Bug [1] reported that wl_display_destroy() doesn't destroy clients, so
client socket file descriptors are being kept open until the compositor
process exits.
Patch [2] proposed to destroy clients in wl_display_destroy(). The
patch was not accepted because doing so changes the ABI.
Thus, a new wl_display_destroy_clients() function is added in this
patch. It should be called by compositors right before
wl_display_destroy().
[1] https://bugs.freedesktop.org/show_bug.cgi?id=99142
[2] https://patchwork.freedesktop.org/patch/128832/
Signed-off-by: Simon Ser <contact@emersion.fr>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Acked-by: Daniel Stone <daniels@collabora.com>
commit 52609ddf79 was intended to
set fds to -1 in the arg list, however it failed to account for
version information at the start of signatures.
Most noticably, this broke mesa's create_prime_buffer by setting
width to -1 instead of the fd, as the width was the argument
following the fd, and the version was one byte long.
This should close https://bugs.kde.org/show_bug.cgi?id=389200
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
In f74c9b98db I added tests.xml to the
repository, but not to the distribution tarball.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>
Reviewed-by: Daniel Stone <daniels@collabora.com>
The client connection is destroyed by the server in several
circumstances. This patch adds log messages in case the connection is
destroyed due to an error other than normal hangup.
Signed-off-by: Mathias Fiedler <mathias_fiedler@mentor.com>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Until recently, if an event attempting to deliver an fd to a zombie
object was demarshalled after the object was made into a zombie, we
leaked the fd and left it in the buffer.
If another event attempting to deliver an fd to a live object was in that
same buffer, the zombie's fd would be delivered instead.
This test recreates that situation.
While this is a ridiculously contrived way to force this race - delivering
an event from a destruction handler - I do have reports of this race
being hit in real world code.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Acked-by: Daniel Stone <daniels@collabora.com>
Until recently, if a client destroying a resource raced with the
server generating an event on that resource that delivered a file
descriptor, we would leak the fd.
This tests for a leaked fd from that race condition.
Reviewed-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
We need to close file descriptors sent to zombie proxies to avoid leaking
them, and perhaps more importantly, to prevent them from being dispatched
in events on other objects (since they would previously be left in the
buffer and potentially fed to following events destined for live proxies)
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Using the singleton zombie object doesn't allow us to posthumously retain
object interface information, which makes it difficult to properly inter
future events destined for the recently deceased proxy.
Notably, this makes it impossible for zombie proxy destined file
descriptors to be properly consumed.
When we create a proxy, we now create a zombie-state object to hold
information about the file descriptors in events it can receive. This
will allow us, in a future patch, to close those FDs.
[daniels: Split Derek's patch into a few smaller ones.]
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Closures created to hold events which will be dispatched on the client,
take a reference to the proxy for the object the event was sent to, as
well as the proxies for all objects referenced in that event.
These references are dropped immediately before dispatch, with the
display lock also being released. This leaves the potential for a
vanishingly small race, where another thread drops the last reference
on one of the proxies used in an event as it is being dispatched.
Fix this by splitting decrease_closure_args_refcount into two functions:
one which validates the objects (to ensure that clients are not returned
objects which they have destroyed), and another which unrefs all proxies
on the closure (object event was sent to, all referenced objects) as
well as the closure itself. For symmetry, increase_closure_args_refcount
is now the place where the refcount for the proxy for the object the
event was sent to, is increased.
This also happens to fix a bug: previously, if an event was sent to a
client-destroyed object, and the event had object arguments, a reference
would be leaked on the proxy for each of the object arguments.
Found by inspection whilst reviewing the zombie-FD-leak series.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Cc: Jonas Ådahl <jadahl@gmail.com>
Cc: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Rather than open-coded decrement-and-maybe-free, introduce a
wl_proxy_unref helper to do this for us. This will come in useful for
future patches, where we may also have to free a zombie object.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Cc: Jonas Ådahl <jadahl@gmail.com>
Commit e273c7cde added a refcount to wl_proxy. The refcount is set to 1
on creation, decreased when the client explicitly destroys the proxy,
and is increased and decreased every time an event referencing that
proxy is queued.
Assuming no bugs, this means the refcount cannot reach 0 without the
proxy being explicitly destroyed. However, some (not all) of the
proxy-unref paths were only destroying the proxy if it had already been
deleted. This should already be enforced by refcounting, so remove the
check and rely solely on the refcount as the arbiter of when to free a
proxy.
Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>
Cc: Jonas Ådahl <jadahl@gmail.com>
Since we now have the WL_MAP_ENTRY_ZOMBIE flag to determine whether or
not a client-side object is a zombie, we can remove the faux object.
[daniels: Extracted from Derek's bespoke-zombie patch as an intermediate
step.]
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
This makes it easier for future patches in the series, which can
possibly return NULL for extant map entries.
[daniels: Extracted from Derek's bespoke-zombie patch as an intermediate
step.]
Reviewed-by: Daniel Stone <daniels@collabora.com>
Add a new map entry flag to indicate that the object received is valid,
but a zombie. Previously this relied on a fixed object pointer, but
future patches in this series will have map entries returning either
NULL, or a different structure type entirely, for zombie objects.
wl_object_is_zombie() now solely uses the new flag to determine whether
or not the object is a zombie.
[daniels: Extracted from Derek's bespoke-zombie patch as an intermediate
step.]
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Add a helper function which determines whether or not an object is a
zombie.
[daniels: Extracted from Derek's bespoke-zombie patch as an intermediate
step.]
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
On the client side we're going to need to know if an object from the
map is a zombie before we attempt to dereference it, so we need to
pass this to the iterator.
Reviewed-by: Daniel Stone <daniels@collabora.com>
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
When we have a closure that can't be dispatched for some reason, and it
contains file descriptors, we must close those descriptors to prevent
leaking them.
Previous commits ensure that only FDs belonging to this invocation of
the closure, i.e. not FDs provided by the client for marshalling, nor
FDs which have already been dispatched to either client or server, will
be left in the closure by destroy time.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
This initializes all the fd arguments in closures to -1 and clears
them back to -1 when they've been dispatched or serialized.
This means that any valid fd in a closure is currently libwayland's
responsibility to close in the case of an error.
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Daniel Stone <daniels@collabora.com>
