2 files changed, 71 insertions, 22 deletions

diff --git a/src/runtime/debug/garbage_test.go b/src/runtime/debug/garbage_test.go
index 69e769ecf2..7213bbe641 100644
--- a/src/runtime/debug/garbage_test.go
+++ b/src/runtime/debug/garbage_test.go
@@ -6,6 +6,7 @@ package debug_test
 
 import (
 	"internal/testenv"
+	"os"
 	"runtime"
 	. "runtime/debug"
 	"testing"
@@ -87,27 +88,71 @@ func TestReadGCStats(t *testing.T) {
 	}
 }
 
-var big = make([]byte, 1<<20)
+var big []byte
 
 func TestFreeOSMemory(t *testing.T) {
-	var ms1, ms2 runtime.MemStats
+	// Tests FreeOSMemory by making big susceptible to collection
+	// and checking that at least that much memory is returned to
+	// the OS after.
 
-	if big == nil {
-		t.Skip("test is not reliable when run multiple times")
-	}
-	big = nil
+	const bigBytes = 32 << 20
+	big = make([]byte, bigBytes)
+
+	// Make sure any in-progress GCs are complete.
 	runtime.GC()
-	runtime.ReadMemStats(&ms1)
+
+	var before runtime.MemStats
+	runtime.ReadMemStats(&before)
+
+	// Clear the last reference to the big allocation, making it
+	// susceptible to collection.
+	big = nil
+
+	// FreeOSMemory runs a GC cycle before releasing memory,
+	// so it's fine to skip a GC here.
+	//
+	// It's possible the background scavenger runs concurrently
+	// with this function and does most of the work for it.
+	// If that happens, it's OK. What we want is a test that fails
+	// often if FreeOSMemory does not work correctly, and a test
+	// that passes every time if it does.
 	FreeOSMemory()
-	runtime.ReadMemStats(&ms2)
-	if ms1.HeapReleased >= ms2.HeapReleased {
-		t.Errorf("released before=%d; released after=%d; did not go up", ms1.HeapReleased, ms2.HeapReleased)
+
+	var after runtime.MemStats
+	runtime.ReadMemStats(&after)
+
+	// Check to make sure that the big allocation (now freed)
+	// had its memory shift into HeapReleased as a result of that
+	// FreeOSMemory.
+	if after.HeapReleased <= before.HeapReleased {
+		t.Fatalf("no memory released: %d -> %d", before.HeapReleased, after.HeapReleased)
+	}
+
+	// Check to make sure bigBytes was released, plus some slack. Pages may get
+	// allocated in between the two measurements above for a variety for reasons,
+	// most commonly for GC work bufs. Since this can get fairly high, depending
+	// on scheduling and what GOMAXPROCS is, give a lot of slack up-front.
+	//
+	// Add a little more slack too if the page size is bigger than the runtime page size.
+	// "big" could end up unaligned on its ends, forcing the scavenger to skip at worst
+	// 2x pages.
+	slack := uint64(bigBytes / 2)
+	pageSize := uint64(os.Getpagesize())
+	if pageSize > 8<<10 {
+		slack += pageSize * 2
+	}
+	if slack > bigBytes {
+		// We basically already checked this.
+		return
+	}
+	if after.HeapReleased-before.HeapReleased < bigBytes-slack {
+		t.Fatalf("less than %d released: %d -> %d", bigBytes, before.HeapReleased, after.HeapReleased)
 	}
 }
 
 var (
-	setGCPercentBallast interface{}
-	setGCPercentSink    interface{}
+	setGCPercentBallast any
+	setGCPercentSink    any
 )
 
 func TestSetGCPercent(t *testing.T) {
diff --git a/src/runtime/debug/mod.go b/src/runtime/debug/mod.go
index 14b99f5735..14a496a8eb 100644
--- a/src/runtime/debug/mod.go
+++ b/src/runtime/debug/mod.go
@@ -57,8 +57,9 @@ type Module struct {
 // BuildSetting describes a setting that may be used to understand how the
 // binary was built. For example, VCS commit and dirty status is stored here.
 type BuildSetting struct {
-	// Key and Value describe the build setting. They must not contain tabs
-	// or newlines.
+	// Key and Value describe the build setting.
+	// Key must not contain an equals sign, space, tab, or newline.
+	// Value must not contain newlines ('\n').
 	Key, Value string
 }
 
@@ -97,10 +98,13 @@ func (bi *BuildInfo) MarshalText() ([]byte, error) {
 		formatMod("dep", *dep)
 	}
 	for _, s := range bi.Settings {
-		if strings.ContainsAny(s.Key, "\n\t") || strings.ContainsAny(s.Value, "\n\t") {
-			return nil, fmt.Errorf("build setting %q contains tab or newline", s.Key)
+		if strings.ContainsAny(s.Key, "= \t\n") {
+			return nil, fmt.Errorf("invalid build setting key %q", s.Key)
 		}
-		fmt.Fprintf(buf, "build\t%s\t%s\n", s.Key, s.Value)
+		if strings.Contains(s.Value, "\n") {
+			return nil, fmt.Errorf("invalid build setting value for key %q: contains newline", s.Value)
+		}
+		fmt.Fprintf(buf, "build\t%s=%s\n", s.Key, s.Value)
 	}
 
 	return buf.Bytes(), nil
@@ -185,14 +189,14 @@ func (bi *BuildInfo) UnmarshalText(data []byte) (err error) {
 			}
 			last = nil
 		case bytes.HasPrefix(line, buildLine):
-			elem := bytes.Split(line[len(buildLine):], tab)
-			if len(elem) != 2 {
-				return fmt.Errorf("expected 2 columns for build setting; got %d", len(elem))
+			key, val, ok := strings.Cut(string(line[len(buildLine):]), "=")
+			if !ok {
+				return fmt.Errorf("invalid build line")
 			}
-			if len(elem[0]) == 0 {
+			if key == "" {
 				return fmt.Errorf("empty key")
 			}
-			bi.Settings = append(bi.Settings, BuildSetting{Key: string(elem[0]), Value: string(elem[1])})
+			bi.Settings = append(bi.Settings, BuildSetting{Key: key, Value: val})
 		}
 		lineNum++
 	}