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

go/types: bad constant integer->string conversion for out-of-bounds integers #42790

Closed
mdempsky opened this issue Nov 23, 2020 · 5 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@mdempsky
Copy link
Member

The package below statically converts the constant 1<<32 to a string (which should be "\uFFFD", because it's outside of the valid Unicode range). It then checks that the length matches the expected length (which is 3).

cmd/compile correctly accepts this package, but go/types and gccgo do not. go/types used to handle this correctly. I believe this is a regression introduced by CL 220844 (6052838).

package p
const _ = -uint(len(string(1<<32)) - len("\uFFFD"))

/cc @griesemer @ianlancetaylor

@mdempsky mdempsky added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 23, 2020
@griesemer griesemer self-assigned this Nov 23, 2020
@griesemer griesemer added this to the Go1.16 milestone Nov 23, 2020
@griesemer
Copy link
Contributor

The go/types conversion code now incorrectly uses string(rune(codepoint)) where in the past it used the correct string(codepoint).

We probably should revisit commit 6052838 to ensure we didn't introduce other bugs elsewhere.

@gopherbot
Copy link

Change https://golang.org/cl/272666 mentions this issue: go/types: fix incorrect string(int) conversion (regression)

@gopherbot
Copy link

Change https://golang.org/cl/272667 mentions this issue: [dev.go2go] go/types, cmd/compile/internal/types2: fix incorrect string(int) conversion (regression)

@gopherbot
Copy link

Change https://golang.org/cl/272669 mentions this issue: [dev.typeparams] go/types, cmd/compile/internal/types2: fix incorrect string(int) conversion (regression)

gopherbot pushed a commit that referenced this issue Nov 24, 2020
…ng(int) conversion (regression)

This is a 1:1 port of the go/types changes in
https://golang.org/cl/272666 (master branch).

Updates #42790.

Change-Id: I633fab83ce9c9895e056592039e1f5e7cb3fe23e
Reviewed-on: https://go-review.googlesource.com/c/go/+/272667
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
gopherbot pushed a commit that referenced this issue Nov 24, 2020
… string(int) conversion (regression)

This is a 1:1 port of the go/types changes in
https://golang.org/cl/272666 (master branch).

Updates #42790.

Change-Id: I5da372961df48129b25777ed705b84d7201393ec
Reviewed-on: https://go-review.googlesource.com/c/go/+/272669
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/272611 mentions this issue: compiler: avoid silent truncation for string(1 << 32)

gopherbot pushed a commit to golang/gofrontend that referenced this issue Nov 25, 2020
In the conversion of a constant integer to a string type, the value of
the constant integer was being silently truncated from unsigned long
to unsigned int, producing the wrong string value.  Add an explicit
overflow check to avoid this problem.

For golang/go#42790

Change-Id: I3407410bb02c24edd114e5f78a1f60ab66378656
Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/272611
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
@golang golang locked and limited conversation to collaborators Nov 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants