opensourcerouting / c-capnproto

C library/compiler for the Cap'n Proto serialization/RPC protocol
MIT License
118 stars 40 forks source link

Fix bug where enums were treated interchangeably with uint16_t. #41

Closed detly closed 3 years ago

detly commented 3 years ago

ANSI C makes no guarantee about the size of an enum, only that it will be the minimum required integer type that can hold all the given values. Treating enums as interchangeable with uint16_t data caused undefined behavoiur on platforms where enums were always at least 32 bits.


This should fix #38. I believe it is impossible to write a Google Test test case that demonstrates this because C++ treats enums differently to C; but I could be wrong about that. I will see if I can do it.

Notwithstanding that, here is a schema and code that demonstrates the bug (on platforms where an enum is minimum 32 bits ie. most modern PCs). It requires Valgrind to fully demonstrate — at the very least, it expects to link with Valgrind's client libraries, and uses them if you run the resulting binary under Valgrind/Memcheck.

Build with Meson, and use it like so:

$ meson build
$ meson compile -C build
$ valgrind build/capnp-test

If you run it without Valgrind, you should at least see:

capnp-test: ../main.c:119: main: Assertion `thing_dec.first == EnumOne_oneA' failed.

Under Valgrind you'll also see the undefined memory it's attempting to access:

Before: ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
After : 00000000ffffffff0000ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff

(The ffs are undefined areas of memory; that is showing where in the decoded struct is defined and undefined.)