Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update vkcube to wayland xdg-shell #469

Closed
cdotstout opened this issue Nov 30, 2020 · 4 comments
Closed

Update vkcube to wayland xdg-shell #469

cdotstout opened this issue Nov 30, 2020 · 4 comments
Assignees

Comments

@cdotstout
Copy link
Contributor

In the following commit the change was reverted. Are there any details of the weston incompatibility or any ticket tracking the steps to reproduce? It would be nice to move off the deprecated protocol.

5ceb7be#diff-4f3c4172a4fe308cebc840da665171f534849b1fca101eaf635d68fb453393db

@jbeich
Copy link
Contributor

jbeich commented May 14, 2021

CC @mstoeckl. Maybe related to the following crash on weston, kwin_wayland, wayfire (wlroots), hikari (wlroots), labwc (wlroots) despite working fine on sway (wlroots), cage (wlroots):

$ vkcube-wayland
Selected GPU 0: Intel(R) HD Graphics 530 (SKL GT2), type: 1
=================================================================
==7240==ERROR: AddressSanitizer: heap-use-after-free on address 0x612000156d00 at pc 0x0000002ca714 bp 0x7fffffffc310 sp 0x7fffffffc308
READ of size 8 at 0x612000156d00 thread T0
    #0 0x2ca713 in demo_update_data_buffer Vulkan-Tools-1.2.178/cube/cube.c:894:66
    #1 0x2f5270 in demo_draw Vulkan-Tools-1.2.178/cube/cube.c:1043:5
    #2 0x2d461c in demo_run Vulkan-Tools-1.2.178/cube/cube.c:2757:13
    #3 0x2cec4d in main Vulkan-Tools-1.2.178/cube/cube.c:4267:5

0x612000156d00 is located 192 bytes inside of 288-byte region [0x612000156c40,0x612000156d60)
freed by thread T0 here:
    #0 0x29b2e2 in free (vkcube-wayland+0x29b2e2)
    #1 0x2e08a5 in demo_resize Vulkan-Tools-1.2.178/cube/cube.c:2447:5
    #2 0x2df646 in handle_surface_configure Vulkan-Tools-1.2.178/cube/cube.c:2768:9
    #3 0x8009730dc in ffi_call_unix64 libffi-3.3/src/x86/unix64.S:101

previously allocated by thread T0 here:
    #0 0x29b42d in malloc (vkcube-wayland+0x29b42d)
    #1 0x2e483b in demo_prepare_buffers Vulkan-Tools-1.2.178/cube/cube.c:1363:36
    #2 0x2d37a0 in demo_prepare Vulkan-Tools-1.2.178/cube/cube.c:2234:5
    #3 0x2cec44 in main Vulkan-Tools-1.2.178/cube/cube.c:4260:5
    #4 0x24123f in _start freebsd/lib/csu/amd64/crt1_c.c:75:7
    #5 0x80031f007  (<unknown module>)

SUMMARY: AddressSanitizer: heap-use-after-free Vulkan-Tools-1.2.178/cube/cube.c:894:66 in demo_update_data_buffer
Shadow bytes around the buggy address:
  0x4c240002ad50: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x4c240002ad60: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x4c240002ad70: fd fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa
  0x4c240002ad80: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x4c240002ad90: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x4c240002ada0:[fd]fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa
  0x4c240002adb0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x4c240002adc0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x4c240002add0: fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa
  0x4c240002ade0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x4c240002adf0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==7240==ABORTING

@mstoeckl
Copy link
Contributor

mstoeckl commented May 14, 2021

OK, I can reproduce this. The following patch seems to fix the issue on Weston + kwin, although more testing is needed to be sure, and I still don't know exactly how the crash indicated above occurs. While this patch improves things, there may be further bugs in the window sizing logic.

diff --git a/cube/cube.c b/cube/cube.c
index a503172a..28b45a63 100644
--- a/cube/cube.c
+++ b/cube/cube.c
@@ -2775,8 +2775,14 @@ static const struct xdg_surface_listener xdg_surface_listener = {handle_surface_
 static void handle_toplevel_configure(void *data, struct xdg_toplevel *xdg_toplevel UNUSED, int32_t width, int32_t height,
                                       struct wl_array *states UNUSED) {
     struct demo *demo = (struct demo *)data;
-    demo->width = width;
-    demo->height = height;
+    /* zero values imply the program may choose its own size, so in that case
+     * stay with the existing value (which on startup is the default) */
+    if (width > 0) {
+        demo->width = width;
+    }
+    if (height > 0) {
+        demo->height = height;
+    }
     /* This should be followed by a surface configure */
 }
 
diff --git a/cube/cube.cpp b/cube/cube.cpp
index e3bed785..44bd35fa 100644
--- a/cube/cube.cpp
+++ b/cube/cube.cpp
@@ -2846,8 +2846,14 @@ static const xdg_surface_listener surface_listener = {handle_surface_configure};
 static void handle_toplevel_configure(void *data, xdg_toplevel *xdg_toplevel, int32_t width, int32_t height,
                                       struct wl_array *states) {
     Demo *demo = (Demo *)data;
-    demo->width = width;
-    demo->height = height;
+    /* zero values imply the program may choose its own size, so in that case
+     * stay with the existing value (which on startup is the default) */
+    if (width > 0) {
+        demo->width = width;
+    }
+    if (height > 0) {
+        demo->height = height;
+    }
     // This will be followed by a surface configure
 }

@jbeich
Copy link
Contributor

jbeich commented May 15, 2021

@mstoeckl, with your fix I can't reproduce the crash anymore. Tested weston, kwin_wayland, wayfire, hikari, labwc and applied downstream.

@TonyBarbour
Copy link
Contributor

Closed with #527

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants