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

[Crash] Variable Link causes SIGSEGV in _LinkBaseExtension::extensionExecute()_ #12281

Closed
2 tasks done
Tracked by #12907
FlachyJoe opened this issue Feb 7, 2024 · 1 comment · Fixed by #12909
Closed
2 tasks done
Tracked by #12907

[Crash] Variable Link causes SIGSEGV in _LinkBaseExtension::extensionExecute()_ #12281

FlachyJoe opened this issue Feb 7, 2024 · 1 comment · Fixed by #12909
Labels
Bug This issue or PR is related to a bug Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD Crash For issues describing crashes or PRs fixing one Status: confirmed The issue was confirmed by others

Comments

@FlachyJoe
Copy link
Contributor

FlachyJoe commented Feb 7, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Problem description

With the file below, setting Caisson001.LinkCopyOnChange to Tracking causes a segmentation error at L330

FreeCAD/src/App/Link.cpp

Lines 322 to 332 in 0dcfe94

PropertyPythonObject *proxy = nullptr;
if(getLinkExecuteProperty()
&& !boost::iequals(getLinkExecuteValue(), "none")
&& (!_LinkOwner.getValue()
|| !container->getDocument()->getObjectByID(_LinkOwner.getValue())))
{
// Check if this is an element link. Do not invoke appLinkExecute()
// if so, because it will be called from the link array.
proxy = Base::freecad_dynamic_cast<PropertyPythonObject>(
linked->getPropertyByName("Proxy"));
}

It seems linked doesn't point to a valid DocumentObject
image

Ping @realthunder

Full version info

OS: Debian GNU/Linux trixie/sid (XFCE/xfce)
Word size of FreeCAD: 64-bit
Version: 0.22.0dev.35823 (Git)
Build type: Release
Branch: main
Hash: 087263547dedb4d58ee31a54d96b8db1509eb44f
Python 3.11.7, Qt 5.15.10, Coin 4.0.2, Vtk 9.1.0, OCC 7.6.3
Locale: English/United States (en_US)

Subproject(s) affected?

Core

Anything else?

Steps to reproduce:

  1. Open VariableLink_CrashTest.zip
  2. Set the LinkCopyOnChange property of Caisson001 object to Tracking
  3. Recompute twice (because of hidden references some objects are still touched after the first)
  4. FreeCAD crash

Code of Conduct

  • I agree to follow this project's Code of Conduct
@luzpaz luzpaz added Bug This issue or PR is related to a bug Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD Crash For issues describing crashes or PRs fixing one labels Feb 10, 2024
@wwmayer wwmayer added the Status: confirmed The issue was confirmed by others label Mar 3, 2024
@wwmayer
Copy link
Contributor

wwmayer commented Mar 3, 2024

It's a typical heap-use-after-free crash.

Inside LinkBaseExtension::extensionExecute() there is the pointer linked and it's checked whether it's null. The call of syncCopyOnChange() removes and deletes many objects -- including the object linked points to. So, at this time it has become a dangling pointer. Since it's used further down it causes undefined behaviour and usually leads to a crash.

A fix would be to replace
DocumentObject* linked = getTrueLinkedObject(true);
with
App::WeakPtrT<DocumentObject> linked(getTrueLinkedObject(true));
and do the necessary changes. After the call of syncCopyOnChange() it must be re-checked if the object is still valid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This issue or PR is related to a bug Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD Crash For issues describing crashes or PRs fixing one Status: confirmed The issue was confirmed by others
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants