Possible "index out of range" in `equalMarks`

[Antonboom]

Hello!

I'm interested in carefree iteration over lists whose lengths can vary:

// equalMarks compares two error markers.
func equalMarks(m1, m2 errorMark) bool {
    if m1.msg != m2.msg {
        return false
    }
    for i, t := range m1.types {
        if !t.Equals(m2.types[i]) {
            return false
        }
    }
    return true
}

And I made an example that breaks this code:

package main

import (
    "fmt"
    "github.com/cockroachdb/errors"
)

type SimpleWrapper struct {
    err error
}

func (w SimpleWrapper) Error() string {
    return "boom!"
}

func (w SimpleWrapper) Unwrap() error {
    return w.err
}

func main() {
    stack := errors.WithStack

    ref := stack(stack(SimpleWrapper{}))
    err := stack(stack(SimpleWrapper{err: stack(errors.New("boom!"))}))

    if errors.IsAny(err, ref) {
        fmt.Println("gotcha!")
    }

    /* panic: runtime error: index out of range [3] with length 3

    goroutine 1 [running]:
    github.com/cockroachdb/errors/markers.equalMarks(...)
            github.com/cockroachdb/errors@v1.9.0/markers/markers.go:205
    github.com/cockroachdb/errors/markers.IsAny({0x102802528, 0x1400000e438}, {0x14000167f48, 0x1, 0x14000167f28?})
            github.com/cockroachdb/errors@v1.9.0/markers/markers.go:186 +0x364
    github.com/cockroachdb/errors.IsAny(...)
            github.com/cockroachdb/errors@v1.9.0/markers_api.go:64
    main.main()
            examples/04-non-standard-modules/cockroach-is-any-bug/main.go:26 +0x318
    */
}

Where am I wrong?

[knz]

Hi, I believe part of the problem is that the ref error has an incomplete SimpleWrapper instance ( its err field is nil), which will mess up the type mark. Does the error persist if you add a proper err to your SimpleWrapper reference?

In any case, I agree the ergonomics are not ideal, so perhaps there's something to improve here.

[Antonboom]

Does the error persist if you add a proper err to your SimpleWrapper reference?

No, now you won't break it quickly :)

You can close the issue, I don't mind, just expressed doubt. Maybe add an explicit check for the lengths of ref lists?