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

bug(gnovm): prohibit conversion of non-convertible typed constants #2681

Open
deelawn opened this issue Aug 12, 2024 · 1 comment · May be fixed by #3117
Open

bug(gnovm): prohibit conversion of non-convertible typed constants #2681

deelawn opened this issue Aug 12, 2024 · 1 comment · May be fixed by #3117
Assignees
Labels
🐞 bug Something isn't working in focus Core team is prioritizing this work 📦 🤖 gnovm Issues or PRs gnovm related

Comments

@deelawn
Copy link
Contributor

deelawn commented Aug 12, 2024

High Level Summary

The Go spec says that constant float values should not be convertible to integer values. Gno guards against conversion of untyped float constants, but not typed constants. For example, both of these examples work in Gno when they should not:

package main

func main() {
    println(int(float64(1.5)))
}
package main

func main() {
    const v float64 = 1.5
    println(int(v))
}

Details

From the Go spec:

A constant value x can be converted to type T if x is representable by a value of T. As a special case, an integer constant x can be explicitly converted to a string type using the same rule as for non-constant x.
-- https://go.dev/ref/spec#Conversions

To interpret the above, it is necessary to know if a typed constant float value x is representable as an integer. Looking at the definition of constant representability:

A constant x is representable by a value of type T, where T is not a type parameter, if one of the following conditions applies:

  • x is in the set of values determined by T.
  • T is a floating-point type and x can be rounded to T's precision without overflow. Rounding uses IEEE 754 round-to-even rules but with an IEEE negative zero further simplified to an unsigned zero. Note that constant values never result in an IEEE negative zero, NaN, or infinity.
  • T is a complex type, and x's components real(x) and imag(x) are representable by values of T's component type (float32 or float64).

-- https://go.dev/ref/spec#Representability

In the case of this particular issue, typed float constant values are not representable as integers because they fail to to satisfy and of the criteria -- most notably that a non-natural number is not contained in the set of possible integer values.

Other Examples

All of these are examples that work in Gno but should not.

Integer Underflow

package main

func main() {
    const a int = -1
    println(uint(a))
}

Integer Overflow

package main

func main() {
    const a int = 270
    println(uint8(a))
}
@deelawn deelawn changed the title bug(gnovm): prohibit conversion of typed constants bug(gnovm): prohibit conversion of non-convertible typed constants Aug 12, 2024
@petar-dambovaliev petar-dambovaliev self-assigned this Aug 12, 2024
@zivkovicmilos zivkovicmilos added 🐞 bug Something isn't working 🌟 must have 🌟 labels Sep 11, 2024
@Kouteki Kouteki added this to the 🚀 Mainnet launch milestone Oct 16, 2024
@Kouteki Kouteki added 📦 🤖 gnovm Issues or PRs gnovm related and removed 🌟 must have 🌟 labels Oct 16, 2024
@Kouteki Kouteki moved this from Triage to Backlog in 🧙‍♂️gno.land core team Oct 22, 2024
@Kouteki Kouteki moved this from Backlog to Todo in 🧙‍♂️gno.land core team Oct 28, 2024
@petar-dambovaliev
Copy link
Contributor

I am starting work on this. I talked to @ltzmaxwell and he said i should take it.

@petar-dambovaliev petar-dambovaliev linked a pull request Nov 13, 2024 that will close this issue
@Kouteki Kouteki moved this from Todo to In Progress in 🧙‍♂️gno.land core team Nov 13, 2024
@Kouteki Kouteki added the in focus Core team is prioritizing this work label Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working in focus Core team is prioritizing this work 📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

5 participants