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

SA9002: panic: runtime error: index out of range [2] with length 1 #930

Closed
ldez opened this issue Feb 11, 2021 · 4 comments
Closed

SA9002: panic: runtime error: index out of range [2] with length 1 #930

ldez opened this issue Feb 11, 2021 · 4 comments
Labels

Comments

@ldez
Copy link
Contributor

ldez commented Feb 11, 2021

$ 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 golangci/golangci-lint#1730 and phuslu/log#28

@ldez ldez added bug needs-triage Newly filed issue that needs triage labels Feb 11, 2021
@dominikh dominikh removed the needs-triage Newly filed issue that needs triage label Feb 11, 2021
@dominikh
Copy link
Owner

Can reproduce, looking into it.

@ldez
Copy link
Contributor Author

ldez commented Feb 11, 2021

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.

@dominikh
Copy link
Owner

dominikh commented Feb 11, 2021

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.

@dominikh
Copy link
Owner

dominikh commented Feb 11, 2021

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.

dominikh added a commit that referenced this issue Feb 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants