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.

Sign up for GitHub

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

Jump to bottom

Segfault when removing bezier spline point #571

Closed
Timmmm opened this issue Mar 28, 2020 · 2 comments · Fixed by #572
Closed

Segfault when removing bezier spline point #571

Timmmm opened this issue Mar 28, 2020 · 2 comments · Fixed by #572
Labels
bug

Comments

@Timmmm
Copy link
Contributor

Timmmm commented Mar 28, 2020

System information

SolveSpace version: 3.0~645353cb
Operating system: Windows 10

Expected behavior

Add a bezier curve with a few segments. Right click one of the vertices, click "Remove spline point" or something like that. It shouldn't crash.

Actual behavior

You get a segfault in mouse.cpp:651 roughly here:

            Request *r = SK.GetRequest(gs.point[0].request());
            int index = r->IndexOfPoint(gs.point[0]);
            if((r->type == Request::Type::CUBIC && (index > 1 && index < r->extraPoints + 2)) ||
                    r->type == Request::Type::CUBIC_PERIODIC) {
                menu->AddItem(_("Remove Spline Point"), [&]() {
                    int index = r->IndexOfPoint(gs.point[0]);

Because r is 0xccccccccc which I think possibly means uninitialised in MSVC? Anyway it's not pointing to a valid object so it crashes later in r->IndexOfPoint().
@Timmmm
Copy link
Contributor Author

Timmmm commented Mar 28, 2020

I'm guessing you probably didn't mean to take a reference to r in that lambda! Changing the capture to [&, r] fixes it. Though really, passing raw pointers around like this is asking for trouble!

@whitequark whitequark added the bug label Mar 28, 2020
@whitequark
Copy link
Contributor

Yes, unfortunately the SolveSpace codebase has a mix of legacy code with more modern C++ and it doesn't always work well. Could you prepare a pull request? I didn't look into your fix closely but it sounds correct to me.

Timmmm added a commit to Timmmm/solvespace that referenced this issue Mar 28, 2020
@Timmmm
Fix segfault in Remove Spline Point context menu

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
49ae6a0 
This was incorrectly capturing `r` by reference and using it after it left its scope. Changed to capture by value, and also explicitly capture `this` in case we were accidentally capturing any other scope variables by reference.

Fixes solvespace#571
@Timmmm Timmmm mentioned this issue Mar 28, 2020
Merged
whitequark pushed a commit that referenced this issue Mar 28, 2020
@Timmmm
Fix segfault in Remove Spline Point context menu

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
485fd01 
This was incorrectly capturing `r` by reference and using it after it left
its scope. Changed to  capture by value, and also explicitly capture `this`
in case we were accidentally capturing any other scope variables by reference.

Fixes #571
devin-ai-integration bot pushed a commit to erkinalp/solvespace that referenced this issue Apr 3, 2025
@Timmmm
Fix segfault in Remove Spline Point context menu
73d0d83 
This was incorrectly capturing `r` by reference and using it after it left
its scope. Changed to  capture by value, and also explicitly capture `this`
in case we were accidentally capturing any other scope variables by reference.

Fixes solvespace#571
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix segfault in Remove Spline Point context menu Timmmm/solvespace
2 participants
@whitequark @Timmmm