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

x/tools/analysis/composite: Add InternalFuzzTarget to unkeyed literals allowlist #51623

Closed
abhinav opened this issue Mar 11, 2022 · 6 comments
Closed
Labels
FrozenDueToAge Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@abhinav
Copy link
Contributor

abhinav commented Mar 11, 2022

What version of Go are you using (go version)?

% go version
go version go1.18rc1 linux/amd64

Does this issue reproduce with the latest release?

With the latest RC, yes. Not with the latest stable.

What operating system and processor architecture are you using (go env)?

Linux, amd64, inside Bazel with rules_go v0.30.0.

go env Output
% go env
GO111MODULE="off"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/user/.cache/go-build"
GOENV="/home/user/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/user/go-code/cache/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/user/go-code/main"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/user/.cache/bazel/_bazel_user/a048389d619430c68eb2be362fdff40d/external/go_sdk_linux_amd64"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/user/.cache/bazel/_bazel_user/a048389d619430c68eb2be362fdff40d/external/go_sdk_linux_amd64/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18rc1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
GOWORK=""
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-build2439313169=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I wrote a minimal fuzz test using the example from the official docs and ran bazel build.

FuzzHex
package fuzzing

import (
        "bytes"
        "encoding/hex"
        "testing"
)

func FuzzHex(f *testing.F) {
        for _, seed := range [][]byte{{}, {0}, {9}, {0xa}, {0xf}, {1, 2, 3, 4}} {
                f.Add(seed)
        }
        f.Fuzz(func(t *testing.T, in []byte) {
                enc := hex.EncodeToString(in)
                out, err := hex.DecodeString(enc)
                if err != nil {
                        t.Fatalf("%v: decode: %v", in, err)
                }
                if !bytes.Equal(in, out) {
                        t.Fatalf("%v: not equal after round trip: %v", in, out)
                }
        })
}

I then ran bazel build on the target:

% bazel build :go_default_test

What did you expect to see?

A successful build.

What did you see instead?

The following error:

[...]/go_default_test_/testmain.go:43:3: testing.InternalFuzzTarget composite literal uses unkeyed fields (composites)

Notes

Sorry, just to clarify: This is not a request to support fuzzing in rules_go. I'm aware that this is not the place to request that.

This is a request to add testing.InternalFuzzTarget to the allowlist in go/analysis/passes/composite, without which tests that use fuzzing do not pass the lint check—at least in Bazel (I was not able to get the same error with vanilla go test -vet=composites). I've verified that adding InternalFuzzTarget to the list resolves this issue in Bazel.

@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Mar 11, 2022
@gopherbot gopherbot added this to the Unreleased milestone Mar 11, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/391875 mentions this issue: go/analysis/passes/composite: allow InternalFuzzTarget

@findleyr
Copy link
Member

@bcmills @matloob do you have any sense for why this would affect bazel, but not go test? I suppose we do not run go vet on the generated test targets.

In any case, the associated CL looks reasonable to me.

@findleyr
Copy link
Member

@abhinav even if we merge now it seems unlikely that this will be fixed for 1.18, due to the imminence of the release. Can you explain further if/how this impacts your workflow?

@abhinav
Copy link
Contributor Author

abhinav commented Mar 14, 2022

@findleyr, oh this is not a blocker for us to upgrade in any way. I've patched our copy of x/tools with the change. Thanks!

@bcmills
Copy link
Contributor

bcmills commented Mar 14, 2022

I suppose we do not run go vet on the generated test targets.

I suspect that's exactly it.

@rysteboe
Copy link

Are there plans to release this as part of a future 1.18.x version?

@golang golang locked and limited conversation to collaborators Jun 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

5 participants