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

Jump to bottom

math/big: new examples that fail the checks in TestFloat64SpecialCases #56164

Open
AdamKorcz opened this issue Oct 11, 2022 · 2 comments · May be fixed by #61531
Open

math/big: new examples that fail the checks in TestFloat64SpecialCases #56164

AdamKorcz opened this issue Oct 11, 2022 · 2 comments · May be fixed by #61531
Labels
help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@AdamKorcz
Copy link

AdamKorcz commented Oct 11, 2022 •
edited

What version of Go are you using (go version)?

go version go1.19.2 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/adam/.cache/go-build"
GOENV="/home/adam/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/adam/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/adam/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/tmp/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/tmp/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19.2"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/tmp/go/src/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2907961955=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I have found a few edge cases that fail the TestFloat64SpecialCases test.

To reproduce:

Add the following edge cases to float64inputs in ratconv_test.go and run the TestFloat64SpecialCases test:

  • "170141183460469231731687303715884105728p-48317",
  • "0000000000000000000000000000031768211457p-9818716",
  • "00000000000000000000000000000000000000000000000000000000001662320182p-801401",
  • "818p-8818873",

The stack trace will be:

--- FAIL: TestFloat64SpecialCases (1.08s)
    ratconv_test.go:589: Rat.SetString("170141183460469231731687303715884105728p-48317").Float64().exact = true, want false
    ratconv_test.go:589: Rat.SetString("0000000000000000000000000000031768211457p-9818716").Float64().exact = true, want false
    ratconv_test.go:589: Rat.SetString("00000000000000000000000000000000000000000000000000000000001662320182p-801401").Float64().exact = true, want false
    ratconv_test.go:589: Rat.SetString("818p-8818873").Float64().exact = true, want false

What did you expect to see?

What did you see instead?
@robpike robpike changed the title Edge cases that fail TestFloat64SpecialCases math/big: failures in TestFloat64SpecialCases Oct 12, 2022
@bcmills bcmills changed the title math/big: failures in TestFloat64SpecialCases math/big: new example that fail the checks in TestFloat64SpecialCases Oct 13, 2022
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 13, 2022
@bcmills bcmills added this to the Backlog milestone Oct 13, 2022
@bcmills bcmills changed the title math/big: new example that fail the checks in TestFloat64SpecialCases math/big: new examples that fail the checks in TestFloat64SpecialCases Oct 13, 2022
@jupj
Copy link
Contributor

jupj commented Oct 19, 2022

The problem seems to be in quotToFloat64() in rat.go.

For these edge cases Ldexp() returns zero on line 262:

f = math.Ldexp(float64(mantissa), exp-Msize1)

quotToFloat64() returns exact zero earlier if a is zero (line 195), so it seems that we need to add a boundary check on the exponent, or add a check for f == 0 here.

When I add the following f == 0 check on line 263, the tests pass:

if math.IsInf(f, 0) || f == 0 {
	exact = false
}

The first and last example break TestFloat32SpecialCases in the same way. A similar fix to quotToFloat32() is needed.

mauri870 added a commit to mauri870/go that referenced this issue Jul 22, 2023
@mauri870
math/big: handle additional edge cases for Rat
a60f722 
New special cases have been added to the Rat testcases.
Updated the function quotToFloat32/64 to check for zero
as well when determining if the value is infinite. This
change ensures that the exact flag is correctly set for
these edge cases, preventing test failures.

Thanks Johan Jansons (@jupj) for the proposed fix on
the ticket description.

Fixes: golang#56164
@mauri870 mauri870 linked a pull request Jul 22, 2023 that will close this issue
Open
@gopherbot
Copy link

Change https://go.dev/cl/512198 mentions this issue: math/big: handle additional edge cases for Rat

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

math/big: handle additional edge cases for Rat mauri870/go
4 participants
@jupj @bcmills @gopherbot @AdamKorcz