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

Spot potential UB #371

Closed
jcoupey opened this issue Jul 30, 2020 · 2 comments · Fixed by #372
Closed

Spot potential UB #371

jcoupey opened this issue Jul 30, 2020 · 2 comments · Fixed by #372

Comments

@jcoupey
Copy link
Collaborator

jcoupey commented Jul 30, 2020

Using GCC Undefined Behavior Sanitizer (enable by compiling with -fsanitize=undefined) on an instance of the Solomon benchmark results in:

/usr/include/c++/7/bits/stl_vector.h:816:34: runtime error: reference binding to null pointer of type 'const struct value_type'
structures/vroom/tw_route.cpp:453:43: runtime error: reference binding to null pointer of type 'const struct Break'

Because there is no break in input, the mentioned line:

const auto& b = v.breaks[current_break];

defines a reference to the first element of an empty vector. In that case, the surrounding loop is designed in such a way that this reference is not actually used when the vector is empty, but this is still UB all right.

We should:

  • investigate other potential runtime reports using other input examples
  • fix spotted UB
  • setup a way to check this periodically in the long run

No really sure how to go for that last point because setting -fsanitize=undefined slows down things to such a degree that it is totally not practically usable in dev mode.

@jcoupey
Copy link
Collaborator Author

jcoupey commented Jul 31, 2020

Running across the usual academic/ad-hoc benchmarks, the error reported above is the only one we get.

@jcoupey
Copy link
Collaborator Author

jcoupey commented Jul 31, 2020

For what it's worth, I also did a few checks raising no error with -fsanitize=address and -fsanitize=thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant