Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

math/big: Int.GCD misbehaves when argument reused for result #11284

Closed
pat42smith opened this issue Jun 19, 2015 · 3 comments
Closed

math/big: Int.GCD misbehaves when argument reused for result #11284

pat42smith opened this issue Jun 19, 2015 · 3 comments
Milestone

Comments

@pat42smith
Copy link

Code of the form x.GCD(nil, nil, y, x) is likely to return an incorrect result. For example, this code:

package main
import "fmt"
import "math/big"
func main() {
    two := big.NewInt(2)
    fmt.Print(big.NewInt(0).GCD(nil, nil, two, big.NewInt(1)))
    x := big.NewInt(1)
    fmt.Print(x.GCD(nil, nil, two, x))
    fmt.Print(x)

    u := big.NewInt(0)
    v := big.NewInt(0)
    fmt.Print(big.NewInt(0).GCD(u, v, two, big.NewInt(1)))
    x = big.NewInt(1)
    fmt.Print(x.GCD(u, v, two, x))
    fmt.Println(x)
}

should print 111111. Instead, it prints 122111.

The cause is clear from examining the start of the binaryGCD method in int.go:

   682  func (z *Int) binaryGCD(a, b *Int) *Int {
   683      u := z
   684      v := new(Int)
   685  
   686      // use one Euclidean iteration to ensure that u and v are approx. the same size
   687      switch {
   688      case len(a.abs) > len(b.abs):
   689          u.Set(b)
   690          v.Rem(a, b)
   691      case len(a.abs) < len(b.abs):
   692          u.Set(a)
   693          v.Rem(b, a)
   694      default:
   695          u.Set(a)
   696          v.Set(b)
   697      }

In the misbehaving call, z and b point to the same object, so the calls to u.Set() will modify the value of b. Since the rest of the function does not use a and b, it should be possible to fix this by simply setting v before u in each of the three instances above.

Lastly, go version returns go version go1.4.2 linux/amd64

@randall77
Copy link
Contributor

@griesemer

@griesemer griesemer added this to the Go1.5 milestone Jun 19, 2015
@griesemer
Copy link
Contributor

Thanks for the analysis @pat42smith !

@mikioh mikioh changed the title math/big.Int.GCD misbehaves when argument reused for result math/big: Int.GCD misbehaves when argument reused for result Jun 19, 2015
@pat42smith
Copy link
Author

From opened to fixed and closed in under 24 hours! Thanks @griesemer !

@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants