Skip to content

math/big: Rat.Denom needlessly mutates receiver #50473

@rsc

Description

@rsc

big.Rat maintains the invariant that b.neg is false.
But then x.Denom needlessly contains

x.b.neg = false

at the top of the function.

If x.b.neg were in question, this would be a clear bug:
calling x.Denom must not change the value of x.

But since x.b.neg is always false, this is just a needless write,
which makes x.Denom trigger (arguably spurious) race reports
in programs that use x.Denom from multiple goroutines
or that use other code that calls it, such as big.Float's SetRat.

You'd certainly expect that given a globally initialized

var x *big.Rat = myValue()

it would be safe to do

f := new(big.Float).SetRat(x)

from multiple goroutines. Yet it is not - the race detector
notices the race writing useless false values to x.b.neg.

The fix is just to delete the assignment.
(Again, if x.b.neg could possibly be true,
then Denom would be changing the value of x!)

% cat x.go
package main

import "math/big"

func main() {
	z := big.NewRat(1, 2)
	c := make(chan int, 5)
	for i := 0; i < 5; i++ {
		go func() {
			z.Denom()
			c <- 1
		}()
	}
	for i := 0; i < 5; i++ {
		<-c
	}
}
% go run -race x.go
==================
WARNING: DATA RACE
Write at 0x00c000112060 by goroutine 8:
  math/big.(*Rat).Denom()
      /Users/rsc/go/src/math/big/rat.go:421 +0x44
  main.main.func1()
      /Users/rsc/x.go:10 +0x6a

Previous write at 0x00c000112060 by goroutine 7:
  math/big.(*Rat).Denom()
      /Users/rsc/go/src/math/big/rat.go:421 +0x44
  main.main.func1()
      /Users/rsc/x.go:10 +0x6a

Goroutine 8 (running) created at:
  main.main()
      /Users/rsc/x.go:9 +0x68

Goroutine 7 (finished) created at:
  main.main()
      /Users/rsc/x.go:9 +0x68
==================
Found 1 data race(s)
exit status 66
% cat xx.go
package main

import "math/big"

func main() {
	z := big.NewRat(1, 2)
	c := make(chan int, 5)
	for i := 0; i < 5; i++ {
		go func() {
			new(big.Float).SetRat(z)
			c <- 1
		}()
	}
	for i := 0; i < 5; i++ {
		<-c
	}
}
% go run -race xx.go
==================
WARNING: DATA RACE
Write at 0x00c000140020 by goroutine 8:
  math/big.(*Rat).Denom()
      /Users/rsc/go/src/math/big/rat.go:421 +0x164
  math/big.(*Float).SetRat()
      /Users/rsc/go/src/math/big/float.go:624 +0x18e
  main.main.func1()
      /Users/rsc/xx.go:10 +0x50

Previous write at 0x00c000140020 by goroutine 7:
  math/big.(*Rat).Denom()
      /Users/rsc/go/src/math/big/rat.go:421 +0x164
  math/big.(*Float).SetRat()
      /Users/rsc/go/src/math/big/float.go:624 +0x18e
  main.main.func1()
      /Users/rsc/xx.go:10 +0x50

Goroutine 8 (running) created at:
  main.main()
      /Users/rsc/xx.go:9 +0x68

Goroutine 7 (finished) created at:
  main.main()
      /Users/rsc/xx.go:9 +0x68
==================
Found 1 data race(s)
exit status 66
% 

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions