Skip to content

Only set _FORTIFY_SOURCE when a higher level of this flag has not been set#4703

Merged
claucambra merged 2 commits into
masterfrom
work/remove-cmake-fortify-source
Jul 12, 2022
Merged

Only set _FORTIFY_SOURCE when a higher level of this flag has not been set#4703
claucambra merged 2 commits into
masterfrom
work/remove-cmake-fortify-source

Conversation

@claucambra

@claucambra claucambra commented Jul 5, 2022

Copy link
Copy Markdown
Collaborator

Fixes #4697

@bnavigator

Signed-off-by: Claudio Cambra claudio.cambra@gmail.com

@claucambra claucambra self-assigned this Jul 5, 2022
@codecov

codecov Bot commented Jul 5, 2022 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #4703 (4532293) into master (53aba48) will increase coverage by 0.12%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4703      +/-   ##
==========================================
+ Coverage   56.43%   56.56%   +0.12%     
==========================================
  Files         138      138              
  Lines       17068    17068              
==========================================
+ Hits         9632     9654      +22     
+ Misses       7436     7414      -22
Impacted Files Coverage Δ
src/libsync/discovery.cpp 84.56% <0.00%> (-0.30%) ⬇️
src/libsync/propagatedownload.cpp 65.33% <0.00%> (+0.14%) ⬆️
src/libsync/syncengine.cpp 85.74% <0.00%> (+0.54%) ⬆️
src/libsync/vfs/cfapi/cfapiwrapper.cpp 77.34% <0.00%> (+1.92%) ⬆️
src/libsync/vfs/cfapi/vfs_cfapi.cpp 86.46% <0.00%> (+2.62%) ⬆️
src/libsync/vfs/cfapi/hydrationjob.cpp 56.45% <0.00%> (+3.76%) ⬆️

@claucambra claucambra force-pushed the work/remove-cmake-fortify-source branch from 6fa545c to 0178318 Compare July 5, 2022 12:17
@mgallien

mgallien commented Jul 6, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator

sounds like it is nor working for me
setting up my build folder that way

cmake .. -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo -DBUILD_UPDATER=ON CFLAGS="-D_FORTIFY_SOURCE=3"

when compiling I get this

/usr/bin/c++ -DQT_CONCURRENT_LIB -DQT_CORE_LIB -DQT_DISABLE_DEPRECATED_BEFORE=0x000000 -DQT_MESSAGELOGCONTEXT -DQT_NO_CAST_FROM_ASCII -DQT_NO_CAST_FROM_BYTEARRAY -DQT_NO_CAST_TO_ASCII -DQT_NO_DEBUG -DQT_NO_URL_CAST_FROM_STRING -DQT_USE_QSTRINGBUILDER -DUNICODE -DWITH_WEBENGINE=1 -D_UNICODE -Dnextcloud_csync_EXPORTS -I/path/desktop/build-release/src/csync -I/path/desktop/src/csync -I/path/desktop/build-release/src/csync/nextcloud_csync_autogen/include -I/path/desktop/build-release -I/path/desktop/src -I/path/desktop/build-release/src -I/path/desktop/src/csync/std -isystem /usr/include/x86_64-linux-gnu/qt5 -isystem /usr/include/x86_64-linux-gnu/qt5/QtCore -isystem /usr/lib/x86_64-linux-gnu/qt5/mkspecs/linux-g++ -isystem /usr/include/x86_64-linux-gnu/qt5/QtConcurrent -Wpedantic -fstack-protector-strong -D_FORTIFY_SOURCE=2 -O2 -g -DNDEBUG -fPIC -fdiagnostics-color=always -fPIC -std=gnu++14 -MD -MT src/csync/CMakeFiles/nextcloud_csync.dir/csync_exclude.cpp.o -MF src/csync/CMakeFiles/nextcloud_csync.dir/csync_exclude.cpp.o.d -o src/csync/CMakeFiles/nextcloud_csync.dir/csync_exclude.cpp.o -c /path/desktop/src/csync/csync_exclude.cpp
/path/desktop/src/csync/csync_exclude.cpp:869:2: error: #error _FORTIFY_SOURCE
  869 | #error _FORTIFY_SOURCE
      |  ^~~~~

so the value is still -D_FORTIFY_SOURCE=2

@claucambra claucambra force-pushed the work/remove-cmake-fortify-source branch 2 times, most recently from 366a47f to 5d21093 Compare July 6, 2022 11:50
@claucambra

claucambra commented Jul 6, 2022

Copy link
Copy Markdown
Collaborator Author

sounds like it is nor working for me setting up my build folder that way

cmake .. -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo -DBUILD_UPDATER=ON CFLAGS="-D_FORTIFY_SOURCE=3"

when compiling I get this

/usr/bin/c++ -DQT_CONCURRENT_LIB -DQT_CORE_LIB -DQT_DISABLE_DEPRECATED_BEFORE=0x000000 -DQT_MESSAGELOGCONTEXT -DQT_NO_CAST_FROM_ASCII -DQT_NO_CAST_FROM_BYTEARRAY -DQT_NO_CAST_TO_ASCII -DQT_NO_DEBUG -DQT_NO_URL_CAST_FROM_STRING -DQT_USE_QSTRINGBUILDER -DUNICODE -DWITH_WEBENGINE=1 -D_UNICODE -Dnextcloud_csync_EXPORTS -I/path/desktop/build-release/src/csync -I/path/desktop/src/csync -I/path/desktop/build-release/src/csync/nextcloud_csync_autogen/include -I/path/desktop/build-release -I/path/desktop/src -I/path/desktop/build-release/src -I/path/desktop/src/csync/std -isystem /usr/include/x86_64-linux-gnu/qt5 -isystem /usr/include/x86_64-linux-gnu/qt5/QtCore -isystem /usr/lib/x86_64-linux-gnu/qt5/mkspecs/linux-g++ -isystem /usr/include/x86_64-linux-gnu/qt5/QtConcurrent -Wpedantic -fstack-protector-strong -D_FORTIFY_SOURCE=2 -O2 -g -DNDEBUG -fPIC -fdiagnostics-color=always -fPIC -std=gnu++14 -MD -MT src/csync/CMakeFiles/nextcloud_csync.dir/csync_exclude.cpp.o -MF src/csync/CMakeFiles/nextcloud_csync.dir/csync_exclude.cpp.o.d -o src/csync/CMakeFiles/nextcloud_csync.dir/csync_exclude.cpp.o -c /path/desktop/src/csync/csync_exclude.cpp
/path/desktop/src/csync/csync_exclude.cpp:869:2: error: #error _FORTIFY_SOURCE
  869 | #error _FORTIFY_SOURCE
      |  ^~~~~

so the value is still -D_FORTIFY_SOURCE=2

Should be fixed now

@claucambra
Only set _FORTIFY_SOURCE when a higher level of this flag has not bee…
850d23c 
…n set

Signed-off-by: Claudio Cambra <claudio.cambra@gmail.com>
@camilasan camilasan force-pushed the work/remove-cmake-fortify-source branch from 5d21093 to 850d23c Compare July 12, 2022 18:23
@camilasan

camilasan commented Jul 12, 2022

Copy link
Copy Markdown
Member

/backport to stable-3.5

@nextcloud-desktop-bot

nextcloud-desktop-bot commented Jul 12, 2022

Copy link
Copy Markdown

AppImage file: Nextcloud-PR-4703-4532293e2a9818f14136c3f3caa56bfa9757e906-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@sonarqubecloud

sonarqubecloud Bot commented Jul 12, 2022

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@claucambra claucambra merged commit ea0ab54 into master Jul 12, 2022
@claucambra claucambra deleted the work/remove-cmake-fortify-source branch July 12, 2022 22:13
@backportbot-nextcloud

backportbot-nextcloud Bot commented Jul 12, 2022

Copy link
Copy Markdown

The backport to stable-3.5 failed. Please do this backport manually.

@claucambra

claucambra commented Jul 12, 2022

Copy link
Copy Markdown
Collaborator Author

/backport to stable-3.5

@backportbot-nextcloud

backportbot-nextcloud Bot commented Jul 12, 2022

Copy link
Copy Markdown

The backport to stable-3.5 failed. Please do this backport manually.

@claucambra claucambra mentioned this pull request Jul 15, 2022
Merged
@mgallien mgallien added this to the 3.6.0 milestone Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mgallien mgallien mgallien approved these changes
@camilasan camilasan camilasan approved these changes
@allexzander allexzander Awaiting requested review from allexzander

Assignees

@claucambra claucambra

Labels

backport-request

Projects

None yet

Milestone

3.6.0

Development

Successfully merging this pull request may close these issues.

[Bug]: hardcoded _FORTIFY_SOURCE=2 conflicts with distribution presets

4 participants

@claucambra @mgallien @camilasan @nextcloud-desktop-bot