fxamacker / cbor

CBOR codec (RFC 8949) with CBOR tags, Go struct tags (toarray, keyasint, omitempty), float64/32/16, big.Int, and fuzz tested billions of execs.
MIT License
687 stars 61 forks source link

feature: Support for compilation via tinygo #295

Open dereulenspiegel opened 2 years ago

dereulenspiegel commented 2 years ago

I tried to build a project using this library via tinygo (because this project would also make sense in microcontrollers), but unfortunately tinygo doesn't support reflect (or at least certain parts of it. The error reported is:

../../../../pkg/mod/github.com/fxamacker/cbor/v2@v2.2.0/decode.go:1019:17: MakeMapWithSize not declared by package reflect
../../../../pkg/mod/github.com/fxamacker/cbor/v2@v2.2.0/tag.go:196:17: contentType.PkgPath undefined (type reflect.Type has no field or method PkgPath)

It would be awesome if we could get this library to compile via tinygo to support microcontrollers and smaller WASM modules. Unfortunately I am not deep enough into the architecture of cbor to understand how hard the dependency to these reflect methods is and how to resolve this.

Is this possible at all?

fxamacker commented 2 years ago

Supporting tinygo seems like a worthwhile feature. I would need to look into it to understand how much work this would take.

dereulenspiegel commented 2 years ago

@fxamacker Awesome thanks. Really looking forward to using your cbor module on something like a NRF52 series microcontroller.

mkungla commented 2 years ago

Supporting tinygo seems like a worthwhile feature. I would need to look into it to understand how much work this would take.

Seems that only following in decoder.go would need some other approach, since tinygo reflect package as for now does not have reflect.MakeMapWithSize method implemented

https://github.com/fxamacker/cbor/blob/9e37184acacd7330c79de271370b26d57c228718/decode.go#L1256-L1262

so for instance following compiles with tinygo

if v.IsNil() {
  v.Set(reflect.MakeMap(tInfo.nonPtrType))
} 
fxamacker commented 2 years ago

Hey @mkungla thanks for looking into this! I'll take a closer look this weekend.

fxamacker commented 2 years ago

TL;DR master branch requires extensive changes to support tinygo but features/stream-mode branch (superset of master branch) would require fewer changes to allow tinygo to use StreamEncoder and StreamDecoder (which don't exist in master yet).

@dereulenspiegel @mkungla I've been looking into tinygo and it's a really cool project! I experimented with this version of tinygo:

tinygo version 0.23.0 linux/amd64 (using go version go1.18.2 and LLVM version 14.0.0)

tinygo's support for reflect is still very much a work in progress and it uses stubs that allow compiling which would panic at runtime.

For example, tinygo has many stubs like these:

image

In tinygo's value.go, there are 30 matches for panic("unimplemented: which allows us to compile but they panic at runtime:

fa@tinygo-amd64:~/go/src/github.com/fxamacker/cbor_main$ ./cbor_main
panic: unimplemented: (reflect.Type).FieldByName()
Aborted (core dumped)

Based on these findings, adding support for tinygo 0.23 would require more extensive changes than anticipated.

I'm going to keep this issue open because tinygo is making progress incrementally adding more support for reflect. In the meantime, I will:

Thanks again for suggesting tinygo!

mkungla commented 1 year ago

@fxamacker in brief look of your code I see that reflect package is primarily used to get underlying builtin type of value and value it self to be encoded or decoded?

If so, it might be possible to get rid of the `reflect' package as a dependency altogether and problem solved 😄. For example, you can create an "internal" package to get types and values that contains something as follows. The following is just a bit of experimental code I have, which of course is not a out of box solution.

package internal

import "unsafe"

// interface for the header of builtin value
type typeiface struct {
    kind *kindinfo
    ptr  unsafe.Pointer
}

// minimal builtin kind info struct needed to get that info
type typeinfo struct {
    size       uintptr
    ptrdata    uintptr // number of bytes in the kinde that can contain pointers
    hash       uint32  // hash of type; avoids computation in hash tables
    tflag      uint8   // extra type information flags
    align      uint8   // alignment of variable with this type
    fieldAlign uint8   // alignment of struct field with this type
    kind       uint8   // enumeration for C
}

type Kind uint

const (
    KindInvalid Kind = iota
    KindBool
    KindInt
    KindInt8
    KindInt16
    KindInt32
    KindInt64
    KindUint
    KindUint8
    KindUint16
    KindUint32
    KindUint64
    KindUintptr
    KindFloat32
    KindFloat64
    KindComplex64
    KindComplex128
    KindArray
    KindChan
    KindFunc
    KindInterface
    KindMap
    KindPointer
    KindSlice
    KindString
    KindStruct
    KindUnsafePointer
)

var kindNames = []string{
    KindInvalid:       "invalid",
    KindBool:          "bool",
    KindInt:           "int",
    KindInt8:          "int8",
    KindInt16:         "int16",
    KindInt32:         "int32",
    KindInt64:         "int64",
    KindUint:          "uint",
    KindUint8:         "uint8",
    KindUint16:        "uint16",
    KindUint32:        "uint32",
    KindUint64:        "uint64",
    KindUintptr:       "uintptr",
    KindFloat32:       "float32",
    KindFloat64:       "float64",
    KindComplex64:     "complex64",
    KindComplex128:    "complex128",
    KindArray:         "array",
    KindChan:          "chan",
    KindFunc:          "func",
    KindInterface:     "interface",
    KindMap:           "map",
    KindPointer:       "ptr",
    KindSlice:         "slice",
    KindString:        "string",
    KindStruct:        "struct",
    KindUnsafePointer: "unsafe.Pointer",
}

func (t Kind) String() (str string) {
    if uint(t) < uint(len(kindNames)) {
        str = kindNames[uint(t)]
    }
    return
}

// Kill "reflect"
func GetUnderlyingValueAndKind(in any, withvalue bool) (value any, kind Kind) {
    e := (*typeiface)(unsafe.Pointer(&in))

    // check whether it is really a pointer or not.
    t := e.kind
    if in == nil || t == nil {
        return nil, KindInvalid
    }

    // there are 27 kinds.
    // check whether t is stored indirectly in an interface value.
    f := uintptr(Kind(t.kind & ((1 << 5) - 1)))
    if t.kind&(1<<5) == 0 {
        f |= uintptr(1 << 7)
        kind = Kind(f & (1<<5 - 1))
    } else {
        kind = Kind(t.kind & ((1 << 5) - 1))
    }

        // return early if you only need to know underlying kind
    if !withvalue {
        return nil, kind
    }
    switch kind {
    case KindBool:
        value = *(*bool)(e.ptr)
    case KindInt:
        value = *(*int)(e.ptr)
    case KindInt8:
        value = *(*int8)(e.ptr)
    case KindInt16:
        value = *(*int16)(e.ptr)
    case KindInt32:
        value = *(*int32)(e.ptr)
    case KindInt64:
        value = *(*int64)(e.ptr)
    case KindUint:
        value = *(*uint)(e.ptr)
    case KindUint8:
        value = *(*uint8)(e.ptr)
    case KindUint16:
        value = *(*uint16)(e.ptr)
    case KindUint32:
        value = *(*uint32)(e.ptr)
    case KindUint64:
        value = *(*uint64)(e.ptr)
    case KindUintptr, KindPointer, KindUnsafePointer:
        value = *(*uintptr)(e.ptr)
    case KindFloat32:
        value = *(*float32)(e.ptr)
    case KindFloat64:
        value = *(*float64)(e.ptr)
    case KindComplex64:
        value = *(*complex64)(e.ptr)
    case KindComplex128:
        value = *(*complex128)(e.ptr)
    case KindString:
        value = *(*string)(e.ptr)
        // ... other supported kinds
    }
    return value, kind
}
mkungla commented 1 year ago

@fxamacker has that issue become obsolete

fxamacker commented 1 year ago

hi @mkungla reflect is used for setting value during decoding, in addition to getting underlying value and type during encoding and decoding. So it is quite a task to replace it.

As of today, both dev and release branches in TinyGo don't yet have all the reflect features needed by this library, such as support for map, etc. TinyGo is refactoring reflect in PR https://github.com/tinygo-org/tinygo/pull/2640 and it got scheduled for TinyGo v0.28 milestone. Not sure yet which reflect features TinyGo will add first after that refactoring.

Ideally, I would love to support TinyGo without adding special workaround for it or providing new API, but that may be necessary (if TinyGo decides not to implement some missing features) so I would like to keep this issue open.

fxamacker commented 1 year ago

Quick update: TinyGo released 0.28 today (June 11, 2023) and it includes 24 improvements related to reflect.

I need to work this weekend but will take a look after wrapping up a couple urgent projects.

At a glance, this looks promising. When time allows, it would be useful to see what reflect support are still missing (if any) in 0.28 that is currently used in this codec.

fxamacker commented 1 year ago

TinyGo 0.28.1 allowed fxamacker/cbor to pass more tests, so that's the good news. :sweat_smile:

The above two items appear to be the main blockers and other remaining issues appear to be limited to tests.

fxamacker commented 1 year ago

I created branch https://github.com/fxamacker/cbor/tree/fxamacker/cbor-tinygo based on fxamacker/cbor v2.5.0-beta4.

It passes tests when compiled with TinyGo 0.28.1 patched with PR https://github.com/tinygo-org/tinygo/pull/3795.

A tradeoff is the removal of one feature: codec cannot decode CBOR tag to Go interface when compiled with TinyGo. Instead, it will return UnmarshalError. This tradeoff is caused by TinyGo v0.28.1 not fully implementing Type.AssignableTo.

I'll update this issue as we make more progress.

fxamacker commented 1 year ago

dgryski's https://github.com/tinygo-org/tinygo/pull/3795 got merged :tada:

x448 commented 4 months ago

@fxamacker Sorry, I didn't see this issue and opened #499 to add build tag for tinygo to move it into main branch.

progrium commented 1 month ago

Is there an issue in TinyGo to track AssignableTo?

fxamacker commented 1 month ago

@progrium Good catch! :+1: I just opened issue at TinyGo to track AssignableTo.