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

Already on GitHub? Sign in to your account

gccgo: math: Ldexp mishandles exp larger than 32-bit #21323

Closed
cherrymui opened this Issue Aug 6, 2017 · 6 comments

Comments

Projects
None yet
4 participants
Contributor

cherrymui commented Aug 6, 2017

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

Go tip (~1.9), gccgo gccgo (GCC) 8.0.0 20170725 (experimental)

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

linux/amd64

What did you do?

package main

import (
        "fmt"
        "math"
)

var i int64 = 1<<32 + 1

func main() {
        fmt.Println(math.Ldexp(1.0, int(i)))
}

What did you expect to see?

With gc toolchain, the program above prints +Inf on linux/amd64. I believe this is the expected result.

What did you see instead?

With gccgo, the program above prints 2 on linux/amd64.

gccgo's math.Ldexp uses libc_ldexp, which is defined in C as double ldexp(double x, int exp). On linux/amd64, C's int is 32-bit whereas Go's int is 64-bit, causing this problem.

@ianlancetaylor

@gopherbot gopherbot added this to the Gccgo milestone Aug 6, 2017

Member

thanm commented Aug 8, 2017

Cherry, I can take a shot at fixing this if you don't already have a CL -- let me know.

Contributor

cherrymui commented Aug 9, 2017

@thanm, thank you.
No, I don't have a CL.

Change https://golang.org/cl/54230 mentions this issue: math: additional tests for Ldexp

@gopherbot gopherbot pushed a commit that referenced this issue Aug 9, 2017

@thanm thanm math: additional tests for Ldexp
Add test cases to verify behavior for Ldexp with exponents outside the
range of Minint32/Maxint32, for a gccgo bug.

Test for issue #21323.

Change-Id: Iea67bc6fcfafdfddf515cf7075bdac59360c277a
Reviewed-on: https://go-review.googlesource.com/54230
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
ff560ee

@thanm thanm self-assigned this Aug 9, 2017

Member

thanm commented Aug 9, 2017

Fix by https://go-review.googlesource.com/c/54250
No sure why, but gopherbot didn't auto-close.

@thanm thanm closed this Aug 9, 2017

Contributor

ianlancetaylor commented Aug 9, 2017

It didn't auto-close because it wasn't written as "golang/go#21323". Sorry I didn't catch that.

Member

thanm commented Aug 9, 2017

Ah ok, thanks. I'll try to remember.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment