I ran the checker against gdb and got an error:
Traceback (most recent call last): File "/home/tromey/Space/Trunk/gcc-python-plugin/libcpychecker/__init__.py", line 33, in on_pass_execution check_pyargs(fun) File "/home/tromey/Space/Trunk/gcc-python-plugin/libcpychecker/PyArg_ParseTuple.py", line 469, in check_pyargs if stmt.fndecl.name == 'PyArg_ParseTuple': AttributeError: 'NoneType' object has no attribute 'name' ../../archer/gdb/cli/cli-dump.c: In function ‘call_dump_func’: ../../archer/gdb/cli/cli-dump.c:787:1: fatal error: Unhandled Python exception raised within callback compilation terminated.
I am not certain, but perhaps this happens due to a call via a function pointer.
The appended patch worked for me, at least in the sense that it removed the error.
Also, the error location could be better here. E.g., in this case, if it used the location of the call statement, I could more easily see what exactly the problem is. As it is the location refers to the closing brace of the function.
Tom
diff --git a/libcpychecker/PyArg_ParseTuple.py b/libcpychecker/PyArg_ParseTuple.py index 13efbf1..2908460 100644 --- a/libcpychecker/PyArg_ParseTuple.py +++ b/libcpychecker/PyArg_ParseTuple.py @@ -466,7 +466,9 @@ def check_pyargs(fun): if isinstance(stmt, gcc.GimpleCall): #log('stmt.fn: %s %r' % (stmt.fn, stmt.fn)) #log('stmt.fndecl: %s %r' % (stmt.fndecl, stmt.fndecl)) - if stmt.fndecl.name == 'PyArg_ParseTuple': + if not hasattr(stmt.fndecl, 'name'): + pass + elif stmt.fndecl.name == 'PyArg_ParseTuple': check_callsite(stmt, 'PyArg_ParseTuple', 1, 2) elif stmt.fndecl.name == 'PyArg_ParseTupleAndKeywords': check_callsite(stmt, 'PyArg_ParseTupleAndKeywords', 2, 4)
On Fri, 2011-06-24 at 12:04 -0600, Tom Tromey wrote:
I ran the checker against gdb and got an error:
Traceback (most recent call last): File "/home/tromey/Space/Trunk/gcc-python-plugin/libcpychecker/__init__.py", line 33, in on_pass_execution check_pyargs(fun) File "/home/tromey/Space/Trunk/gcc-python-plugin/libcpychecker/PyArg_ParseTuple.py", line 469, in check_pyargs if stmt.fndecl.name == 'PyArg_ParseTuple': AttributeError: 'NoneType' object has no attribute 'name' ../../archer/gdb/cli/cli-dump.c: In function ‘call_dump_func’: ../../archer/gdb/cli/cli-dump.c:787:1: fatal error: Unhandled Python exception raised within callback compilation terminated.
I am not certain, but perhaps this happens due to a call via a function pointer.
The appended patch worked for me, at least in the sense that it removed the error.
As it happens, I just ran into this one myself! (in my case, I'm trying to run it on python itself).
Yes, my checker was barfing on calls through a function pointer.
I've already pushed this patch for this: http://git.fedorahosted.org/git/?p=gcc-python-plugin.git;a=commit;h=896b4c8f...
I'm running into another problem, with format codes that specify a PyObject subclass e.g "S" I'd coded these to expect e.g. a PyStringObject**, but this is overspecifying it; there's plenty of valid code that passes PyObject**. (but PyStringObject** would also be valid).
So I'm working on fixing that now.
Also, the error location could be better here. E.g., in this case, if it used the location of the call statement, I could more easily see what exactly the problem is. As it is the location refers to the closing brace of the function.
Agreed.
BTW, was this function the final one in the source file?
There are two problems here: - if a Python unhandled exception happens, the plugin reports it (giving the Python traceback), and then issues a gcc error, which is reported at the level of the C source code. Somehow we need to get gcc to give a better location for that error: it knows which function the error happened in. But it's better to fix the python script so that the exception doesn't happen, of course. - when issuing the warning from the script, we have to use the gcc.Location. I've tried using using that of the arguments themselves, but it tends to give me the location of the call. I'm not quite sure where the information is getting lost.
I'm excited that you're trying it out on gdb. Are you able to compile all of gdb with the checker script?
Cheers Dave
David> BTW, was this function the final one in the source file?
Yes.
David> There are two problems here: David> - if a Python unhandled exception happens, the plugin reports it David> (giving the Python traceback), and then issues a gcc error, which is David> reported at the level of the C source code. Somehow we need to get gcc David> to give a better location for that error: it knows which function the David> error happened in. But it's better to fix the python script so that the David> exception doesn't happen, of course.
Yeah, I see.
David> - when issuing the warning from the script, we have to use the David> gcc.Location. I've tried using using that of the arguments themselves, David> but it tends to give me the location of the call. I'm not quite sure David> where the information is getting lost.
Sometimes GCC drops information surprisingly early. So it could just be a GCC bug; you'd have to debug into GCC to see.
David> I'm excited that you're trying it out on gdb. Are you able to compile David> all of gdb with the checker script?
Just about! It caught a few real bugs (mostly char** -> const char **, but also some int -> Py_ssize_t), but I also found some plugin bugs.
Tom
David> I'm excited that you're trying it out on gdb. Are you able to compile David> all of gdb with the checker script?
Tom> Just about! It caught a few real bugs (mostly char** -> const char **, Tom> but also some int -> Py_ssize_t), but I also found some plugin bugs.
I fixed up all the gdb bugs and will check the fix in shortly.
The only remaining problems come from the typedef thing.
Tom
On Fri, 2011-06-24 at 13:38 -0600, Tom Tromey wrote:
David> I'm excited that you're trying it out on gdb. Are you able to compile David> all of gdb with the checker script?
Tom> Just about! It caught a few real bugs (mostly char** -> const char **, Tom> but also some int -> Py_ssize_t), but I also found some plugin bugs.
I fixed up all the gdb bugs and will check the fix in shortly.
Note to self: I see that you posted the resulting gdb patch here: http://sourceware.org/ml/gdb-patches/2011-06/msg00376.html
(Obviously I'm interested in bugs found and fixed using this tool!)
David> Note to self: I see that you posted the resulting gdb patch here: David> http://sourceware.org/ml/gdb-patches/2011-06/msg00376.html David> (Obviously I'm interested in bugs found and fixed using this tool!)
I thought of CCing this list but in the end just CCd you personally. But I am happy to CC the plugin list for future changes...
Tom
On Fri, 2011-06-24 at 14:14 -0600, Tom Tromey wrote:
David> Note to self: I see that you posted the resulting gdb patch here: David> http://sourceware.org/ml/gdb-patches/2011-06/msg00376.html David> (Obviously I'm interested in bugs found and fixed using this tool!)
I thought of CCing this list but in the end just CCd you personally. But I am happy to CC the plugin list for future changes...
Unfortunately I've just run into a good reason for sending such patches to this list: I fear that the checker may have given you false results, and that there might thus be a bug in the gdb patch above.
Specifically, the PyArg_Parse* checker had a bug in how it coped with the various "#" codes: it assumed that the code had a #define PY_SSIZE_T_CLEAN before the initial #include <Python.h>
The presence or absence of this macro affects all of the "#" format codes: without it, they should be "int", with it, they should be "Py_ssize_t". (This is documented much better in the py3k version of the docs [1], than in the python 2 version [2]).
I believe I've now fixed it in git, with: http://git.fedorahosted.org/git/?p=gcc-python-plugin.git;a=commit;h=ad8afdda...
Sorry about this Dave
[1] http://docs.python.org/py3k/c-api/arg.html [2] http://docs.python.org/c-api/arg.html
David> Specifically, the PyArg_Parse* checker had a bug in how it coped with David> the various "#" codes: it assumed that the code had a David> #define PY_SSIZE_T_CLEAN David> before the initial David> #include <Python.h>
Thanks. I was totally unaware of this. I went ahead and changed GDB to always define it.
David> Sorry about this
It is no problem.
Tom
gcc-python-plugin@lists.stg.fedorahosted.org