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

Add sanitizer option to user presets #81

Open
avitase opened this issue Feb 1, 2023 · 5 comments
Open

Add sanitizer option to user presets #81

avitase opened this issue Feb 1, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@avitase
Copy link

avitase commented Feb 1, 2023

I would like to see an option to enable the address (and UB?) sanitizer(s) in CMakeUserPresets.json or even enable it/them by default for (at least) the unit tests. The address sanitizer is available for MSVC so this one can safetly be move as a OS agnostic option.

@friendlyanon
Copy link
Owner

I am actually in the process of adding hardening flags to the presets, so I guess this is covered by that. I'll think about whether to make sanitizers the default or not. Right now, there is a separate sanitizer build config for CI that one can also use locally.

@friendlyanon friendlyanon added the enhancement New feature or request label Feb 1, 2023
@friendlyanon
Copy link
Owner

0.35.0 added hardening flags for all platforms, however I'm not sure of the usefulness of adding a separate sanitizer user preset. Basically it comes down to the fact that Windows does not really have feature parity with Linux and Mac, but I also don't want to put too many things in the user presets. I want the user presets to be easily reproducible using the contribution documents.

@avitase
Copy link
Author

avitase commented Feb 15, 2023

IMO, the address sanitizer on its own is already very helpful, and it is available for MSVC.

@friendlyanon
Copy link
Owner

I'd like to think that based on how ci-sanitize is put together, one could do the same for MSVC locally. Maybe once MSVC gets LSAN and UBSAN, sanitizers could be made the default, but until then I'd like to keep the presets consistent. There is also the issue with sanitizers being the default is that MSVC for example has reduced capabilities:

However, it isn't compatible with edit-and-continue, incremental linking, and /RTC.

For anyone interested, adding sanitizer based on the existing ci-sanitize preset for Windows would look like this:

    {
      "name": "dev",
      "binaryDir": "${sourceDir}/build/dev",
      "inherits": "dev-win64",
      "cacheVariables": {
        "CMAKE_CONFIGURATION_TYPES": "Sanitize;RelWithDebInfo;Release;Debug",
        "CMAKE_CXX_FLAGS_SANITIZE": "/fsanitize=address /MDd /Zi /Od",
        "CMAKE_MAP_IMPORTED_CONFIG_SANITIZE": "Sanitize;RelWithDebInfo;Release;Debug;",
        "CMAKE_EXE_LINKER_FLAGS_SANITIZE": "/debug /INCREMENTAL:NO",
        "CMAKE_SHARED_LINKER_FLAGS_SANITIZE": "/debug /INCREMENTAL:NO"
      }
    },

The "configuration" property in the build and test presets were appropriately adjusted to "Sanitize". Adjusting CMAKE_CONFIGURATION_TYPES is necessary, because that's how multi-config generators know what configs to allow at build time and what configs to allow when looking up dependencies, which is also related to CMAKE_MAP_IMPORTED_CONFIG.

For other platforms, you can likely just copy paste the CI preset and adjust for your platform.


Granted, the above adds a new preset to the usual ones, because I don't want to overload the other configurations. Sanitizers can be used for for debug and release builds, so that's why I don't put this in say Release or Debug.

@sharadhr
Copy link

sharadhr commented Sep 7, 2023

There is also the issue with sanitizers being the default is that MSVC for example has reduced capabilities:

For the record, the various sanitisers are also incompatible with -D_FORTIFY_SOURCE (google/sanitizers#247), which is used in flags-linux.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants