search

awesome-gocui / gocui

Minimalist Go package aimed at creating Console User Interfaces.
BSD 3-Clause "New" or "Revised" License
350 stars 39 forks source link

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

Closed josharian closed 5 years ago

josharian commented 5 years ago

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 https://github.com/golang/go/issues/33841.

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

mjarkk commented 5 years ago

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 commented 5 years ago

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).