Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upEnable Link Time Optimization (LTO) with clang #2920
Conversation
* Change the LLVM formula to build the LLVMgold plugin * Pass "-flto" for osquery and all dependencies when building with clang on Unix * Pass "-flto" to modules when they are built * Set `ranlib` and `ar` to their LLVM verions when building on Linux
|
ok to test |
Updat the revision on llvm formula in llvm.rb to force a rebuild. Re-enable bottle code since rebuilds are now triggered by the revision change.
|
@artemdinaburg updated the pull request - view changes |
| @@ -142,6 +148,20 @@ else() | |||
| endif() | |||
| endif() | |||
|
|
|||
| if(UNIX AND NOT APPLE) | |||
theopolis
Jan 20, 2017
Member
Maybe just if(LINUX)
Maybe just if(LINUX)
theopolis
Jan 20, 2017
Member
Could you add the CMAKE_CXX_COMPILER MATCHES "clang" to this conditional too? It seems more consistent with the rest of the CMake logic changes.
Could you add the CMAKE_CXX_COMPILER MATCHES "clang" to this conditional too? It seems more consistent with the rest of the CMake logic changes.
artemdinaburg
Jan 21, 2017
Author
Contributor
I kept it at UNIX AND NOT APPLE since I figured other platforms like FreeBSD can also build with LTO; although I have not tested. If FreeBSD and other UNIXes are not a worry, I can change it to IF(LINUX)
I kept it at UNIX AND NOT APPLE since I figured other platforms like FreeBSD can also build with LTO; although I have not tested. If FreeBSD and other UNIXes are not a worry, I can change it to IF(LINUX)
theopolis
Jan 21, 2017
Member
Right, there're too many conditionals using LINUX, other UNIXes have been left behind.
Right, there're too many conditionals using LINUX, other UNIXes have been left behind.
| @@ -33,6 +33,12 @@ else() | |||
| -Wno-unused-parameter | |||
| -Wno-gnu-case-range | |||
| ) | |||
| if (CMAKE_CXX_COMPILER MATCHES "clang") | |||
theopolis
Jan 20, 2017
Member
You should be able to nix this.
You should be able to nix this.
artemdinaburg
Jan 21, 2017
Author
Contributor
Are we guaranteed to be on clang at this point? I added it since if someone were to set CC=gcc as an environment variable, -flto would cause issues.
Are we guaranteed to be on clang at this point? I added it since if someone were to set CC=gcc as an environment variable, -flto would cause issues.
theopolis
Jan 21, 2017
Member
Ah, I meant the whole body, the CMakeLists.txt in the top level should have added this compile option right?
Ah, I meant the whole body, the CMakeLists.txt in the top level should have added this compile option right?
| # When doing a clang on Unix LTO build, the link step also needs | ||
| # -flto on the comand line | ||
| ADD_OSQUERY_LINK_CORE("-flto") | ||
| ADD_OSQUERY_LINK_ADDITIONAL("-flto") |
theopolis
Jan 20, 2017
Member
You can nix this, core's links should be added to additional's IIRC.
You can nix this, core's links should be added to additional's IIRC.
|
@artemdinaburg updated the pull request - view changes |
ranlibandarto their LLVM versions when building on LinuxIn my quick tests, an LTO build results in an
osquerydbinary thats about 2MiB smaller than the equivalent non-LTO build.Enabling LTO is also a first step to turning on Control Flow Integrity.