BurntSushi / xgb

The X Go Binding is a low-level API to communicate with the X server. It is modeled on XCB and supports many X extensions.
Other
489 stars 75 forks source link

Panic on sync.SystemcounterReadList #11

Open syndtr opened 11 years ago

syndtr commented 11 years ago
    panic: runtime error: slice bounds out of range

    goroutine 1 [running]:
    runtime.panic(0x557480, 0xc21000a2c0)
            /home/syndtr/go/src/pkg/runtime/panic.c:266 +0xb6
    github.com/BurntSushi/xgb/sync.SystemcounterReadList(0xc210057420, 0x350, 0x350, 0xc210057800, 0x1b, ...)
            /home/syndtr/gocode/src/github.com/BurntSushi/xgb/sync/sync.go:526 +0x148
    github.com/BurntSushi/xgb/sync.listSystemCountersReply(0xc210057400, 0x370, 0x370, 0xc2100512a0)
            /home/syndtr/gocode/src/github.com/BurntSushi/xgb/sync/sync.go:1586 +0x26d
    github.com/BurntSushi/xgb/sync.ListSystemCountersCookie.Reply(0xc21001d570, 0xc21001d570, 0x0, 0xc21004d440)
            /home/syndtr/gocode/src/github.com/BurntSushi/xgb/sync/sync.go:1564 +0xae
    main.main()
            /home/syndtr/playground/zetapmd/zpmx.go:25 +0x207

This seems to fix it:

--- a/sync/sync.go
+++ b/sync/sync.go
@@ -512,10 +512,10 @@ func SystemcounterRead(buf []byte, v *Systemcounter) int {
                byteString := make([]byte, v.NameLen)
                copy(byteString[:v.NameLen], buf[b:])
                v.Name = string(byteString)
-               b += xgb.Pad(int(v.NameLen))
+               b += int(v.NameLen)
        }

-       return b
+       return xgb.Pad(b)
 }

 // SystemcounterReadList reads a byte slice into a list of Systemcounter values.
BurntSushi commented 11 years ago

Could you please confirm that you're using the latest commit? My guess is that this bug was fixed in #5 (commit eb7c38953b074e33f86861a3da4c05623cd44fc6).

This seems to fix it:

Sadly, that is not a fix. sync/sync.go (along with almost everything else) is automatically generated by xgb/xgbgen. The only code not automatically generated is xgbgen itself and the base xgb module.

BurntSushi commented 11 years ago

If you are on the latest commit and still getting this panic, could you please produce a code sample (preferably minimal) that provokes this error?

syndtr commented 11 years ago

I'm on the latest commit.

Here you go:

package main

import (
    "github.com/BurntSushi/xgb"
    "github.com/BurntSushi/xgb/sync"
    "log"
)

func main() {
    X, err := xgb.NewConn()
    if err != nil {
        log.Fatal(err)
    }
    if err := sync.Init(X); err != nil {
        log.Fatal(err)
    }
    _, err = sync.ListSystemCounters(X).Reply()
    if err != nil {
        log.Fatal(err)
    }
}
BurntSushi commented 10 years ago

While the fix presented here will work, the problem is that the code being patched is auto-generated. The bug should be fixed in the generator.

However, in the protocol specification for the SYSTEMCOUNTER type, the padding is computed on the length of the name and the name itself, combined. But the XML description doesn't specify this. So I'm not sure how to discover this fact. I think we'd need to ask the XCB folks (or dig into their generator).

BurntSushi commented 10 years ago

Note that the sync extension has been removed from XGB until the generator supports the switch XML field. (A use of the switch field has been added to the XML recently.)