From 7cc17225e694741720e3da7fb87164b6d6e1b8a3 Mon Sep 17 00:00:00 2001 From: Jing Chen Date: Wed, 16 Oct 2024 18:17:15 +0000 Subject: [PATCH] Remove references to math/rand package's Read function. The helper function is deprecated. The package gvisor.dev/gvisor/pkg/rand depends on crypto/rand which performs worse thatn math/rand, the changes are fine since they are not at any gVisor's hot path. The ultimate goal is to migrate math/rand to math/rand/v2. --- pkg/aio/BUILD | 1 + pkg/aio/aio_test.go | 2 +- pkg/buffer/BUILD | 1 + pkg/buffer/buffer_test.go | 2 +- pkg/buffer/view_test.go | 2 +- pkg/lisafs/BUILD | 1 + pkg/lisafs/sock_test.go | 2 +- pkg/lisafs/testsuite/BUILD | 1 + pkg/lisafs/testsuite/testsuite.go | 2 +- pkg/p9/p9test/BUILD | 1 + pkg/p9/p9test/client_test.go | 3 ++- pkg/tcpip/BUILD | 1 + pkg/tcpip/link/fdbased/BUILD | 1 + pkg/tcpip/link/fdbased/endpoint_test.go | 2 +- pkg/tcpip/network/internal/testutil/BUILD | 1 + .../network/internal/testutil/testutil.go | 2 +- pkg/tcpip/tcpip.go | 2 +- pkg/test/testutil/BUILD | 1 + pkg/test/testutil/testutil.go | 19 ++----------------- test/benchmarks/tcp/BUILD | 1 + test/benchmarks/tcp/tcp_proxy.go | 2 +- test/cmd/test_app/BUILD | 1 + test/cmd/test_app/main.go | 3 ++- test/packetimpact/testbench/BUILD | 1 + test/packetimpact/testbench/testbench.go | 3 ++- test/packetimpact/tests/BUILD | 2 ++ .../tests/ipv4_fragment_reassembly_test.go | 2 +- .../tests/ipv6_fragment_reassembly_test.go | 2 +- tools/go_marshal/analysis/BUILD | 3 +++ tools/go_marshal/analysis/analysis_unsafe.go | 3 ++- 30 files changed, 38 insertions(+), 32 deletions(-) diff --git a/pkg/aio/BUILD b/pkg/aio/BUILD index 850235267d..06a6098543 100644 --- a/pkg/aio/BUILD +++ b/pkg/aio/BUILD @@ -25,6 +25,7 @@ go_test( library = ":aio", deps = [ "//pkg/bitmap", + "//pkg/rand", "@org_golang_x_sys//unix:go_default_library", ], ) diff --git a/pkg/aio/aio_test.go b/pkg/aio/aio_test.go index fc7b5d1b62..37a229bfe0 100644 --- a/pkg/aio/aio_test.go +++ b/pkg/aio/aio_test.go @@ -17,12 +17,12 @@ package aio import ( "bytes" "io" - "math/rand" "os" "testing" "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/bitmap" + "gvisor.dev/gvisor/pkg/rand" ) func TestRead(t *testing.T) { diff --git a/pkg/buffer/BUILD b/pkg/buffer/BUILD index ecf6230bbb..f62991780f 100644 --- a/pkg/buffer/BUILD +++ b/pkg/buffer/BUILD @@ -62,6 +62,7 @@ go_test( ], library = ":buffer", deps = [ + "//pkg/rand", "//pkg/state", "//pkg/tcpip/checksum", "@com_github_google_go_cmp//cmp:go_default_library", diff --git a/pkg/buffer/buffer_test.go b/pkg/buffer/buffer_test.go index e8f18481fa..6e304db9e6 100644 --- a/pkg/buffer/buffer_test.go +++ b/pkg/buffer/buffer_test.go @@ -19,11 +19,11 @@ import ( "context" "fmt" "io" - "math/rand" "slices" "strings" "testing" + "gvisor.dev/gvisor/pkg/rand" "gvisor.dev/gvisor/pkg/state" "gvisor.dev/gvisor/pkg/tcpip/checksum" ) diff --git a/pkg/buffer/view_test.go b/pkg/buffer/view_test.go index 1f660c4e25..d0763ee4ef 100644 --- a/pkg/buffer/view_test.go +++ b/pkg/buffer/view_test.go @@ -16,10 +16,10 @@ package buffer import ( "bytes" - "math/rand" "testing" "github.com/google/go-cmp/cmp" + "gvisor.dev/gvisor/pkg/rand" ) func TestNewView(t *testing.T) { diff --git a/pkg/lisafs/BUILD b/pkg/lisafs/BUILD index bc7faafb33..a37e38f30f 100644 --- a/pkg/lisafs/BUILD +++ b/pkg/lisafs/BUILD @@ -125,6 +125,7 @@ go_test( library = ":lisafs", deps = [ "//pkg/marshal", + "//pkg/rand", "//pkg/sync", "//pkg/unet", "@org_golang_x_sys//unix:go_default_library", diff --git a/pkg/lisafs/sock_test.go b/pkg/lisafs/sock_test.go index 387f4b7a8a..e9825b3dff 100644 --- a/pkg/lisafs/sock_test.go +++ b/pkg/lisafs/sock_test.go @@ -16,12 +16,12 @@ package lisafs import ( "bytes" - "math/rand" "reflect" "testing" "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/marshal" + "gvisor.dev/gvisor/pkg/rand" "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/unet" ) diff --git a/pkg/lisafs/testsuite/BUILD b/pkg/lisafs/testsuite/BUILD index 00f10d2f4d..eb71f7d063 100644 --- a/pkg/lisafs/testsuite/BUILD +++ b/pkg/lisafs/testsuite/BUILD @@ -14,6 +14,7 @@ go_library( "//pkg/abi/linux", "//pkg/context", "//pkg/lisafs", + "//pkg/rand", "//pkg/refs", "//pkg/unet", "@com_github_syndtr_gocapability//capability:go_default_library", diff --git a/pkg/lisafs/testsuite/testsuite.go b/pkg/lisafs/testsuite/testsuite.go index e77bc5be9b..f1565258db 100644 --- a/pkg/lisafs/testsuite/testsuite.go +++ b/pkg/lisafs/testsuite/testsuite.go @@ -19,7 +19,6 @@ package testsuite import ( "bytes" "fmt" - "math/rand" "os" "testing" "time" @@ -29,6 +28,7 @@ import ( "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/lisafs" + "gvisor.dev/gvisor/pkg/rand" "gvisor.dev/gvisor/pkg/refs" "gvisor.dev/gvisor/pkg/unet" ) diff --git a/pkg/p9/p9test/BUILD b/pkg/p9/p9test/BUILD index e9044af97e..8bbf757752 100644 --- a/pkg/p9/p9test/BUILD +++ b/pkg/p9/p9test/BUILD @@ -87,6 +87,7 @@ go_test( deps = [ "//pkg/fd", "//pkg/p9", + "//pkg/rand", "//pkg/sync", "@com_github_golang_mock//gomock:go_default_library", "@org_golang_x_sys//unix:go_default_library", diff --git a/pkg/p9/p9test/client_test.go b/pkg/p9/p9test/client_test.go index 79fdb48664..c6eaf4da1a 100644 --- a/pkg/p9/p9test/client_test.go +++ b/pkg/p9/p9test/client_test.go @@ -29,6 +29,7 @@ import ( "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/fd" "gvisor.dev/gvisor/pkg/p9" + gvisorrand "gvisor.dev/gvisor/pkg/rand" "gvisor.dev/gvisor/pkg/sync" ) @@ -2192,7 +2193,7 @@ func TestReadWriteConcurrent(t *testing.T) { // Initialize random data for each instance. for i := 0; i < instances; i++ { - if _, err := rand.Read(dataSets[i][:]); err != nil { + if _, err := gvisorrand.Read(dataSets[i][:]); err != nil { t.Fatalf("error initializing dataSet#%d, got %v", i, err) } } diff --git a/pkg/tcpip/BUILD b/pkg/tcpip/BUILD index 9e4e0b8cfb..a11a63cdde 100644 --- a/pkg/tcpip/BUILD +++ b/pkg/tcpip/BUILD @@ -49,6 +49,7 @@ go_library( deps = [ "//pkg/atomicbitops", "//pkg/buffer", + "//pkg/rand", "//pkg/sync", "//pkg/waiter", "@org_golang_x_sys//unix:go_default_library", diff --git a/pkg/tcpip/link/fdbased/BUILD b/pkg/tcpip/link/fdbased/BUILD index f61365fce2..94a69f2d26 100644 --- a/pkg/tcpip/link/fdbased/BUILD +++ b/pkg/tcpip/link/fdbased/BUILD @@ -42,6 +42,7 @@ go_test( library = ":fdbased", deps = [ "//pkg/buffer", + "//pkg/rand", "//pkg/refs", "//pkg/tcpip", "//pkg/tcpip/header", diff --git a/pkg/tcpip/link/fdbased/endpoint_test.go b/pkg/tcpip/link/fdbased/endpoint_test.go index 3cb1560b06..cb4fea9157 100644 --- a/pkg/tcpip/link/fdbased/endpoint_test.go +++ b/pkg/tcpip/link/fdbased/endpoint_test.go @@ -20,7 +20,6 @@ package fdbased import ( "bytes" "fmt" - "math/rand" "os" "slices" "testing" @@ -30,6 +29,7 @@ import ( "github.com/google/go-cmp/cmp" "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/buffer" + "gvisor.dev/gvisor/pkg/rand" "gvisor.dev/gvisor/pkg/refs" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/header" diff --git a/pkg/tcpip/network/internal/testutil/BUILD b/pkg/tcpip/network/internal/testutil/BUILD index 148c3d71b3..eb28df1a19 100644 --- a/pkg/tcpip/network/internal/testutil/BUILD +++ b/pkg/tcpip/network/internal/testutil/BUILD @@ -19,6 +19,7 @@ go_library( ], deps = [ "//pkg/buffer", + "//pkg/rand", "//pkg/tcpip", "//pkg/tcpip/checker", "//pkg/tcpip/header", diff --git a/pkg/tcpip/network/internal/testutil/testutil.go b/pkg/tcpip/network/internal/testutil/testutil.go index 1cfd1da449..2ac2db8aa4 100644 --- a/pkg/tcpip/network/internal/testutil/testutil.go +++ b/pkg/tcpip/network/internal/testutil/testutil.go @@ -18,11 +18,11 @@ package testutil import ( "fmt" - "math/rand" "testing" "github.com/google/go-cmp/cmp" "gvisor.dev/gvisor/pkg/buffer" + "gvisor.dev/gvisor/pkg/rand" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/checker" "gvisor.dev/gvisor/pkg/tcpip/header" diff --git a/pkg/tcpip/tcpip.go b/pkg/tcpip/tcpip.go index b89481733f..7839bfb267 100644 --- a/pkg/tcpip/tcpip.go +++ b/pkg/tcpip/tcpip.go @@ -35,7 +35,6 @@ import ( "io" "math" "math/bits" - "math/rand" "net" "reflect" "strconv" @@ -43,6 +42,7 @@ import ( "time" "gvisor.dev/gvisor/pkg/atomicbitops" + "gvisor.dev/gvisor/pkg/rand" "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/waiter" ) diff --git a/pkg/test/testutil/BUILD b/pkg/test/testutil/BUILD index 04d1ad490f..f9d4c5cb17 100644 --- a/pkg/test/testutil/BUILD +++ b/pkg/test/testutil/BUILD @@ -17,6 +17,7 @@ go_library( visibility = ["//:sandbox"], deps = [ "//pkg/sentry/watchdog", + "//pkg/rand", "//pkg/sync", "//runsc/config", "//runsc/flag", diff --git a/pkg/test/testutil/testutil.go b/pkg/test/testutil/testutil.go index df1def1396..16e5c08336 100644 --- a/pkg/test/testutil/testutil.go +++ b/pkg/test/testutil/testutil.go @@ -25,7 +25,6 @@ import ( "io" "log" "math" - "math/rand" "net/http" "os" "os/exec" @@ -40,6 +39,7 @@ import ( "github.com/cenkalti/backoff" specs "github.com/opencontainers/runtime-spec/specs-go" "golang.org/x/sys/unix" + "gvisor.dev/gvisor/pkg/rand" "gvisor.dev/gvisor/pkg/sentry/watchdog" "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/runsc/config" @@ -352,28 +352,13 @@ func writeSpec(dir string, spec *specs.Spec) error { return os.WriteFile(filepath.Join(dir, "config.json"), b, 0755) } -// idRandomSrc is a pseudo random generator used to in RandomID. -var idRandomSrc = rand.New(rand.NewSource(time.Now().UnixNano())) - -// idRandomSrcMtx is the mutex protecting idRandomSrc.Read from being used -// concurrently in different goroutines. -var idRandomSrcMtx sync.Mutex - // RandomID returns 20 random bytes following the given prefix. func RandomID(prefix string) string { - // Read 20 random bytes. b := make([]byte, 20) - // Rand.Read is not safe for concurrent use. Packetimpact tests can be run in - // parallel now, so we have to protect the Read with a mutex. Otherwise we'll - // run into name conflicts. - // https://golang.org/pkg/math/rand/#Rand.Read - idRandomSrcMtx.Lock() // "[Read] always returns len(p) and a nil error." --godoc - if _, err := idRandomSrc.Read(b); err != nil { - idRandomSrcMtx.Unlock() + if _, err := rand.Read(b); err != nil { panic("rand.Read failed: " + err.Error()) } - idRandomSrcMtx.Unlock() if prefix != "" { prefix = prefix + "-" } diff --git a/test/benchmarks/tcp/BUILD b/test/benchmarks/tcp/BUILD index 6e239ae2af..1db1cdcc44 100644 --- a/test/benchmarks/tcp/BUILD +++ b/test/benchmarks/tcp/BUILD @@ -13,6 +13,7 @@ go_binary( ], visibility = ["//:sandbox"], deps = [ + "//pkg/rand", "//pkg/tcpip", "//pkg/tcpip/adapters/gonet", "//pkg/tcpip/link/fdbased", diff --git a/test/benchmarks/tcp/tcp_proxy.go b/test/benchmarks/tcp/tcp_proxy.go index eef0a7f108..a2df5d75eb 100644 --- a/test/benchmarks/tcp/tcp_proxy.go +++ b/test/benchmarks/tcp/tcp_proxy.go @@ -21,7 +21,6 @@ import ( "fmt" "io" "log" - "math/rand" "net" "os" "os/signal" @@ -33,6 +32,7 @@ import ( "time" "golang.org/x/sys/unix" + "gvisor.dev/gvisor/pkg/rand" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/adapters/gonet" "gvisor.dev/gvisor/pkg/tcpip/link/fdbased" diff --git a/test/cmd/test_app/BUILD b/test/cmd/test_app/BUILD index 6035980c44..bc2169db18 100644 --- a/test/cmd/test_app/BUILD +++ b/test/cmd/test_app/BUILD @@ -19,6 +19,7 @@ go_binary( "//test/syscalls/linux:__pkg__", ], deps = [ + "//pkg/rand", "//pkg/test/testutil", "//pkg/unet", "//runsc/flag", diff --git a/test/cmd/test_app/main.go b/test/cmd/test_app/main.go index a213b87791..09db930acf 100644 --- a/test/cmd/test_app/main.go +++ b/test/cmd/test_app/main.go @@ -34,6 +34,7 @@ import ( "github.com/google/subcommands" "github.com/kr/pty" + gvisorrand "gvisor.dev/gvisor/pkg/rand" "gvisor.dev/gvisor/pkg/test/testutil" "gvisor.dev/gvisor/runsc/flag" ) @@ -104,7 +105,7 @@ func (c *fsTreeCreator) Execute(ctx context.Context, f *flag.FlagSet, args ...an } data := make([]byte, fileSize) - rand.Read(data) + gvisorrand.Read(data) for i := uint(0); i < depth; i++ { for j := uint(0); j < numFilesPerLevel; j++ { filePath := filepath.Join(curDir, fmt.Sprintf("file%d", j)) diff --git a/test/packetimpact/testbench/BUILD b/test/packetimpact/testbench/BUILD index bf7b6af28f..7c1fda27aa 100644 --- a/test/packetimpact/testbench/BUILD +++ b/test/packetimpact/testbench/BUILD @@ -21,6 +21,7 @@ go_library( "//pkg/binary", "//pkg/buffer", "//pkg/hostarch", + "//pkg/rand", "//pkg/tcpip", "//pkg/tcpip/checksum", "//pkg/tcpip/header", diff --git a/test/packetimpact/testbench/testbench.go b/test/packetimpact/testbench/testbench.go index 38ae9c1d7c..822f495aae 100644 --- a/test/packetimpact/testbench/testbench.go +++ b/test/packetimpact/testbench/testbench.go @@ -20,10 +20,11 @@ import ( "encoding/json" "flag" "fmt" - "math/rand" "net" "testing" "time" + + "gvisor.dev/gvisor/pkg/rand" ) var ( diff --git a/test/packetimpact/tests/BUILD b/test/packetimpact/tests/BUILD index 9fcedb80f4..f024a99429 100644 --- a/test/packetimpact/tests/BUILD +++ b/test/packetimpact/tests/BUILD @@ -281,6 +281,7 @@ packetimpact_testbench( name = "ipv4_fragment_reassembly", srcs = ["ipv4_fragment_reassembly_test.go"], deps = [ + "//pkg/rand", "//pkg/tcpip/checksum", "//pkg/tcpip/header", "//test/packetimpact/testbench", @@ -293,6 +294,7 @@ packetimpact_testbench( name = "ipv6_fragment_reassembly", srcs = ["ipv6_fragment_reassembly_test.go"], deps = [ + "//pkg/rand", "//pkg/tcpip", "//pkg/tcpip/checksum", "//pkg/tcpip/header", diff --git a/test/packetimpact/tests/ipv4_fragment_reassembly_test.go b/test/packetimpact/tests/ipv4_fragment_reassembly_test.go index b76796358a..150c0d0b04 100644 --- a/test/packetimpact/tests/ipv4_fragment_reassembly_test.go +++ b/test/packetimpact/tests/ipv4_fragment_reassembly_test.go @@ -16,11 +16,11 @@ package ipv4_fragment_reassembly_test import ( "flag" - "math/rand" "testing" "time" "github.com/google/go-cmp/cmp" + "gvisor.dev/gvisor/pkg/rand" "gvisor.dev/gvisor/pkg/tcpip/checksum" "gvisor.dev/gvisor/pkg/tcpip/header" "gvisor.dev/gvisor/test/packetimpact/testbench" diff --git a/test/packetimpact/tests/ipv6_fragment_reassembly_test.go b/test/packetimpact/tests/ipv6_fragment_reassembly_test.go index 9ed96141fa..cd537b00a4 100644 --- a/test/packetimpact/tests/ipv6_fragment_reassembly_test.go +++ b/test/packetimpact/tests/ipv6_fragment_reassembly_test.go @@ -16,11 +16,11 @@ package ipv6_fragment_reassembly_test import ( "flag" - "math/rand" "testing" "time" "github.com/google/go-cmp/cmp" + "gvisor.dev/gvisor/pkg/rand" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/checksum" "gvisor.dev/gvisor/pkg/tcpip/header" diff --git a/tools/go_marshal/analysis/BUILD b/tools/go_marshal/analysis/BUILD index 69699a09a8..a0f2b7cafc 100644 --- a/tools/go_marshal/analysis/BUILD +++ b/tools/go_marshal/analysis/BUILD @@ -8,6 +8,9 @@ go_library( name = "analysis", testonly = 1, srcs = ["analysis_unsafe.go"], + deps = [ + "//pkg/rand", + ], visibility = [ "//:sandbox", ], diff --git a/tools/go_marshal/analysis/analysis_unsafe.go b/tools/go_marshal/analysis/analysis_unsafe.go index 8fb3a1ff85..f939082b9b 100644 --- a/tools/go_marshal/analysis/analysis_unsafe.go +++ b/tools/go_marshal/analysis/analysis_unsafe.go @@ -26,10 +26,11 @@ package analysis import ( "fmt" - "math/rand" "reflect" "testing" "unsafe" + + "gvisor.dev/gvisor/pkg/rand" ) // RandomizeValue assigns random value(s) to an abitrary type. This is intended