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

Use address and UB sanitizers #1082

Open
jcoupey opened this issue Apr 8, 2024 · 3 comments
Open

Use address and UB sanitizers #1082

jcoupey opened this issue Apr 8, 2024 · 3 comments
Milestone

Comments

@jcoupey
Copy link
Collaborator

jcoupey commented Apr 8, 2024

For testing/debug purposes, we should probably turn on sanitizers in order to try and catch potential code problems. A good place to start would be to add -fsanitize=address -fno-omit-frame-pointer and -fsanitize=undefined to the default (non-release) build and see what happens when running on various benchmarks.

My current understanding is that both gcc and clang support those flags. From this gcc doc it looks like the UB checks would be the same under the hood. Not sure about the address checking, probably worth checking with both compilers.

@jcoupey jcoupey added this to the v1.15.0 milestone Apr 8, 2024
@jcoupey
Copy link
Collaborator Author

jcoupey commented Apr 12, 2024

I did a simple test using clang-15 and basic VRPTW benchmark instances. The good news is that nothing is caught by the sanitizers, but the slowdown is somewhere in the x15 to x20 range!

This makes it impracticable when doing proper benchmarking, which happens quite a lot during dev phases. So I have mixed feelings about that: on one hand it would be nice to always enable the sanitizers in dev mode, on the other hand it is only realistic to do so occasionally for targeted tests.

Maybe a good trade-off would be to define a subset of the usual academic benchmarks (e.g. a few instances per type), then periodically run on those with the sanitizers enabled. This could be done before releases, on even on CI if it is fast enough.

@kkarbowiak
Copy link
Contributor

I think adding some checks in a CI is a very good idea. If the slowdown is this big, those could be limited only to release branches (or even release tags). If they catch an error, it can later be backtraced using git bisect command.

An imortant thing would be to try and exercise as many paths in code as possible (so use instances of many types).

One additional sanitizer that could be useful is array-bounds, but I'd have to check whether it's supported by both gcc and clang or just one of them.

@jcoupey
Copy link
Collaborator Author

jcoupey commented Apr 25, 2024

The time it takes to solve a bunch of small instances from various benchmarks would not be that big compared to the overall build time, so probably worth it. I can work on selecting a variety of such instances.

Ideally we would build with those flags on CI in order to run the tests, but keep the flags out of the way for local dev builds. Not sure exactly how this could be achieved conveniently.

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

No branches or pull requests

2 participants