[CMake] Fix LLVM build non-determinism on RHEL

[amyk]

Summary

On RHEL, the OS tooling (ar, ranlib) is not deterministic by default. Therefore, we cannot get bit-for-bit identical builds.

The goal of this patch is that it adds the flags required to force determinism.

[hubert.reinterpretcast]

llvm/cmake/modules/HandleLLVMOptions.cmake

155

Checking for > 2.22 introduces the possibility that the check passes for patch/fix levels of 2.22 when we actually cared about 2.23.

[amyk]

llvm/cmake/modules/HandleLLVMOptions.cmake

155

@hubert.reinterpretcast
The regex used only picks up the first two 2.22, 2.23, etc. It doesn't pick up any subversion, e.g. 2.23.1

Here's some example output:

$ ranlib --version
GNU ranlib (GNU Binutils for Ubuntu) 2.26.1

Within cmake, ([0-9]+\\.[0-9]+) will pick up 2.26, so the check would be correct. Is this an acceptable change?

[hubert.reinterpretcast]

This patch LGTM.

llvm/cmake/modules/HandleLLVMOptions.cmake

155

Yes; thanks for pointing that out. Sorry for the noise.

[MaskRay]

I think it might be better checking ${CMAKE_RANLIB} rD t.a works, because the user may specify llvm-ar as CMAKE_RANLIB.

[MaskRay]

binutils 2.22 was released in 2011. Do we want to support binutils that old? If not, we should probably not do such change, but rather bump the version requirement https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library

CC @hans why may know more about these problems.

[hans]

In D64817#1637401, @MaskRay wrote:

binutils 2.22 was released in 2011. Do we want to support binutils that old? If not, we should probably not do such change, but rather bump the version requirement https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library

CC @hans why may know more about these problems.

I don't really. JF is the master of supported versions of things.

[jfb]

In D64817#1637436, @hans wrote:

In D64817#1637401, @MaskRay wrote:

binutils 2.22 was released in 2011. Do we want to support binutils that old? If not, we should probably not do such change, but rather bump the version requirement https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library

CC @hans why may know more about these problems.

I don't really. JF is the master of supported versions of things.

We have a process for toolchain bumps. Someone needs to write a proposal and shepherd it through. I’m on vacation, won’t be me 🙂

[tstellar]

Which version of RHEL is this patch meant for? Since the jump to C++14, on RHEL <= 7, you will need to compile llvm with the devtoolset toolchain, which has a newer version of binutils. This fix is probably only needed on the release/9.x branch (which can still be built by the system toolchain on RHEL7) and not trunk.

[hubert.reinterpretcast]

In D64817#1638218, @tstellar wrote:

Which version of RHEL is this patch meant for? Since the jump to C++14, on RHEL <= 7, you will need to compile llvm with the devtoolset toolchain, which has a newer version of binutils. This fix is probably only needed on the release/9.x branch (which can still be built by the system toolchain on RHEL7) and not trunk.

It is possible to build with Clang and libc++ and not use a newer version of binutils for trunk.

[tstellar]

In D64817#1638283, @hubert.reinterpretcast wrote:

In D64817#1638218, @tstellar wrote:

Which version of RHEL is this patch meant for? Since the jump to C++14, on RHEL <= 7, you will need to compile llvm with the devtoolset toolchain, which has a newer version of binutils. This fix is probably only needed on the release/9.x branch (which can still be built by the system toolchain on RHEL7) and not trunk.

It is possible to build with Clang and libc++ and not use a newer version of binutils for trunk.

Is this the use case this patch is intended to fix?

[hubert.reinterpretcast]

In D64817#1638218, @tstellar wrote:

Which version of RHEL is this patch meant for? Since the jump to C++14, on RHEL <= 7, you will need to compile llvm with the devtoolset toolchain, which has a newer version of binutils. This fix is probably only needed on the release/9.x branch (which can still be built by the system toolchain on RHEL7) and not trunk.

It is possible to build with Clang and libc++ and not use a newer version of binutils for trunk.

Is this the use case this patch is intended to fix?

I may be out of date on the info, but earlier in the year, we did have both devtoolset and Clang+libc++ builds on RHEL <= 7.

[amyk]

The intention was simply to fix non determinism when building on RHEL, and the version checks were simply to make sure that this patch only gets applied to versions of ar/ranlib that support it.

I didn't realize that the r option on ar also creates a new archive that we can simply test for support on, so I think simply removing all notions of versioning is the correct route to go. If any of the steps fail (non-support), it falls back to using the default so should not cause any new failures. Even with DTS7, we still do not get reproducible builds, so this patch should still apply to all branches.

[amyk]

Updated with appropriate diff.

[tstellar]

llvm/cmake/modules/HandleLLVMOptions.cmake

149–150

Can you make this comment more specific about which versions of RHEL and also ar and randlib this is intended for? If we ever drop support for these older tools, this would make it easier to identify and remove this section.

Otherwise, LGTM.

[amyk]

Updated with more specific comment.

[amyk]

llvm/cmake/modules/HandleLLVMOptions.cmake

149–150

Thanks for pointing that out. The comment has been updated.

[amyk]

Ping.

Does anyone else have any specific comments regarding this patch?

[tstellar]

LGTM. Should we also backport this to the 9.0 branch?

[MaskRay]

According to https://reviews.llvm.org/D64817#1638218 , shall we just push the change to the release/9.x branch? binutils 2.22 becomes more and more irrelevant.

[amyk]

@tstellar Yes, I will commit this patch and also open a bugzilla for back porting.

[MaskRay]

llvm/cmake/modules/HandleLLVMOptions.cmake

148

This overrides the user-specified -DCMAKE_CXX_ARCHIVE_CREATE='<CMAKE_AR> <LINK_FLAGS> qcTD <TARGET> <OBJECTS>'. See D116850

llvm/cmake/modules/HandleLLVMOptions.cmake

   set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} -Wl,-flat_namespace -Wl,-undefined -Wl,suppress")
 endif()
 
+if(${CMAKE_SYSTEM_NAME} MATCHES "Linux")
+  # RHEL has ar and ranlib being non-deterministic by default. The D flag forces determinism,
+  # however only GNU version of ar and ranlib have this option
+  execute_process(COMMAND ${CMAKE_RANLIB} --version
+                  WORKING_DIRECTORY ${PROJECT_SOURCE_DIR} RESULT_VARIABLE RANLIB_RESULT OUTPUT_VARIABLE RANLIB_OUTPUT)
+  if((${RANLIB_RESULT} EQUAL 0) AND ("${RANLIB_OUTPUT}" MATCHES "GNU ranlib") AND ("${RANLIB_OUTPUT}" MATCHES "([0-9]+\\.[0-9]+)"))
+    # Version 2.23 is the first version of binutils where both ar and ranlib have the D flag
+    if("${CMAKE_MATCH_0}" VERSION_GREATER "2.22")
+      set(CMAKE_C_ARCHIVE_CREATE "<CMAKE_AR> Dqc <TARGET> <LINK_FLAGS> <OBJECTS>")
+      set(CMAKE_C_ARCHIVE_APPEND "<CMAKE_AR> Dq  <TARGET> <LINK_FLAGS> <OBJECTS>")
+      set(CMAKE_C_ARCHIVE_FINISH "<CMAKE_RANLIB> -D <TARGET>")
+  
+      set(CMAKE_CXX_ARCHIVE_CREATE "<CMAKE_AR> Dqc <TARGET> <LINK_FLAGS> <OBJECTS>")
+      set(CMAKE_CXX_ARCHIVE_APPEND "<CMAKE_AR> Dq  <TARGET> <LINK_FLAGS> <OBJECTS>")
+      set(CMAKE_CXX_ARCHIVE_FINISH "<CMAKE_RANLIB> -D <TARGET>")
+    endif()
+  endif()
+endif()
+
 if(${CMAKE_SYSTEM_NAME} MATCHES "AIX")
   if(NOT LLVM_BUILD_32_BITS)
     if (CMAKE_CXX_COMPILER_ID MATCHES "XL")