Re: [PATCH xserver v2] xwayland: avoid using freed xwl_window on unrealize
Hi Daniel, On Thu, Apr 19, 2018 at 5:24 PM, Daniel Stone wrote: > [...] > The callback is passed as an argument to the function directly, so you > could just destroy that instead. > Sure, but that wouldn't solve the problem which is the callback occurs after the “xwl_window” is freed, meaning that we access freed memory here. In other words, we could do that (and I agree it would be more elegant) but that would probably crash on the next line or so. Removing all frame callbacks when un-realizing the window as done with the other patch that was merged (thanks Adam!) is safer. Cheers, Olivier ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [ANNOUNCE] xorg-server 1.19.99.904
On Wed, Apr 18, 2018 at 10:13:36AM -0500, Jeff Smith wrote:
> Here is something that may help in tracking down the cause of the
> disappearing symbol.
> 1: apply the following changes to sdksyms.sh
> 2: re-run the build
> (this will output what is being fed into awk from the pre-processor
> into sdksyms.i)
> 3: search for drm_format_for_depth in sdksyms.i
> (if not found, search for dri3_send_open_reply instead)
> 4: post the relevant section of sdksyms.i
>
> -${CPP:-cpp} "$@" sdksyms.c | ${AWK:-awk} -v topdir=$topdir '
> +${CPP:-cpp} "$@" sdksyms.c | tee sdksyms.i | ${AWK:-awk} -v topdir=$topdir '
Here's the requested data:
typedef struct dri3_screen_info {
# 88 "../../dri3/dri3.h" 3 4
__uint32_t
# 88 "../../dri3/dri3.h"
version;
dri3_open_proc open;
dri3_pixmap_from_fd_proc pixmap_from_fd;
dri3_fd_from_pixmap_proc fd_from_pixmap;
dri3_open_client_proc open_client;
dri3_pixmap_from_fds_proc pixmap_from_fds;
dri3_fds_from_pixmap_proc fds_from_pixmap;
dri3_get_formats_proc get_formats;
dri3_get_modifiers_proc get_modifiers;
dri3_get_drawable_modifiers_proc get_drawable_modifiers;
} dri3_screen_info_rec, *dri3_screen_info_ptr;
extern __attribute__((visibility("default"))) Bool
dri3_screen_init(ScreenPtr screen, const dri3_screen_info_rec *info);
extern __attribute__((visibility("default"))) int
dri3_send_open_reply(ClientPtr client, int fd);
extern __attribute__((visibility("default")))
# 112 "../../dri3/dri3.h" 3 4
__uint32_t
# 113 "../../dri3/dri3.h"
drm_format_for_depth(
# 113 "../../dri3/dri3.h" 3 4
__uint32_t
# 113 "../../dri3/dri3.h"
depth,
# 113 "../../dri3/dri3.h" 3 4
__uint32_t
# 113 "../../dri3/dri3.h"
bpp);
# 106 "sdksyms.c" 2
# 120 "sdksyms.c"
I don't see the problem, do you?
The whole .i file is available at
http://danbala.tuwien.ac.at/~wiz/tmp/xdksyms.output.gz
Thomas
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xf86-input-libinput] Implement the custom acceleration curve options
One new property, and the existing accel profile gets extended to keep one extra value. The new property libinput Accel Curve Points is a list of pairs of points to be added to the acceleration curve. libinput only supports adding points to the curve so we simply declare the behavior as undefined when the curve is set multiple times. Also helps to identify those that bother to read the man page before playing with random driver values. Signed-off-by: Peter Hutterer --- Note, the matching libinput patch is not yet on git master. configure.ac | 17 include/libinput-properties.h | 8 ++ man/libinput.man | 53 +- src/xf86libinput.c| 227 +- 4 files changed, 298 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index 34f2274..5892c5e 100644 --- a/configure.ac +++ b/configure.ac @@ -47,6 +47,23 @@ XORG_DEFAULT_OPTIONS PKG_CHECK_MODULES(XORG, [xorg-server >= 1.10] xproto [inputproto >= 2.2]) PKG_CHECK_MODULES(LIBINPUT, [libinput >= 1.4.901]) +AC_MSG_CHECKING([if libinput_device_config_accel_set_curve_point is available]) +OLD_LIBS=$LIBS +OLD_CFLAGS=$CFLAGS +LIBS="$LIBS $LIBINPUT_LIBS" +CFLAGS="$CFLAGS $LIBINPUT_CFLAGS" +AC_LINK_IFELSE( + [AC_LANG_PROGRAM([[#include ]], + [[libinput_device_config_accel_set_curve_point(NULL, 0, 0)]])], + [AC_MSG_RESULT([yes]) +AC_DEFINE(HAVE_LIBINPUT_CUSTOM_ACCEL_CURVE, [1], + [libinput_device_config_accel_set_curve_point() is available]) +[libinput_have_custom_accel_curve=yes]], + [AC_MSG_RESULT([no]) +[libinput_have_custom_accel_curve=no]]) +LIBS=$OLD_LIBS +CFLAGS=$OLD_CFLAGS + # Define a configure option for an alternate input module directory AC_ARG_WITH(xorg-module-dir, AC_HELP_STRING([--with-xorg-module-dir=DIR], diff --git a/include/libinput-properties.h b/include/libinput-properties.h index a701316..3169cfb 100644 --- a/include/libinput-properties.h +++ b/include/libinput-properties.h @@ -73,6 +73,14 @@ only one is enabled at a time at max */ #define LIBINPUT_PROP_ACCEL_PROFILE_ENABLED "libinput Accel Profile Enabled" +/* Pointer accel custom curve: FLOAT, n values for n>=0 && n%2 == 0, where + each pair of values are the x/y coordinates of the acceleration curve. + The exact behavior depends on the acceleration profile. + + This property should be considered write-only, the value of the property may + not reflect the actual curve points used in libinput. */ +#define LIBINPUT_PROP_ACCEL_CURVE_POINTS "libinput Accel Curve Points" + /* Natural scrolling: BOOL, 1 value */ #define LIBINPUT_PROP_NATURAL_SCROLL "libinput Natural Scrolling Enabled" diff --git a/man/libinput.man b/man/libinput.man index dbf7dee..acb447d 100644 --- a/man/libinput.man +++ b/man/libinput.man @@ -45,11 +45,19 @@ are supported: Sets the pointer acceleration profile to the given profile. Permitted values are .BI adaptive, -.BI flat. +.BI flat, +.BI device-speed-curve. Not all devices support this option or all profiles. If a profile is unsupported, the default profile for this device is used. For a description on the profiles and their behavior, see the libinput documentation. .TP 7 +.BI "Option \*qAccelCurvePoints\*q \*q" string \*q +A string of space-separated pairs of floats, each pair value separated by a +colon, i.e. the string looks like this "A:B C:D". These pairs represent the +x/y value of a point on the acceleration curve. See +.B CUSTOM POINTER ACCELERATION CURVES +for details. +.TP 7 .BI "Option \*qAccelSpeed\*q \*q" float \*q Sets the pointer acceleration speed within the range [-1, 1] .TP 7 @@ -204,16 +212,26 @@ on the device. The following properties are provided by the driver. .TP 7 .BI "libinput Accel Profiles Available" -2 boolean values (8 bit, 0 or 1), in order "adaptive", "flat". -Indicates which acceleration profiles are available on this device. +3 boolean values (8 bit, 0 or 1), in order "adaptive", "flat", +"device-speed-curve". Indicates which acceleration profiles are available on +this device. .TP 7 .BI "libinput Accel Profile Enabled" -2 boolean values (8 bit, 0 or 1), in order "adaptive", "flat". -Indicates which acceleration profile is currently enabled on this device. +3 boolean values (8 bit, 0 or 1), in order "adaptive", "flat", +"device-speed-curve". Indicates which acceleration profile is currently +enabled on this device. .TP 7 .BI "libinput Accel Speed" 1 32-bit float value, defines the pointer speed. Value range -1, 1 .TP 7 +.BI "libinput Accel Curve Points" +n 32-bit float values, n is a multiple of 2, each pair represents one point +on the acceleration curve. If the acceleration profile is +\fBdevice-speed-curve\fR, each value x is the device speed in units/ms and each +value x+1 is the acceleration factor to apply. This property should o
Re: [PATCH xserver] meson: Ensure we always build Xext/hashtable.c for glx
On Wed, 2018-04-18 at 18:09 -0400, Lyude Paul wrote: > Seems that while glxvnd relies on some of the hashtable functions in > Xext, we only build hashtable support for Xext if we're also building > the res extension. This leads to some errors if you try to build glx > without res enabled: > > glx/liblibglxvnd.a(vndcmds.c.o): In function `LookupVendorPrivDispatch': > /home/lyudess/Projects/xserver/glx/vndcmds.c:65: undefined reference to > `ht_find' > /home/lyudess/Projects/xserver/glx/vndcmds.c:67: undefined reference to > `ht_add' > glx/liblibglxvnd.a(vndcmds.c.o): In function `GlxDispatchInit': > /home/lyudess/Projects/xserver/glx/vndcmds.c:405: undefined reference to > `ht_generic_compare' > /home/lyudess/Projects/xserver/glx/vndcmds.c:405: undefined reference to > `ht_generic_hash' > /home/lyudess/Projects/xserver/glx/vndcmds.c:405: undefined reference to > `ht_create' > glx/liblibglxvnd.a(vndcmds.c.o): In function `GlxDispatchReset': > /home/lyudess/Projects/xserver/glx/vndcmds.c:468: undefined reference to > `ht_destroy' > collect2: error: ld returned 1 exit status > ninja: build stopped: subcommand failed. > > So, make sure that hashtable.c gets both for both glx and res > > Signed-off-by: Lyude Paul Merged these two, thanks: remote: I: patch #217776 updated using rev 4e28a6a223c4f9d0f5defe0313a94e22e0416787. remote: I: patch #217775 updated using rev fe4d1876b4f01c0b0e1916d548c398789f196164. remote: I: 2 patch(es) updated to state Accepted. To ssh://git.freedesktop.org/git/xorg/xserver 3b4671f9e9..fe4d1876b4 master -> master - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver v2] xwayland: avoid using freed xwl_window on unrealize
On Thu, 2018-04-19 at 17:33 +0200, Olivier Fourdan wrote: > Hi, > > On Thu, Apr 19, 2018 at 5:05 PM, Olivier Fourdan wrote: > > Looks like we're not done yet :/ > > > > I'll try that other patch I sent and see if that makes a difference here, > > could be related according to the backtrace I guess... > > > > So yes, https://patchwork.freedesktop.org/patch/217833/ prevents the crash. Merged, thanks: remote: I: patch #217833 updated using rev 3b4671f9e9c85f23e7593652e1482b11dc3ad4af. remote: I: 1 patch(es) updated to state Accepted. To ssh://git.freedesktop.org/git/xorg/xserver 8b8f9007cc..3b4671f9e9 master -> master - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver v2] xwayland: avoid using freed xwl_window on unrealize
Hi, On Thu, Apr 19, 2018 at 5:05 PM, Olivier Fourdan wrote: > Looks like we're not done yet :/ > > I'll try that other patch I sent and see if that makes a difference here, > could be related according to the backtrace I guess... > So yes, https://patchwork.freedesktop.org/patch/217833/ prevents the crash. I found skypeforlinux to be a excellent test for these issues, like moving the pointer pover the toplevel menu bar while a menu is opened to map/unmap the menus quickly would have led to a crash previously. Not anymore, with https://patchwork.freedesktop.org/patch/217833/ added. But there are still oddities, like some window remaining black for like 2~3 seconds after re-mapping. Closing the skypeforlinux toplevel and restoring it by running it again triggers the black window for a few second issue (skype doesn't really quit, it remains running, until it's recalled again, so this is the same X11 client) Cheers, Olivier ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver v2] xwayland: avoid using freed xwl_window on unrealize
Hi,
On 19 April 2018 at 16:05, Olivier Fourdan wrote:
> (gdb) f 9
> #9 xwl_present_frame_callback (data=0x184cb10, callback=,
> time=) at xwayland-present.c:192
> 192wl_callback_destroy(xwl_window->present_frame_callback);
> (gdb) list
> 187 struct wl_callback *callback,
> 188 uint32_t time)
> 189{
> 190struct xwl_window *xwl_window = data;
> 191
> 192wl_callback_destroy(xwl_window->present_frame_callback);
> 193xwl_window->present_frame_callback = NULL;
> 194
> 195if (xwl_window->present_timer_firing) {
> 196/* If the timer is firing, this frame callback is too late */
>
> Looks like we're not done yet :/
>
> I'll try that other patch I sent and see if that makes a difference here,
> could be related according to the backtrace I guess...
The callback is passed as an argument to the function directly, so you
could just destroy that instead.
Cheers,
Daniel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver v2] xwayland: avoid using freed xwl_window on unrealize
Hi,
On Thu, Apr 19, 2018 at 9:15 AM, Olivier Fourdan wrote:
> Sure, I've sent a new patch to do that, but I reckon this can wait after
> the release, we might leak a frame callback in some cases (although
> valgrind did not really complain yet), but we won't crash anymore.
>
Sorry I kinda take that back, I just had a crash related to
xwayland-present again:
gdb) bt
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1 0x7f023c612591 in __GI_abort () at abort.c:79
#2 0x0058d850 in OsAbort () at utils.c:1350
#3 0x00592ac9 in AbortServer () at log.c:877
#4 0x0059393d in FatalError (f=f@entry=0x5b86b0 "Caught signal %d
(%s). Server aborting\n") at log.c:1015
#5 0x0058aba5 in OsSigHandler (signo=11, sip=,
unused=) at osinit.c:156
#6
#7 0x7f023eac1b55 in wl_proxy_destroy (proxy=0x31) at
src/wayland-client.c:530
#8 0x00434774 in wl_callback_destroy (wl_callback=)
at /usr/include/wayland-client-protocol.h:1154
#9 xwl_present_frame_callback (data=0x184cb10, callback=,
time=) at xwayland-present.c:192
#10 0x7f023bf5603e in ffi_call_unix64 () at ../src/x86/unix64.S:76
#11 0x7f023bf559ff in ffi_call () at ../src/x86/ffi64.c:525
#12 0x7f023eac52dd in wl_closure_invoke (closure=0x18837d0, flags=1,
target=, opcode=0,
data=) at src/connection.c:996
#13 0x7f023eac1a39 in dispatch_event (display=display@entry=0x980010,
queue=) at src/wayland-client.c:1434
#14 0x7f023eac2f5c in dispatch_queue (queue=0x9800d8, display=0x980010)
at src/wayland-client.c:1580
#15 wl_display_dispatch_queue_pending (display=0x980010, queue=0x9800d8) at
src/wayland-client.c:1822
#16 0x7f023eac2fc0 in wl_display_dispatch_pending (display=) at src/wayland-client.c:1885
#17 0x0042a83b in xwl_read_events (xwl_screen=0x977c20) at
xwayland.c:800
#18 0x0058b591 in ospoll_wait (ospoll=0x96d0d0, timeout=) at ospoll.c:651
#19 0x00584e2c in WaitForSomething (are_ready=) at
WaitFor.c:208
#20 0x00554970 in Dispatch () at ../include/list.h:220
#21 0x00558c26 in dix_main (argc=12, argv=0x7ffce44bebc8,
envp=) at main.c:276
---Type to continue, or q to quit---
#22 0x7f023c6141bb in __libc_start_main (main=0x429f30 , argc=12,
argv=0x7ffce44bebc8, init=,
fini=, rtld_fini=,
stack_end=0x7ffce44bebb8) at ../csu/libc-start.c:308
#23 0x00429f6a in _start ()
(gdb) f 9
#9 xwl_present_frame_callback (data=0x184cb10, callback=,
time=) at xwayland-present.c:192
192wl_callback_destroy(xwl_window->present_frame_callback);
(gdb) list
187 struct wl_callback *callback,
188 uint32_t time)
189{
190struct xwl_window *xwl_window = data;
191
192wl_callback_destroy(xwl_window->present_frame_callback);
193xwl_window->present_frame_callback = NULL;
194
195if (xwl_window->present_timer_firing) {
196/* If the timer is firing, this frame callback is too late */
Looks like we're not done yet :/
I'll try that other patch I sent and see if that makes a difference here,
could be related according to the backtrace I guess...
Cheers,
Olivier
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] xwayland: Clean up all frame callbacks
Reviewed-by: Roman Gilg
On Thu, Apr 19, 2018 at 9:13 AM, Olivier Fourdan wrote:
> Regardless of the order we un-realize windows.
>
> Suggested-by: Roman Gilg
> Signed-off-by: Olivier Fourdan
> ---
> hw/xwayland/xwayland-present.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
> index e835a1399..c41a8a2d1 100644
> --- a/hw/xwayland/xwayland-present.c
> +++ b/hw/xwayland/xwayland-present.c
> @@ -77,7 +77,7 @@ xwl_present_cleanup(struct xwl_window *xwl_window,
> WindowPtr window)
> {
> struct xwl_present_event *event, *tmp;
>
> -if (xwl_window->present_window == window) {
> +if (xwl_window->present_window == window || xwl_window->window ==
> window) {
> if (xwl_window->present_frame_callback) {
> wl_callback_destroy(xwl_window->present_frame_callback);
> xwl_window->present_frame_callback = NULL;
> --
> 2.17.0
>
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver v2] xwayland: avoid using freed xwl_window on unrealize
Hi Roman, On 18 April 2018 at 20:34, Roman Gilg wrote: > > I believe this check should be ORed with xwl_window->window == window. > Otherwise in case the top parent window is unrealized first, > xwl_window->present_window != window holds here and afterwards > xwl_present_cleanup won't be called again since the xwl_window is > already gone. I.e. an existing frame callback wouldn't be destroyed in > this case. > Sure, I've sent a new patch to do that, but I reckon this can wait after the release, we might leak a frame callback in some cases (although valgrind did not really complain yet), but we won't crash anymore. Cheers, Olivier ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver] xwayland: Clean up all frame callbacks
Regardless of the order we un-realize windows.
Suggested-by: Roman Gilg
Signed-off-by: Olivier Fourdan
---
hw/xwayland/xwayland-present.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
index e835a1399..c41a8a2d1 100644
--- a/hw/xwayland/xwayland-present.c
+++ b/hw/xwayland/xwayland-present.c
@@ -77,7 +77,7 @@ xwl_present_cleanup(struct xwl_window *xwl_window, WindowPtr
window)
{
struct xwl_present_event *event, *tmp;
-if (xwl_window->present_window == window) {
+if (xwl_window->present_window == window || xwl_window->window == window) {
if (xwl_window->present_frame_callback) {
wl_callback_destroy(xwl_window->present_frame_callback);
xwl_window->present_frame_callback = NULL;
--
2.17.0
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
