[Fixed] Crash when typing expressions

Post here for help on using FreeCAD's graphical user interface (GUI).
Forum rules
and Helpful information
IMPORTANT: Please click here and read this first, before asking for help

Also, be nice to others! Read the FreeCAD code of conduct!
wmayer
Site Admin
Posts: 16857
Joined: Thu Feb 19, 2009 10:32 am

Re: Crash when typing epressions

Postby wmayer » Mon Aug 26, 2019 9:37 am

FYI, there is currently a different behaviour of PyCXX for Py2 and Py3. The constructors of PyCXX for Py2 in general don't allow a null pointer as argument and when doing so an exception is raised.

For Py3 null pointers are accepted. But this means then when passing the pointer to any Py function it's up to the caller to verify that the pointer is not null if Py itself doesn't check for it.

And that's exactly what happens here. The identifier res has a null pointer but the function PyModule_Check doesn't check for null and directly accesses the type object. That's why a segfault occurs.
realthunder
Posts: 1847
Joined: Tue Jan 03, 2017 10:55 am

Re: Crash when typing epressions

Postby realthunder » Mon Aug 26, 2019 10:05 am

wmayer wrote:
Mon Aug 26, 2019 9:37 am
For Py3 null pointers are accepted. But this means then when passing the pointer to any Py function it's up to the caller to verify that the pointer is not null if Py itself doesn't check for it.
Isn't that a questionable implementation? This means a lot of functions of PyCXX cannot be trusted, such as getItem(), etc? I don't think it's feasible to check null pointer for all those situations. By the look of it, PyCXX is designed with the assumption that the pointer is not null. I think at least Object::accepts() shouldn't accept null pointer if PyErr_Occured().
Try Assembly3 (latest version 0.11) along with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
wmayer
Site Admin
Posts: 16857
Joined: Thu Feb 19, 2009 10:32 am

Re: Crash when typing epressions

Postby wmayer » Mon Aug 26, 2019 1:39 pm

Isn't that a questionable implementation?
It's for sure not ideal. If it was only that you must check the pointer when you access it and use the Python C-API on your own I could understand it but it's even worse. The base class Py::Object has methods like type(), str(), dir(), ... and it even crashes if you call them in case the internal pointer is null.

However, in the past it wasn't allowed to create a Py::Object from a null pointer. The restriction has been lifted when the new type Py::Null was introduced. For the Py3 implementation the virtual method accepts() of Py::Object now is:

Code: Select all

        // Can pyob be used in this object's constructor?
        virtual bool accepts( PyObject * /* pyob */ ) const
        {
            // allow any object or NULL
            return true;
        }
But all other types (except of Py::Null) still require a non-null pointer in their constructors.

Maybe it's worth to have a look at later PyCXX versions. Possibly this issue has been solved in a smarter way there.
realthunder
Posts: 1847
Joined: Tue Jan 03, 2017 10:55 am

Re: Crash when typing epressions

Postby realthunder » Tue Aug 27, 2019 12:28 am

wmayer wrote:
Mon Aug 26, 2019 1:39 pm
Maybe it's worth to have a look at later PyCXX versions. Possibly this issue has been solved in a smarter way there.
I just checked the sourceforge project page, Object::accepts() is still the same. Looks like the project is still maintained with recent commit. Don't know why the problem still persists. Additionally, the Python3 PyCXX document clearly mentioned that Object does not accept null pointer.

So what to do, shall we fix it?
Try Assembly3 (latest version 0.11) along with my custom build of FreeCAD at here.
And if you'd like to show your support, you can donate through patreon, liberapay, or paypal
wmayer
Site Admin
Posts: 16857
Joined: Thu Feb 19, 2009 10:32 am

Re: Crash when typing expressions

Postby wmayer » Tue Aug 27, 2019 9:52 am

So what to do, shall we fix it?
This is no so easy because Debian or Fedora package maintainers have the policy not to use the code copies of 3rd party libraries of an upstream project when already an official package for such a library exists for their distribution.

So, this means when we change the local PyCXX code then Debian, Fedora, ... folks won't benefit from it. IMO, the best way would be to open a bug report or start a discussion on the PyCXX project.

In the meantime we have to add the security checks in our code. But I don't think that now we must go through every single use of Py::Object and add this check since in 99.9% of all cases it's guaranteed that the passed pointer is not null. So, I would just add the check where we know it currently crashes.
wmayer
Site Admin
Posts: 16857
Joined: Thu Feb 19, 2009 10:32 am

Re: Crash when typing expressions

Postby wmayer » Wed Aug 28, 2019 11:11 am

I had a look at the PyCXX code too and for Py3 it was from the beginning on that Py::Object accepts null. And it's already more than 3 years that the Py2 implementation has been changed, too: https://sourceforge.net/p/cxx/code/354/

So, apparently the Py2 version inside the FreeCAD code base is quite outdated.

The reason that null is accepted is that it has become possible to delete attributes of a Python extension object. When you have created an instance of an extension object and you type del myextobject.attribute then the callback handler for setattr is called where the second argument is a Py::Object with a null pointer inside.

But it's still a bit odd behaviour to accept null within Py::Object because for Py3 the new type Py::Null (which is missing in Py2) has been added which explicitly allows null.