-
-
Notifications
You must be signed in to change notification settings - Fork 280
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
ci: run sanitizers #955
ci: run sanitizers #955
Conversation
6333dce
to
d97d112
Compare
ASAN is now triggered, but I'm not sure if it is configured properly or useful:
EDIT: it only works for the linker, there is a problem with |
The tests are green, but the leak I added on purpose is undetected 😢 |
@dunglas Did you try with an obvious buffer overflow to see if ASAN is functional:
? |
@TimWolla interesting, I added your snippet: the error is caught on standard builds... but not on ASAN builds! |
Where are you actually building frankenphp? Is this part of |
@iluuu1994 Yes |
Can you dump the compiler output somehow? Are you sure the flags are actually passed properly? |
I printed as many details as possible (and caught a typo in |
You have both an -O2 and -O0, it's possible that the problematic code is optimized away. |
Good catch @iluuu1994, that was the problem and the sanitizer now works! Thank you very much. I'll have to refactor our build process as it doesn't look possible to override the options defined directly in the |
No worries. Note that you may run into issues with msan. msan requires instrumenting all code, it is not enough to instrument some code. In other words, all static and dynamic libraries must be recompiled with msan enabled. Our msan job builds limited extensions for exactly this reason, e.g. no openssl. |
Replaces #806.