all: use slices.Sort

[austinderek]

This is a follow-up for CL 709756.

Split into two CLs to avoid a single CL being too large.

Updates golang#57433

---

🔄 This is a mirror of upstream PR golang#75789

[staging]

PullRequest network review has been cancelled

You can reactivate the code review job from the PullRequest dashboard - or - by adding [pr] to the title of this code review.

[staging]

HackerOne Code Security Review

🟢 Scan Complete: 136 Issue(s)

Here's how the code changes were interpreted and info about the tools used for scanning.

📖 Summary of Changes

The changes involve migrating from the traditional `sort` package to the newer `slices` package for sorting operations across multiple internal files. Specifically, `sort.Strings()` calls have been replaced with `slices.Sort()` in coverage formatting, profile encoding, and profile merging methods, demonstrating a consistent update to Go's standard library sorting approach.

File	Summary
src/internal/coverage/cformat/format.go	Removed the "sort" import and replaced "sort.Strings(pkgs)" with "slices.Sort(pkgs)" in the EmitTextual method, maintaining the same sorting functionality with the slices package.
src/internal/profile/encode.go	Replaced sort.Strings() with slices.Sort() in the preEncode method, indicating a migration to the newer Go standard library sorting function.
src/internal/profile/merge.go	The file was updated to replace the sort package with the new slices package for sorting, specifically in the key() methods of Sample and other structs. This change reflects Go's newer standard library sorting approach.

ℹ️ Issues Detected

NOTE: These may not require action!

Below are unvalidated results from the Analysis Tools that ran during the latest scan for transparency. We investigate each of these for accuracy and relevance before surfacing them as a potential problem.

How will I know if something is a problem?
When validation completes, any concerns that warrant attention prior to merge will be posted as inline comments. These will show up in 2 ways:

• Expert review (most cases): Issues will be posted by experts who manually reviewed and validated them. These are real HackerOne engineers (not bots) reviewing through an integrated IDE-like tool. You can communicate with them like any other reviewer. They'll stay assigned and get notified with commit & comment updates.
• Automatically: In cases where our validation checks have highest confidence the problem is legitimate and urgent. These will include a description of contextual reasoning why & actionable next steps.

File & Line	Issue
src/archive/tar/writer_test.go Line 1367	Changed from using sort.Strings(names) to slices.Sort(names). This is a safe change as it's just switching to the newer slices package for sorting, which has equivalent functionality for this use case.
src/go/ast/commentmap_test.go Line 135	Changed from using sort.Strings() to slices.Sort() for string sorting. This is a safe change as the slices package provides equivalent functionality.
src/internal/profile/encode.go Line 35	The change from sort.Strings() to slices.Sort() is generally safe, but it's important to note that slices.Sort() was introduced in Go 1.21. If this code needs to be compatible with older Go versions, this change could cause compilation failures. While not a direct security vulnerability, compatibility issues can lead to deployment problems or force users to use older, potentially less secure versions of the software.
src/internal/coverage/cformat/format.go Line 185	The change replaces sort.Strings(pkgs) with slices.Sort(pkgs). This is a safe change as both functions sort string slices in ascending order. The slices.Sort function was introduced in Go 1.21 as a more generic sorting function. There are no security implications from this change.
src/internal/profile/merge.go Line 9	The change from sort.Strings() to slices.Sort() is generally safe, but there's a potential issue if the Go version used doesn't support the slices package (introduced in Go 1.21). If this code is compiled with an older Go version, it will fail to compile. This is a compatibility concern rather than a direct security vulnerability.
src/go/types/api_test.go Line 3164	The change replaces sort.Strings(got) with slices.Sort(got), which is a safe modernization. The slices package is already imported, and this change simply uses the newer, more idiomatic API instead of the older sort package function. This is not a security issue.
src/cmp/cmp_test.go Line 98	Changed from using sort.Float64s() to slices.Sort() for float64 sorting. This is a safe change as the slices package provides equivalent functionality.
src/crypto/cipher/ctr_aes_test.go Line 288	Changed from using sort.Ints() to slices.Sort() for integer sorting. This is a safe change as the slices package provides equivalent functionality.
doc/go_spec.html Line 3176	There's a typo in line 3176 where the closing tag for the link to 'Underlying_types' is missing, which has been fixed in the changed line. This is a minor documentation issue and doesn't present any security concerns.
src/runtime/proc.go Line 812	The changes in lines 792-821 modify the getGodebugEarly function to return a boolean indicating success, but there's a potential issue at line 812 where gostringnocopy is used instead of the original gostring. This could lead to memory safety issues if the string is modified while it's being used.
src/cmd/link/internal/ld/xcoff.go Line 1782	The change replaces a manual loop over all symbols to check for the AttrCgoExport attribute with a call to ldr.ForAllCgoExportStatic(). This is a safer approach as it uses a dedicated iterator function that's designed to return only the symbols with the CgoExport attribute. However, the code still performs the same checks on the returned symbols, so the security posture remains unchanged. This is a refactoring improvement rather than a security issue.
src/crypto/internal/fips140/entropy/entropy.go Line 115	The touchMemory function uses unsafe pointer operations to manipulate memory. This could lead to memory corruption if the index calculation is incorrect. The function uses atomic operations but there's a risk of buffer overflow if idx is not properly bounds-checked against the ScratchBuffer size.
src/crypto/internal/fips140/mlkem/mlkem1024.go Line 295	Added FIPS self-test call without checking the return value. If the self-test fails, the function will continue execution potentially using insecure cryptographic operations.
src/crypto/internal/fips140/mlkem/mlkem768.go Line 175	The added fipsSelfTest() function is called in multiple places (lines 175, 189, 342, 354, 460) but is not defined in the provided code. If this function is not properly implemented or doesn't perform adequate validation, it could lead to security issues where cryptographic operations proceed without proper self-testing, potentially allowing compromised or weak implementations to be used.
src/runtime/select.go Line 86	The code changes from direct field access to using getter methods for sudog fields (c and elem). While this change itself is not inherently problematic, it introduces a potential race condition if the getter methods aren't properly synchronized. Without seeing the implementation of these getter methods, there's a risk that concurrent access to these fields could lead to data races or memory corruption, especially in a concurrent runtime environment like Go's scheduler.
src/cmd/compile/internal/walk/walk.go Line 283	The added code in mayCall function introduces a special case for literal values, returning false to indicate they don't require function calls. While this optimization is generally safe, it could potentially lead to incorrect behavior if the race detector or other instrumentation is expected to run on all expressions including literals. The change bypasses the instrumentation check on line 288 for literal values, which could lead to race conditions being missed if literals are involved in a race.
src/runtime/testdata/testgoroutineleakprofile/goker/istio16224.go Line 38	In the Run method, there's a potential deadlock when stopping. The code attempts to read from m.eventCh after receiving from the stop channel, but doesn't properly handle the case where the channel might be empty, potentially causing a goroutine leak.
src/cmd/go/internal/modget/get.go Line 301	The change on line 301 modifies how the ForceUseModules flag is accessed. Instead of directly setting a global variable modload.ForceUseModules, it's now setting the property on a state object modload.LoaderState.ForceUseModules. This change itself doesn't introduce a security vulnerability, but it represents a change in how module loading state is managed - moving from a global variable to a state object. This is generally a positive change from a security perspective as it reduces the risks associated with global state.
src/runtime/testdata/testgoroutineleakprofile/goker/moby17176.go Line 35	This code contains a mutex deadlock vulnerability. In the cleanupDeletedDevices() method, when nrDeletedDevices == 0, the function returns without unlocking the mutex that was locked at the beginning of the function. This will cause any subsequent attempts to acquire the lock to block indefinitely, leading to goroutine leaks.
src/runtime/mgc.go Line 379	The code changes introduce goroutine leak detection functionality. The change from regular uint32 variables to atomic.Uint32 for markrootNext and markrootJobs could lead to race conditions if code elsewhere assumes these fields can be accessed without atomic operations. This is particularly concerning since the code previously accessed these fields directly without atomic operations, and existing code might still do so.
src/crypto/internal/fips140/mlkem/cast.go Line 15	The code changes the initialization pattern from a direct init() function to using sync.OnceFunc(), but doesn't actually call the function anywhere in the provided code. This means the FIPS self-test might never be executed, potentially bypassing required security validation in FIPS mode.
src/cmd/compile/internal/wasm/ssa.go Line 427	The code changes remove conditional logic that was checking for the availability of saturating conversion instructions in WebAssembly. Previously, the code would use helper functions (WasmTruncS and WasmTruncU) when saturating conversions weren't available. Now the code unconditionally uses the native WebAssembly instructions (I64TruncSatF32S, I64TruncSatF64S, I64TruncSatF32U, I64TruncSatF64U) without checking if they're supported. This could cause runtime failures on WebAssembly environments that don't support these instructions.
src/net/http/pprof/pprof.go Line 357	The code adds a new profile type 'goroutineleak' to the profileSupportsDelta map and conditionally adds its description based on the goexperiment.GoroutineLeakProfile flag. While this doesn't introduce a direct security vulnerability, it's worth noting that exposing leak profiling information could potentially reveal sensitive application information to attackers if the /debug/pprof/ endpoint is accessible without proper authentication. This is a general concern with all profiling endpoints, but adding a new one extends the attack surface.
src/runtime/traceback.go Line 1230	Added support for a new goroutine status '_Gleaked' but the waitreason check in the goroutineheader function was only updated for this status in one place (line 1230) but not in the waitfor calculation (line 1236). This could lead to incorrect wait time reporting for leaked goroutines.
src/runtime/testdata/testgoroutineleakprofile/goker/moby28462.go Line 82	The CloseMonitorChannel method sends to s.stop channel without checking if there's a receiver, which could block indefinitely if the monitor goroutine has already exited or isn't listening, causing a goroutine leak.

🧰 Analysis tools

• [ ✅ ] HackerOne AI Code Analysis
• [ ✅ ] HackerOne AI Code Validation
• [ ✅ ] semgrep
• [ ✅ ] gosec
• [ ✅ ] bandit

⏱️ Latest scan covered changes up to commit 96340a0 (latest)