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

Nogo should use original source file instead of coverage instrumented ones for static analysis #3769

Closed
emmaxy opened this issue Nov 28, 2023 · 0 comments · Fixed by #3770
Closed

Comments

@emmaxy
Copy link
Contributor

emmaxy commented Nov 28, 2023

What version of rules_go are you using?

0.39.1

What version of gazelle are you using?

0.28.0

What version of Bazel are you using?

6.3.0

Does this issue reproduce with the latest releases of all the above?

What operating system and processor architecture are you using?

MacOS M1 Ventura and Linux AMD64

Any other potentially useful information about your toolchain?

What did you do?

With nogo setup in monorepo, I'm trying to enable a gochecknoinits analyer, which check that no inits functions are present in Go code. When I ran bazel coverage //..., nogo reports errors for init() functions it found on non-existing lines in some source files.

What did you expect to see?

nogo should not report errors on files where there is no init() function in the original source file.

What did you see instead?

When coverage mode is set, the source file is modified with an init() function added to register files to coverdata, which is where nogo static analysis reported the error.

// Append an init function.
var buf = bytes.NewBuffer(editor.Bytes())
fmt.Fprintf(buf, `
func init() {
%s.RegisterFile(%q,
%[3]s.Count[:],
%[3]s.Pos[:],
%[3]s.NumStmt[:])
}
`, coverdataName, srcName, varName)

Ideally nogo static analysis should not run against coverage instrumented source files, but the original ones. Under GoCompilePkg actions, the nogoSrcsOrigin map was introduced in #3216 for this purpose. But this mechanism doesn't work when cgo is enabled, as all source files path would change after cgo compilation:

srcDir, goSrcs, objFiles, err = cgo2(goenv, goSrcs, cgoSrcs, cSrcs, cxxSrcs, objcSrcs, objcxxSrcs, sSrcs, hSrcs, packagePath, packageName, cc, cppFlags, cFlags, cxxFlags, objcFlags, objcxxFlags, ldFlags, cgoExportHPath)

hence no file would match the nogoSrcsOrigin map, resulting in nogo uses the coverage instrumented source files instead:

// If source is found in the origin map, that means it's likely to be a generated source file
// so feed the original source file to static analyzers instead of the generated one.
//
// If origin is empty, that means the generated source file is not based on a user-provided source file
// thus ignore that entry entirely.
if originSrc, ok := nogoSrcsOrigin[goSrc]; ok {
if originSrc != "" {
nogoSrcs = append(nogoSrcs, originSrc)
}
continue
}

I'd like to propose a fix to this issue by having cgo compile source files that are not coverage instrumented for nogo's use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant