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 when copying selection that runs to end of window #1

Closed
phinze opened this issue Aug 6, 2022 · 4 comments
Closed

Crash when copying selection that runs to end of window #1

phinze opened this issue Aug 6, 2022 · 4 comments

Comments

@phinze
Copy link
Collaborator

phinze commented Aug 6, 2022

πŸ‘‹ πŸ˜„ Hi! Nice terminal emulator you've got here. ✨ I've broken it for you. πŸ™ƒ

Steps to reproduce (on darwin/arm64 but I'm betting it happens everywhere):

  • Make a selection that runs to the end of the window
  • Try to copy w/ CMD+C

Expect:

  • Copied text

Observe:


Stepping around with a debugger, it looks like this assertion is failing, because sel.x actually does have a value larger than the number of columns in the window.

I'm only just getting the hang of the codebase so I'm not sure if this is a bug in the selection capture code or just a quirk that we need to handle by "truncating" the selection to the end of the window.

@mitchellh
Copy link
Contributor

While trying to reproduce this I figured out another crash, its odd, digging into it now:

  1. cat src/Window.zig (any file that causes the screen to scroll)
  2. Select any of the TRAILING blank space
  3. Copy

I added some logging and the char its getting is a 2863311530 which is 0xAAAAAAAA which is what Zig sets as the undefined value for debug builds so... somewhere we're setting char to undefined. Bad! This is a diff bug since its not the assertion you're seeing tripped, but I'm going to look at this first...

@mitchellh
Copy link
Contributor

mitchellh commented Aug 6, 2022

Fixed my issue, now looking into yours: 9d26ab2 (Note: this case should be tested, but we're going to have to redo all the resize stuff for reflow and I'm going to get to that Soon(tm) so... ignoring)

@mitchellh
Copy link
Contributor

Reproduced, here is the stack trace:

gdb $ bt
#0  0x0000fffff7e038d8 in __pthread_kill_implementation () from /nix/store/6afvzgx3himw7584k9zmfxmqd86b1z7j-glibc-2.34-210/lib/libc.so.6
#1  0x0000fffff7dc12b4 in raise () from /nix/store/6afvzgx3himw7584k9zmfxmqd86b1z7j-glibc-2.34-210/lib/libc.so.6
#2  0x0000fffff7daefd8 in abort () from /nix/store/6afvzgx3himw7584k9zmfxmqd86b1z7j-glibc-2.34-210/lib/libc.so.6
#3  0x0000000000447420 in std.os.abort () at /nix/store/a44c1nv4qqwqshc5li4gk8nwd99lgr4r-zig-0.10.0-dev.3445+18440cb23/lib/zig/std/os.zig:505
#4  0x0000000000444a9c in std.debug.panicImpl (trace=0x0, first_trace_addr=..., msg=...) at /nix/store/a44c1nv4qqwqshc5li4gk8nwd99lgr4r-zig-0.10.0-dev.3445+18440cb23/lib/zig/std/debug.zig:386
#5  0x0000000000441ddc in std.builtin.default_panic (msg=<error reading variable: Cannot access memory at address 0x6>, error_return_trace=0x0) at /nix/store/a44c1nv4qqwqshc5li4gk8nwd99lgr4r-zig-0.10.0-dev.3445+18440cb23/lib/zig/std/builtin.zig:844
#6  0x0000000000442690 in std.debug.assert (ok=false) at /nix/store/a44c1nv4qqwqshc5li4gk8nwd99lgr4r-zig-0.10.0-dev.3445+18440cb23/lib/zig/std/debug.zig:281
#7  0x000000000049027c in terminal.Screen.selectionSlices (self=..., sel=...) at /home/mitchellh/code/go/src/github.com/mitchellh/ghostty/src/terminal/Screen.zig:493
#8  0x000000000046d6d8 in terminal.Screen.selectionString (self=<error reading variable: Cannot access memory at address 0x0>, alloc=<error reading variable: Cannot access memory at address 0x6>, sel=<error reading variable: Cannot access memory at address 0x159894>)
    at /home/mitchellh/code/go/src/github.com/mitchellh/ghostty/src/terminal/Screen.zig:408
#9  0x000000000046ea50 in Window.keyCallback (window=..., key=c, scancode=54, action=press, mods=...) at Window.zig:462
#10 .glfw.Window.CWrapper.keyCallbackWrapper (handle=0xb4dc70, key=67, scancode=54, action=1, mods=8) at /home/mitchellh/code/go/src/github.com/mitchellh/ghostty/vendor/mach/glfw/src/Window.zig:2112
#11 0x0000000000512294 in _glfwInputKey (window=0xb4dc70, key=67, scancode=54, action=1, mods=8) at vendor/mach/glfw/upstream/glfw/src/input.c:305
#12 0x000000000057dbb8 in processEvent (event=0xffffffff76e8) at vendor/mach/glfw/upstream/glfw/src/x11_window.c:1206
#13 0x0000000000563bd0 in _glfwPollEventsX11 () at vendor/mach/glfw/upstream/glfw/src/x11_window.c:2735
#14 0x0000000000563f44 in _glfwWaitEventsX11 () at vendor/mach/glfw/upstream/glfw/src/x11_window.c:2759
#15 0x000000000053ed6c in glfwWaitEvents () at vendor/mach/glfw/upstream/glfw/src/window.c:1119
#16 0x00000000004467f8 in .glfw.waitEvents () at /home/mitchellh/code/go/src/github.com/mitchellh/ghostty/vendor/mach/glfw/src/main.zig:414
#17 App.run (self=...) at App.zig:95
#18 0x000000000044545c in main () at main.zig:81
#19 0x0000000000446e1c in std.start.callMain () at /nix/store/a44c1nv4qqwqshc5li4gk8nwd99lgr4r-zig-0.10.0-dev.3445+18440cb23/lib/zig/std/start.zig:576
#20 std.start.initEventLoopAndCallMain () at /nix/store/a44c1nv4qqwqshc5li4gk8nwd99lgr4r-zig-0.10.0-dev.3445+18440cb23/lib/zig/std/start.zig:510
#21 std.start.callMainWithArgs (argc=5, argv=0xffffffffa4b8, envp=...) at /nix/store/a44c1nv4qqwqshc5li4gk8nwd99lgr4r-zig-0.10.0-dev.3445+18440cb23/lib/zig/std/start.zig:460
#22 std.start.main (c_argc=5, c_argv=0xffffffffa4b8, c_envp=0xffffffffa4e8) at /nix/store/a44c1nv4qqwqshc5li4gk8nwd99lgr4r-zig-0.10.0-dev.3445+18440cb23/lib/zig/std/start.zig:475

@mitchellh
Copy link
Contributor

mitchellh commented Aug 6, 2022

Oh fun, I never thought about this but the mouse xpos ypos values can be NEGATIVE if you click and drag OFF the window (to the top/left) or greater than the width if you drag off right/bottom. Commit 1da5ca0 bounds this on both sides.

Praise assertions validating our assumptions and finding something that breaks em.

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

2 participants