Support --enable-sanitizer for MSVC builds #16999
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
While it is already possible to enable ASan for MSVC (assuming Visual Studio 2019 16.10 or later) by passing `/fsanitizer=address` in the `CFLAGS`, it is only usable if `ZEND_DEBUG` is also enabled; otherwise there are `STATUS_BACK_STACK` errors all the time. Since it makes some sense to combine ASan instrumentation with debug assertions enabled anyway (typical for fuzzing), we support the configure option `--enable-sanitizer`, which is already supported for Clang builds, also for MSVC builds. Note that MSVC supports only ASan for now; contrary to Clang which additionally supports UBSan on Windows. Since ASan reports can be pretty useless without debug symbol information, we require such builds to also produce PDBs (i.e. `--enable-debug-pack`), but forbid actual debug builds (for performance reasons, and because the way it is implemented it would not make sense; that was already an issue with Clang builds with sanitizers enabled).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While it is already possible to enable ASan for MSVC (assuming Visual Studio 2019 16.10 or later) by passing
/fsanitizer=addressin theCFLAGS, it is only usable ifZEND_DEBUGis also enabled; otherwise there areSTATUS_BACK_STACKerrors all the time.Since it makes some sense to combine ASan instrumentation with debug assertions enabled anyway (typical for fuzzing), we support the configure option
--enable-sanitizer, which is already supported for Clang builds, also for MSVC builds. Note that MSVC supports only ASan for now; contrary to Clang which additionally supports UBSan on Windows.Since ASan reports can be pretty useless without debug symbol information, we require such builds to also produce PDBs (i.e.
--enable-debug-pack), but forbid actual debug builds (for performance reasons, and because the way it is implemented it would not make sense; that was already an issue with Clang builds with sanitizers enabled).Note that there are basically no changes regarding Clang sanitizers (besides that
--enable-debug --enable-sanitzeris explicitly forbidden; previously that would not properly enable sanitizers anyway). I have some doubts that the flags which are set for Clang sanitizers make much sense (they even might not properly work), but that could be addressed by another patch (if anyone cares about that).Also note that phpize builds do not properly support sanitizers (with or without this patch). It is not even quite clear how that should be done (probably, if a phpize build uses an instrumented PHP build, the extension should automaticallybe instrumented as well). On the other hand, it might not make much sense to support this for phpize at all; after all, we do not provide prebuilt ASan instrumentation on Windows, and those who want to use this, might be better off doing in-tree builds, anyway. I'm not sure, but I guess it's pretty much the same for phpize builds on POSIX; these use release builds of PHP (neither ASan instrumented nor debug builds).
My main reason for suggesting this change is to be able to have ASan builds running in CI (and possibly for fuzz testing). See #15978 for a CI run. Since this is pretty slow (tests take more than an hour), it is unsuitable for pushes, but appears to be a sensible addition for nightly runs, particularly to catch memory issues in Windows only code paths.
If one needs a full debug build with ASan instrumentation for whatever reason, it is still possible to set
CFLAGS=/fsanitizer=addressand toconfigure --enable-debug.