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

false staticcheck reports not reproducible with standalone staticcheck #1768

Closed
3 tasks done
dvyukov opened this issue Feb 22, 2021 · 11 comments
Closed
3 tasks done

false staticcheck reports not reproducible with standalone staticcheck #1768

dvyukov opened this issue Feb 22, 2021 · 11 comments
Labels
bug Something isn't working

Comments

@dvyukov
Copy link

dvyukov commented Feb 22, 2021

Thank you for creating the issue!

  • Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
  • Yes, I've searched similar issues on GitHub and didn't find any.
  • Yes, I've included all information below (version, config, etc).

Please include the following information:

Version of golangci-lint
$ golangci-lint --version
golangci-lint has version v1.37.1 built from (unknown, mod sum: "h1:Unt38FmBltdoILo6oQUMp6PuxwWbJ1YyNSStCrvA7+8=") on (unknown)
Config file
$ cat .golangci.yml
# Copyright 2019 syzkaller project authors. All rights reserved.
# Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.

run:
  deadline: 8m
  skip-dirs:
    - pkg/kd
    - tools/syz-trace2syz
  # Autogenerated files take too much time and memory to load,
  # even if we skip them with skip-dirs.
  # So we define this tag and use it in the autogenerated files.
  build-tags:
    - codeanalysis

output:
  print-linter-name: false

linters:
  enable:
    - lll
    - vet
    - gofmt
    - golint
    - structcheck
    - unconvert
    - deadcode
    - goconst
    - unused
    - gosimple
    - varcheck
    - misspell
    - gocyclo
    - vetshadow
    - megacheck
    - stylecheck
    - govet
    - whitespace
    - nestif
    - goprintffuncname
    - godot
    - gocognit
    - funlen
    - dupl
    - staticcheck
    #- syz-linter
  disable:
    - bodyclose
    - depguard
    - dogsled
    - gochecknoglobals
    - gochecknoinits
    - godox
    - goimports
    - gomnd
    - gomodguard
    - gosec
    - maligned
    - rowserrcheck
    - testpackage
    - typecheck
    - ineffassign
    # errcheck would be good to enable, but we need to fix existing warnings first.
    - errcheck
    - interfacer
    - unparam
    - nakedret
    - prealloc
    - scopelint
    - gocritic
    - wsl

linters-settings:
  lll:
    line-length: 120
  gocyclo:
    # TODO: consider reducing this value.
    min-complexity: 24
  dupl:
    threshold: 60
  goconst:
    min-len: 3
    min-occurrences: 3
  nestif:
    # TODO: consider reducing this value.
    min-complexity: 12
  godot:
    check-all: true
  gocognit:
    # TODO: consider reducing this value.
    min-complexity: 70
  funlen:
    # TODO: consider reducing these value.
    lines: 140
    statements: 80
  custom:
    syz-linter:
      path: bin/syz-linter.so

issues:
  exclude-use-default: false
  max-same-issues: 0
  exclude:
    - "exported .* should have comment"
    - "comment on .* should be of the form"
    - "at least one file in a package should have a package comment"
    # This check gives false positives related to the standard log.Fatalf
    # (which is strange, standard log package should be supported).#
    #- "SA5011: possible nil pointer dereference"
  exclude-rules:
    - path: (pkg/csource/generated.go|pkg/build/linux_generated.go)
      linters:
        - lll
    - path: (sys/.*/init.*|sys/targets/common.go)
      text: "don't use ALL_CAPS in Go names|should not use ALL_CAPS in Go names"
    - path: (prog/.*)
      text: "methods on the same type should have the same receiver name"
    - path: (dashboard/app/.*_test\.go)
      linters:
        - dupl
    - path: (prog/.*_test\.go)
      linters:
        - goconst
    - path: (.*_test\.go)
      text: "Function '.*' is too long"
Go environment
$ go version && go env
go version go1.16 linux/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/.cache/go-build"
GOENV="/home/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/go1.16"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/go1.16/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/go/src/github.com/google/syzkaller/go.mod"
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-build1111402449=/tmp/go-build -gno-record-gcc-switches"
Verbose output of running
$ golangci-lint cache clean
$ golangci-lint run -v
syzkaller$ golangci-lint cache clean

syzkaller$ golangci-lint run -v
INFO [config_reader] Config search paths: [./ /home/dvyukov/go/src/github.com/google/syzkaller /home/dvyukov/go/src/github.com/google /home/dvyukov/go/src/github.com /home/dvyukov/go/src /home/dvyukov/go /home/dvyukov /home /] 
INFO [config_reader] Used config file .golangci.yml 
INFO Loaded bin/syz-linter.so: syz-linter         
INFO [lintersdb] Active 22 linters: [deadcode dupl funlen gocognit goconst gocyclo godot gofmt golint goprintffuncname gosimple govet lll misspell nestif staticcheck structcheck stylecheck unconvert unused varcheck whitespace] 
INFO [loader] Using build tags: [codeanalysis]    
INFO [loader] Go packages loading at mode 575 (types_sizes|files|imports|name|compiled_files|deps|exports_file) took 2.757195184s 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 44.045785ms 
INFO [linters context/goanalysis] analyzers took 1m32.020864178s with top 10 stages: buildir: 24.540296988s, dupl: 5.807368928s, gofmt: 3.943640542s, unconvert: 2.831761317s, inspect: 1.653510331s, golint: 1.378760885s, whitespace: 1.258429192s, the_only_name: 1.204208885s, ctrlflow: 1.100919231s, printf: 951.530516ms 
INFO [linters context/goanalysis] analyzers took 8.638602766s with top 10 stages: buildir: 6.583385026s, U1000: 2.05521774s 
INFO [runner/skip dirs] Skipped 23 issues from dir tools/syz-trace2syz/proggen by pattern tools/syz-trace2syz 
INFO [runner/skip dirs] Skipped 45 issues from dir pkg/kd by pattern pkg/kd 
INFO [runner/skip dirs] Skipped 17 issues from dir tools/syz-trace2syz/parser by pattern tools/syz-trace2syz 
INFO [runner] Issues before processing: 1622, after processing: 3 
INFO [runner] Processors filtering stat (out/in): sort_results: 3/3, exclude: 430/1480, nolint: 3/235, diff: 3/3, max_from_linter: 3/3, max_per_file_from_linter: 3/3, autogenerated_exclude: 1480/1537, exclude-rules: 235/430, max_same_issues: 3/3, source_code: 3/3, filename_unadjuster: 1622/1622, path_prettifier: 1622/1622, skip_files: 1622/1622, skip_dirs: 1537/1622, path_shortener: 3/3, severity-rules: 3/3, cgo: 1622/1622, identifier_marker: 1480/1480, uniq_by_line: 3/3, path_prefixer: 3/3 
INFO [runner] processing took 80.194485ms with stages: nolint: 32.84438ms, identifier_marker: 31.266005ms, path_prettifier: 5.38418ms, autogenerated_exclude: 4.074242ms, exclude: 3.980446ms, exclude-rules: 1.420827ms, skip_dirs: 984.961µs, cgo: 108.51µs, filename_unadjuster: 71.76µs, source_code: 49.752µs, uniq_by_line: 2.255µs, max_from_linter: 1.978µs, path_shortener: 1.957µs, max_same_issues: 1.264µs, max_per_file_from_linter: 690ns, diff: 327ns, sort_results: 302ns, severity-rules: 238ns, skip_files: 226ns, path_prefixer: 185ns 
INFO [runner] linters took 9.72484146s with stages: goanalysis_metalinter: 7.900493804s, unused: 1.744060286s 
sys/fuchsia/fidlgen/main.go:27:17: SA5011: possible nil pointer dereference
	arch := target.KernelHeaderArch
	               ^
tools/syz-runtest/runtest.go:271:6: SA5011: possible nil pointer dereference
	req.Output = a.Output
	    ^
tools/syz-runtest/runtest.go:272:6: SA5011: possible nil pointer dereference
	req.Info = a.Info
	    ^
INFO File cache stats: 369 entries of total size 3.3MiB 
INFO Memory: 126 samples, avg is 488.8MB, max is 812.6MB 
INFO Execution took 12.531268835s                 

Repro instructions:

$ git clone https://github.com/google/syzkaller.git
$ cd syzkaller
$ git checkout fcc6d71be2c3ce7d9305c04fc2e87af554571bac

Apply the following change:

diff --git a/.golangci.yml b/.golangci.yml
index cb0306b56650..3dcedd7390b8 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -42,7 +42,7 @@ linters:
     - funlen
     - dupl
     - staticcheck
-    - syz-linter
+    #- syz-linter
   disable:
     - bodyclose
     - depguard
@@ -105,7 +105,7 @@ issues:
     - "at least one file in a package should have a package comment"
     # This check gives false positives related to the standard log.Fatalf
     # (which is strange, standard log package should be supported).#
-    - "SA5011: possible nil pointer dereference"
+    #- "SA5011: possible nil pointer dereference"
   exclude-rules:
     - path: (pkg/csource/generated.go|pkg/build/linux_generated.go)
       linters:

Then:

$ go install github.com/golangci/golangci-lint/cmd/golangci-lint (vendored)
$ golangci-lint run ./tools/syz-runtest
tools/syz-runtest/runtest.go:271:6: SA5011: possible nil pointer dereference
	req.Output = a.Output
	    ^
tools/syz-runtest/runtest.go:272:6: SA5011: possible nil pointer dereference
	req.Info = a.Info

These are false reports, the line is preceded by log.Fatalf and standalone staticcheck does not produce these warnings:

$ go install honnef.co/go/tools/cmd/staticcheck@latest
$ staticcheck ./tools/syz-runtest

I've found #1189 but it seems to be only about reporting context/related info.

@dominikh

@dvyukov dvyukov added the bug Something isn't working label Feb 22, 2021
dvyukov referenced this issue in google/syzkaller Feb 22, 2021
Currently it produces false reports with Go1.16:

tools/syz-runtest/runtest.go:271:6: SA5011: possible nil pointer dereference
	req.Output = a.Output
	    ^
tools/syz-runtest/runtest.go:272:6: SA5011: possible nil pointer dereference
	req.Info = a.Info
	    ^
sys/fuchsia/fidlgen/main.go:27:17: SA5011: possible nil pointer dereference
	arch := target.KernelHeaderArch
	               ^

All these are preceeded with log.Fatalf, which is strange...

Update #2446
@ldez
Copy link
Member

ldez commented Feb 22, 2021

Duplicate of #1189

@ldez ldez marked this as a duplicate of #1189 Feb 22, 2021
@ldez ldez closed this as completed Feb 22, 2021
@dvyukov
Copy link
Author

dvyukov commented Feb 22, 2021

Duplicate of #1189

Isn't it only about reporting context/related info?

@ldez
Copy link
Member

ldez commented Feb 22, 2021

Maybe I misread.

I'm trying to reproduce.

@dvyukov
Copy link
Author

dvyukov commented Feb 22, 2021

One thing I forgot to mention:
The false warnings appeared only after switch to Go 1.16. Warnings did not show up with Go 1.15.
The same happened with golangci-lint 1.31. I've updated to 1.37 just to not report issues on older releases, but FWIW everything was exactly the same with 1.31.

@ldez
Copy link
Member

ldez commented Feb 22, 2021

Currently, I followed your steps, but I'm not able to run the linter on your project

$ golangci-lint run ./tools/syz-runtest
WARN [runner] Can't run linter goanalysis_metalinter: deadcode: analysis skipped: errors in package: [/home/ldez/sources/go/src/github.com/google/syzkaller/tools/syz-runtest/runtest.go:21:2: could not import github.com/google/syzkaller/pkg/instance (/home/ldez/sources/go/src/github.com/google/syzkaller/pkg/instance/instance.go:18:2: could not import github.com/google/syzkaller/pkg/build (/home/ldez/sources/go/src/github.com/google/syzkaller/pkg/build/build.go:17:2: could not import github.com/google/syzkaller/pkg/report (/home/ldez/sources/go/src/github.com/google/syzkaller/pkg/report/fuzz.go:9:2: could not import github.com/google/syzkaller/pkg/mgrconfig (/home/ldez/sources/go/src/github.com/google/syzkaller/pkg/mgrconfig/load.go:18:4: could not import github.com/google/syzkaller/sys (/home/ldez/sources/go/src/github.com/google/syzkaller/sys/sys.go:8:4: could not import github.com/google/syzkaller/sys/akaros/gen (sys/sys.go:8:2: cannot find package "." in:
	/home/ldez/sources/go/src/github.com/google/syzkaller/sys/akaros/gen)))))) /home/ldez/sources/go/src/github.com/google/syzkaller/tools/syz-runtest/runtest.go:22:2: could not import github.com/google/syzkaller/pkg/mgrconfig (/home/ldez/sources/go/src/github.com/google/syzkaller/pkg/mgrconfig/load.go:18:4: could not import github.com/google/syzkaller/sys (/home/ldez/sources/go/src/github.com/google/syzkaller/sys/sys.go:8:4: could not import github.com/google/syzkaller/sys/akaros/gen (sys/sys.go:8:2: cannot find package "." in:
	/home/ldez/sources/go/src/github.com/google/syzkaller/sys/akaros/gen))) /home/ldez/sources/go/src/github.com/google/syzkaller/tools/syz-runtest/runtest.go:24:2: could not import github.com/google/syzkaller/pkg/report (/home/ldez/sources/go/src/github.com/google/syzkaller/pkg/report/fuzz.go:9:2: could not import github.com/google/syzkaller/pkg/mgrconfig (/home/ldez/sources/go/src/github.com/google/syzkaller/pkg/mgrconfig/load.go:18:4: could not import github.com/google/syzkaller/sys (/home/ldez/sources/go/src/github.com/google/syzkaller/sys/sys.go:8:4: could not import github.com/google/syzkaller/sys/akaros/gen (sys/sys.go:8:2: cannot find package "." in:
	/home/ldez/sources/go/src/github.com/google/syzkaller/sys/akaros/gen)))) /home/ldez/sources/go/src/github.com/google/syzkaller/tools/syz-runtest/runtest.go:25:2: could not import github.com/google/syzkaller/pkg/rpctype (/home/ldez/sources/go/src/github.com/google/syzkaller/pkg/rpctype/rpctype.go:9:2: could not import github.com/google/syzkaller/pkg/host (/home/ldez/sources/go/src/github.com/google/syzkaller/pkg/host/features.go:10:2: could not import github.com/google/syzkaller/pkg/csource (/home/ldez/sources/go/src/github.com/google/syzkaller/pkg/csource/options.go:14:2: could not import github.com/google/syzkaller/pkg/mgrconfig (/home/ldez/sources/go/src/github.com/google/syzkaller/pkg/mgrconfig/load.go:18:4: could not import github.com/google/syzkaller/sys (/home/ldez/sources/go/src/github.com/google/syzkaller/sys/sys.go:8:4: could not import github.com/google/syzkaller/sys/akaros/gen (sys/sys.go:8:2: cannot find package "." in:
	/home/ldez/sources/go/src/github.com/google/syzkaller/sys/akaros/gen)))))) /home/ldez/sources/go/src/github.com/google/syzkaller/tools/syz-runtest/runtest.go:26:2: could not import github.com/google/syzkaller/pkg/runtest (/home/ldez/sources/go/src/github.com/google/syzkaller/pkg/runtest/run.go:27:2: could not import github.com/google/syzkaller/pkg/csource (/home/ldez/sources/go/src/github.com/google/syzkaller/pkg/csource/options.go:14:2: could not import github.com/google/syzkaller/pkg/mgrconfig (/home/ldez/sources/go/src/github.com/google/syzkaller/pkg/mgrconfig/load.go:18:4: could not import github.com/google/syzkaller/sys (/home/ldez/sources/go/src/github.com/google/syzkaller/sys/sys.go:8:4: could not import github.com/google/syzkaller/sys/akaros/gen (sys/sys.go:8:2: cannot find package "." in:
	/home/ldez/sources/go/src/github.com/google/syzkaller/sys/akaros/gen))))) /home/ldez/sources/go/src/github.com/google/syzkaller/tools/syz-runtest/runtest.go:28:4: could not import github.com/google/syzkaller/sys (/home/ldez/sources/go/src/github.com/google/syzkaller/sys/sys.go:8:4: could not import github.com/google/syzkaller/sys/akaros/gen (sys/sys.go:8:2: cannot find package "." in:
	/home/ldez/sources/go/src/github.com/google/syzkaller/sys/akaros/gen)) /home/ldez/sources/go/src/github.com/google/syzkaller/tools/syz-runtest/runtest.go:29:2: could not import github.com/google/syzkaller/vm (/home/ldez/sources/go/src/github.com/google/syzkaller/vm/vm.go:18:2: could not import github.com/google/syzkaller/pkg/mgrconfig (/home/ldez/sources/go/src/github.com/google/syzkaller/pkg/mgrconfig/load.go:18:4: could not import github.com/google/syzkaller/sys (/home/ldez/sources/go/src/github.com/google/syzkaller/sys/sys.go:8:4: could not import github.com/google/syzkaller/sys/akaros/gen (sys/sys.go:8:2: cannot find package "." in:
	/home/ldez/sources/go/src/github.com/google/syzkaller/sys/akaros/gen)))) /home/ldez/sources/go/src/github.com/google/syzkaller/tools/syz-runtest/runtest.go:15:2: "net" imported but not used] 
WARN [runner] Can't run linter unused: buildir: failed to load package : could not load export data: no export data for "github.com/google/syzkaller/sys/akaros/gen" 
ERRO Running error: buildir: failed to load package : could not load export data: no export data for "github.com/google/syzkaller/sys/akaros/gen" 

@dvyukov
Copy link
Author

dvyukov commented Feb 22, 2021

oh, sorry, please run make descriptions after check out

@ldez
Copy link
Member

ldez commented Feb 22, 2021

The problem will be fixed by the next version.

$ golangci-lint run ./tools/syz-runtest
tools/syz-runtest/runtest.go:271:6: SA5011: possible nil pointer dereference
	req.Output = a.Output
	    ^
tools/syz-runtest/runtest.go:272:6: SA5011: possible nil pointer dereference
	req.Info = a.Info
	    ^
$ ./golangci-lint run ./tools/syz-runtest

@ldez
Copy link
Member

ldez commented Feb 22, 2021

So it's not a duplicate 😉

For now, I don't know if it's related to the update of staticcheck #1756 or to something else

@ldez
Copy link
Member

ldez commented Feb 22, 2021

You tried to trick me 😄 :

$ go install honnef.co/go/tools/cmd/staticcheck@latest

But I also tried with v0.0.1-2020.1.6 and I have the same output as the latest

@ldez
Copy link
Member

ldez commented Feb 22, 2021

I confirm the problem has been fixed by #1756

@dvyukov
Copy link
Author

dvyukov commented Feb 23, 2021

You tried to trick me smile :

That was @dominikh who tried to trick both me and you :)
https://github.com/golangci/golangci-lint/pull/1756/files

So I need to wait for new golangci-lint version.
Trying to figure out if there is any workaround, e.g. manually import staticcheck 0.1.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants