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 CMake option to toggle stack protection #286

Merged
merged 1 commit into from
Jul 12, 2020

Conversation

chrisdalke
Copy link
Contributor

Using the MinGW-W64 compiler on Windows results in a segfault when a database is initially opened. This seems to be the result of the -fstack-protector flag, which has patchy support in MinGW.

Note some other major projects that have encountered this, and don't add this flag when compiling with MinGW on Windows:
bitcoin/bitcoin#8732
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86832

I can reproduce this with the example program and embedded sqlite3 library -- Building with MSVC works fine, and building with MinGW-W64 with the flag removed works fine, but segfaults at sqlite3_open_v2() with the flag added.

This PR adds a CMake option, SQLITECPP_USE_STACK_PROTECTION, defaulting to enabled, which allows disabling of this flag. Some other pull requests on this project to improve hardening (see #262) also mention this flag. In KOLANICH's PR, he excludes this flag unless the platform is Linux. I think that a good short-term solution short of refactoring all the hardening flags is to just keep this behind an option and document it with a comment in the CMakeFile.

@chrisdalke chrisdalke changed the title Add CMake option to toggle stack protection hardening Add CMake option to toggle stack protection Jul 12, 2020
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 1aa8286 on chrisdalke:master into f757b64 on SRombauts:master.

@SRombauts SRombauts self-assigned this Jul 12, 2020
@SRombauts SRombauts merged commit 9106e8d into SRombauts:master Jul 12, 2020
@SRombauts
Copy link
Owner

Thanks a lot, I'll also have a look at the aforementioned PR!

Cheers!
Sébastien

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

Successfully merging this pull request may close these issues.

3 participants