-
Notifications
You must be signed in to change notification settings - Fork 1.5k
added -fsanitizer=integer to UBSAN #2922
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
base: main
Are you sure you want to change the base?
Conversation
@danmar The UBSAN build found an issue - could you please have a look? Thanks!
|
Compiling a sanitized build with GCC is much slower than Clang it seems. Maybe we should switch compilers for those. |
I will try to look ASAP! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this
I could not reproduce this immediately. I fear I don't want to spend a lot more time on clang importer now before the release. |
I could reproduce it. |
Thanks! Now I can also see the problem. |
I have updated clangimport.cpp upstream so please try it out.. |
0c84436
to
0a75871
Compare
@danmar A new UBSAN finding while using Clang as compiler...
Submitted #2927 |
UBSAN compilation is also much faster with Clang - 5 minutes instead of 20. ASAN as well under 4 compared to over 6. Also something interesting in the Clang ASAN (actually LSAN) build:
|
Turns out we never waited for all child processes to be finished. We waited for all the pipes no longer being in use and assumed for each pipe there was one finished child process which is not the case. A difference can be seen by the Still the issue reported by LSAN isn't finished yet. |
Requires explicit conversion - will prepare a PR. |
9629d68
to
1f5323f
Compare
Will clean this up tomorrow so we can merge it. |
Requires explicit conversions. Will prepare a PR. |
@danmar This is in the code which you just adjusted in 8a1c16a |
I can reproduce this. |
There's no |
A fix 9aa6966 |
|
|
|
Usage of |
|
Sanitizer passes now - might still need to fix some unit tests. |
Please leave this open until this whole mess is sorted out. I open a different PR with the non-integer flags. |
e295f4e
to
dfbfe95
Compare
Will clean this up and try to activate this with less checks. |
No description provided.