(Go: >> BACK << -|- >> HOME <<)
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.

Sign up for GitHub

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

Jump to bottom

feature: add support for 32-bit architectures #302

Open
gibmat opened this issue Dec 1, 2021 · 5 comments
Open

feature: add support for 32-bit architectures #302

gibmat opened this issue Dec 1, 2021 · 5 comments
Labels
enhancement New feature or request undecided Final decision hasn't been made yet

Comments

@gibmat
Copy link
gibmat commented Dec 1, 2021

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:

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
@x448
Copy link
Contributor
x448 commented Dec 1, 2021

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.

@gibmat
Copy link
Author
gibmat commented Dec 2, 2021

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.

@fxamacker
Copy link
Owner

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

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

@fxamacker fxamacker added enhancement New feature or request undecided Final decision hasn't been made yet labels Dec 2, 2021
@fxamacker fxamacker changed the title bug: Tests fail on 32bit architectures feature: add support for 32-bit architectures Dec 6, 2021
@gibmat
Copy link
Author
gibmat commented Jan 8, 2022

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.

@fxamacker
Copy link
Owner

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.

@jas4711 jas4711 mentioned this issue Jan 15, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request undecided Final decision hasn't been made yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@gibmat @fxamacker @x448