connection: fix demarshal of invalid header

The size argument to wl_connection_demarshal() is taken from the message by the
caller wl_client_connection_data(), therefore 'size' is untrusted data
controllable by a Wayland client. The size should always be at least the header
size, otherwise the header is invalid.

If the size is smaller than header size, it leads to reading past the end of
allocated memory. Furthermore if size is zero, wl_closure_init() changes
behaviour and leaves num_arrays uninitialized, leading to access of arbitrary
memory.

Check that 'size' fits at least the header. The space for arguments is already
properly checked.

This makes the request_bogus_size test free of errors under Valgrind.

Fixes: https://gitlab.freedesktop.org/wayland/wayland/issues/52

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Reviewed-by: Simon Ser <contact@emersion.fr>

tests: add request_bogus_size

This attempts to reproduce the error conditions from
https://gitlab.freedesktop.org/wayland/wayland/issues/52 and make it crash.
While the crash was repeatable in my tests, it depends on garbage on stack
leading to access of invalid memory, which is not guaranteed.

This is a FAIL_TEST, so that the following fix commit can be verified.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Reviewed-by: Simon Ser <contact@emersion.fr>

contributing: use Gitlab merge request workflow

The experience from Weston shows that the Gitlab merge request based workflow
works really well. Recently there have also been issues with the mailing list
that have made the email based workflow more painful than it used to be. Those
issues might have been temporary or occasional, but they probably are only
going to increase.

The MR workflow is different, it has its issues
(https://gitlab.freedesktop.org/freedesktop/freedesktop/issues/74) and we
likely lose the explicit Reviewed-by etc. tags from commit messages, but it is
also much easier to work with: no more whitespace damaged patches, lost email,
setting up git-send-email; we gain automated CI before any human reviewer even
looks at anything, and people can jump in to an ongoing discussion even if they
weren't subscribed before.

If you still want email, you can subscribe to that selectively(!) in Gitlab
yourself.

This text has been copied from Weston's CONTRIBUTING.md of the 5.0.91 release
and slightly altered for Wayland.

Fixes: https://gitlab.freedesktop.org/wayland/wayland/issues/49

v2: fixed two left-over mentions of Weston

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
v1 Reviewed-by: Simon Ser <contact@emersion.fr>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Scott Anderson <scott.anderson@collabora.com>
Acked-by: Ian Ray <ian.ray@ge.com>
Acked-by: Derek Foreman <derek.foreman.wayland@gmail.com>

wayland-util.h: add forward declaration for wl_object

The definition of wl_argument in wayland-util.h references wl_object,
so wl_object ought to be defined in wayland-util.h. This resolves
gitlab issue #78.

Fixes: https://gitlab.freedesktop.org/wayland/wayland/issues/78

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

configure.ac: bump to version 1.16.92 for the beta release

tests: fix main symbol duplication

So far I got these errors before patching:

libtool: link: cc -o .libs/headers-test -pthread -Wall -Wextra -Wno-unused-parameter -g -Wstrict-prototypes -Wmissing-prototypes -fvisibility=hidden -O2 -pipe tests/headers-test.o tests/headers-protocol-test.o tests/headers-protocol-core-test.o /tmp/obj/wayland-1.16.0/build-amd64/.libs/libtest-runner.a -L.libs -lwayland-client -lffi -lm -lwayland-server -lkvm -Wl,-rpath-link,/usr/local/lib
ld: error: duplicate symbol: main
>>> defined at headers-test.c:53 (/tmp/obj/wayland-1.16.0/wayland-1.16.0/tests/headers-test.c:53)
>>>            tests/headers-test.o:(main)
>>> defined at test-runner.c:377 (/tmp/obj/wayland-1.16.0/wayland-1.16.0/tests/test-runner.c:377)
>>>            test-runner.o:(.text+0x250) in archive /tmp/obj/wayland-1.16.0/build-amd64/.libs/libtest-runner.a

libtool: link: cc -o .libs/exec-fd-leak-checker -pthread -Wall -Wextra -Wno-unused-parameter -g -Wstrict-prototypes -Wmissing-prototypes -fvisibility=hidden -O2 -pipe tests/exec-fd-leak-checker.o /tmp/obj/wayland-1.16.0/build-amd64/.libs/libtest-runner.a -L.libs -lwayland-client -lffi -lm -lwayland-server -lkvm -Wl,-rpath-link,/usr/local/lib
ld: error: duplicate symbol: main
>>> defined at exec-fd-leak-checker.c:57 (/tmp/obj/wayland-1.16.0/wayland-1.16.0/tests/exec-fd-leak-checker.c:57)
>>>            tests/exec-fd-leak-checker.o:(main)
>>> defined at test-runner.c:377 (/tmp/obj/wayland-1.16.0/wayland-1.16.0/tests/test-runner.c:377)
>>>            test-runner.o:(.text+0x250) in archive /tmp/obj/wayland-1.16.0/build-amd64/.libs/libtest-runner.a

Makefile.am: error: object 'tests/test-helpers.$(OBJEXT)' created both with libtool and without

libtool: link: cc -o .libs/fixed-benchmark -pthread -Wall -Wextra -Wno-unused-parameter -g -Wstrict-prototypes -Wmissing-prototypes -fvisibility=hidden -O2 -pipe tests/fixed-benchmark.o /tmp/obj/wayland-1.16.0/build-amd64/.libs/libtest-runner.a -L.libs -lwayland-client -lffi -lm -lwayland-server -lkvm -Wl,-rpath-link,/usr/local/lib
ld: error: duplicate symbol: main
>>> defined at fixed-benchmark.c:100 (/tmp/obj/wayland-1.16.0/wayland-1.16.0/tests/fixed-benchmark.c:100)
>>>            tests/fixed-benchmark.o:(main)
>>> defined at test-runner.c:377 (/tmp/obj/wayland-1.16.0/wayland-1.16.0/tests/test-runner.c:377)
>>>            test-runner.o:(.text+0x250) in archive /tmp/obj/wayland-1.16.0/build-amd64/.libs/libtest-runner.a

This commit fixes all of that.

Signed-off-by: Leonid Bobrov <mazocomp@disroot.org>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>

protocol: warn clients about some wl_output properties

All wl_output properties don't always make sense for all
compositors.

Some compositors might not implement a "global compositor space",
(e.g. 3D compositors) in which case properties like x and y don't
make sense.

Some compositors might expose virtual outputs, in which case modes,
make and model are not relevant.

In a lot of these situations, information from xdg_output is better
suited.

Compositors also expose output refresh rate, which shouldn't be used
for synchronization purposes.

Signed-off-by: Simon Ser <contact@emersion.fr>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Jonas Ådahl <jadahl@gmail.com>
Reviewed-by: Derek Foreman <derek.foreman.wayland@gmail.com>

configure.ac: bump version to 1.16.91 for the alpha release

TODO: remove "SDL port", it's been done by now

Upstream SDL supports Wayland since v2.0.4 (June 2015):
https://forums.libsdl.org/viewtopic.php?t=11294

Just set SDL_VIDEODRIVER=wayland and SDL will do the right thing :)

Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>

Print NULL strings as "nil" in wl_closure_print

Calling printf("%s", NULL) is undefined behaviour.

Signed-off-by: Simon Ser <contact@emersion.fr>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>

proto, server: Add internal server error message. (v2)

Many languages such as C++ or Rust have an unwinding error-reporting
mechanism. Code in these languages can (and must!) wrap request handling
callbacks in unwind guards to avoid undefined behaviour.

As a consequence such code will detect internal server errors, but have
no way to communicate such failures to the client.

This adds a WL_DISPLAY_ERROR_IMPLEMENTATION error to wl_display so that
such code can notify (and disconnect) clients which hit internal bugs.
While servers can currently abuse other wl_display errors for the same
effect, adding an explicit error code allows clients to tell the
difference between errors which are their fault and errors which are the
server's fault. This is particularly interesting for automated bug
reporting.

v2: Rename error from "internal" to "implementation", in sympathy with
    X11's BadImplementation error.
    Add more justification in the commit message.

Signed-off-by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>

server: Split out varargs version of wl_resource_post_error.

This will allow other wrappers around wl_resource_post_error to accept
variable argument lists.

Signed-off-by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>

protocol: prefer wl_surface.damage_buffer

This commit makes wl_surface.damage_buffer preferred over wl_surface.damage.
wl_surface.damage can be implemented in a non-optimal way by the compositor
(e.g. by always damaging the whole buffer).

Having two requests makes it complicated for the compositor to handle damage,
making it necessary to transform one into the other's coordinates.

Moreover, integration with wp_viewporter is tricky.

Signed-off-by: Simon Ser <contact@emersion.fr>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Derek Foreman <derek.foreman.samsung@gmail.com>

protocol: Bump seat to version 7 and require keymaps be private

Weston commit 76829fc4eaea329d2a525c3978271e13bd76c078 (and similar
commits for other compositors) protects the compositor's keyboard
mapping from client damage by duplicating the keymap for every
client.

On some systems there are other potential fixes for this - such as
using sealed memfds on linux - but we can't use them since
essentially all client code anywhere has mapped the keyboard map
with a MAP_SHARED mmap() call.

While we can't break years worth of code, we can require any future
clients to use MAP_PRIVATE if they use a seat version above 6.

If a compositor can't use sealing or a similar facility, it should
still protect itself with copied keymaps, but clients must always
assume shared mapping of a keymap will fail.

Signed-off-by: Derek Foreman <derek.foreman.samsung@gmail.com>
Reviewed-by: Simon Ser <contact@emersion.fr>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Philipp Kerling <pkerling@casix.org>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>

tests: Remove memory leak checking infrastructure

There are far better ways to detect memory leaks, such as either
valgrind or ASan. Having Meson makes it really easy to use these tools
in our tests, and we can do that in CI as well.

Having these local wrappers actually completely broke ASan usage, so
remove them in favour of using the more powerful options.

Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

tests: Overly elaborate compiler warning workaround

Clang will rightly point out that example_sockaddr_un in socket-test
will get discarded from the compilation unit as it is completely unused.
Put in a couple of lines which of no value other than stopping Clang
from complaining.

Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

tests: Use volatile pointer for NULL dereference

Clang warns that it can silently discard a non-volatile write to a NULL
pointer (perhaps it constitutes undefined behaviour?), and recommends
changing it to volatile.

This patch slavishly complies with the demand of the unfeeling machine.

Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

scanner: Reverse expat/libxml include order

libxml2 unconditonally defines XMLCALL to nothing. Expat does not
redefine XMLCALL if it is already defined, but if it is not, and we are
building with gcc on i386 (not x86-64), it will define it as 'cdecl'.

Including Expat before libxml thus results in a warning about XMLCALL
being redefined. Luckily we can get around this by just reversing the
include order: cdecl is a no-op on Unix-like systems, so by having
libxml first define XMLCALL to nothing and including Expat afterwards,
we avoid the warning and lose nothing.

Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

scanner: Mark fail() as noreturn

Help static analysers by letting them know that once we fail(),
execution will terminally complete.

Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

scanner: Plug two memory leaks

Found with both ASan leak sanitizer and Valgrind. We were trivially
leaking the enum name for every arg parsed by the scanner which had one.
If libxml-based DTD validation was enabled, we would also leak the DTD
itself, despite diligently freeing the document, context, etc.

Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

configure.ac: Reopen master for regular development

configure.ac: bump to version 1.16.0 for the official release

tests: Demarshalling of very long array/string lengths.

Attempting to demarshal message with array or string longer than its
body should return failure. Handling the length correctly is tricky when
it gets to near-UINT32_MAX values. Unexpected overflows can cause
crashes and other security issues.

These tests verify that demarshalling such message gives failure instead
of crash.

v2: Added consts, serialized opcode and size properly, updated style.
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Acked-by: Derek Foreman <derek.foreman.samsung@gmail.com>

configure.ac: bump to version 1.15.94 for the RC2 release

connection: Prevent pointer overflow from large lengths.

If the remote side sends sufficiently large `length` field, it will
overflow the `p` pointer. Technically it is undefined behavior, in
practice it makes `p < end`, so the length check passes. Attempts to
access the data later causes crashes.

This issue manifests only on 32bit systems, but the behavior is
undefined everywhere.
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Derek Foreman <derek.foreman.samsung@gmail.com>

connection: Prevent integer overflow in DIV_ROUNDUP.

The DIV_ROUNDUP macro would overflow when trying to round values higher
than MAX_UINT32 - (a - 1). The result is 0 after the division. This is
potential security issue when demarshalling an array because the length
check is performed with the overflowed value, but then the original huge
value is stored for later use.

The issue was present only on 32bit platforms. The use of size_t in the
DIV_ROUNDUP macro already promoted everything to 64 bit size on 64 bit
systems.
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Derek Foreman <derek.foreman.samsung@gmail.com>
Style changes by Derek Foreman

configure.ac: bump to version 1.15.93 for the RC1 release

contributing: Weston now uses GitLab MRs

Note that Weston uses GitLab MRs for review, not mail.

Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>

configure.ac: bump to version 1.15.92 for the beta release

spec: Delete old unused directory

The protocol spec used to live here, but it's now part of the regular
doc build. The PNG files are created as part of the doc build. Delete
the pre-generated versions.

Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Derek Foreman <derek.foreman.samsung@gmail.com>

build: Remove support for non-pkg-config Expat

The Expat XML library has shipped a pkg-config file for long enough to
be in Debian's oldstable (Jessie, April 2015) and Ubuntu's oldest
supported LTS (Trusty, 14.04). The pkg-config file was added in Expat
upstream's commit 352cfc8f59a7, in September 2007.

Drop build support for versions of Expat which do not ship a
pkg-config file.

Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Derek Foreman <derek.foreman.samsung@gmail.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

build: Remove execinfo.h check

The check for the execinfo.h header is only advisory; the build will not
fail if it is not present, and set HAVE_EXECINFO_H if it is. The check
was added in commit bc3e020475e ("build: Add declaration checks to check for
required syscall flags") with no obvious use or reasoning.

Remove the no-op check.

Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Derek Foreman <derek.foreman.samsung@gmail.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

wayland-server: Finally remove deprecated struct wl_buffer definition

commit d94a8722cb29d8b897672be66ff3c9ff79eab6fe
warned this was coming, back in 2013.

I've seen libraries that have wayland client and server using functions
in the same file.  Since struct wl_buffer still exists as an opaque
entity in client code, the vestigial deprecated wl_buffer from the
server include will generate warnings when not building with
WL_HIDE_DEPRECATED.

Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Acked-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>

protocol: allow to send a zero physical output size

Physical size doesn't always make sense for all outputs. In case
it's not available or not relevant, allow compositors to send zero.

Acked-by: Daniel Stone <daniels@collabora.com>
Acked-by: Jonas Ådahl <jadahl@gmail.com>
Acked-by: Olivier Fourdan <ofourdan@redhat.com>

configure.ac: bump to version 1.15.91 for the alpha release

contributing: document the release cycle freezes

These should be the conventions we have been using since 1.0, written
down more accurately.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Derek Foreman <derek.foreman.samsung@gmail.com>

contributing: about re-sending patches

This is what is generally expected from people who re-send patches,
whether the patches are their own or not.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Derek Foreman <derek.foreman.samsung@gmail.com>

contributing: review rules for bugs

Half of the ideas came from Daniel but most of them are reworded, the
rest are my thoughts.

Mention compiler warnings specifically, and be more explicit on what
kind of code or bugs or bug fixes are acceptable or not. Clarify commit
scope.

v2: move the "In a patch series" rule to the bottom, reworded.

Cc: Daniel Stone <daniels@collabora.com>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Derek Foreman <derek.foreman.samsung@gmail.com>

contributing: how to read the review rules

This is to avoid fighting around the letter of the guidelines. This is
not a protocol spec.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Derek Foreman <derek.foreman.samsung@gmail.com>

client: remove definition of wl_global

Nothing on the client side uses it since
9fe75537ad207c1496e6d9be41a8f5af4b876506 which was just before the 0.99
release.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-By: Markus Ongyerth <wl@ongy.net>

doc: update IANA MIME types registry URL

Use a more official one, served over HTTP rather than FTP.

Reviewed-by: Matheus Santana <embs@cin.ufpe.br>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>

contributing: commit rights

This has been copied from
https://cgit.freedesktop.org/xorg/app/intel-gpu-tools/tree/CONTRIBUTING?id=eccae1360d6d01e73c6af2bd97122cef708207ef
and slightly edited to better with Wayland and Weston.

The intention is to make it easier to give out commit access to new
people, let them know what is expected of them, and help the community
to grow. Hopefully this will in time improve the patch review throughput
and timeliness.

The original text was introduced in
https://cgit.freedesktop.org/xorg/app/intel-gpu-tools/commit/CONTRIBUTING?id=0350f0e7f6a0e07281445fc3082aa70419f4aac7

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Matheus Santana <embs@cin.ufpe.br>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Derek Foreman <derek.foreman.samsung@gmail.com>

contributing: add review guidelines

This sets up the standards for patch review, and defines when a patch
can be merged. I believe these are the practises we have been using
already for a long time, now they are just written down explicitly.

It's not an exhaustive list of criteria and likely cannot ever be, but
it should give a good idea of what level of review we want to have.

It has been written in general terms, so that we can easily apply the
same text not just to Wayland, but also Weston and other projects as
necessary.

This addition is not redundant with
https://wayland.freedesktop.org/reviewing.html .

The web page is a friendly introduction and encouragement for people to
get involved. The guidelines here are more specific and aimed for people
who seek commit rights or maintainership.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Matheus Santana <embs@cin.ufpe.br>
Reviewed-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Derek Foreman <derek.foreman.samsung@gmail.com>

.gitlab-ci.yml: collect the distcheck error logs

Currently we issue both check and distcheck, as reportedly there has
been cases in the past one works, while the other doesn't.

Yet we only collect the check artefacts (test logs).

Correct that, by picking the distcheck ones as well.
Note: the build-*/wayland*/ directory is purged by distcheck if it runs
successfully.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>

contributing: format as markdown

Format for nice viewing through Gitlab.

Conduct and Licensing were raised to first level headings as they are
not technical guidelines for contributing patches. It's nice to use the
first level headings more.

Reformat patchwork link and add Xorg patchwork link for Xwayland.

v2: Unfortunately Gitlab harcodes a tab character to mean 4 spaces, so we
cannot reasonably spell the coding style examples correctly. Hence, tab
characters have been replaced with eight spaces so that they at least
look right in both the file and through gitlab web UI.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Daniel Stone <daniels@collabora.com>

doc: move Contributing

Gitlab expects a CONTRIBUTING.md in the root directory, so move our
guide there.

Conversion to proper markup is a follow-up patch.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Daniel Stone <daniels@collabora.com>

doc: Update URLs for GitLab transition

Update bug and Git URLs for GitLab; the site has also been served over
HTTPS for quite some time.

Signed-off-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>

scanner: allow referencing foreign enums

It's already possible to reference foreign interfaces, so it
should also be possible to reference foreign enums.

Signed-off-by: Simon Ser <contact@emersion.fr>
Reviewed-by: Silvan Jegen <s.jegen@gmail.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>

Add .gitlab-ci.yml

Add a GitLab CI configuration which tests building, 'make check', and
'make distcheck' of the tree inside a Debian Stretch container. The
choice of distribution base was arbitrary and may easily be changed.

When commits are pushed to upstream, the commits will run this CI
pipeline to run these tests, and capture the result as an artifact
bundle, including the compiled binaries and full test suite logs.

Results can be seen at:
    https://gitlab.freedesktop.org/wayland/wayland/pipelines/

Signed-off-by: Daniel Stone <daniels@collabora.com>

tests: Add free-without-remove test

[Derek Foreman <derekf@osg.samsung.com> moved this into resources-test]

Reviewed-by: Derek Foreman <derekf@osg.samsung.com>

server: Add special case destroy signal emitter

In the past much code (weston, efl/enlightenment, mutter) has
freed structures containing wl_listeners from destroy handlers
without first removing the listener from the signal.  As the
destroy notifier only fires once, this has largely gone
unnoticed until recently.

Other code does not (Qt, wlroots) - and removes itself from
the signal before free.

If somehow a destroy signal is listened to by code from both
kinds of callers, those that free will corrupt the lists for
those that don't, and Bad Things will happen.

To avoid these bad things, remove every item from the signal list
during destroy emit, and put it in a list all its own.  This way
whether the listener is removed or not has no impact on the
following emits.

Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Simon Ser <contact@emersion.fr>
Reviewed-by: Markus Ongyerth <wl@ongy.net>

tests: Test for use after free in resource destruction signals

For years it's been common practice to free the object containing
the wl_listener inside resource destruction notifiers, but not
remove the listener from the list.

That is: It's been safe to assume (when only one listener is present)
that the wl_listener will never be touched again, since this is
a destruction callback.

Recently some patches were reviewed that made some positive changes
to our internal signal handling code, but would've violated this
assumption, and changed free()d memory in several existing compositors
(weston, mutter, enlightenment).

Since the breakage was extremely subtle, codify this assumption in
a test case (thus promoting it to an ABI promise).

Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-by: Markus Ongyerth <wl@ongy.net>
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>

wayland-server: Properly handle EAGAIN from wl_connection_read()

commit 3cddb3c692acd3536a7cc8542a29f0cc3c0ac3d6 casted len to an
unsigned value to compare to sizeof results.  However,
wl_connection_read() can fail, setting errno to EAGAIN and returning
a value of -1.

When cast to an unsigned type this leads to a loop condition of true
when it should be false.

Signed-off-by: Dipen Somani <dipen.somani@samsung.com>
Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>

configure.ac: Reopen master for regular development

configure.ac: bump to version 1.15.0 for the official release

configure.ac: bump to version 1.14.93 for the RC1 release

.gitignore: add wayland-egl-abi-check

Instruct git go ignore the file, in case we've done an in-tree build.

Cc: Derek Foreman <derekf@osg.samsung.com>
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>

wayland-egl: Ignore underscored symbols in ABI check

Rather than a hard-coded list of platform symbols, just ignore anything
prefaced with an underscore. This fixes breakage on ARM, which declares
several slightly different platform symbols to x86.

Signed-off-by: Daniel Stone <daniels@collabora.com>
Fixes: 21b1f22eb056 ("wayland-egl: enhance the symbol test")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105620
Cc: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com>

configure.ac: bump to version 1.14.92 for the beta release

wayland-egl: Make symbol test fail on failure

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: 21b1f22eb056 ("wayland-egl: enhance the symbol test")
Cc: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

wayland-egl: Pass nm path to check script

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: 6903e4d53925 ("wayland-egl: use correct `nm` path when cross-compiling")
Cc: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Quentin Glidic <sardemff7+git@sardemff7.net>

wayland-egl: bump the version number to 18.1.0

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>

wayland-egl: enhance the symbol test

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>

wayland-egl: fail the symbol check if lib is missing

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>

wayland-egl: set the correct path to libwayland-egl.so

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: 85cb5ed64aa ("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>

configure.ac: don't install the static libraries

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>

tests: disable coredumps on sanity-test

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>

walyand-client: Fix trivial build break from previous commit

previous commit, a9187853d44db41206b5d16a770d4db108972812 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>

client: Don't inappropriatly close fds for zombie objects

commit 239ba39331420f953de35c337ae57db35573f9cb 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>

configure.ac: bump to version 1.14.91 for the alpha release

tests: fix scanner private-code test reference data

Commit e501230d1dc8a5015616e104ec0e08886a0b88df "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>

scanner: Fix broken private-code generation

Missing a closing bracket.

Reviewed-by: Daniel Stone <daniels@collabora.com>

wayland-egl: use correct `nm` path when cross-compiling

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>

tests: add code, public-code and private-code tests

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>

scanner: make use of __has_attribute()

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>

build: use public-code when using the local wayland-scanner

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>

scanner: introduce "public-code" and "private-code"

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>

build: remove white space in -uninstalled.pc.in files

v2: Rebase, address wayland-client-uninstalled

Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Derek Foreman <derekf@osg.samsung.com> (v1)

connection: Fix broken log message when demarshalling short closure

Like the similar wl_log() message further into this function that was
fixed in commit 2fc248dc2c877d02694db40aad52180d71373d5a 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>

wayland-egl: rename wl_egl_window::private to driver_private

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>

wayland-egl: move the wayland-egl{, -core}.h headers to egl/

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>

wayland-egl-symbols-check: pass the DSO name via the build system

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>

build: wire-up wayland-egl

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>

wayland-egl: add a note about keeping the backend version in sync

Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Arnaud Vrac <avrac@freebox.fr>

wayland-egl: introduce wayland-egl-backend.pc

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>

wayland-egl: correct wayland-egl.pc description/version

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>

wayland-egl: reuse the existing WL_EXPORT macro

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>

wayland-egl: import libwayland-egl.so frontend library from Mesa

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>

shm: provide actual error on mmap failed

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>

server: add wl_display_destroy_clients()

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>

doc: Document behavior of non-nullable object arguments in clients

Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>

connection: Clear correct args when clearing fds to -1

commit 52609ddf79a96fee0465006e2c6339a3a5d23a87 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>

tests: Add missing file to distribution

In f74c9b98db49ce16e037c3012590c4a24a4fc32e 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>

server: add log message when client connection is destroyed due to an error

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>

tests: Check for wrong fd delivery with zombie objects

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>

tests: Add a test for fd leaks on zombie objects

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>

client: Consume file descriptors destined for zombie proxies

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>

client: Replace the singleton zombie with bespoke zombies

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>

client: Plug a race in proxy destruction vs. dispatch

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>

client: Add wl_proxy_unref helper

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>