math/big: fix spurious race in Rat.Denom, Float.SetRat

Rat maintains the invariant that x.b.neg is always false,
but Rat.Denom was writing x.b.neg = false itself too.
That makes Rat.Denom a writing operation, when it should
be a read-only operation. That in turn makes it unsafe to
use from multiple goroutines, which is highly unexpected.
Make it read-only and therefore race-free again.

Fixes #50473.

Change-Id: I97b87913954511e5200c0665d16b9ed63422e505
Reviewed-on: https://go-review.googlesource.com/c/go/+/375935
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

2 files changed, 19 insertions, 1 deletions

diff --git a/src/math/big/rat.go b/src/math/big/rat.go
index d35cd4cbd1..731a979ff7 100644
--- a/src/math/big/rat.go
+++ b/src/math/big/rat.go
@@ -418,7 +418,7 @@ func (x *Rat) Num() *Int {
 // If the result is a reference to x's denominator it
 // may change if a new value is assigned to x, and vice versa.
 func (x *Rat) Denom() *Int {
-	x.b.neg = false // the result is always >= 0
+	// Note that x.b.neg is guaranteed false.
 	if len(x.b.abs) == 0 {
 		// Note: If this proves problematic, we could
 		//       panic instead and require the Rat to
diff --git a/src/math/big/rat_test.go b/src/math/big/rat_test.go
index 02569c1b16..d98c89b357 100644
--- a/src/math/big/rat_test.go
+++ b/src/math/big/rat_test.go
@@ -726,3 +726,21 @@ func TestIssue34919(t *testing.T) {
 		}
 	}
 }
+
+func TestDenomRace(t *testing.T) {
+	x := NewRat(1, 2)
+	const N = 3
+	c := make(chan bool, N)
+	for i := 0; i < N; i++ {
+		go func() {
+			// Denom (also used by Float.SetRat) used to mutate x unnecessarily,
+			// provoking race reports when run in the race detector.
+			x.Denom()
+			new(Float).SetRat(x)
+			c <- true
+		}()
+	}
+	for i := 0; i < N; i++ {
+		<-c
+	}
+}