lloyd / yajl

A fast streaming JSON parsing library in C.
http://lloyd.github.com/yajl
ISC License
2.15k stars 435 forks source link

yajl_config() and yajl_gen_config() trigger undefined behavior with varargs #188

Open haberman opened 8 years ago

haberman commented 8 years ago

There is a subtle but important issue with the varargs code in yajl_config() and yajl_gen_config(). The problem is with this code:

int
yajl_config(yajl_handle h, yajl_option opt, ...)
{
    int rv = 1;
    va_list ap;
    va_start(ap, opt);

The opt parameter is declared as the enum type yajl_option. But when you pass "opt" to va_start, it undergoes integer promotion, which results in undefined behavior. Clang is adding a warning for this, which is how this issue came to our attention.

You can see a bit of information about this here. The key part is:

// Undefined by C99 7.15.1.4p4 (va_start): // "If the parameter parmN is declared with the register storage // class, with a function or array type, or with a type that is // not compatible with the type that results after application of // the default argument promotions, the behavior is undefined."

Enum gets promoted to int, which is not compatible with int.

There are a couple possible solutions:

  1. The easiest one would be to change the declared type of the parameter to int or unsigned int. That should be interface-compatible with existing clients.
  2. Another option would be to rearrange the parameters so that the yajl_handle parameter would be passed to va_start() instead. Since that type doesn't undergo promotion, it doesn't trigger the undefined behavior. However this would be an API-incompatible change.