#178 fix strict aliasing warning in Python's bool handling

closed-accepted
nobody
None
5
2007-09-17
2007-05-24
No

I've run into a problem with Swig's use of Py_True and Py_False when passing bool values to C code. Swig 1.3.31 generates this as part of the wrapper:

SWIGINTERN int
SWIG_AsVal_bool (PyObject *obj, bool *val)
{
if (obj == Py_True) {
if (val) *val = true;
return SWIG_OK;
} else if (obj == Py_False) {
if (val) *val = false;
return SWIG_OK;
} else {
long v = 0;
int res = SWIG_AddCast(SWIG_AsVal_long (obj, val ? &v : 0));
if (SWIG_IsOK(res) && val) *val = v ? true : false;
return res;
}
}

Compiling this with GCC 4.1 (with -O2 -Wall -Werror) complains:

In function ‘SWIG_AsVal_bool’:
blah_wrap.c:3432: warning: dereferencing type-punned pointer will break
strict-aliasing rules
blah_wrap.c:3435: warning: dereferencing type-punned pointer will break
strict-aliasing rules

The warnings are caused by any comparison between a PyObject * and Py_True or Py_False. Strictly speaking, this could be considered a bug in the way the Python headers define Py_True and Py_False (I'm using Python 2.4.4), but I think it's worth having a fix for this in Swig. The attached patch changes the code from that above to the much cleaner version below:

SWIGINTERN int
SWIG_AsVal_bool (PyObject *obj, bool *val)
{
int r = PyObject_IsTrue(obj);
if (r == -1)
return SWIG_ERROR;
if (val) *val = r ? true : false;
return SWIG_OK;
}

The PyObject_IsTrue function also avoids the need to cast non-boolean values to long.

Discussion

  • Olly Betts

    Olly Betts - 2007-05-27

    Logged In: YES
    user_id=14972
    Originator: NO

    I've read a few reports which suggest it's prudent to compile the code SWIG generates (for Python at least) with -fno-strict-aliasing when using GCC. For example:

    http://article.gmane.org/gmane.comp.gcc.devel/74692

    But regardless of that, I think your patch improves the generated code so is worth applying. I'll take a look at applying when I have a suitable number of spare moments (unless someone else deals with it first).

     
  • Olly Betts

    Olly Betts - 2007-09-17

    Logged In: YES
    user_id=14972
    Originator: NO

    Thanks for your patch - I've just applied it to SVN.

     
  • Olly Betts

    Olly Betts - 2007-09-17
    • status: open --> closed-accepted
     

Log in to post a comment.