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

SEGFAULT in Debug mode with MSVC in merge_insertion_sort #184

Closed
Morwenn opened this issue Feb 15, 2021 · 2 comments
Closed

SEGFAULT in Debug mode with MSVC in merge_insertion_sort #184

Morwenn opened this issue Feb 15, 2021 · 2 comments
Labels
Milestone

Comments

@Morwenn
Copy link
Owner

Morwenn commented Feb 15, 2021

merge_insertion_sort often segfaults with MSVC for an obscure reason. I managed to track the issue down to that specific line prior to the pairwise swaps:

auto end = has_stray ? std::prev(last) : last;

The line itself doesn't cause the segfault, but the destruction of end at the end of the algorithm scope does. I am so far unsure why, more tests are needed.

@Morwenn Morwenn added the bug label Feb 15, 2021
@Morwenn Morwenn changed the title SEGFAULT with MSVC in merge_insertion_sort SEGFAULT in Debug mode with MSVC in merge_insertion_sort Feb 16, 2021
Morwenn added a commit that referenced this issue Mar 4, 2021
The way it was written, the destructor never actually destructed the
node's values. The loop variable was constructed with sentinel_node_,
which was also used in the loop condition in such a way that the loop
body was never executed.

Might be the cause of issue #184
@Morwenn
Copy link
Owner Author

Morwenn commented Mar 4, 2021

The previous commit wasn't a silver bullet, but it did help. There are no segfaults anymore, and we're down to two failing tests:

175 - test most sorters with no_post_iterator - cppsort::merge_insertion_sorter (Timeout)
759 - merge_insertion_sorter tests with projections (Timeout)

The timeouts make me suspect an infinite loop, but that's an investigation for another day. Unfortunately, the PRNG seed wasn't logged. Maybe we should log it directly from CMake in order to make extra sure.

@Morwenn
Copy link
Owner Author

Morwenn commented Mar 5, 2021

It looks like the commit 1160154 made the timeout issues disappear. I don't know how they're linked, but it doesn't seem extremely surprising that they are.

I'm gonna keep the issue opened a bit longer, and close it if the problem doesn't reappear.

@Morwenn Morwenn added this to the 1.10.0 milestone Mar 8, 2021
@Morwenn Morwenn closed this as completed Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant