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.

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

Fix integer overflow warning in python.cxx #70

Closed
wants to merge 1 commit into from

Conversation

vigsterkr
Copy link
Contributor

When compiling SWIG generated cxx file in c++11 mode ('-std=c++11') with '-Wc++11-narrowing' flag one will get an error (see the example below) since the 'tp_dictoffset' is basically 'unsigned long' to 'long', which could lead to integer overflow. Using 'static_cast<Py_ssize_t>(...)' can prevent this.

error example (with clang -std=c++11):
non-constant-expression cannot be narrowed from type 'size_t'
(aka 'unsigned long') to 'Py_ssize_t' (aka 'long') in initializer list
-Wc++11-narrowing(((char_)&((SwigPyObject ) 64L)->dict) - (char_) 64L), /...
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

When compiling SWIG generated cxx file in c++11 mode ('-std=c++11') with '-Wc++11-narrowing' flag one will get an error (see the example below) since the 'tp_dictoffset' is basically 'unsigned long' to 'long', which could lead to integer overflow. Using 'static_cast<Py_ssize_t>(...)' can prevent this.

error example (with clang -std=c++11):
non-constant-expression cannot be narrowed from type 'size_t'
      (aka 'unsigned long') to 'Py_ssize_t' (aka 'long') in initializer list
      [-Wc++11-narrowing]
    (size_t)(((char*)&((SwigPyObject *) 64L)->dict) - (char*) 64L), /*...
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@ojwb
Copy link
Member

ojwb commented Aug 23, 2013

It won't ever actually overflow, as it's computing the offset of a struct member, which will always be a fairly small number. It would be good to fix the warning though.

Does this code only get generated in C++ mode? If not, then static_cast<> isn't safe to use and we should just use a C-style cast to Py_ssize_t. What options are you passing to swig? I couldn't figure out the option combination to actually get this code generated...

The whole casting of 64L is rather nasty, and using offsetof() would be saner (and possibly more portable), but that's really a separate issue.

@vigsterkr
Copy link
Contributor Author

@ojwb i dont know if this code is generated only in c++ mode.
i'm using the following flags for swig: -w473 -w454 -w312 -w325 -fvirtual builtin -modern -modernargs -c++

and i'm trying to compile the c++ generated code with clang and C++11 extension, i.e.:
clang++ -std=c++11

@ojwb
Copy link
Member

ojwb commented Aug 23, 2013

OK, thanks for the flags. This code can be generated in C mode too - e.g. by this interface file:

%module foo
struct A { int x; };
typedef struct A A;

I've committed these two changes, which should fix this warning, and make this just use offsetof:

commit 628b4710e5309379475f55c95a42e1cf21c9be87
Author: Olly Betts <olly@survex.com>
Date:   Sat Aug 24 08:40:08 2013 +1200

    [Python] Fix clang++ warning in generated wrapper code.

commit b477cb66be5739a0acc2c2de15e988187971ce40
Author: Olly Betts <olly@survex.com>
Date:   Sat Aug 24 08:34:50 2013 +1200

    Use offsetof() rather than icky homemade equivalent

Thanks for reporting this issue.

@ojwb ojwb closed this Aug 23, 2013
iglesias pushed a commit to iglesias/shogun that referenced this pull request Nov 20, 2013
read more about this bug at:
swig/swig#70
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants