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

crash on swaymsg command #4239

Closed
Tormen opened this issue Jun 9, 2019 · 5 comments · Fixed by #4246
Closed

crash on swaymsg command #4239

Tormen opened this issue Jun 9, 2019 · 5 comments · Fixed by #4246
Labels
bug Not working as intended

Comments

@Tormen
Copy link

Tormen commented Jun 9, 2019

  • Sway Version: sway version 1.1-rc1-34-g5b1a8d62 (Jun 9 2019, branch 'master')

  • Debug Log: of ./build/sway/sway -d -c /etc/sway/config &>/tmp/sway.log: http://sprunge.us/KcmhqR

  • Default Config File (untouched): /etc/sway/config http://sprunge.us/jsajhn

  • CUSTOM Config File: /etc/sway/config.d/99-custom.mine: http://sprunge.us/YxWBgg

  • Steps to reproduce:
    1.) start sway as user me with standard config + above 99-custom.mine config: with ./build/sway/sway -d -c /etc/sway/config
    2.) start sakura with app_id my-shell using custom shortcut $mod+shift+return
    3.) switch to empty/fresh workspace with default shortcut $mod+2
    4.) try to get sakura shell over using custom shortcut $mod+shift+s

  • Stack Trace, if sway crashes:
    coredumpctl gdb sway with bt full: http://sprunge.us/HIv9ZV

@Tormen
Copy link
Author

Tormen commented Jun 9, 2019

Now with meson build -Db_sanitize=address:

  • Sway Version: sway version 1.1-rc1-34-g5b1a8d62 (Jun 9 2019, branch 'master')

  • Debug Log: of ./build/sway/sway -d -c /etc/sway/config &>/tmp/sway.log: http://sprunge.us/t9jPLp

  • Default Config File (untouched): /etc/sway/config http://sprunge.us/jsajhn

  • CUSTOM Config File: /etc/sway/config.d/99-custom.mine: http://sprunge.us/YxWBgg

  • Steps to reproduce:
    1.) start sway as user me with standard config + above 99-custom.mine config: with ./build/sway/sway -d -c /etc/sway/config
    2.) start sakura with app_id my-shell using custom shortcut $mod+shift+return
    3.) switch to empty/fresh workspace with default shortcut $mod+2
    4.) try to get sakura shell over using custom shortcut $mod+shift+s

  • Stack Trace, if sway crashes:
    coredumpctl gdb sway with bt full: http://sprunge.us/tVqrHX

@emersion
Copy link
Member

emersion commented Jun 9, 2019

This is helpful:

==6165==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f5a6394393a bp 0x7ffeab55ac50 sp 0x7ffeab55a3b0 T0)
==6165==The signal is caused by a READ memory access.
==6165==Hint: address points to the zero page.
    #0 0x7f5a63943939 in __interceptor_strcmp /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:369
    #1 0x55bf31596533 in cmd_focus ../sway/commands/focus.c:328
    #2 0x55bf31517814 in execute_command ../sway/commands.c:293
    #3 0x55bf31531140 in ipc_client_handle_command ../sway/ipc-server.c:619
    #4 0x55bf3152f11d in ipc_client_handle_readable ../sway/ipc-server.c:269
    #5 0x7f5a632ac7f1 in wl_event_loop_dispatch (/usr/lib/libwayland-server.so.0+0xa7f1)
    #6 0x7f5a632ab39b in wl_display_run (/usr/lib/libwayland-server.so.0+0x939b)
    #7 0x55bf31536b26 in server_run ../sway/server.c:202
    #8 0x55bf31535373 in main ../sway/main.c:397
    #9 0x7f5a62fd7ce2 in __libc_start_main (/usr/lib/libc.so.6+0x23ce2)
    #10 0x55bf3151612d in _start (/data/appData/archlinux/sway-git/src/sway/build/sway/sway+0x3912d)

@emersion emersion added the bug Not working as intended label Jun 9, 2019
@Tormen
Copy link
Author

Tormen commented Jun 10, 2019

FYI: I let memtest86-efi run: all tests with 4 passes: 0% errors

@mcoffin
Copy link
Contributor

mcoffin commented Jun 10, 2019

This is hopefully irrelevant, but strcmp is implemented as an ifunc. It appears the resolution is working fine, but I got to learn something about function resolution today!

It's pretty interesting that it is implemented how it is, since there are runtime implications (albeit minimal) to using that to have multiple implementations selected at runtime.

I've been investigating, but I can't reproduce this yet.

EDIT: This has nothing to do with it. The block before that code checks for argc == 0 && container, and in his example, container is NULL, and argc is 0. I'll work on a fix

EDIT2: Even with the exact config above, I cannot reproduce.

EDIT3: I successfully reproduced and bisected. The bad commit is f0f5de9

EDIT4: Identified root cause in option string parsing code, and I'm working on a patch as we speak.

EDIT5: patch is working, but I need to clean it up a bit prior to submitting. This should work for you in the meantime. mcoffin/sway/command-parsing#4239

mcoffin added a commit to mcoffin/sway that referenced this issue Jun 11, 2019
This patch fixes faulty command parsing introduced by
f0f5de9. When that commit allowed
criteria reset on ';' delimeters in commands lists, it failed to account
for its inner ','-parsing loop eating threw the entire rest of the
string.

This patch refactors argsep to use a list of multiple separators, and
(optionally) return the separator that it matched against in this
iteration via a pointer. This allows it to hint at the command parser
which separator was used at the end of the last command, allowing it to
trigger a potential secondary read of the criteria.

Fixes swaywm#4239
mcoffin added a commit to mcoffin/sway that referenced this issue Jun 11, 2019
This patch fixes faulty command parsing introduced by
f0f5de9. When that commit allowed
criteria reset on ';' delimeters in commands lists, it failed to account
for its inner ','-parsing loop eating threw the entire rest of the
string.

This patch refactors argsep to use a list of multiple separators, and
(optionally) return the separator that it matched against in this
iteration via a pointer. This allows it to hint at the command parser
which separator was used at the end of the last command, allowing it to
trigger a potential secondary read of the criteria.

Fixes swaywm#4239
mcoffin added a commit to mcoffin/sway that referenced this issue Jun 11, 2019
This patch fixes faulty command parsing introduced by
f0f5de9. When that commit allowed
criteria reset on ';' delimeters in commands lists, it failed to account
for its inner ','-parsing loop eating threw the entire rest of the
string.

This patch refactors argsep to use a list of multiple separators, and
(optionally) return the separator that it matched against in this
iteration via a pointer. This allows it to hint at the command parser
which separator was used at the end of the last command, allowing it to
trigger a potential secondary read of the criteria.

Fixes swaywm#4239
mcoffin added a commit to mcoffin/sway that referenced this issue Jun 11, 2019
This patch fixes faulty command parsing introduced by
f0f5de9. When that commit allowed
criteria reset on ';' delimeters in commands lists, it failed to account
for its inner ','-parsing loop eating threw the entire rest of the
string.

This patch refactors argsep to use a list of multiple separators, and
(optionally) return the separator that it matched against in this
iteration via a pointer. This allows it to hint at the command parser
which separator was used at the end of the last command, allowing it to
trigger a potential secondary read of the criteria.

Fixes swaywm#4239
mcoffin added a commit to mcoffin/sway that referenced this issue Jun 11, 2019
This patch fixes faulty command parsing introduced by
f0f5de9. When that commit allowed
criteria reset on ';' delimeters in commands lists, it failed to account
for its inner ','-parsing loop eating threw the entire rest of the
string.

This patch refactors argsep to use a list of multiple separators, and
(optionally) return the separator that it matched against in this
iteration via a pointer. This allows it to hint at the command parser
which separator was used at the end of the last command, allowing it to
trigger a potential secondary read of the criteria.

Fixes swaywm#4239
RedSoxFan pushed a commit that referenced this issue Jun 11, 2019
This patch fixes faulty command parsing introduced by
f0f5de9. When that commit allowed
criteria reset on ';' delimeters in commands lists, it failed to account
for its inner ','-parsing loop eating threw the entire rest of the
string.

This patch refactors argsep to use a list of multiple separators, and
(optionally) return the separator that it matched against in this
iteration via a pointer. This allows it to hint at the command parser
which separator was used at the end of the last command, allowing it to
trigger a potential secondary read of the criteria.

Fixes #4239
@Tormen
Copy link
Author

Tormen commented Jun 15, 2019

Thanks a lot @emersion , @mcoffin , @RedSoxFan !!!

lolzballs pushed a commit to lolzballs/sway that referenced this issue Dec 5, 2019
This patch fixes faulty command parsing introduced by
f0f5de9. When that commit allowed
criteria reset on ';' delimeters in commands lists, it failed to account
for its inner ','-parsing loop eating threw the entire rest of the
string.

This patch refactors argsep to use a list of multiple separators, and
(optionally) return the separator that it matched against in this
iteration via a pointer. This allows it to hint at the command parser
which separator was used at the end of the last command, allowing it to
trigger a potential secondary read of the criteria.

Fixes swaywm#4239
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Not working as intended
Development

Successfully merging a pull request may close this issue.

3 participants