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

out-of-bounds sequence access when iterating over the result of set_symmetric_difference with a range for. #228

Closed
indiosmo opened this issue Dec 13, 2024 · 3 comments · Fixed by #229
Labels
bug Something isn't working

Comments

@indiosmo
Copy link

indiosmo commented Dec 13, 2024

Getting an out-of-bounds sequence access error when trying to iterate on the result of a set_symmetric_difference.
I'm using trunk.

#include "flux.hpp"
#include <iostream>
#include <cassert>

int main()
{
  std::array arr1{0, 1, 2, 3, 4, 5};
  std::array arr2{   1,    3,    5, 6, 7};
  auto merged = flux::set_symmetric_difference(flux::ref(arr1), flux::ref(arr2));

  assert(flux::equal(merged, std::array{0, 2, 4, 6, 7}));

  for (auto i : merged) {
    // crashes here
    std::cout << i << "\n";
  }

  return 0;
}

Link to the example: https://godbolt.org/z/z6E1eashz

I tried several variations, like reifying into a vector, also crashes

auto vec = merged.to<std::vector>(); // crashes

Interestingly I have a much more complex example using scan and map on the result of set_symmetric_difference and it works.
https://godbolt.org/z/9ex3se6hY

@tcbrindle tcbrindle added the bug Something isn't working label Dec 29, 2024
@tcbrindle
Copy link
Owner

Hi @indiosmo, thanks for the detailed bug report.

I think I can see what the problem is: set_symmetric_difference_adaptor's cursor_type should compare equal when both underlying cursors are the same; but the defaulted equality operator is also comparing the state_ field. This means that when using iterators, the end check never returns true and so iteration tries to run off the end of the underlying array, which (fortunately) gets caught by the bounds check.

tcbrindle added a commit that referenced this issue Dec 29, 2024
Manually implement `set_symmetric_difference_adaptor::cursor_type`'s `operator==` so that it doesn't compare the `state_` field

Fixes #228
tcbrindle added a commit that referenced this issue Dec 31, 2024
@tcbrindle
Copy link
Owner

Hi @indiosmo, this should now be fixed. Thanks again for the bug report, please let me know if you come across any more problems

@indiosmo
Copy link
Author

Cool, I'll test it in a few days. Thanks for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants