linter crashes with panic

[gucio321]

• Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
• Yes, I've included all information below (version, config, etc).
• Yes, I've tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)
• Yes, I've searched similar issues on GitHub and didn't find any.

Description of the problem

linter crashes on OpenDiablo2 project.
here is a workflow run, which reproduces this behavior

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.40.0 built from 5c6adb63 on 2021-05-10T10:45:21Z

Config file

$ cat .golangci.yml
---
linters-settings:
  dupl:
    threshold: 100
  funlen:
    lines: 100
    statements: 50
  goconst:
    min-len: 2
    min-occurrences: 2
  gocritic:
    enabled-tags:
      - diagnostic
      - experimental
      - opinionated
      - performance
      - style
    disabled-checks:
  gocyclo:
    min-complexity: 15
  gofmt:
    simplify: true
  goimports:
    local-prefixes: github.com/OpenDiablo2/OpenDiablo2
  golint:
    min-confidence: 0.8
  govet:
    enable-all: true
    check-shadowing: true
    disable:
      # While struct sizes could be smaller if fields aligned properly, that also leads
      # to possibly non-intuitive layout of struct fields (harder to read). Disable
      # `fieldalignment` check here until we evaluate if it is worthwhile.
      - fieldalignment
  lll:
    line-length: 140
  misspell:
    locale: US

linters:
  disable-all: true
  enable:
    - bodyclose
    - deadcode
    - depguard
    - dogsled
    - dupl
    - errcheck
    - funlen
    - gochecknoglobals
    - gochecknoinits
    - gocognit
    - goconst
    - gocritic
    - gocyclo
    - godox
    - gofmt
    - goimports
    - golint
    - gomnd
    - goprintffuncname
    - gosec
    - gosimple
    - govet
    - ineffassign
    - lll
    - misspell
    - nakedret
    - prealloc
    - rowserrcheck
    - staticcheck
    - structcheck
    - stylecheck
    - typecheck
    - unconvert
    - unparam
    - unused
    - varcheck
    - whitespace
    - wsl

run:
  timeout: 5m
  tests: true
  skip-dirs:
    - .github
    - build
    - web

issues:
  exclude-rules:
    - linters:
        - funlen
      # Disable 'funlen' linter for test functions.
      # It's common for table-driven tests to be more than 60 characters long
      source: "^func Test"
  max-issues-per-linter: 0
  max-same-issues: 0
  exclude-use-default: false

Go environment

$ go version && go env
go version go1.16.4 linux/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/user/.cache/go-build"
GOENV="/home/user/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/user/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/user/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16.4"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3225948580=/tmp/go-build -gno-record-gcc-switches"

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v ./...
INFO [config_reader] Config search paths: [./ /home/mszeptuch/git/d2/me/OpenDiablo2 /home/mszeptuch/git/d2/me /home/mszeptuch/git/d2 /home/mszeptuch/git /home/mszeptuch /home /] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 38 linters: [bodyclose deadcode depguard dogsled dupl errcheck funlen gochecknoglobals gochecknoinits gocognit goconst gocritic gocyclo godox gofmt goimports golint gomnd goprintffuncname gosec gosimple govet ineffassign lll misspell nakedret prealloc rowserrcheck staticcheck structcheck stylecheck typecheck unconvert unparam unused varcheck whitespace wsl] 
INFO [loader] Go packages loading at mode 575 (exports_file|files|imports|name|compiled_files|deps|types_sizes) took 454.737027ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 163.235443ms 
INFO [linters context/goanalysis] analyzers took 1m25.799685893s with top 10 stages: the_only_name: 4.987980064s, buildir: 4.764309399s, buildssa: 4.169827884s, gocritic: 4.089240111s, dupl: 3.977163207s, gosec: 3.601953723s, directives: 2.657086921s, whitespace: 2.580385854s, gocyclo: 2.355753677s, goimports: 2.350630905s 
WARN [linters context] Panic: bodyclose: package "d2app" (isInitialPkg: true, needAnalyzeSource: true): interface conversion: interface {} is nil, not *buildssa.SSA: goroutine 10588 [running]:
runtime/debug.Stack(0x11f2447, 0x3c, 0xc0030fe9c8)
	runtime/debug/stack.go:24 +0x9f
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func1(0xc001a938c0)
	github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:101 +0x1be
panic(0x1000a00, 0xc00b7c7920)
	runtime/panic.go:965 +0x1b9
github.com/timakin/bodyclose/passes/bodyclose.runner.run(0xc00a7b5d40, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xc00a7b5d40, 0x0, ...)
	github.com/timakin/bodyclose@v0.0.0-20200424151742-cb6215831a94/passes/bodyclose/bodyclose.go:45 +0x837
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyze(0xc001a938c0)
	github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:187 +0x9f2
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func2()
	github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:105 +0x2a
github.com/golangci/golangci-lint/pkg/timeutils.(*Stopwatch).TrackStage(0xc002e55770, 0x1159d21, 0x9, 0xc0046bef70)
	github.com/golangci/golangci-lint/pkg/timeutils/stopwatch.go:111 +0x4d
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe(0xc001a938c0)
	github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:104 +0x91
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze.func2(0xc009cf8df0, 0xc001a938c0)
	github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_loadingpackage.go:80 +0x65
created by github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze
	github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_loadingpackage.go:75 +0x316 
WARN [runner] Can't run linter goanalysis_metalinter: goanalysis_metalinter: bodyclose: package "d2app" (isInitialPkg: true, needAnalyzeSource: true): interface conversion: interface {} is nil, not *buildssa.SSA 
INFO [runner] processing took 3.476µs with stages: max_same_issues: 675ns, skip_dirs: 425ns, nolint: 317ns, cgo: 278ns, max_from_linter: 242ns, autogenerated_exclude: 212ns, uniq_by_line: 162ns, identifier_marker: 161ns, filename_unadjuster: 151ns, path_prettifier: 133ns, diff: 118ns, skip_files: 116ns, exclude: 68ns, path_shortener: 65ns, max_per_file_from_linter: 62ns, source_code: 62ns, severity-rules: 59ns, sort_results: 59ns, exclude-rules: 56ns, path_prefixer: 55ns 
INFO [runner] linters took 3.753465733s with stages: goanalysis_metalinter: 3.753419759s 
INFO File cache stats: 188 entries of total size 919.5KiB 
INFO Memory: 44 samples, avg is 237.9MB, max is 344.7MB 
INFO Execution took 4.378323222s   

Code example or link to a public repository

https://github.com/OpenDiablo2/OpenDiablo2

[boring-cyborg]

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

[ldez]

Hello,

The problem seems related to govet, as a workaround you can disable it.
I will investigate.

[ldez]

It seems to be a bug with the new nilness analyzer, then I recommend disabling this analyzer.

  govet:
    enable-all: true
    disable:
      - nilness

[ashanbrown]

I've also seen some other panics that seem to be fixed by not running gotvet. This one doesn't appear to be related to nilness (although removing that fixed a different panic):

WARN [linters context] Panic: unusedwrite: package "main" (isInitialPkg: true, needAnalyzeSource: true): interface conversion: interface {} is nil, not *buildssa.SSA: goroutine 20325 [running]:
runtime/debug.Stack(0x1df377c, 0x3c, 0xc003307af8)
	runtime/debug/stack.go:24 +0x9f
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func1(0xc00305bcd0)
	github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:101 +0x1be
panic(0x1c014e0, 0xc03eec8990)
	runtime/panic.go:965 +0x1b9
golang.org/x/tools/go/analysis/passes/unusedwrite.run(0xc0234131e0, 0x356c6d1c1, 0x26b3120, 0xc0136ed9b8, 0x2)
	golang.org/x/tools@v0.1.1-0.20210430200834-7a6108e9b210/go/analysis/passes/unusedwrite/unusedwrite.go:74 +0x1c5
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyze(0xc00305bcd0)
	github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:187 +0x9f2
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func2()
	github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:105 +0x2a
github.com/golangci/golangci-lint/pkg/timeutils.(*Stopwatch).TrackStage(0xc00131f5e0, 0x1d91c5a, 0xb, 0xc003625f70)
	github.com/golangci/golangci-lint/pkg/timeutils/stopwatch.go:111 +0x4d
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe(0xc00305bcd0)
	github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:104 +0x91
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze.func2(0xc06efe3f00, 0xc00305bcd0)
	github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_loadingpackage.go:80 +0x65
created by github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze
	github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_loadingpackage.go:75 +0x316
WARN [runner] Can't run linter goanalysis_metalinter: goanalysis_metalinter: unusedwrite: package "main" (isInitialPkg: true, needAnalyzeSource: true): interface conversion: interface {} is nil, not *buildssa.SSA

[ashanbrown]

What is more concerning is that when we panic, I'm still seeing an exit status code of 0.

[ldez]

It's related to the same problem as nilness, the important information is interface conversion: interface {} is nil, not *buildssa.SSA:

2 "passes" are affected by this problem: nilness and unusedwrite

FYI I know the root cause of all those panics.

[ldez]

What is more concerning is that when we panic, I'm still seeing an exit status code of 0.

It's expected because the panics are reported as a warning. #1540

[ldez]

The root cause is sigchanyzer but I'm still trying to understand why.

It only happens when the sigchanyzer reports a problem.

so the workaround:

  govet:
    enable-all: true
    disable:
      - sigchanyzer

[ashanbrown]

@ldez Why is it desirable to have panics reported as warnings? Will it still run the other linters at least?

[ldez]

@ashanbrown more details in #1540

It's a bit offtopic, I will stay focus on the root cause of the panic.

[ashanbrown]

@ldez I didn't see any comments on #1540 explaining current the behavior. Should I open a new issue or just comment on that issue? I generally prefer not to comment on closed issues. I updated golangci-lint on several repos today, only to find out that they don't do anything because they are invisibly panicking in the pre-commit hooks that run them, so the current behavior is kind of frustrating. Thanks.

[ldez]

@ashanbrown in #1540 description there is #1537 (comment)
It explains the base of the behavior.

I generally prefer not to comment on closed issues.

I share this opinion, you can open a new issue if you need.

[ldez]

Back to the main topic, the problem is in sigchanyzer, it's related to the following lines:
https://github.com/golang/tools/blob/be4aaae4cf865bd1e65ae3a22df28c1f47b6ccfd/go/analysis/passes/sigchanyzer/sigchanyzer.go#L62-L65

I think it's because it alters the AST.