Expression Crash bug FreeCAD0.18.1 Py3 on kubuntu 18.04

[jmaustpc]

Hi all
I have found a way to crash FreeCAD

OS: Kubuntu 18.04.2 LTS
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 0.18.1.
Build type: Release
Python version: 3.6.7
Qt version: 5.9.5
Coin version: 4.0.0a
OCC version: 7.3.0
Locale: English/Australia (en_AU)

click on the property Placement X of the default Part Sphere, start creating an Expression, type "Body.Sh" it will offer "ShapeType", press the down arrow keyboard or mouse click to select "ShapeType", you get an instant crash.

ShapeType would presumably return a string where a float would be needed, and is obviously an inappropriate input ...but FreeCAD should not crash.

centreofmass.FCStd

(18.25 KiB) Downloaded 57 times

Jim

[chrisb]

I can confirm, although reproducing is slightly different. As you talk about Part->Sphere you don't have a body. So it is (for the sake of less clicking I take the radius instead of placement)
- select Part WB
- create Sphere
- enter expression in radius field, select Shape.ShapeType
->crash

OS: macOS 10.13
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 0.19.16207 (Git)
Build type: Release
Branch: master
Hash: 741533941c67da2f721bf1b7c2d55c987fd22a0c
Python version: 3.7.2
Qt version: 5.6.2
Coin version: 4.0.0a
OCC version: 7.3.0
Locale: English/UnitedStates (en_US)

[eivindkvedalen]

Hi all
I have found a way to crash FreeCAD

...

click on the property Placement X of the default Part Sphere, start creating an Expression, type "Body.Sh" it will offer "ShapeType", press the down arrow keyboard or mouse click to select "ShapeType", you get an instant crash.

ShapeType would presumably return a string where a float would be needed, and is obviously an inappropriate input ...but FreeCAD should not crash.

centreofmass.FCStd

Jim

I get a bad_cast message here, but it does not crash. Not sure where the type error should be handled though. Please add it to the bug tracker.

Eivind

[wmayer]

I can confirm the crash too when using the debug build. It happens inside ObjectIdentifier::getValue() where the value of the expression is stored to the Python identifier _path_value_temp_. This identifier is of type "str" (for Python3) and when trying to convert it to a utf-8 encoded string it stops with an abort.

[wmayer]

After a bit more digging the issue only seems to happen with Py3 builds. I was able to fix the abort() crash but now it segfaults because a null pointer is passed (the second argument) to the constructor of StringExpression. So, the question is why it creates at some point this null pointer for a char* string.

[wmayer]

Fixed with git commit 88f637d8f

The one error responsible for the abort() is the missing instantiation of Base::PyGILStateLocker to acquire the mutex.
The second error is the Python to C string conversion. There PyUnicode_AsUTF8String is used which however creates a bytes object in Py3. The following call of PyUnicode_AsUTF8 then of course returns null. The solution is to call directly PyUnicode_AsUTF8.

[jmaustpc]

Fixed with git commit 88f637d8f

The one error responsible for the abort() is the missing instantiation of Base::PyGILStateLocker to acquire the mutex.
The second error is the Python to C string conversion. There PyUnicode_AsUTF8String is used which however creates a bytes object in Py3. The following call of PyUnicode_AsUTF8 then of course returns null. The solution is to call directly PyUnicode_AsUTF8.

thanks Werner, both for the fix and the explanation.

Would it be helpful if we collected a list of bugs that probably should be back ported to 0.18.2 if/when we tag one? If so, what is the best way to do that? I was wondering if I should create a topic in the developer's forum (pin it to the top) where we can post links to topics like this, if we think they are a bug of a nature that should be backported?

Would you agree that pretty much any "crash bug" should be a candidate for such a list?

Jim

[wmayer]

Would it be helpful if we collected a list of bugs that probably should be back ported to 0.18.2 if/when we tag one?

Would you agree that pretty much any "crash bug" should be a candidate for such a list?

Yes, it makes a lot of sense because this is a serious one and the likelihood that someone else will have it is quite high.

I was wondering if I should create a topic in the developer's forum (pin it to the top) where we can post links to topics like this, if we think they are a bug of a nature that should be backported?

There is already a topic about this and up to my knowledge all serious issues have been back ported to 0.18. So, this will be the only issue at the moment that must be fixed in 0.18, too.