Go-tools: SA9002: panic: runtime error: index out of range [2] with length 1

Created on 11 Feb 2021  路  4Comments  路  Source: dominikh/go-tools

$ staticcheck -version
staticcheck (no version)
$ staticcheck -debug.version
staticcheck (no version)

Compiled with Go version: go1.15.8
Main module:
        honnef.co/go/tools
Dependencies:
        github.com/BurntSushi/[email protected] (sum: h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=)
        golang.org/x/[email protected] (sum: h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=)
        golang.org/x/[email protected] (sum: h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k=)
        golang.org/x/[email protected] (sum: h1:po9/4sTYwZU9lPhi1tOrb4hCv3qrhiQ77LZfGa2OjwY=)
        golang.org/x/[email protected] (sum: h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=)
$ go version 
go version go1.15.8 linux/amd64
$ staticcheck --checks="SA9002" .
panic: runtime error: index out of range [2] with length 1

goroutine 360 [running]:
honnef.co/go/tools/staticcheck.CheckNonOctalFileMode.func1(0x9df540, 0xc0020e7180)
        /home/ldez/sources/go/src/honnef.co/go/tools/staticcheck/lint.go:2928 +0x7b5
golang.org/x/tools/go/ast/inspector.(*Inspector).Preorder(0xc000ba2500, 0xc000318dd8, 0x1, 0x1, 0xc000064de8)
        /home/ldez/sources/go/pkg/mod/golang.org/x/[email protected]/go/ast/inspector/inspector.go:77 +0xa2
honnef.co/go/tools/analysis/code.Preorder(...)
        /home/ldez/sources/go/src/honnef.co/go/tools/analysis/code/visit.go:16
honnef.co/go/tools/staticcheck.CheckNonOctalFileMode(0xc001658f70, 0x20996c8c, 0xc4b100, 0xc001658ee8, 0x0)
        /home/ldez/sources/go/src/honnef.co/go/tools/staticcheck/lint.go:2947 +0xcc
honnef.co/go/tools/lintcmd/runner.(*analyzerRunner).do(0xc001a7aa50, 0x9ea7e0, 0xc0004e5400, 0x0, 0x0)
        /home/ldez/sources/go/src/honnef.co/go/tools/lintcmd/runner/runner.go:938 +0x61e
honnef.co/go/tools/lintcmd/runner.genericHandle(0x9ea7e0, 0xc0004e5400, 0x9ea7e0, 0xc0004d8000, 0xc000c6b4a0, 0xc0002ca0b8, 0xc000d17120)
        /home/ldez/sources/go/src/honnef.co/go/tools/lintcmd/runner/runner.go:763 +0x18b
created by honnef.co/go/tools/lintcmd/runner.(*subrunner).runAnalyzers
        /home/ldez/sources/go/src/honnef.co/go/tools/lintcmd/runner/runner.go:1004 +0x5ca

The code is on https://github.com/phuslu/log

Related to https://github.com/golangci/golangci-lint/issues/1730 and https://github.com/phuslu/log/issues/28

bug

Most helpful comment

This bug report has resulted in the commits 2715787..b9870cd and the plan to move more checks away from AST-based checks to IR-based ones, which don't suffer from this issue (at least not now that 28022ce exists.)

Thanks for catching this gap in Staticcheck's understanding of the language.

All 4 comments

Can reproduce, looking into it.

With the following modifications, the panic disappears:

diff --git i/file.go w/file.go
index fe01944..d147db4 100644
--- i/file.go
+++ w/file.go
@@ -152,7 +152,8 @@ func (w *FileWriter) Rotate() (err error) {

 func (w *FileWriter) rotate() (err error) {
        var file *os.File
-       file, err = os.OpenFile(w.fileargs(timeNow()))
+       args, flag, perm := w.fileargs(timeNow())
+       file, err = os.OpenFile(args, flag, perm)
        if err != nil {
                return err
        }
@@ -216,7 +217,8 @@ func (w *FileWriter) rotate() (err error) {
 }

 func (w *FileWriter) create() (err error) {
-       w.file, err = os.OpenFile(w.fileargs(timeNow()))
+       args, flag, perm := w.fileargs(timeNow())
+       w.file, err = os.OpenFile(args, flag, perm)
        if err != nil {
                return err
        }

I tried to create a small and isolated snippet, but I failed to reproduce it.

A simple reproducer is the following:

package bar

import "os"

func fn() (string, int, os.FileMode) {
    return "", 0, 0
}

func fn2() {
    os.OpenFile(fn())
}

The bug is caused by us using indices from os.OpenFile's signature to index into the actual call's (as written in the code) arguments. The signature has 3 arguments, but the call has 1. Will fix.

This bug report has resulted in the commits 2715787..b9870cd and the plan to move more checks away from AST-based checks to IR-based ones, which don't suffer from this issue (at least not now that 28022ce exists.)

Thanks for catching this gap in Staticcheck's understanding of the language.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

dominikh picture dominikh  路  3Comments

ryboe picture ryboe  路  3Comments

Shikkic picture Shikkic  路  4Comments

stamm picture stamm  路  4Comments

tonyghita picture tonyghita  路  4Comments