cbor icon indicating copy to clipboard operation
cbor copied to clipboard

feature: add support for 32-bit architectures

Open gibmat opened this issue 3 years ago • 5 comments

What version of fxamacker/cbor are you using?

2.3.0

Does this issue reproduce with the latest release?

Yes, this is the latest release

What OS and CPU architecture are you using (go env)?

This was observed in build logs from Debian's buildd instances for i386 and armhf:

  • https://ci.debian.net/data/autopkgtest/testing/armhf/g/golang-github-fxamacker-cbor/17101841/log.gz
  • https://ci.debian.net/data/autopkgtest/testing/i386/g/golang-github-fxamacker-cbor/17102133/log.gz

What did you do?

Build the library and run the tests on any 32bit system.

What did you expect to see?

All tests pass.

What did you see instead?

github.com/fxamacker/cbor
   dh_auto_test -O--builddirectory=_build -O--buildsystem=golang
	cd _build && go test -vet=off -v -p 160 github.com/fxamacker/cbor
# github.com/fxamacker/cbor [github.com/fxamacker/cbor.test]
src/github.com/fxamacker/cbor/bench_test.go:281:15: constant 18446744073709551615 overflows uint
src/github.com/fxamacker/cbor/bench_test.go:292:3: constant 18446744073709551615 overflows uint
src/github.com/fxamacker/cbor/bench_test.go:303:10: constant 18446744073709551615 overflows uint
src/github.com/fxamacker/cbor/bench_test.go:314:3: constant 18446744073709551615 overflows uint
src/github.com/fxamacker/cbor/bench_test.go:325:7: constant 18446744073709551615 overflows uint
src/github.com/fxamacker/cbor/bench_test.go:336:3: constant 18446744073709551615 overflows uint
src/github.com/fxamacker/cbor/decode_test.go:110:44: constant 1000000000000 overflows uint
src/github.com/fxamacker/cbor/decode_test.go:110:86: constant 1000000000000 overflows int
src/github.com/fxamacker/cbor/decode_test.go:116:51: constant 18446744073709551615 overflows uint
src/github.com/fxamacker/cbor/decode_test.go:3202:29: constant 2147483648 overflows int
src/github.com/fxamacker/cbor/decode_test.go:3202:29: too many errors
FAIL	github.com/fxamacker/cbor [build failed]
FAIL

gibmat avatar Dec 01 '21 21:12 gibmat

Unless I'm mistaken, 32-bit platforms were never officially supported by this library so this is a feature request (not a bug).

The README lists these 64-bit platforms under the Current Status section:

Passed all tests – v2.x passed all 375+ tests on amd64, arm64, ppc64le and s390x with linux.

@fxamacker please correct me if I'm mistaken.

x448 avatar Dec 01 '21 22:12 x448

Well, there are a lot of 32bit platforms still out there! :) From my initial look, I hope it wouldn't be too hard to update the variable types to (u)int64, so the sizes would be the same regardless of the architecture.

This library is one of the (many) dependencies of packaging LXD for Debian, and I'd like to make it available on as many different architectures supported by Debian as possible.

gibmat avatar Dec 02 '21 01:12 gibmat

@x448 yes, 32-bit was never officially supported.

@gibmat thanks for suggesting this. I'll try to make time to look into it.

fxamacker avatar Dec 02 '21 04:12 fxamacker

Spent a little bit of time this afternoon investigating further, and came up with this commit: 7a8157a1230c532f89a908a314047f502747a504. With that applied, the build and all tests succeed on both amd64 and i386 architectures running on Linux/golang1.17.5.

gibmat avatar Jan 08 '22 00:01 gibmat

Spent a little bit of time this afternoon investigating further, and came up with this commit: 7a8157a. With that applied, the build and all tests succeed on both amd64 and i386 architectures running on Linux/golang1.17.5.

Very interesting!

BTW, changing data type from int to int64 or vice versa can introduce security issues or other runtime problems, so it usually involves a lot more work after initial coding changes.

E.g. depending on the programming language, there might be runtime errors decoding > 2^32 items into map or other container data types on 32-bit arch. So adding support for 32-bit arch can require adding more tests and fuzzing on 32-bit & 64-bit platforms for multiple weeks each. And CI may need to be updated to add 32-bit platforms, etc.

fxamacker avatar Jan 22 '22 18:01 fxamacker