[solved] Weekly 021-31527 TechDraw dimensions cause crashes

[wandererfan]

Version: 0.21.0.31453 (Git)

Crash report is for >= 0.21.0.31527. When you get a minute, can you try with a newer build?

[chennes]

Strange, that's a fresh pull from this morning. Might be I messed up my cMake, let me rebuild from scratch

[tanderson69]

I bet address sanitizer will find this bug on any platform. cmake

[chennes]

Tried again with:

Code: Select all

OS: macOS 13.0
Word size of FreeCAD: 64-bit
Version: 0.21.0.31624 (Git)
Build type: Release
Branch: master
Hash: b548c19325999dbf82dcc99e8357a3ddaacd9f6d
Python 3.10.8, Qt 5.15.7, Coin 4.0.0, Vtk 9.2.2, OCC 7.6.3
Locale: English/United States (en_US)

Still no crash (but as @tanderson69 suggests, ASAN might catch it, on any platform). I did not try with the sanitizer on.

[chrisb]

Still no crash

Can you reproduce using the official 31598?

[chennes]

Can you reproduce using the official 31598?

Yes, with that version it crashes in TechDraw::validateDimSelection, same as everyone else.

[chrisb]

Yes, with that version it crashes in TechDraw::validateDimSelection, same as everyone else.

So it's not your hardware which makes it not crash.

[chennes]

Some interesting results. As @tanderson69 predicted, turning on ASAN causes a crash, even in my self-compiled version that was not crashing before. Interestingly, the first time I clicked on an edge and told TD to dimension it, I got an error dialog that said "getGeomTypeFromName - empty geometry name". The second time I tried to insert the dimension (for the same edge, just deselecting and reselecting it), I got a crash with the following (abbreviated) backtrace:

Code: Select all

#8	0x0000000212c2e340 in TechDraw::checkGeometryOccurences() at src/Mod/TechDraw/Gui/DimensionValidators.cpp:240
#9	0x0000000212c2c099 in TechDraw::validateDimSelection() at src/Mod/TechDraw/Gui/DimensionValidators.cpp:149
#10	0x0000000212b5dcc1 in execDistance(Gui::Command*) at src/Mod/TechDraw/Gui/CommandCreateDims.cpp:610

The snippet of code that's crashing is:

Code: Select all

bool TechDraw::checkGeometryOccurences(StringVector subNames,
                             GeomCountMap keyedMinimumCounts)
{
    //how many of each geometry descriptor are input
    GeomCountMap foundCounts;
    for (auto& sub : subNames) {
        std::string geometryType = DrawUtil::getGeomTypeFromName(sub); // Crash is here

My debugger says that "subNames" has one element, a std::string containing the text "Edge1", which certainly seems reasonable, but it is not happy with "sub" there, and it's not obvious to me why not, though it does appear that the function call to getGeomTypeFromName is making a copy of the string, and it's the copy constructor that's failing. Setting aside the "why is this being passed as a std::string and not a const std::string & ???", I don't see why that call should fail, unless it's something bizarre like the underlying data not being null-terminated.

So, in the long tradition of "just change stuff and see what happens", I converted the getGeomTypeFromName to use a const ref (and the loop to use a const auto & for good measure), figuring this would at least change the backtrace. By eliminating the copy of the string, I pushed the error out to here:

Code: Select all

std::string DrawUtil::getGeomTypeFromName(const std::string &geomName)
{
    boost::regex re("^[a-zA-Z]*");//one or more letters at start of string
    boost::match_results<std::string::const_iterator> what;
    boost::match_flag_type flags = boost::match_default;
    std::string::const_iterator begin = geomName.begin();

with backtrace

Code: Select all

#5	0x0000000219fe3533 in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::__is_long() const at c++/v1/string:1459
#6	0x0000000219fe3839 in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::__get_pointer() const at /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.1.sdk/usr/include/c++/v1/string:1581
#7	0x0000000219fba780 in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::begin() const at c++/v1/string:943
#8	0x0000000219fba03f in TechDraw::DrawUtil::getGeomTypeFromName() at src/Mod/TechDraw/App/DrawUtil.cpp:108
#9	0x00000002182f87f1 in TechDraw::checkGeometryOccurences() at src/Mod/TechDraw/Gui/DimensionValidators.cpp:240

(I'm going to hit "submit" now because I don't trust the forums software )

[chennes]

Continued...

Up to this point I had been creating the geometry from scratch every time. That got old, so I saved the file just prior to adding dimensions. After doing so, I got the crash on the first attempt every time, not on the second. So the missing subName seems to be related to whether the document has been saved or not (it's not clear to my why it exists the second time through, though).

So at any rate, now I've got a clean crash, every time. I'm now stepping through the code in validateDimSelection and keeping a close eye on the subName string. It is starting out null-terminated, so everything looks fine there. It gets copied successfully in a call to getSubName(), so the original data is being copied at least once without issue.

Once again, I'm going to hit submit before carrying on...

[wandererfan]

I believe git commit 7fc59a68a0 fixes this, but I don't have the ability to test on mac. Using the "sanitizer" I was able to generate a failure on linux at the same point as was reported for mac, so fingers crossed.

Big props to @chennes and @tanderson69 for their assistance.