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

Performance issue with gosec on v1.54.2 #4039

Closed
5 tasks done
tobias-kuendig opened this issue Aug 24, 2023 · 9 comments
Closed
5 tasks done

Performance issue with gosec on v1.54.2 #4039

tobias-kuendig opened this issue Aug 24, 2023 · 9 comments
Labels
bug Something isn't working dependencies Relates to an upstream dependency feedback required Requires additional feedback

Comments

@tobias-kuendig
Copy link

tobias-kuendig commented Aug 24, 2023

Welcome

Description of the problem

Using 1.54.2 of golang-ci-lint, the gosec linter suddenly is a lot slower than before. I am running this on a relatively large codebase:

v1.54.1 (runs in a few seconds)

$ curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.54.1
$ golangci-lint version
golangci-lint has version 1.54.1 built with go1.21.0 from a9378d9b on 2023-08-11T12:49:16Z

$ golangci-lint run -v --no-config --disable-all --enable gosec ./...                                                             
INFO [lintersdb] Active 1 linters: [gosec]        
INFO [loader] Go packages loading at mode 575 (compiled_files|exports_file|files|deps|imports|name|types_sizes) took 1.53940274s 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 90.851188ms 
INFO [linters_context/goanalysis] analyzers took 976.350117ms with top 10 stages: gosec: 974.013444ms, typecheck: 2.336673ms 
INFO [runner] Issues before processing: 55, after processing: 0 
INFO [runner] Processors filtering stat (out/in): exclude: 46/46, cgo: 55/55, skip_files: 55/55, exclude-rules: 9/46, nolint: 0/9, filename_unadjuster: 55/55, path_prettifier: 55/55, skip_dirs: 55/55, autogenerated_exclude: 46/55, identifier_marker: 46/46 
INFO [runner] processing took 4.736721ms with stages: nolint: 2.332846ms, autogenerated_exclude: 1.196407ms, exclude-rules: 373.173µs, identifier_marker: 328.37µs, skip_dirs: 250.949µs, path_prettifier: 239.91µs, cgo: 8.717µs, filename_unadjuster: 3.366µs, max_same_issues: 545ns, fixer: 359ns, source_code: 311ns, diff: 285ns, uniq_by_line: 283ns, skip_files: 260ns, path_shortener: 189ns, severity-rules: 186ns, exclude: 181ns, sort_results: 150ns, max_from_linter: 100ns, max_per_file_from_linter: 70ns, path_prefixer: 64ns 
INFO [runner] linters took 905.767983ms with stages: goanalysis_metalinter: 900.987883ms 
INFO File cache stats: 0 entries of total size 0B 
INFO Memory: 27 samples, avg is 136.6MB, max is 398.2MB 
INFO Execution took 2.539893412s 

v1.54.2 (takes minutes, hitting timeout)

I've had runs that passed, this one hit the timeout:

$ curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.54.2
$ golangci-lint version
golangci-lint has version 1.54.2 built with go1.21.0 from 411e0bbb on 2023-08-21T12:04:32Z

$ golangci-lint run -v --no-config --disable-all --enable gosec ./...                                      
INFO [lintersdb] Active 1 linters: [gosec]        
INFO [loader] Go packages loading at mode 575 (imports|types_sizes|deps|exports_file|name|compiled_files|files) took 1.278350021s 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 98.160985ms 

INFO Memory: 601 samples, avg is 441.7MB, max is 453.7MB 
INFO Execution took 1m0.000260426s                
INFO [linters_context/goanalysis] analyzers took 6m59.362370889s with top 10 stages: gosec: 6m59.360517105s, typecheck: 1.853784ms 
INFO [runner] Issues before processing: 56, after processing: 0 
INFO [runner] Processors filtering stat (out/in): exclude-rules: 10/47, identifier_marker: 47/47, nolint: 0/10, skip_files: 56/56, autogenerated_exclude: 47/56, exclude: 47/47, filename_unadjuster: 56/56, path_prettifier: 56/56, cgo: 56/56, skip_dirs: 56/56 
INFO [runner] processing took 5.211167ms with stages: nolint: 2.466589ms, autogenerated_exclude: 1.393037ms, exclude-rules: 541.334µs, identifier_marker: 390.287µs, path_prettifier: 250.543µs, skip_dirs: 157.991µs, cgo: 5.333µs, filename_unadjuster: 2.913µs, max_same_issues: 909ns, skip_files: 365ns, fixer: 350ns, diff: 293ns, sort_results: 263ns, severity-rules: 188ns, exclude: 187ns, uniq_by_line: 185ns, max_from_linter: 113ns, source_code: 95ns, max_per_file_from_linter: 67ns, path_shortener: 63ns, path_prefixer: 62ns 
INFO [runner] linters took 6m58.151346821s with stages: goanalysis_metalinter: 6m58.146074082s 
INFO File cache stats: 0 entries of total size 0B 
ERRO Timeout exceeded: try increasing it by passing --timeout option 

Running my default linter set with gosec disabled finishes in seconds:

$ golangci-lint version
golangci-lint has version 1.54.2 built with go1.21.0 from 411e0bbb on 2023-08-21T12:04:32Z

$ golangci-lint run -v ./...      

INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 46 linters: [asasalint asciicheck bidichk bodyclose decorder dogsled durationcheck errcheck errorlint execinquery exportloopref gochecknoinits gofmt goheader goimports gomoddirectives gomodguard goprintffuncname gosimple govet grouper importas loggercheck makezero misspell nakedret nestif nilerr noctx nolintlint nosprintfhostport predeclared promlinter reassign revive rowserrcheck staticcheck stylecheck tenv tparallel unconvert unparam unused usestdlibvars wastedassign whitespace] 
INFO [loader] Go packages loading at mode 575 (compiled_files|deps|types_sizes|exports_file|files|imports|name) took 1.334268938s 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 92.982021ms 
INFO [linters_context] importas settings found, but no aliases listed. List aliases under alias: key. 
INFO [linters_context/goanalysis] analyzers took 0s with no stages 
INFO [runner/skip_dirs] Skipped 626 issues from dir applications/<test>/graphql/gqlserver by pattern applications/<test>/graphql/gqlserver 
INFO [runner] Issues before processing: 1595, after processing: 1 
INFO [runner] Processors filtering stat (out/in): exclude-rules: 462/889, fixer: 1/1, max_from_linter: 1/1, source_code: 1/1, path_prefixer: 1/1, sort_results: 1/1, nolint: 2/462, cgo: 1595/1595, filenameunadjuster: 1595/1595, path_prettifier: 1595/1595, skip_files: 1587/1595, skip_dirs: 961/1587, max_per_file_from_linter: 1/1, max_same_issues: 1/1, path_shortener: 1/1, autogenerated_exclude: 889/961, identifier_marker: 889/889, exclude: 889/889, uniq_by_line: 1/2, diff: 1/1, severity-rules: 1/1 
INFO [runner] processing took 40.59585ms with stages: nolint: 17.332562ms, exclude-rules: 11.281626ms, identifier_marker: 5.99838ms, path_prettifier: 2.925171ms, autogenerated_exclude: 2.204377ms, skip_dirs: 617.777µs, cgo: 93.45µs, skip_files: 73.536µs, filename_unadjuster: 46.562µs, source_code: 16.282µs, max_same_issues: 1.457µs, uniq_by_line: 1.375µs, path_shortener: 728ns, max_from_linter: 618ns, max_per_file_from_linter: 536ns, fixer: 337ns, exclude: 304ns, sort_results: 266ns, severity-rules: 222ns, diff: 218ns, path_prefixer: 66ns 
INFO [runner] linters took 153.458431ms with stages: goanalysis_metalinter: 112.812073ms 
INFO File cache stats: 1 entries of total size 10.8KiB 
INFO Memory: 17 samples, avg is 34.7MB, max is 101.6MB 
INFO Execution took 1.584228822s 

Running gosec manually runs quickly as well:

$ time gosec --exclude-dir=applications/<test>/graphql/gqlserver ./...  
35.50s user 14.62s system 1714% cpu 2.923 total

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.54.2 built with go1.21.0 from 411e0bbb on 2023-08-21T12:04:32Z

Configuration

golangci-lint run --no-config --disable-all --enable gosec ./...

Go environment

$ go version && go env
go version go1.20.4 linux/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/<me>/.cache/go-build"
GOENV="/home/<me>/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS="-mod=mod"
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/<me>/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/<me>/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.20.4"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/<me>/code/<project>/go.mod"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build123119041=/tmp/go-build -gno-record-gcc-switches"

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v --no-config --disable-all --enable gosec ./...
# paste output here

A minimal reproducible example or link to a public repository

// N/A

Validation

  • Yes, I've included all information above (version, config, etc.).
@tobias-kuendig tobias-kuendig added the bug Something isn't working label Aug 24, 2023
@ldez
Copy link
Member

ldez commented Aug 24, 2023

Hello,

The 2 sections ("Verbose output of running", and "A minimal reproducible example or link to a public repository") are missing and it's a problem to analyze the issue.

In the section "Verbose output of running" we ask to clean the cache before running to avoid misinterpretation related to the cache.
In the section "A minimal reproducible example or link to a public repository" we ask for a reproducible context to be able to check the issue.

Can you run again the commands and clean the cache before each run?

Also, we don't know the version of gosec you are using.

@tobias-kuendig
Copy link
Author

The 2 sections ("Verbose output of running", and "A minimal reproducible example or link to a public repository") are missing and it's a problem to analyze the issue.

The outputs that are pasted above are using the -v flag. Here again:

$ golangci-lint cache clean

$ golangci-lint version
golangci-lint has version 1.54.1 built with go1.21.0 from a9378d9b on 2023-08-11T12:49:16Z

$ golangci-lint run -v --no-config --disable-all   --enable gosec ./...       

INFO [lintersdb] Active 1 linters: [gosec]        
INFO [loader] Go packages loading at mode 575 (files|exports_file|imports|name|types_sizes|compiled_files|deps) took 2.409055854s 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 64.608045ms 
INFO [linters_context/goanalysis] analyzers took 961.812676ms with top 10 stages: gosec: 960.178769ms, typecheck: 1.633907ms 
INFO [runner] Issues before processing: 55, after processing: 0 
INFO [runner] Processors filtering stat (out/in): skip_dirs: 46/55, identifier_marker: 46/46, filename_unadjuster: 55/55, skip_files: 55/55, exclude: 46/46, cgo: 55/55, path_prettifier: 55/55, autogenerated_exclude: 46/46, exclude-rules: 9/46, nolint: 0/9 
INFO [runner] processing took 3.598443ms with stages: nolint: 2.119488ms, identifier_marker: 370.119µs, path_prettifier: 308.073µs, exclude-rules: 298.499µs, autogenerated_exclude: 278.973µs, skip_dirs: 179.959µs, cgo: 35.917µs, filename_unadjuster: 3.564µs, max_same_issues: 1.175µs, fixer: 421ns, sort_results: 363ns, uniq_by_line: 352ns, diff: 339ns, exclude: 245ns, skip_files: 231ns, severity-rules: 223ns, path_shortener: 156ns, source_code: 120ns, max_from_linter: 92ns, max_per_file_from_linter: 71ns, path_prefixer: 63ns 
INFO [runner] linters took 877.093409ms with stages: goanalysis_metalinter: 873.455546ms 
INFO File cache stats: 0 entries of total size 0B 
INFO Memory: 35 samples, avg is 110.1MB, max is 398.7MB 
INFO Execution took 3.354412491s  
$ golangci-lint cache clean

$ golangci-lint version
golangci-lint has version 1.54.2 built with go1.21.0 from 411e0bbb on 2023-08-21T12:04:32Z

$ golangci-lint run -v --no-config --disable-all   --enable gosec ./...       

INFO [lintersdb] Active 1 linters: [gosec]        
INFO [loader] Go packages loading at mode 575 (types_sizes|compiled_files|files|imports|name|deps|exports_file) took 837.308762ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 67.999232ms 
INFO [linters_context/goanalysis] analyzers took 6m55.501595662s with top 10 stages: gosec: 6m55.499226636s, typecheck: 2.369026ms 
INFO [runner] Issues before processing: 56, after processing: 1 
INFO [runner] Processors filtering stat (out/in): severity-rules: 1/1, fixer: 1/1, skip_dirs: 47/56, nolint: 1/10, max_per_file_from_linter: 1/1, max_same_issues: 1/1, source_code: 1/1, cgo: 56/56, filename_unadjuster: 56/56, exclude-rules: 10/47, diff: 1/1, max_from_linter: 1/1, sort_results: 1/1, path_prettifier: 56/56, skip_files: 56/56, exclude: 47/47, uniq_by_line: 1/1, path_prefixer: 1/1, autogenerated_exclude: 47/47, identifier_marker: 47/47, path_shortener: 1/1 
INFO [runner] processing took 4.146831ms with stages: nolint: 2.549208ms, exclude-rules: 545.712µs, identifier_marker: 384µs, path_prettifier: 248.6µs, autogenerated_exclude: 241.708µs, skip_dirs: 153.23µs, source_code: 13.11µs, cgo:  4.602µs, filename_unadjuster: 1.714µs, max_same_issues: 940ns, uniq_by_line: 899ns, diff: 556ns, path_shortener: 551ns, sort_results: 333ns, fixer: 327ns, max_per_file_from_linter: 313ns, max_from_linter: 301ns, skip_files: 227ns, exclude: 210ns, severity-rules: 174ns, path_prefixer: 116ns 
INFO [runner] linters took 6m54.278035864s with stages: goanalysis_metalinter: 6m54.273848798s 
INFO File cache stats: 1 entries of total size 10.8KiB 
INFO Memory: 4153 samples, avg is 461.6MB, max is 467.7MB 
INFO Execution took 6m55.186614563s 

gosec is version:

$ gosec --version        
Version: 2.17.0
Git tag: v2.17.0
Build date: unknown

Providing a reproduction repo is not easy since I am running the linter against a fairly large repo. I have tried to lint only specific folders and the problem seems to be related to a large generated Graphql server (using gqlgen, about 80k lines). Strangely enough, running golangci-lint on the graphql folder but specifically excluding that large file still takes long, even though it is the only file in that directory:

$ ls applications/<myapp>/graphql/gqlserver        
gen_server.go

$ golangci-lint run  -v --no-config --disable-all --skip-files applications/<myapp>/graphql/gqlserver/gen_server.go --enable gosec applications/<myapp>/graphql/gqlserver/...

INFO [lintersdb] Active 1 linters: [gosec]        
INFO [loader] Go packages loading at mode 575 (types_sizes|deps|name|files|imports|compiled_files|exports_file) took 101.050076ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 54.762526ms 
INFO [linters_context/goanalysis] analyzers took 7m17.371501993s with top 10 stages: gosec: 7m17.371494857s, typecheck: 7.136µs 
INFO [runner] Issues before processing: 9, after processing: 0 
INFO [runner] Processors filtering stat (out/in): cgo: 9/9, skip_files: 0/9, filename_unadjuster: 9/9, path_prettifier: 9/9 
INFO [runner] processing took 23.688µs with stages: path_prettifier: 14.825µs, skip_files: 3.735µs, cgo: 1.341µs, filename_unadjuster: 473ns, nolint: 420ns, skip_dirs: 371ns, autogenerated_exclude: 345ns, max_same_issues: 306ns, fixer: 293ns, uniq_by_line: 247ns, exclude-rules: 225ns, exclude: 182ns, severity-rules: 164ns, max_from_linter: 159ns, identifier_marker: 136ns, diff: 134ns, sort_results: 73ns, max_per_file_from_linter: 71ns, path_shortener: 64ns, path_prefixer: 62ns, source_code: 62ns 
INFO [runner] linters took 7m17.704762618s with stages: goanalysis_metalinter: 7m17.704705012s 
INFO File cache stats: 0 entries of total size 0B 
INFO Memory: 4380 samples, avg is 325.8MB, max is 331.8MB 
INFO Execution took 7m17.863160752s  

@ldez
Copy link
Member

ldez commented Aug 24, 2023

FYI --skip-files doesn't exclude a file of the analysis, it just ignores reports from this file.

@ldez
Copy link
Member

ldez commented Aug 24, 2023

I don't really reproduce can you try to disable the rules (one by one):

linters-settings:
  gosec:
    excludes:
      - G101
linters-settings:
  gosec:
    excludes:
      - G602

@ldez ldez added feedback required Requires additional feedback dependencies Relates to an upstream dependency labels Aug 24, 2023
@tobias-kuendig
Copy link
Author

Thank you for looking into this! It looks like disabling G602 does the trick:

$ golangci-lint run  -v applications/<myapp>/graphql/gqlserver/...

INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 47 linters: [asasalint asciicheck bidichk bodyclose decorder dogsled durationcheck errcheck errorlint execinquery exportloopref gochecknoinits gofmt goheader goimports gomoddirectives gomodguard goprintffuncname gosec gosimple govet grouper importas loggercheck makezero misspell nakedret nestif nilerr noctx nolintlint nosprintfhostport predeclared promlinter reassign revive rowserrcheck staticcheck stylecheck tenv tparallel unconvert unparam unused usestdlibvars wastedassign whitespace] 
INFO [loader] Go packages loading at mode 575 (files|imports|types_sizes|compiled_files|deps|exports_file|name) took 134.738156ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 54.377788ms 
INFO [linters_context] importas settings found, but no aliases listed. List aliases under alias: key. 
INFO [linters_context/goanalysis] analyzers took 29.695692936s with top 10 stages: buildir: 7.852710964s, whitespace: 2.814758794s, nilness: 1.813657727s, gosec: 1.052963717s, goimports: 715.386193ms, errorlint: 613.462779ms, commentmap: 596.384583ms, directives: 581.074949ms, buildssa: 536.033524ms, typedness: 510.871629ms 
INFO [runner] Issues before processing: 635, after processing: 0 
INFO [runner] Processors filtering stat (out/in): skip_files: 635/635, skip_dirs: 635/635, autogenerated_exclude: 0/635, cgo: 635/635, path_prettifier: 635/635, filename_unadjuster: 635/635 
INFO [runner] processing took 794.101µs with stages: autogenerated_exclude: 511.773µs, path_prettifier: 156.164µs, skip_dirs: 45.203µs, skip_files: 42.015µs, cgo: 19.6µs, filename_unadjuster: 15.961µs, fixer: 662ns, max_same_issues: 511ns, nolint: 436ns, exclude-rules: 280ns, uniq_by_line: 276ns, diff: 246ns, exclude: 170ns, severity-rules: 167ns, identifier_marker: 136ns, max_from_linter: 118ns, sort_results: 101ns, path_shortener: 77ns, max_per_file_from_linter: 74ns, path_prefixer: 66ns, source_code: 65ns 
INFO [runner] linters took 4.778701237s with stages: goanalysis_metalinter: 4.777868382s 
INFO File cache stats: 1 entries of total size 2.5MiB 
INFO Memory: 51 samples, avg is 814.2MB, max is 1135.6MB 
INFO Execution took 4.970457739s  

Enabling the G602 rule does lead to a timeout again, so it is definitely it.

@ldez
Copy link
Member

ldez commented Aug 24, 2023

can you try gosec (the binary) v2.26.0 versus v2.27.0?

@tobias-kuendig
Copy link
Author

I guess you meant v2.16 vs v2.17:

v2.16

$ gosec --version
Version: 2.16.0
Git tag: v2.16.0
Build date: unknown

$ time gosec
1.77s user 0.25s system 241% cpu 0.839 total

v2.17

$ gosec --version
Version: 2.17.0
Git tag: v2.17.0
Build date: unknown

$ time gosec
<forever> (I exited after 2 minutes)

v2.17 without G602

$ time gosec --exclude=G602 
1.83s user 0.30s system 265% cpu 0.806 total

This is definitely an upstream issue.

@ldez
Copy link
Member

ldez commented Aug 24, 2023

can you open an issue on the gosec repository?

@tobias-kuendig
Copy link
Author

I'll do so tomorrow, I'll try to analyze the offending PR first: securego/gosec#973

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies Relates to an upstream dependency feedback required Requires additional feedback
Projects
None yet
Development

No branches or pull requests

2 participants