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

FYI: gocui triggers a bug in Go 1.13 (pre-release) #33

Closed
josharian opened this issue Aug 26, 2019 · 2 comments · Fixed by #36
Closed

FYI: gocui triggers a bug in Go 1.13 (pre-release) #33

josharian opened this issue Aug 26, 2019 · 2 comments · Fixed by #36
Labels
bug Something isn't working

Comments

@josharian
Copy link

This is a heads up that gocui appears to trigger a bug in Go 1.13 (at least rc1), so if folks come to this repo and complain about crashes, you'll know what's happening.

The issue is reported upstream at golang/go#33841.

Feel free to close this issue any time once you've seen it.

@josharian josharian added the bug Something isn't working label Aug 26, 2019
@mjarkk
Copy link
Member

mjarkk commented Aug 26, 2019

Thanks for taking the time making the issue, we will take note of that.

I’ll leaf this issue open till the bug is fixed unless others think this can be closed.

@randall77
Copy link

I think this is a bug in gocui. In gui.go, you do:

var sz struct {
		rows uint16
                cols uint16
        }
        ...
            	_, _, _ = syscall.Syscall(syscall.SYS_IOCTL,
			out.Fd(), uintptr(syscall.TIOCGWINSZ), uintptr(unsafe.Pointer(&sz)))

But the manpage for ioctl says the structure should be:

 struct winsize {
               unsigned short ws_row;
               unsigned short ws_col;
               unsigned short ws_xpixel;   /* unused */
               unsigned short ws_ypixel;   /* unused */
           };

The kernel is actually writing to those unused fields, which clobbers a random stack slot next to the sz variable. Adding 4 bytes of padding to the sz struct will fix the issue.

The stack layout of the getTermWindowSize method changed from 1.12 to 1.13, which is why we only see the issue in 1.13 (the overwritten data in 1.12 probably wasn't critical - in 1.13 it is).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants