One year ago I implemented a simple Go linter which leverages the power of the Go compiler to highlight potential bugs and it was able to find plenty of issues in the popular open source projects (go-spacemesh, cloudflared, wal-g, ssh3, answer, pdfcpu). Go Go go-spacemesh cloudflared wal-g ssh3 answer pdfcpu The linter uses a quite interesting approach as it directly uses Go compiler output to find places of code which were removed by the compiler – allowing it to detect non-trivial issues as long as the Go compiler can performs clever optimizations. Go Go So, let me tell a little story about linter development and meanwhile you can run it (govanish) on your project and check if your code vanished from the binary without your approval! govanish The idea of using Go compilerzoutput for a linter is not unique. For example, gcassert linter uses similar approach and validates that all functions marked with special comment will be inlined by the compiler. The idea of using Go compilerzoutput for a linter is not unique. For example, gcassert linter uses similar approach and validates that all functions marked with special comment will be inlined by the compiler. gcassert Beginning Everything started with me playing around with Go and suddenly discovering tricky nuance of how Go interfaces are working. Go Go Internally, every interface instance has two components: one that represents the underlying concrete type of the instance and another that represents the actual value of the instance. While this is not a surprise, it turns out that for interfaces to be considered equal, the Go compiler checks both components for equality. This behavior starting to play weirdly when you operate with nil values and nil interfaces (more detailed explanation can be found here). type value Go both nil nil here Look at the following code snippet: package main import "net/url" func alwaysNilType() *url.Error { return nil } func alwaysNilInterface() error { return alwaysNilType() } func main() { if err := alwaysNilInterface(); err == nil { panic("never called") } } package main import "net/url" func alwaysNilType() *url.Error { return nil } func alwaysNilInterface() error { return alwaysNilType() } func main() { if err := alwaysNilInterface(); err == nil { panic("never called") } } Turns out that alwaysNilInterface always returns interface instance which has nil value but non-nil type information (because type is *url.Error) and this makes the value from this function unequal to a nil interface which has both type and value components set to nil! alwaysNilInterface nil non- nil *url.Error nil nil That’s why you should always use error interface in Go instead of more specific types despite that in other languages you usually return the most specific type, accept the most generic type) That’s why you should always use error interface in Go instead of more specific types despite that in other languages you usually return the most specific type, accept the most generic type) return the most specific type, accept the most generic type Assembly After discovering this nuance, I wondered how interface comparison works, which led me to examine the generated assembly for the snippet above (by the way, Golang uses Plan9 assembler syntax which is annoying sometimes as it’s very hard to find description of instructions). by the way, Golang uses Plan9 assembler syntax which is annoying sometimes as it’s very hard to find description of instructions Plan9 Luckily, Go provides an easy way to inspect the generated assembler code: we can just pass -gcflags=-S argument to the go build command. The output of the compiler looks like following: -gcflags=-S go build ... main.main STEXT nosplit size=1 args=0x0 locals=0x0 funcid=0x0 align=0x0 0x0000 00000 (/home/sivukhin/code/go-nil-interface/main.go:8) TEXT main.main(SB), NOSPLIT|NOFRAME|ABIInternal, $0-0 0x0000 00000 (/home/sivukhin/code/go-nil-interface/main.go:8) FUNCDATA $0, gclocals·g2BeySu+wFnoycgXfElmcg==(SB) 0x0000 00000 (/home/sivukhin/code/go-nil-interface/main.go:8) FUNCDATA $1, gclocals·g2BeySu+wFnoycgXfElmcg==(SB) 0x0000 00000 (/home/sivukhin/code/go-nil-interface/main.go:12) RET 0x0000 c3 . rel 0+0 t=R_USEIFACE type:*net/url.Error+0 rel 0+0 t=R_USEIFACE type:string+0 ... ... main.main STEXT nosplit size=1 args=0x0 locals=0x0 funcid=0x0 align=0x0 0x0000 00000 (/home/sivukhin/code/go-nil-interface/main.go:8) TEXT main.main(SB), NOSPLIT|NOFRAME|ABIInternal, $0-0 0x0000 00000 (/home/sivukhin/code/go-nil-interface/main.go:8) FUNCDATA $0, gclocals·g2BeySu+wFnoycgXfElmcg==(SB) 0x0000 00000 (/home/sivukhin/code/go-nil-interface/main.go:8) FUNCDATA $1, gclocals·g2BeySu+wFnoycgXfElmcg==(SB) 0x0000 00000 (/home/sivukhin/code/go-nil-interface/main.go:12) RET 0x0000 c3 . rel 0+0 t=R_USEIFACE type:*net/url.Error+0 rel 0+0 t=R_USEIFACE type:string+0 ... That’s interesting, because compiler completely removed conditional code from the main function body and just generates “no-op” executable. This is kind of expected behaviour: optimizing compiler should simplify code and remove “useless” operations – that’s why we love them. But from the developer perspective it’s actually bit suspicious, because the removal of code from the binary may indicate that something unexpected has happened: we wrote code err == nil condition on purpose and it’s shouldn’t result in constant result independent of the function inputs. completely removed main err == nil There is another interesting bit in the generated assembly – you can notice that every instruction annotated with the line of code which “produced” the sequence of assembler operations. That’s cool! Let’s look at which lines are actually in use in the final assembler code based on the compiler output: $> go test -c -gcflags=-S main.go 2>&1 | grep -Po "main.go:\d+" | uniq main.go:5 main.go:6 main.go:8 main.go:12 $> go test -c -gcflags=-S main.go 2>&1 | grep -Po "main.go:\d+" | uniq main.go:5 main.go:6 main.go:8 main.go:12 We can see that line 10 is missing in the output which is the hint that compiler completely removed logic from this line in the final assembly. That’s actually nice! So, what if we try to utilize this combination of compiler output introspection and the fact that sometimes compiler can delete parts of the code which we definitely wanted to see in the binary in some form? Can we create a linter out from that idea which will allow us to find non-trivial bugs in the go code? How noisy this linter will be? Let’s find out! Govanish So, that’s how govanish linter actually works! govanish It builds your project with the gcflags=-S option and analyzes which lines of code are missed from the generated assembly. Not surprisingly, on a big projects linter is pretty noisy, that’s why few heuristics were implemented to ignore from analysis few common cases when Go compiler remove or replace chunk of code logic in the final assembler listing: gcflags=-S Go Code like for k := range m { delete(m, k) } is not analyzed because Go compiler is clever enough to use runtime.mapclear() function in this case Constant true/false conditions are not analyzed, because they are usually written for local debugging and intent is clear that the code should be removed by compiler Type constructors are not analyzed because in most cases they are no-op (e.g. []byte(net.IP{})) Platform dependent code is not analyzed, because it will always trigger govanish on a wrong platform Conditions which uses functions with constant returns (e.g. return nil) are not analyzed if function is defined in the module Code like for k := range m { delete(m, k) } is not analyzed because Go compiler is clever enough to use runtime.mapclear() function in this case for k := range m { delete(m, k) } Go runtime.mapclear() Constant true/false conditions are not analyzed, because they are usually written for local debugging and intent is clear that the code should be removed by compiler true false Type constructors are not analyzed because in most cases they are no-op (e.g. []byte(net.IP{})) []byte(net.IP{}) Platform dependent code is not analyzed, because it will always trigger govanish on a wrong platform govanish Conditions which uses functions with constant returns (e.g. return nil) are not analyzed if function is defined in the module return nil Still, even with these heuristics in place, govanish produces a lot of false positives especially around error checking code, because a lot of Go functions follow generic approach to return an error while actually they are never failing (e.g. func (b *strings.Builder) Write(p []byte) (int, error)). But on the other hand I find it fun to run govanish from time to time on popular projects and discover some bugs with it. govanish func (b *strings.Builder) Write(p []byte) (int, error) govanish Govanish in action Let’s see govanish in action on some real project – I decided to pick the rook repository (there is an obviously an element of cherry-picking as I ran govanish on a several projects before rook and detected no real issues there). govanish rook govanish rook So, you can clone the repository, install govanish and run it: govanish $> go install github.com/sivukhin/govanish@latest $> git clone https://github.com/rook/rook $> cd rook $> govanish -format github 2025/02/23 15:21:34 module path: /home/sivukhin/code/kekek/rook 2025/02/23 15:21:34 ready to compile project at path '/home/sivukhin/code/kekek/rook' for assembly inspection 2025/02/23 15:21:34 ready to parse assembly output 2025/02/23 15:21:40 ready to normalize assembly lines (size 428) 2025/02/23 15:21:41 built func registry: 2446 entries 2025/02/23 15:21:41 ready to analyze module AST ::warning file=pkg/operator/k8sutil/pod.go,line=167,endLine=167::seems like code vanished from compiled binary ::warning file=pkg/operator/k8sutil/pod.go,line=185,endLine=185::seems like code vanished from compiled binary ::warning file=pkg/operator/discover/discover.go,line=440,endLine=444::seems like code vanished from compiled binary ::warning file=pkg/operator/test/client.go,line=99,endLine=99::seems like code vanished from compiled binary $> go install github.com/sivukhin/govanish@latest $> git clone https://github.com/rook/rook $> cd rook $> govanish -format github 2025/02/23 15:21:34 module path: /home/sivukhin/code/kekek/rook 2025/02/23 15:21:34 ready to compile project at path '/home/sivukhin/code/kekek/rook' for assembly inspection 2025/02/23 15:21:34 ready to parse assembly output 2025/02/23 15:21:40 ready to normalize assembly lines (size 428) 2025/02/23 15:21:41 built func registry: 2446 entries 2025/02/23 15:21:41 ready to analyze module AST ::warning file=pkg/operator/k8sutil/pod.go,line=167,endLine=167::seems like code vanished from compiled binary ::warning file=pkg/operator/k8sutil/pod.go,line=185,endLine=185::seems like code vanished from compiled binary ::warning file=pkg/operator/discover/discover.go,line=440,endLine=444::seems like code vanished from compiled binary ::warning file=pkg/operator/test/client.go,line=99,endLine=99::seems like code vanished from compiled binary govanish identified 4 code regions which do not match ignore heuristics and vanished from the final assembler listing. Let’s look more closely all of them. govanish In pkg/operator/k8sutil/pod.go file at line 167 there is a return value which were removed by Go for some reason. In pkg/operator/k8sutil/pod.go file at line 167 there is a return value which were removed by Go for some reason. pkg/operator/k8sutil/pod.go 167 func GetRunningPod(ctx context.Context, clientset kubernetes.Interface) (*v1.Pod, error) { podName := os.Getenv(PodNameEnvVar) if podName == "" { return nil, fmt.Errorf("...") } podNamespace := os.Getenv(PodNamespaceEnvVar) if podName == "" { return nil, fmt.Errorf("...") } ... func GetRunningPod(ctx context.Context, clientset kubernetes.Interface) (*v1.Pod, error) { podName := os.Getenv(PodNameEnvVar) if podName == "" { return nil, fmt.Errorf("...") } podNamespace := os.Getenv(PodNamespaceEnvVar) if podName == "" { return nil, fmt.Errorf("...") } ... Here, we actually spotted a real bug, because the code obviously need to check podNamespace variable at line 165 instead of duplicate check of podName variable (by the way, Goland also warns about this issue with Condition is always false rule) bug podNamespace 165 podName by the way, Goland Condition is always false In pkg/operator/k8sutil/pod.go line 185 disappeared from the final assembly but actually compiler emitted necessary logic but attributed assembly listing slightly differently which led govanish to think that this line actually disappeared. func GetMatchingContainer(containers []v1.Container, name string) (v1.Container, error) { var result *v1.Container if len(containers) == 1 { // if there is only one pod, use its image rather than require a set container name result = &containers[0] } ... In pkg/operator/k8sutil/pod.go line 185 disappeared from the final assembly but actually compiler emitted necessary logic but attributed assembly listing slightly differently which led govanish to think that this line actually disappeared. func GetMatchingContainer(containers []v1.Container, name string) (v1.Container, error) { var result *v1.Container if len(containers) == 1 { // if there is only one pod, use its image rather than require a set container name result = &containers[0] } ... In pkg/operator/k8sutil/pod.go line 185 disappeared from the final assembly but actually compiler emitted necessary logic but attributed assembly listing slightly differently which led govanish to think that this line actually disappeared. pkg/operator/k8sutil/pod.go 185 govanish func GetMatchingContainer(containers []v1.Container, name string) (v1.Container, error) { var result *v1.Container if len(containers) == 1 { // if there is only one pod, use its image rather than require a set container name result = &containers[0] } ... func GetMatchingContainer(containers []v1.Container, name string) (v1.Container, error) { var result *v1.Container if len(containers) == 1 { // if there is only one pod, use its image rather than require a set container name result = &containers[0] } ... In pkg/operator/discover/discover.go file range between lines 440-444 were removed by Go and this is also a bug because this is an else if condition which comes after else if len(filter) >= 0 which is always true, making the subsequent else if condition redundant! (here, Goland is silent and do not see any issue with the code) In pkg/operator/discover/discover.go file range between lines 440-444 were removed by Go and this is also a bug because this is an else if condition which comes after else if len(filter) >= 0 which is always true, making the subsequent else if condition redundant! (here, Goland is silent and do not see any issue with the code) pkg/operator/discover/discover.go 440-444 bug else if else if len(filter) >= 0 else if here, Goland } else if len(filter) >= 0 { for i := range nodeDevices { //TODO support filter based on other keys matched, err := regexp.Match(filter, []byte(nodeDevices[i].Name)) if err == nil && matched { d := cephv1.Device{ Name: nodeDevices[i].Name, } claimedDevices = append(claimedDevices, nodeDevices[i]) results = append(results, d) } } } else if useAllDevices { for i := range nodeDevices { ... } else if len(filter) >= 0 { for i := range nodeDevices { //TODO support filter based on other keys matched, err := regexp.Match(filter, []byte(nodeDevices[i].Name)) if err == nil && matched { d := cephv1.Device{ Name: nodeDevices[i].Name, } claimedDevices = append(claimedDevices, nodeDevices[i]) results = append(results, d) } } } else if useAllDevices { for i := range nodeDevices { ... In pkg/operator/test/client.go file cast check code were removed from the binary because compiler proved that failure condition can never happen. This is not a bug and pretty frequent false positive case for govanish. fd, ok := d.(*fakediscovery.FakeDiscovery) if !ok { panic(fmt.Errorf("failed to get fake clientset's fake discovery")) } In pkg/operator/test/client.go file cast check code were removed from the binary because compiler proved that failure condition can never happen. This is not a bug and pretty frequent false positive case for govanish. fd, ok := d.(*fakediscovery.FakeDiscovery) if !ok { panic(fmt.Errorf("failed to get fake clientset's fake discovery")) } In pkg/operator/test/client.go file cast check code were removed from the binary because compiler proved that failure condition can never happen. This is not a bug and pretty frequent false positive case for govanish. pkg/operator/test/client.go govanish fd, ok := d.(*fakediscovery.FakeDiscovery) if !ok { panic(fmt.Errorf("failed to get fake clientset's fake discovery")) } fd, ok := d.(*fakediscovery.FakeDiscovery) if !ok { panic(fmt.Errorf("failed to get fake clientset's fake discovery")) } Overall, in the rook repository, govanish found two bugs with a 50% false positive rate – which is pretty cool! While the average false positive rate of the linter is higher, govanish has still proven to be useful and capable of finding bugs that other linters miss. Give it a try and see how much code Go removes without your approval! rook govanish govanish Go
