tinyjson: Int32 produces float operation

[fdymylja]

The following function produces float ops:
https://github.com/CosmWasm/tinyjson/blob/master/jlexer/lexer.go#L862

func (r *Lexer) Int32() int32 {
	s := r.number()
	if !r.Ok() {
		return 0
	}

	n, err := strconv.ParseInt(s, 10, 32)
	if err != nil {
		r.addNonfatalError(&LexerError{
			Offset: r.start,
			Reason: err.Error(),
			Data:   s,
		})
	}
	return int32(n)
}

I'd say it comes from strconv.ParseInt.

Will try to remove the strconv.ParseInt part and check if floats still leak in the code.

[fdymylja]

Floating point operations are coming from strconv.ParseInt.

Asserted by:
replacing the tinyjson go import with a local one and removed the strconv.ParseInt part.

Will inspect the tinygo repo to try to remove float ops from strconv.

[fdymylja]

I think floats are leaking due to this: https://github.com/golang/go/blob/master/src/strconv/atof.go#L413 <- it's a var declared not on a function but at top level file - so maybe the go compiler is not removing those from code, even if unused since no function used is referencing those. [THIS IS WRONG LOOK COMMENT DOWN]

I can see two approaches here:

• patch tinyjson not to make it use strconv.
• patch tinygo and make it not use the go's strconv repository but a custom one -> https://github.com/confio/tinygo/blob/cosmwasm-v3/loader/goroot.go#L220

[fdymylja]

Ok so the real issue... is the one that you had before @ethanfrey !

https://github.com/golang/go/blob/master/src/strconv/atoi.go#L204

Removing the err.(*NumError).Err != ErrRange removes the problem.

[fdymylja]

Issue lays in interface comparison, probably we need a generalised solution for this... not sure how it could be handled.. considering the real problem is the Golang codebase that uses interface comparisons for errors and so on.
Minimum reproducible case:

package main

import (
	"github.com/cosmwasm/cosmwasm-go/std"
	"github.com/cosmwasm/cosmwasm-go/std/types"
	"unsafe"
)
func main() {}

type x interface { x() }

type a struct {}

func (a *a) x() {}

func (b *b) x() {}

type b struct {}

//export instantiate
func instantiate(envPtr, infoPtr, msgPtr uint32) unsafe.Pointer {
	return std.DoInstantiate(func(deps *std.Deps, env types.Env, messageInfo types.MessageInfo, messageBytes []byte) (*types.Response, error) {
		var a x = &a{}
		var b x = &b{}

		panic(a == b) // generates float ops
	}, envPtr, infoPtr, msgPtr)
}

[ethanfrey]

Issue lays in interface comparison

Yeah, it is built in deeply in reflect.

I think we either need a float-less fork of reflect and fmt 😬
OR some other solution like stripping out floats from wasm (assuming only those in the std lib) or (one day) enabling floats in wasm contracts

I need to think more on this. Thanks for highlighting the issue

[fdymylja]

Issue lays in interface comparison

Yeah, it is built in deeply in reflect.

I think we either need a float-less fork of reflect and fmt 😬 OR some other solution like stripping out floats from wasm (assuming only those in the std lib) or (one day) enabling floats in wasm contracts

I need to think more on this. Thanks for highlighting the issue

unfortunately i think stripping floats is not always the correct solution, for instance in this case if you stripped out floats, what would happen? the error comparison would fail? it would panic? it might create logic issues?

I'm not scared about contracts, we can just say to not do interface comparisons, but the problem lays in the go stdlib that does error checking and they not always lead to an early return (like in strconv case).

[ethanfrey]

unfortunately i think stripping floats is not always the correct solution, for instance in this case if you stripped out floats, what would happen? the error comparison would fail? it would panic? it might create logic issues?

"stripping" replaces all floats with "unreachable" (panic). this is not ideal if those paths ever get executed.

[ethanfrey]

Fixed by @fdymylja here CosmWasm/tinygo#5 (comment) 💪

This is now available in comswasm/tinygo:0.19.3 or cosmwasm/go-optimizer:0.3.0 🎉