revive is not playing nice with cgo

[veger]

Welcome

• 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).
• Yes, I've tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)

Description of the problem

When using cgo an import "C" is required to active cgo (and import/define C code). The blank-imports check does not accept this and throws the following warning:

blank-imports: a blank import should be only in a main or test package, or have a comment justifying it (revive)

When running with revive by itself the issue is gone: See mgechev/revive#635 for more details on report on the revive issue tracker.

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.44.2 built from d58dbde5 on 2022-02-17T20:58:06Z

Configuration file

$ cat .golangci.yml
linters-settings:
  revive:
    ignore-generated-header: true
    rules:
      - name: blank-imports
        disabled: false

Go environment

$ go version && go env
go version go1.17.7 linux/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/some-user/.cache/go-build"
GOENV="/home/some-user/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/some-user/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/some-user/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.7"
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-build342790889=/tmp/go-build -gno-record-gcc-switches

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v --disable-all -E revive --config=golangci.yml ./pkg/...
INFO [config_reader] Used config file golangci.yml 
INFO [lintersdb] Active 1 linters: [revive]       
INFO [loader] Go packages loading at mode 7 (files|compiled_files|name) took 44.333638ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 133.973µs 
INFO [linters context/goanalysis] analyzers took 426.733µs with top 10 stages: the_only_name: 426.733µs 
INFO [runner] Issues before processing: 2, after processing: 1 
INFO [runner] Processors filtering stat (out/in): max_from_linter: 1/1, path_prefixer: 1/1, skip_dirs: 1/1, identifier_marker: 1/1, exclude: 1/1, max_per_file_from_linter: 1/1, path_prettifier: 1/1, exclude-rules: 1/1, diff: 1/1, severity-rules: 1/1, sort_results: 1/1, skip_files: 1/1, filename_unadjuster: 1/1, autogenerated_exclude: 1/1, nolint: 1/1, uniq_by_line: 1/1, max_same_issues: 1/1, source_code: 1/1, path_shortener: 1/1, cgo: 1/2 
INFO [runner] processing took 171.187µs with stages: exclude-rules: 62.068µs, identifier_marker: 32.43µs, path_prettifier: 18.586µs, autogenerated_exclude: 18.147µs, nolint: 16.529µs, source_code: 10.408µs, skip_dirs: 5.063µs, path_shortener: 1.799µs, uniq_by_line: 1.459µs, max_same_issues: 1.11µs, cgo: 738ns, max_from_linter: 726ns, filename_unadjuster: 714ns, max_per_file_from_linter: 359ns, severity-rules: 218ns, skip_files: 202ns, diff: 190ns, exclude: 190ns, sort_results: 147ns, path_prefixer: 104ns 
INFO [runner] linters took 6.615019ms with stages: revive: 6.390022ms 
pkg/blank-import-cgo.go:5:8: blank-imports: a blank import should be only in a main or test package, or have a comment justifying it (revive)
import "C"
       ^
INFO File cache stats: 1 entries of total size 76B 
INFO Memory: 2 samples, avg is 48.8MB, max is 48.8MB 
INFO Execution took 55.307828ms                   

Code example or link to a public repository

// Package pkg ...
package pkg

// #include <stdlib.h>
import "C"

[boring-cyborg]

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

[ldez]

Hello,

I don't reproduce your problem:

main.go

package main

import "unsafe"

// #include <stdlib.h>
import "C"

func main() {
	cStr := C.CString("test")

	C.free(unsafe.Pointer(cStr))
}

.golangci.yml

linters:
  disable-all: true
  enable:
    - revive
    - typecheck

linters-settings:
  revive:
    severity: warning
    confidence: 0.1
    ignore-generated-header: true
    rules:
      - name: blank-imports
        disabled: false

$ golangci-lint version
golangci-lint has version 1.44.2 built from d58dbde5 on 2022-02-17T20:58:06Z

$ golangci-lint run

[ldez]

oops I tested in the main package I will test it again.

Edit: With another package, I can reproduce the problem.

$ golangci-lint run
pkg/pkg.go:6:8: blank-imports: a blank import should be only in a main or test package, or have a comment justifying it (revive)
import "C"
       ^

[veger]

The main() -part was to reproduce another issue that I seem to have with revive + golangci-lint, but I was not (yet) able to reproduce in a smaller example.
I hope that it will sorted by the same fix as this issue (so I do not need to figure out how to reproduce...)

Also nlreturn and gocritic give me issues (similar to the ones provided by revive), but didn't yet try to reproduce them (depending on issue, might be solved as well hopefully).

The other (cgo-related) issues:

• dupSubExpr: suspicious identical LHS and RHS for == operator (gocritic) <-- there is not comparison going on in the line
• return with no blank line before (nlreturn) <- there is not even a return in the line
• constant-logical-expr: expression always evaluates to true (revive) <-- I am setting a struct member field with a value returned from C, nothing that 'evaluates'

all on code similar to the second line in the example main() function you used initially. But as said, this simple function was not able to reproduce, so something else is influencing it...

[ldez]

seems related to #2449

[veger]

Might be as there seems to be something weird with cgo going on.

But in my case the issue arises with import "C" added: all kinds on incorrect linting issues are shown, whereas in the linked issue the linter issues are gone/hidden. So it looks a little reversed?

[ldez]

revive:

imp.Path "C"
imp.Path "unsafe"

revive inside golangci-lint:

imp.Path "runtime/cgo"
imp.Path "runtime/cgo" _
imp.Path "unsafe"
imp.Path "unsafe" _

[ldez]

revive

pkg/pkg.go
package pkg

import (
        "fmt"
        "unsafe"
)

// #include <stdlib.h>
import "C"

func Foo() {
        cStr := C.CString("test")

        C.free(unsafe.Pointer(cStr))
        fmt.Println("foo")
}

revive inside golangci-lint

/home/ldez/.cache/go-build/56/56a837003f7c4d84b25b7fcca59e280574ded82f5a8520ccd7a732348008126a-d
//go:cgo_ldflag "-g"
//go:cgo_ldflag "-O2"
// Code generated by cmd/cgo; DO NOT EDIT.

package pkg

import "unsafe"

import _ "runtime/cgo"

import "syscall"

var _ syscall.Errno
func _Cgo_ptr(ptr unsafe.Pointer) unsafe.Pointer { return ptr }

//go:linkname _Cgo_always_false runtime.cgoAlwaysFalse
var _Cgo_always_false bool
//go:linkname _Cgo_use runtime.cgoUse
func _Cgo_use(interface{})
type _Ctype__GoString_ string

type _Ctype_char int8

type _Ctype_intgo = _Ctype_ptrdiff_t

type _Ctype_long int64

type _Ctype_ptrdiff_t = _Ctype_long

type _Ctype_void [0]byte

//go:linkname _cgo_runtime_cgocall runtime.cgocall
func _cgo_runtime_cgocall(unsafe.Pointer, uintptr) int32

//go:linkname _cgoCheckPointer runtime.cgoCheckPointer
func _cgoCheckPointer(interface{}, interface{})

//go:linkname _cgoCheckResult runtime.cgoCheckResult
func _cgoCheckResult(interface{})


func _Cfunc_CString(s string) *_Ctype_char {
        p := _cgo_cmalloc(uint64(len(s)+1))
        pp := (*[1<<30]byte)(p)
        copy(pp[:], s)
        pp[len(s)] = 0
        return (*_Ctype_char)(p)
}
//go:cgo_import_static _cgo_3d6db73808d3_Cfunc_free
//go:linkname __cgofn__cgo_3d6db73808d3_Cfunc_free _cgo_3d6db73808d3_Cfunc_free
var __cgofn__cgo_3d6db73808d3_Cfunc_free byte
var _cgo_3d6db73808d3_Cfunc_free = unsafe.Pointer(&__cgofn__cgo_3d6db73808d3_Cfunc_free)

//go:cgo_unsafe_args
func _Cfunc_free(p0 unsafe.Pointer) (r1 _Ctype_void) {
        _cgo_runtime_cgocall(_cgo_3d6db73808d3_Cfunc_free, uintptr(unsafe.Pointer(&p0)))
        if _Cgo_always_false {
                _Cgo_use(p0)
        }
        return
}

//go:cgo_import_static _cgo_3d6db73808d3_Cfunc__Cmalloc
//go:linkname __cgofn__cgo_3d6db73808d3_Cfunc__Cmalloc _cgo_3d6db73808d3_Cfunc__Cmalloc
var __cgofn__cgo_3d6db73808d3_Cfunc__Cmalloc byte
var _cgo_3d6db73808d3_Cfunc__Cmalloc = unsafe.Pointer(&__cgofn__cgo_3d6db73808d3_Cfunc__Cmalloc)

//go:linkname runtime_throw runtime.throw
func runtime_throw(string)

//go:cgo_unsafe_args
func _cgo_cmalloc(p0 uint64) (r1 unsafe.Pointer) {
        _cgo_runtime_cgocall(_cgo_3d6db73808d3_Cfunc__Cmalloc, uintptr(unsafe.Pointer(&p0)))
        if r1 == nil {
                runtime_throw("runtime: C malloc failed")
        }
        return
}

/home/ldez/.cache/go-build/ac/aca88097c12702655e6fedd11b136612c34a0e16093472459dbcf2dc8d633e0d-d
// Code generated by cmd/cgo; DO NOT EDIT.

//line /home/ldez/sources/go/src/github.com/golangci/sandbox/pkg/pkg.go:1:1
package pkg

import (
        "fmt"
        "unsafe"
)

// #include <stdlib.h>
import _ "unsafe"

func Foo() {
        cStr := ( /*line :12:10*/_Cfunc_CString /*line :12:18*/)("test")

        func() { _cgo0 := /*line :14:9*/unsafe.Pointer(cStr); _cgoCheckPointer(_cgo0, nil); _Cfunc_free(_cgo0); }()
        fmt.Println("foo")
}

/home/ldez/.cache/go-build/a5/a55556eced9241dd430120d61dc485042b7bbaf97df622c5d6419e794d0bc66d-d
package pkg
//go:cgo_import_dynamic free free#GLIBC_2.2.5 "libc.so.6"
//go:cgo_import_dynamic _ITM_deregisterTMCloneTable _ITM_deregisterTMCloneTable ""
//go:cgo_import_dynamic __libc_start_main __libc_start_main#GLIBC_2.2.5 "libc.so.6"
//go:cgo_import_dynamic __gmon_start__ __gmon_start__ ""
//go:cgo_import_dynamic malloc malloc#GLIBC_2.2.5 "libc.so.6"
//go:cgo_import_dynamic _ITM_registerTMCloneTable _ITM_registerTMCloneTable ""
//go:cgo_import_dynamic __cxa_finalize __cxa_finalize#GLIBC_2.2.5 "libc.so.6"
//go:cgo_import_dynamic _ _ "libpthread.so.0"
//go:cgo_import_dynamic _ _ "libc.so.6"

So the problem seems related to the "cache".

[veger]

golangci-lint + revive processes the file through cgo and lints that file, whereas revive is not invoking cgo and processes the original.

This (cgo processing) might also cause the other linter issues I mentioned..? 🤔

[ldez]

golangci-lint + cgo => because of the cgo processing, all linters that aren't really an Analyzer can be impacted (revive, go-critic, ...) and the other linters also (depending on the Analyzer implementation)