math/big: Rat.Float64 not safe for concurrent use

[rsc]

In math/big, Rat.Float64 modifies x.b.abs by calling set(natNone), which makes it unsafe for concurrent use.
But you would really expect that a reading method like Float64 would be safe.
In fact go/constant's Float64Val assumes this, unless you also want to say that constant.Float64Val is not safe for concurrent use either.
But then you'd have to track all the calls to that, like go/types.roundFloat64, called from the types.Checker, and so on.
It seems like the solution has to be to make Rat.Float64 safe for concurrent use.
It would be fine to just not modify x.b.abs there and do 'b = natOne', no?

This is the root cause of the original go/packages race reported in #31749.

It may also be a mistake to have fixed #33792 by declaring Denom to be unsafe for concurrent use,
for many of the same reasons.
That could have returned a fixed 1 int as well.

[rsc]

As another data point, if z is a Float and x is a Rat, z.SetRat(x) calls x.Denom, meaning that z.SetRat(x) is unsafe to call concurrently with other uses of x, even though it seems to only be reading x.

[elagergren-spideroak]

WRT to #33792, big.(*Rat).Denom cannot return a fixed 1 because it's documented as returning a reference to x's denominator. In the CL I assumed changing the behavior would be a non-starter.

[griesemer]

@ericlagergren Indeed, but the primary reason for returning a reference was speed, if I remember correctly. In retrospect, and that's entirely my fault, the implementation probably should have returned a fixed (new) one.

I'm looking into these.

[gopherbot]

Change https://golang.org/cl/201205 mentions this issue: math/big: make Rat accessors safe for concurrent use

[dmitshur]

I'm trying to determine if this qualifies for backporting. It looks like a serious issue, however I understand it (along with #33792) has existed for many years.

The only workaround I can think of right now is to rewrite code to use Rat.Float64 and Rat.Denom differently for Go 1.13 and older. Are there other workarounds available for Go 1.13 and 1.12?

Based on analysis in #36605 (comment), this issue is causing data races in x/tools/go/packages on release-branch.go1.13, which is causing red on the dashboard and contributing to #11811. /cc @matloob

[gopherbot]

Change https://golang.org/cl/233321 mentions this issue: [release-branch.go1.13] math/big: make Rat accessors safe for concurrent use