Skip to content

Conversation

@austinderek
Copy link

Irregularly typedmemmove and bulkBarrierPreWrite crashes on unaligned
arguments. By aligning the arguments this is fixed.

Fixes golang#46893


🔄 This is a mirror of upstream PR #74868

@austinderek austinderek force-pushed the master branch 30 times, most recently from 121e5bb to 7a1679d Compare August 5, 2025 07:04
@austinderek austinderek force-pushed the master branch 18 times, most recently from d5b9503 to 5bf50a0 Compare October 3, 2025 12:02
@austinderek austinderek closed this Oct 3, 2025
@austinderek austinderek deleted the feature/fix-unaligned-arguments branch October 3, 2025 12:03
@staging
Copy link

staging bot commented Oct 3, 2025

HackerOne Code Security Review

🟢 Scan Complete: 35 Issue(s)
🟠 Validation Complete: One or more Issues looked potentially actionable, so this was escalated to our network of engineers for manual review. Once this is complete you'll see an update posted.

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

ℹ️ 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/encoding/xml/read.go Line 261 The code changes replace type assertions using Interface().(Type) with the new reflect.TypeAssert[Type] function. While this is generally safer, the implementation now checks only if CanInterface() is true before attempting the type assertion, but doesn't verify that the type actually implements the interface before the assertion. The original code explicitly checked both CanInterface() AND Type().Implements(interfaceType) before performing the assertion, which was a more defensive approach. This could potentially lead to runtime panics if a value that doesn't implement the expected interface is processed.
src/cmd/link/dwarf_test.go Line 389 The change adds a check to skip tests on platforms that don't support DWARF symbol tables in executables, and adds a special case for Solaris in the TestFlagW function. This is a test-only change that improves platform compatibility and doesn't introduce security issues.
src/runtime/rt0_netbsd_arm64.s Line 10 The change replaces a BL main(SB) instruction with JMP main(SB). While BL is a branch with link (saves return address), JMP doesn't save the return address. This means the function will not properly return to its caller. In security-sensitive code, this could potentially lead to stack corruption or unexpected behavior if something relies on proper function return.
src/crypto/internal/fips140/entropy/entropy.go Line 128 The entropy source uses a linear congruential generator (LCG) with fixed parameters on lines 128-129 to generate memory access patterns. While this is used only for creating timing variations and not directly for cryptographic purposes, LCGs are predictable and could potentially be exploited to predict the memory access pattern, which might reduce the effectiveness of the entropy collection in certain environments.
src/crypto/internal/fips140/entropy/entropy.go Line 118 The code uses unsafe.Pointer in touchMemory function (line 118) to perform unaligned memory access. While this is intentional for the entropy generation, it could potentially cause memory corruption or undefined behavior on some architectures that don't support unaligned access. The alignment check on line 116 helps mitigate this, but the use of unsafe operations always warrants careful review.
src/runtime/proc.go Line 812 The function getGodebugEarly() has been modified to return a boolean indicating whether the OS provides environment variables early in the initialization sequence. Previously, it would return an empty string if the environment variable wasn't found, which could be ambiguous. Now it returns both the value and a boolean success flag. However, the function also now uses gostringnocopy instead of gostring when extracting the GODEBUG value. This could potentially lead to memory safety issues if the underlying memory for the environment variable is modified while the string is still in use, as gostringnocopy doesn't make a copy of the string data.
src/cmd/link/internal/ld/lib.go Line 1455 The change on line 1455 modifies the condition for adding the -Wl,-S flag to suppress debugging symbols. Previously, this flag was not added for AIX, but now it's also not added for Solaris. This is because the Solaris linker's -S flag has a different meaning than what's intended here. Without this fix, the linker would pass an incorrect flag to the Solaris linker, potentially causing unexpected behavior or errors during linking.
src/crypto/internal/fips140/drbg/rand.go Line 33 The code changes how entropy is obtained for the FIPS 140-3 cryptographic module. The previous implementation used entropy.Depleted to get entropy from an external source, but now it uses a new internal entropy source. If the new entropy source (getEntropy()) is not properly implemented or doesn't meet FIPS requirements, this could compromise the security of all random number generation in FIPS mode. Additionally, the error handling in the new implementation might allow for continued operation after multiple entropy source failures.
src/crypto/internal/fips140/mlkem/mlkem768.go Line 175 The code adds calls to fipsSelfTest() in several functions (generateKey, GenerateKeyInternal768, Encapsulate, EncapsulateInternal, and Decapsulate) but there's no implementation of this function visible in the file. If this function is meant to perform security-critical validation before cryptographic operations, its absence or failure to execute properly could lead to using the ML-KEM implementation without proper validation. This could potentially bypass security checks intended to ensure the implementation meets FIPS requirements.
src/debug/elf/file.go Line 1834 The new putUint function has a potential integer overflow vulnerability. In line 1834, the check math.MaxUint64-start < length is intended to prevent integer overflow when calculating start+length, but it's not sufficient. If start is very large and length is small, the subtraction could overflow, causing the check to fail. A safer approach would be to check if start > math.MaxUint64-length.
src/cmd/cgo/internal/testcshared/cshared_test.go Line 276 The change removes '_cgo_dummy_export' from the Windows DLL export definition file. This symbol was previously exported but is now removed. The code has been refactored to use a new function 'checkNumberOfExportedSymbolsWindows' that properly accounts for this change. The code now correctly handles the case where '_cgo_stub_export' is exported when there are no other symbols exported. This change improves the security posture by reducing the attack surface through fewer exported symbols.
src/cmd/link/internal/ld/pe.go Line 1788 The function peCreateExportFile writes a file to a path specified by *flagTmpdir without proper validation. This could lead to path traversal vulnerabilities if flagTmpdir contains malicious input. The function should validate that the path is safe and within expected boundaries before writing to it. Additionally, the file is created with 0666 permissions, which may be overly permissive depending on the security context.
src/crypto/tls/handshake_server.go Line 360 The change on line 360 modifies an error message to use %q instead of %s when formatting the clientProtos variable. While this is a security improvement, it's a minor one. Using %q ensures that special characters in protocol names are properly escaped in the error message, which prevents potential log injection attacks if these error messages are logged. Without proper quoting, maliciously crafted ALPN protocol names could potentially be used to inject content into logs.
src/crypto/internal/fips140/mlkem/mlkem1024.go Line 116 The code adds calls to fipsSelfTest() in several functions (GenerateKey1024, GenerateKeyInternal1024, Encapsulate, EncapsulateInternal, Decapsulate) but there's no implementation of this function visible in the code. If this function is meant to verify FIPS compliance or perform security checks, but fails silently or doesn't properly validate security properties, it could create a false sense of security while allowing insecure operations to proceed.
src/cmd/link/internal/loader/loader.go Line 1115 The change introduces a new method ForAllCgoExportStatic that returns an iterator over symbols marked with the 'cgo_export_static' compiler directive. This method uses the new Go 'iter' package (imported on line 19) to create an iterator sequence. While the implementation itself is secure, there's a potential issue if the caller doesn't properly handle the iteration. If the underlying map l.attrCgoExportStatic is modified during iteration (by another goroutine), this could lead to undefined behavior or potential crashes. The code should be used in a single-threaded context or with proper synchronization.
src/cmd/link/internal/ld/macho.go Line 109 The changes add two new Mach-O ARM64 relocation types: MACHO_ARM64_RELOC_SUBTRACTOR (line 109) and MACHO_ARM64_RELOC_POINTER_TO_GOT (line 115). These are standard relocation types in the Mach-O format and don't introduce any security vulnerabilities. The changes are simply adding constants to support these relocation types in the Go linker.
src/net/tcpsock_test.go Line 515 The test has been modified to use t.Errorf instead of t.Fatalf in the TestTCPReadWriteAllocs function. While this change itself is not a security vulnerability, it could potentially mask memory allocation issues in the TCP implementation. Memory allocations in network code can sometimes lead to performance degradation under load or even resource exhaustion attacks. However, since this is just a test file and the change only affects how test failures are reported (error vs fatal), the security impact is minimal.
src/internal/poll/fd_windows.go Line 241 The code introduces runtime.Pinner to pin memory during I/O operations, but there's a potential race condition in the pin/unpin mechanism. The pin() method pins memory for read/write operations, but if an error occurs between pin() and execIO(), the memory might be unpinned too early or not at all. While execIO() has a deferred unpin, there are code paths where pin() is called but execIO() might not be called, potentially leading to memory leaks or use-after-free scenarios.
src/cmd/compile/internal/ssa/rewrite.go Line 2750 The added function isDictArgSym checks if a symbol is a dictionary argument by comparing its name to a constant LocalDictName. While this is not a vulnerability itself, it's worth noting that the function doesn't perform any null checks before dereferencing the symbol as an *ir.Name. If sym is nil or not of type *ir.Name, this would cause a panic. This is a defensive programming issue rather than a security vulnerability.
src/encoding/xml/marshal.go Line 448 The code changes replace the use of type assertions via Type.Implements() with the new reflect.TypeAssert function. While this is generally a good change, there's a potential issue with the error handling. The original code checked if a type implements an interface before attempting to use it, but the new code uses reflect.TypeAssert which returns a boolean indicating success. If the type assertion fails, the code continues execution rather than returning an error. This could lead to unexpected behavior if the code assumes the assertion succeeded when it actually failed.
src/cmd/cgo/out.go Line 1061 The code adds a maxAlign variable to track the maximum alignment among all fields in a struct, and then uses this value to explicitly align the struct with __attribute__((aligned(maxAlign))). This fixes a potential security issue where improper alignment could lead to memory access violations or undefined behavior when passing data between Go and C. Without proper alignment, the struct could be accessed in ways that trigger undefined behavior, especially on architectures with strict alignment requirements.
src/internal/buildcfg/cfg.go Line 450 The code changes remove conditional checks for WASM features (SatConv and SignExt) and make them always enabled. While this change itself doesn't introduce security vulnerabilities, it modifies the behavior of the gogoarchTags() function to always include these features in the tags list regardless of the actual GOWASM configuration. This could potentially lead to unexpected behavior if any security-sensitive code relies on these tags for feature detection or conditional compilation. The change should be carefully reviewed to ensure it doesn't break any security assumptions in dependent code.
src/cmd/link/internal/ld/symtab.go Line 657 The change on line 657 replaces a custom slice creation with sliceSym(pcln.pclntab). The original code was using slice(pcln.pclntab, uint64(ldr.SymSize(pcln.pclntab))) which is functionally equivalent to sliceSym but inconsistent with the pattern used elsewhere. While not a direct security issue, inconsistent memory handling patterns can lead to maintenance errors that introduce vulnerabilities.
src/cmd/link/internal/ld/symtab.go Line 648 The change on line 648 replaces sliceSym(pcln.cutab) with slice(pcln.cutab, uint64(ldr.SymSize(pcln.cutab))/4). This divides the size by 4, which suggests the original code was incorrectly calculating the slice length. The cutab likely contains 4-byte elements, but the original code was treating the byte size as the element count. This could lead to accessing memory beyond the intended bounds, potentially causing crashes or memory corruption in the runtime.
src/runtime/runtime1.go Line 405 The code refactoring splits the debug variable parsing into multiple functions, but introduces a potential issue where parseRuntimeDebugVars no longer initializes the godebugEnv variable immediately. This could lead to a race condition where other parts of the runtime might access the uninitialized godebugEnv variable between the calls to parseRuntimeDebugVars and finishDebugVarsSetup. If any code depends on godebugEnv being initialized immediately after parsing debug variables, it could lead to unexpected behavior.
🧰 Analysis tools

⏱️ Latest scan covered changes up to commit 46ae8b9 (latest)

austinderek pushed a commit that referenced this pull request Oct 15, 2025
This change adds a generated 8-bit bitmask for use in functions
shouldEscape and ishex.

Function shouldEscape is now inlineable. Function escape is now much faster;
function unescape is a bit faster. Here are some benchmark results (no change
to allocations):

goos: darwin
goarch: amd64
pkg: net/url
cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
                    │     old      │                 new                 │
                    │    sec/op    │   sec/op     vs base                │
QueryEscape/#00-8      58.38n ± 1%   35.98n ± 1%  -38.38% (p=0.000 n=20)
QueryEscape/#1-8     303.50n ± 0%   94.77n ± 0%  -68.77% (p=0.000 n=20)
QueryEscape/#2-8     202.90n ± 0%   78.66n ± 1%  -61.23% (p=0.000 n=20)
QueryEscape/#3-8      444.5n ± 0%   145.9n ± 0%  -67.17% (p=0.000 n=20)
QueryEscape/#4-8     2678.0n ± 0%   913.7n ± 0%  -65.88% (p=0.000 n=20)
PathEscape/#00-8       81.34n ± 0%   44.64n ± 1%  -45.12% (p=0.000 n=20)
PathEscape/#1-8      307.65n ± 0%   96.71n ± 1%  -68.56% (p=0.000 n=20)
PathEscape/#2-8      200.80n ± 1%   78.25n ± 0%  -61.03% (p=0.000 n=20)
PathEscape/#3-8       450.1n ± 1%   145.5n ± 0%  -67.67% (p=0.000 n=20)
PathEscape/#4-8      2663.5n ± 0%   876.5n ± 0%  -67.09% (p=0.000 n=20)
QueryUnescape/#00-8    53.32n ± 1%   51.67n ± 1%   -3.09% (p=0.000 n=20)
QueryUnescape/#1-8    161.0n ± 1%   136.2n ± 1%  -15.40% (p=0.000 n=20)
QueryUnescape/#2-8    126.1n ± 1%   118.3n ± 1%   -6.23% (p=0.000 n=20)
QueryUnescape/#3-8    294.6n ± 0%   273.1n ± 0%   -7.30% (p=0.000 n=20)
QueryUnescape/#4-8    1.511µ ± 0%   1.411µ ± 0%   -6.62% (p=0.000 n=20)
PathUnescape/#00-8     63.84n ± 1%   53.59n ± 1%  -16.05% (p=0.000 n=20)
PathUnescape/#1-8     163.6n ± 3%   137.9n ± 1%  -15.71% (p=0.000 n=20)
PathUnescape/#2-8     126.4n ± 1%   119.1n ± 1%   -5.78% (p=0.000 n=20)
PathUnescape/#3-8     294.2n ± 0%   273.3n ± 0%   -7.12% (p=0.000 n=20)
PathUnescape/#4-8     1.554µ ± 0%   1.417µ ± 0%   -8.78% (p=0.000 n=20)
geomean                277.8n        162.7n       -41.44%

This change draws heavy inspiration from CL 174998, which showed promise but
stalled years ago.

Updates golang#17860
austinderek pushed a commit that referenced this pull request Oct 15, 2025
This change adds a generated 8-bit bitmask for use in functions
shouldEscape and ishex.

Function shouldEscape is now inlineable. Function escape is now much faster;
function unescape is a bit faster. Here are some benchmark results (no change
to allocations):

goos: darwin
goarch: amd64
pkg: net/url
cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
                    │     old      │                 new                 │
                    │    sec/op    │   sec/op     vs base                │
QueryEscape/#00-8      58.38n ± 1%   35.98n ± 1%  -38.38% (p=0.000 n=20)
QueryEscape/#1-8     303.50n ± 0%   94.77n ± 0%  -68.77% (p=0.000 n=20)
QueryEscape/#2-8     202.90n ± 0%   78.66n ± 1%  -61.23% (p=0.000 n=20)
QueryEscape/#3-8      444.5n ± 0%   145.9n ± 0%  -67.17% (p=0.000 n=20)
QueryEscape/#4-8     2678.0n ± 0%   913.7n ± 0%  -65.88% (p=0.000 n=20)
PathEscape/#00-8       81.34n ± 0%   44.64n ± 1%  -45.12% (p=0.000 n=20)
PathEscape/#1-8      307.65n ± 0%   96.71n ± 1%  -68.56% (p=0.000 n=20)
PathEscape/#2-8      200.80n ± 1%   78.25n ± 0%  -61.03% (p=0.000 n=20)
PathEscape/#3-8       450.1n ± 1%   145.5n ± 0%  -67.67% (p=0.000 n=20)
PathEscape/#4-8      2663.5n ± 0%   876.5n ± 0%  -67.09% (p=0.000 n=20)
QueryUnescape/#00-8    53.32n ± 1%   51.67n ± 1%   -3.09% (p=0.000 n=20)
QueryUnescape/#1-8    161.0n ± 1%   136.2n ± 1%  -15.40% (p=0.000 n=20)
QueryUnescape/#2-8    126.1n ± 1%   118.3n ± 1%   -6.23% (p=0.000 n=20)
QueryUnescape/#3-8    294.6n ± 0%   273.1n ± 0%   -7.30% (p=0.000 n=20)
QueryUnescape/#4-8    1.511µ ± 0%   1.411µ ± 0%   -6.62% (p=0.000 n=20)
PathUnescape/#00-8     63.84n ± 1%   53.59n ± 1%  -16.05% (p=0.000 n=20)
PathUnescape/#1-8     163.6n ± 3%   137.9n ± 1%  -15.71% (p=0.000 n=20)
PathUnescape/#2-8     126.4n ± 1%   119.1n ± 1%   -5.78% (p=0.000 n=20)
PathUnescape/#3-8     294.2n ± 0%   273.3n ± 0%   -7.12% (p=0.000 n=20)
PathUnescape/#4-8     1.554µ ± 0%   1.417µ ± 0%   -8.78% (p=0.000 n=20)
geomean                277.8n        162.7n       -41.44%

This change draws heavy inspiration from CL 174998, which showed promise but
stalled years ago.

Updates golang#17860
austinderek pushed a commit that referenced this pull request Oct 16, 2025
This change adds a generated 8-bit bitmask for use in functions
shouldEscape and ishex.

Function shouldEscape is now inlineable. Function escape is now much faster;
function unescape is a bit faster. Here are some benchmark results (no change
to allocations):

goos: darwin
goarch: amd64
pkg: net/url
cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
                    │     old      │                 new                 │
                    │    sec/op    │   sec/op     vs base                │
QueryEscape/#00-8      58.38n ± 1%   35.98n ± 1%  -38.38% (p=0.000 n=20)
QueryEscape/#1-8     303.50n ± 0%   94.77n ± 0%  -68.77% (p=0.000 n=20)
QueryEscape/#2-8     202.90n ± 0%   78.66n ± 1%  -61.23% (p=0.000 n=20)
QueryEscape/#3-8      444.5n ± 0%   145.9n ± 0%  -67.17% (p=0.000 n=20)
QueryEscape/#4-8     2678.0n ± 0%   913.7n ± 0%  -65.88% (p=0.000 n=20)
PathEscape/#00-8       81.34n ± 0%   44.64n ± 1%  -45.12% (p=0.000 n=20)
PathEscape/#1-8      307.65n ± 0%   96.71n ± 1%  -68.56% (p=0.000 n=20)
PathEscape/#2-8      200.80n ± 1%   78.25n ± 0%  -61.03% (p=0.000 n=20)
PathEscape/#3-8       450.1n ± 1%   145.5n ± 0%  -67.67% (p=0.000 n=20)
PathEscape/#4-8      2663.5n ± 0%   876.5n ± 0%  -67.09% (p=0.000 n=20)
QueryUnescape/#00-8    53.32n ± 1%   51.67n ± 1%   -3.09% (p=0.000 n=20)
QueryUnescape/#1-8    161.0n ± 1%   136.2n ± 1%  -15.40% (p=0.000 n=20)
QueryUnescape/#2-8    126.1n ± 1%   118.3n ± 1%   -6.23% (p=0.000 n=20)
QueryUnescape/#3-8    294.6n ± 0%   273.1n ± 0%   -7.30% (p=0.000 n=20)
QueryUnescape/#4-8    1.511µ ± 0%   1.411µ ± 0%   -6.62% (p=0.000 n=20)
PathUnescape/#00-8     63.84n ± 1%   53.59n ± 1%  -16.05% (p=0.000 n=20)
PathUnescape/#1-8     163.6n ± 3%   137.9n ± 1%  -15.71% (p=0.000 n=20)
PathUnescape/#2-8     126.4n ± 1%   119.1n ± 1%   -5.78% (p=0.000 n=20)
PathUnescape/#3-8     294.2n ± 0%   273.3n ± 0%   -7.12% (p=0.000 n=20)
PathUnescape/#4-8     1.554µ ± 0%   1.417µ ± 0%   -8.78% (p=0.000 n=20)
geomean                277.8n        162.7n       -41.44%

This change draws heavy inspiration from CL 174998, which showed promise but
stalled years ago.

Updates golang#17860
austinderek pushed a commit that referenced this pull request Oct 17, 2025
This change adds a generated 8-bit bitmask for use in functions
shouldEscape and ishex.

Function shouldEscape is now inlineable. Function escape is now much faster;
function unescape is a bit faster. Here are some benchmark results (no change
to allocations):

goos: darwin
goarch: amd64
pkg: net/url
cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
                    │     old      │                 new                 │
                    │    sec/op    │   sec/op     vs base                │
QueryEscape/#00-8      58.38n ± 1%   35.98n ± 1%  -38.38% (p=0.000 n=20)
QueryEscape/#1-8     303.50n ± 0%   94.77n ± 0%  -68.77% (p=0.000 n=20)
QueryEscape/#2-8     202.90n ± 0%   78.66n ± 1%  -61.23% (p=0.000 n=20)
QueryEscape/#3-8      444.5n ± 0%   145.9n ± 0%  -67.17% (p=0.000 n=20)
QueryEscape/#4-8     2678.0n ± 0%   913.7n ± 0%  -65.88% (p=0.000 n=20)
PathEscape/#00-8       81.34n ± 0%   44.64n ± 1%  -45.12% (p=0.000 n=20)
PathEscape/#1-8      307.65n ± 0%   96.71n ± 1%  -68.56% (p=0.000 n=20)
PathEscape/#2-8      200.80n ± 1%   78.25n ± 0%  -61.03% (p=0.000 n=20)
PathEscape/#3-8       450.1n ± 1%   145.5n ± 0%  -67.67% (p=0.000 n=20)
PathEscape/#4-8      2663.5n ± 0%   876.5n ± 0%  -67.09% (p=0.000 n=20)
QueryUnescape/#00-8    53.32n ± 1%   51.67n ± 1%   -3.09% (p=0.000 n=20)
QueryUnescape/#1-8    161.0n ± 1%   136.2n ± 1%  -15.40% (p=0.000 n=20)
QueryUnescape/#2-8    126.1n ± 1%   118.3n ± 1%   -6.23% (p=0.000 n=20)
QueryUnescape/#3-8    294.6n ± 0%   273.1n ± 0%   -7.30% (p=0.000 n=20)
QueryUnescape/#4-8    1.511µ ± 0%   1.411µ ± 0%   -6.62% (p=0.000 n=20)
PathUnescape/#00-8     63.84n ± 1%   53.59n ± 1%  -16.05% (p=0.000 n=20)
PathUnescape/#1-8     163.6n ± 3%   137.9n ± 1%  -15.71% (p=0.000 n=20)
PathUnescape/#2-8     126.4n ± 1%   119.1n ± 1%   -5.78% (p=0.000 n=20)
PathUnescape/#3-8     294.2n ± 0%   273.3n ± 0%   -7.12% (p=0.000 n=20)
PathUnescape/#4-8     1.554µ ± 0%   1.417µ ± 0%   -8.78% (p=0.000 n=20)
geomean                277.8n        162.7n       -41.44%

This change draws heavy inspiration from CL 174998, which showed promise but
stalled years ago.

Updates golang#17860
austinderek pushed a commit that referenced this pull request Oct 20, 2025
This change adds benchmarks for Encode and reverts what CL 617356 did in
this package. At the moment, using maps.Keys in conjunction with
slices.Sorted indeed causes a bunch of closures to escape to heap.
Moreover, all other things being equal, pre-sizing the slice in which
we collect the keys is beneficial to performance when they are "many" (>8)
keys because it results in fewer allocations than if we don't pre-size the
slice.

Here are some benchmark results:

goos: darwin
goarch: amd64
pkg: net/url
cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
                                                           │     old      │                 new                 │
                                                           │    sec/op    │   sec/op     vs base                │
EncodeQuery/#00-8                                             2.051n ± 1%   2.343n ± 1%  +14.24% (p=0.000 n=20)
EncodeQuery/#1-8                                             2.337n ± 1%   2.458n ± 4%   +5.16% (p=0.000 n=20)
EncodeQuery/oe=utf8&q=puppies-8                               489.6n ± 0%   284.5n ± 0%  -41.88% (p=0.000 n=20)
EncodeQuery/q=dogs&q=%26&q=7-8                                397.2n ± 1%   231.7n ± 1%  -41.66% (p=0.000 n=20)
EncodeQuery/a=a1&a=a2&a=a3&b=b1&b=b2&b=b3&c=c1&c=c2&c=c3-8    743.1n ± 0%   519.0n ± 0%  -30.16% (p=0.000 n=20)
EncodeQuery/a=a&b=b&c=c&d=d&e=e&f=f&g=g&h=h&i=i-8            1324.0n ± 0%   931.0n ± 0%  -29.68% (p=0.000 n=20)
geomean                                                       98.57n        75.38n       -23.53%

                                                           │      old      │                 new                  │
                                                           │     B/op      │    B/op     vs base                  │
EncodeQuery/#00-8                                             0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=20) ¹
EncodeQuery/#1-8                                             0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=20) ¹
EncodeQuery/oe=utf8&q=puppies-8                              168.00 ± 0%     56.00 ± 0%  -66.67% (p=0.000 n=20)
EncodeQuery/q=dogs&q=%26&q=7-8                               112.00 ± 0%     32.00 ± 0%  -71.43% (p=0.000 n=20)
EncodeQuery/a=a1&a=a2&a=a3&b=b1&b=b2&b=b3&c=c1&c=c2&c=c3-8    296.0 ± 0%     168.0 ± 0%  -43.24% (p=0.000 n=20)
EncodeQuery/a=a&b=b&c=c&d=d&e=e&f=f&g=g&h=h&i=i-8             680.0 ± 0%     264.0 ± 0%  -61.18% (p=0.000 n=20)
geomean                                                                  ²               -47.48%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                                                           │      old      │                 new                  │
                                                           │   allocs/op   │ allocs/op   vs base                  │
EncodeQuery/#00-8                                             0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=20) ¹
EncodeQuery/#1-8                                             0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=20) ¹
EncodeQuery/oe=utf8&q=puppies-8                               8.000 ± 0%     3.000 ± 0%  -62.50% (p=0.000 n=20)
EncodeQuery/q=dogs&q=%26&q=7-8                                7.000 ± 0%     3.000 ± 0%  -57.14% (p=0.000 n=20)
EncodeQuery/a=a1&a=a2&a=a3&b=b1&b=b2&b=b3&c=c1&c=c2&c=c3-8   10.000 ± 0%     5.000 ± 0%  -50.00% (p=0.000 n=20)
EncodeQuery/a=a&b=b&c=c&d=d&e=e&f=f&g=g&h=h&i=i-8            12.000 ± 0%     5.000 ± 0%  -58.33% (p=0.000 n=20)
geomean                                                                  ²               -43.23%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

Change-Id: Ia0d7579f90434f0546d93b680ab18b47a1ffbdac
GitHub-Last-Rev: f25be71
GitHub-Pull-Request: golang#75874
Reviewed-on: https://go-review.googlesource.com/c/go/+/711280
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Florian Lehner <lehner.florian86@gmail.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Sean Liao <sean@liao.dev>
Reviewed-by: t hepudds <thepudds1460@gmail.com>
austinderek pushed a commit that referenced this pull request Oct 23, 2025
This change adds a generated 8-bit bitmask for use in functions
shouldEscape and ishex.

Function shouldEscape is now inlineable. Function escape is now much faster;
function unescape is a bit faster. Here are some benchmark results (no change
to allocations):

goos: darwin
goarch: amd64
pkg: net/url
cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
                    │     old      │                 new                 │
                    │    sec/op    │   sec/op     vs base                │
QueryEscape/#00-8      58.38n ± 1%   35.98n ± 1%  -38.38% (p=0.000 n=20)
QueryEscape/#1-8     303.50n ± 0%   94.77n ± 0%  -68.77% (p=0.000 n=20)
QueryEscape/#2-8     202.90n ± 0%   78.66n ± 1%  -61.23% (p=0.000 n=20)
QueryEscape/#3-8      444.5n ± 0%   145.9n ± 0%  -67.17% (p=0.000 n=20)
QueryEscape/#4-8     2678.0n ± 0%   913.7n ± 0%  -65.88% (p=0.000 n=20)
PathEscape/#00-8       81.34n ± 0%   44.64n ± 1%  -45.12% (p=0.000 n=20)
PathEscape/#1-8      307.65n ± 0%   96.71n ± 1%  -68.56% (p=0.000 n=20)
PathEscape/#2-8      200.80n ± 1%   78.25n ± 0%  -61.03% (p=0.000 n=20)
PathEscape/#3-8       450.1n ± 1%   145.5n ± 0%  -67.67% (p=0.000 n=20)
PathEscape/#4-8      2663.5n ± 0%   876.5n ± 0%  -67.09% (p=0.000 n=20)
QueryUnescape/#00-8    53.32n ± 1%   51.67n ± 1%   -3.09% (p=0.000 n=20)
QueryUnescape/#1-8    161.0n ± 1%   136.2n ± 1%  -15.40% (p=0.000 n=20)
QueryUnescape/#2-8    126.1n ± 1%   118.3n ± 1%   -6.23% (p=0.000 n=20)
QueryUnescape/#3-8    294.6n ± 0%   273.1n ± 0%   -7.30% (p=0.000 n=20)
QueryUnescape/#4-8    1.511µ ± 0%   1.411µ ± 0%   -6.62% (p=0.000 n=20)
PathUnescape/#00-8     63.84n ± 1%   53.59n ± 1%  -16.05% (p=0.000 n=20)
PathUnescape/#1-8     163.6n ± 3%   137.9n ± 1%  -15.71% (p=0.000 n=20)
PathUnescape/#2-8     126.4n ± 1%   119.1n ± 1%   -5.78% (p=0.000 n=20)
PathUnescape/#3-8     294.2n ± 0%   273.3n ± 0%   -7.12% (p=0.000 n=20)
PathUnescape/#4-8     1.554µ ± 0%   1.417µ ± 0%   -8.78% (p=0.000 n=20)
geomean                277.8n        162.7n       -41.44%

This change draws heavy inspiration from CL 174998, which showed promise but
stalled years ago.

Updates golang#17860
austinderek pushed a commit that referenced this pull request Oct 23, 2025
This change adds a generated 8-bit bitmask for use in functions shouldEscape and ishex.

Function shouldEscape is now inlineable. Function escape is now much faster;
function unescape is a bit faster. Here are some benchmark results (no change
to allocations):

goos: darwin
goarch: amd64
pkg: net/url
cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
                    │     old      │                 new                 │
                    │    sec/op    │   sec/op     vs base                │
QueryEscape/#00-8      58.38n ± 1%   35.98n ± 1%  -38.38% (p=0.000 n=20)
QueryEscape/#1-8     303.50n ± 0%   94.77n ± 0%  -68.77% (p=0.000 n=20)
QueryEscape/#2-8     202.90n ± 0%   78.66n ± 1%  -61.23% (p=0.000 n=20)
QueryEscape/#3-8      444.5n ± 0%   145.9n ± 0%  -67.17% (p=0.000 n=20)
QueryEscape/#4-8     2678.0n ± 0%   913.7n ± 0%  -65.88% (p=0.000 n=20)
PathEscape/#00-8       81.34n ± 0%   44.64n ± 1%  -45.12% (p=0.000 n=20)
PathEscape/#1-8      307.65n ± 0%   96.71n ± 1%  -68.56% (p=0.000 n=20)
PathEscape/#2-8      200.80n ± 1%   78.25n ± 0%  -61.03% (p=0.000 n=20)
PathEscape/#3-8       450.1n ± 1%   145.5n ± 0%  -67.67% (p=0.000 n=20)
PathEscape/#4-8      2663.5n ± 0%   876.5n ± 0%  -67.09% (p=0.000 n=20)
QueryUnescape/#00-8    53.32n ± 1%   51.67n ± 1%   -3.09% (p=0.000 n=20)
QueryUnescape/#1-8    161.0n ± 1%   136.2n ± 1%  -15.40% (p=0.000 n=20)
QueryUnescape/#2-8    126.1n ± 1%   118.3n ± 1%   -6.23% (p=0.000 n=20)
QueryUnescape/#3-8    294.6n ± 0%   273.1n ± 0%   -7.30% (p=0.000 n=20)
QueryUnescape/#4-8    1.511µ ± 0%   1.411µ ± 0%   -6.62% (p=0.000 n=20)
PathUnescape/#00-8     63.84n ± 1%   53.59n ± 1%  -16.05% (p=0.000 n=20)
PathUnescape/#1-8     163.6n ± 3%   137.9n ± 1%  -15.71% (p=0.000 n=20)
PathUnescape/#2-8     126.4n ± 1%   119.1n ± 1%   -5.78% (p=0.000 n=20)
PathUnescape/#3-8     294.2n ± 0%   273.3n ± 0%   -7.12% (p=0.000 n=20)
PathUnescape/#4-8     1.554µ ± 0%   1.417µ ± 0%   -8.78% (p=0.000 n=20)
geomean                277.8n        162.7n       -41.44%

This change draws heavy inspiration from CL 174998, which showed promise but stalled years ago.

Updates golang#17860

Change-Id: Idcbb1696608998b9e2fc91e1f2a488d8f1f6028c
GitHub-Last-Rev: ff360c2
GitHub-Pull-Request: golang#75914
Reviewed-on: https://go-review.googlesource.com/c/go/+/712200
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Jorropo <jorropo.pgm@gmail.com>
Reviewed-by: Takuto Nagami <logica0419@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
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 this pull request may close these issues.