Skip to contentSkip to conversation

package/llvm-project/compiler-rt: fix build with Ninja

PATCH In review Thomas Devoogdt 4 participants 8 messages Aug 8, 2023 – Sep 12, 2023

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 package/llvm-project/compiler-rt: fix build with Ninja

ninja: error: dependency cycle: include/sanitizer/allocator_interface.h -> include/sanitizer/allocator_interface.h

It's not clear why this cycle happens, but compiler-rt also proposes to create a separate build folder on their site: https://compiler-rt.llvm.org/.

---
 package/llvm-project/compiler-rt/compiler-rt.mk | 1 +
 1 file changed, 1 insertion(+)
package/llvm-project/compiler-rt/compiler-rt.mk +1 -0
index 03dc76eaea..5d9c3d8fa8 100644
@@ -11,6 +11,7 @@ COMPILER_RT_LICENSE = NCSA MIT
 COMPILER_RT_LICENSE_FILES = LICENSE.TXT
 COMPILER_RT_CPE_ID_VENDOR = llvm
 COMPILER_RT_DEPENDENCIES = host-clang llvm
+COMPILER_RT_SUPPORTS_IN_SOURCE_BUILD = NO
 
 COMPILER_RT_INSTALL_STAGING = YES
 COMPILER_RT_INSTALL_TARGET = NO
Signature · 6 lines
2.34.1

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#1 lore raw
Quoted 6 lines · On Tue, 8 Aug 2023 11:04:07 +0200 Thomas Devoogdt <[email protected]> wrote:

ninja: error: dependency cycle: include/sanitizer/allocator_interface.h -> include/sanitizer/allocator_interface.h

It's not clear why this cycle happens, but compiler-rt also proposes to create a separate build folder on their site: https://compiler-rt.llvm.org/.

Not sure to understand: the default backend for CMake is still makefiles, and compiler-rt was not switched to using ninja. So?

Thomas

Signature · 7 lines
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#2 lore raw

Hi Thomas, Yann, All,

Quoted 2 lines

Not sure to understand: the default backend for CMake is still makefiles, and compiler-rt was not switched to using ninja. So?

No, ninja is not yet the default.
Yann did some tests to find out if it makes sense to make ninja the default. If only a few packages break on it, then we can force them to use make. But if there are too many of them, then we can just keep make as the default.

Either way, see this mail:
https://lore.kernel.org/buildroot/20230806205447.GW421096@scaer/,
where Yann told me that compiler-rt fails on ninja, so that's the reason for the patch.

Kind regards,

Thomas
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#3 lore raw
Quoted 8 lines · On Tue, 8 Aug 2023 13:22:21 +0200 Thomas Devoogdt <[email protected]> wrote:
Quoted 2 lines

Not sure to understand: the default backend for CMake is still makefiles, and compiler-rt was not switched to using ninja. So?

No, ninja is not yet the default.
Yann did some tests to find out if it makes sense to make ninja the default. If only a few packages break on it, then we can force them to use make. But if there are too many of them, then we can just keep make as the default.

Quoted 4 lines

Either way, see this mail:
https://lore.kernel.org/buildroot/20230806205447.GW421096@scaer/,
where Yann told me that compiler-rt fails on ninja, so that's the reason for the patch.

Right, but changing compiler-rt to use out of tree build makes sense together with switching to ninja. So in the same patch, you should have:

COMPILER_RT_SUPPORTS_IN_SOURCE_BUILD = NO
COMPILER_RT_CMAKE_BACKEND = ninja

And then later, when we are ready to make ninja the default backend of cmake, we can remove in one batch all "<pkg>_CMAKE_BACKEND = ninja" statements from all packages. We can also progressively mark as "<pkg>_CMAKE_BACKEND = make" the packages that we know are not compatible with ninja.

At least, that's how I see the transition period. What do you think?

Thomas

Signature · 7 lines
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#4 lore raw
In reply to Thomas Devoogdt

Thomas D., All,

On 2023-08-08 13:22 +0200, Thomas Devoogdt spake thusly:

Quoted 8 lines
Quoted 2 lines

Not sure to understand: the default backend for CMake is still makefiles, and compiler-rt was not switched to using ninja. So?

No, ninja is not yet the default.
Yann did some tests to find out if it makes sense to make ninja the default. If only a few packages break on it, then we can force them to use make. But if there are too many of them, then we can just keep make as the default.

Quoted 4 lines

Either way, see this mail:
https://lore.kernel.org/buildroot/20230806205447.GW421096@scaer/,
where Yann told me that compiler-rt fails on ninja, so that's the reason for the patch.

Yes, but as Thomas P. explains, we only need to fix compiler-rt with ninja if it is actually switched over to using ninja, which your patch does not do.

Presumably, the build of compiler-rt succeeds now, so if it ain't broken, so why would we need to fix it by building out-of-source?

But if you had tested, you'd have noticed that the build *is* already broken even with makefiles. But make is (insanely) forgiving about circular dependencies, and just arbitrarily drops a circular dependency:

Quoted 1 line
Quoted 1 line
Quoted 1 line

compiler-rt 15.0.3 Building

PATH="..." /home/ymorin/dev/buildroot/O/next/per-package/compiler-rt/host/bin/cmake --build /home/ymorin/dev/buildroot/O/next/build/compiler-rt-15.0.3/ -j9 -- make[4]: Circular include/sanitizer/allocator_interface.h <- include/sanitizer/allocator_interface.h dependency dropped.
make[4]: Circular include/sanitizer/asan_interface.h <- include/sanitizer/asan_interface.h dependency dropped.
make[4]: Circular include/sanitizer/common_interface_defs.h <- include/sanitizer/common_interface_defs.h dependency dropped. make[4]: Circular include/sanitizer/coverage_interface.h <- include/sanitizer/coverage_interface.h dependency dropped.
make[4]: Circular include/sanitizer/dfsan_interface.h <- include/sanitizer/dfsan_interface.h dependency dropped.
make[4]: Circular include/sanitizer/hwasan_interface.h <- include/sanitizer/hwasan_interface.h dependency dropped.
make[4]: Circular include/sanitizer/linux_syscall_hooks.h <- include/sanitizer/linux_syscall_hooks.h dependency dropped.
make[4]: Circular include/sanitizer/lsan_interface.h <- include/sanitizer/lsan_interface.h dependency dropped.
make[4]: Circular include/sanitizer/msan_interface.h <- include/sanitizer/msan_interface.h dependency dropped.
make[4]: Circular include/sanitizer/netbsd_syscall_hooks.h <- include/sanitizer/netbsd_syscall_hooks.h dependency dropped.
make[4]: Circular include/sanitizer/scudo_interface.h <- include/sanitizer/scudo_interface.h dependency dropped.
make[4]: Circular include/sanitizer/tsan_interface.h <- include/sanitizer/tsan_interface.h dependency dropped.
make[4]: Circular include/sanitizer/tsan_interface_atomic.h <- include/sanitizer/tsan_interface_atomic.h dependency dropped. make[4]: Circular include/sanitizer/ubsan_interface.h <- include/sanitizer/ubsan_interface.h dependency dropped.
make[4]: Circular include/fuzzer/FuzzedDataProvider.h <- include/fuzzer/FuzzedDataProvider.h dependency dropped.
make[4]: Circular include/sanitizer/memprof_interface.h <- include/sanitizer/memprof_interface.h dependency dropped.
make[4]: Circular include/profile/MemProfData.inc <- include/profile/MemProfData.inc dependency dropped.
make[4]: Circular include/xray/xray_interface.h <- include/xray/xray_interface.h dependency dropped.
make[4]: Circular include/xray/xray_log_interface.h <- include/xray/xray_log_interface.h dependency dropped.
make[4]: Circular include/xray/xray_records.h <- include/xray/xray_records.h dependency dropped.
make[4]: Circular include/orc/c_api.h <- include/orc/c_api.h dependency dropped.
make[4]: Circular include/profile/InstrProfData.inc <- include/profile/InstrProfData.inc dependency dropped.

In this case, this is OK because the file depends on itself, so breaking the circular dependency actually removes the spurious dependency.

However, this is not correct, and thus we do need to build compiler-rt with out-of-source, even with makefile, because it *is* broken.

So, the rationale is wrong: it is not broken because of ninja. It is broken despite ninja, but ninja exposes the breakage.

Really, when we have a patch that fixes something, what is broken and why it is broken, is of the utmost most importance; the explanations for the technical fix is important too, but it only comes second, with the code actually being the less interesting thing.

Can you please resubmit the patch, but with a corrected commit log that summarises the above explanations, please?

Regards,
Yann E. MORIN.

Signature · 10 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#5 lore raw

PATCH v2 1/1 package/llvm-project/compiler-rt: fix circular dependency warning

make[4]: Circular include/sanitizer/allocator_interface.h <- include/sanitizer/allocator_interface.h dependency dropped. make[4]: Circular include/sanitizer/asan_interface.h <- include/sanitizer/asan_interface.h dependency dropped. make[4]: Circular include/sanitizer/common_interface_defs.h <- include/sanitizer/common_interface_defs.h dependency dropped. make[4]: Circular include/sanitizer/coverage_interface.h <- include/sanitizer/coverage_interface.h dependency dropped. make[4]: Circular include/sanitizer/dfsan_interface.h <- include/sanitizer/dfsan_interface.h dependency dropped. make[4]: Circular include/sanitizer/hwasan_interface.h <- include/sanitizer/hwasan_interface.h dependency dropped. make[4]: Circular include/sanitizer/linux_syscall_hooks.h <- include/sanitizer/linux_syscall_hooks.h dependency dropped. make[4]: Circular include/sanitizer/lsan_interface.h <- include/sanitizer/lsan_interface.h dependency dropped. make[4]: Circular include/sanitizer/msan_interface.h <- include/sanitizer/msan_interface.h dependency dropped. make[4]: Circular include/sanitizer/netbsd_syscall_hooks.h <- include/sanitizer/netbsd_syscall_hooks.h dependency dropped. make[4]: Circular include/sanitizer/scudo_interface.h <- include/sanitizer/scudo_interface.h dependency dropped. make[4]: Circular include/sanitizer/tsan_interface.h <- include/sanitizer/tsan_interface.h dependency dropped. make[4]: Circular include/sanitizer/tsan_interface_atomic.h <- include/sanitizer/tsan_interface_atomic.h dependency dropped. make[4]: Circular include/sanitizer/ubsan_interface.h <- include/sanitizer/ubsan_interface.h dependency dropped. make[4]: Circular include/fuzzer/FuzzedDataProvider.h <- include/fuzzer/FuzzedDataProvider.h dependency dropped. make[4]: Circular include/sanitizer/memprof_interface.h <- include/sanitizer/memprof_interface.h dependency dropped. make[4]: Circular include/profile/MemProfData.inc <- include/profile/MemProfData.inc dependency dropped. make[4]: Circular include/xray/xray_interface.h <- include/xray/xray_interface.h dependency dropped. make[4]: Circular include/xray/xray_log_interface.h <- include/xray/xray_log_interface.h dependency dropped. make[4]: Circular include/xray/xray_records.h <- include/xray/xray_records.h dependency dropped. make[4]: Circular include/orc/c_api.h <- include/orc/c_api.h dependency dropped.
make[4]: Circular include/profile/InstrProfData.inc <- include/profile/InstrProfData.inc dependency dropped.

It's not clear why this cycle happens, but compiler-rt also proposes to create a separate build folder on their site: https://compiler-rt.llvm.org/.

---
v2:
- This patch was orignally submitted to fix the ninja build, which does a hard assert on circular dependencies. Make is less strict on it, and retries with arbitrary dropping dependencies. The oringal git message was wrong to mention that it fixes the ninja build since ninja is not enabled by default anyway. So in v2, I just changed the git message to only mention the circular dependency warning.

- Original Ninja error:

ninja: error: dependency cycle: include/sanitizer/allocator_interface.h -> include/sanitizer/allocator_interface.h

---
 package/llvm-project/compiler-rt/compiler-rt.mk | 1 +
 1 file changed, 1 insertion(+)
package/llvm-project/compiler-rt/compiler-rt.mk +1 -0
index 03dc76eaea..5d9c3d8fa8 100644
@@ -11,6 +11,7 @@ COMPILER_RT_LICENSE = NCSA MIT
 COMPILER_RT_LICENSE_FILES = LICENSE.TXT
 COMPILER_RT_CPE_ID_VENDOR = llvm
 COMPILER_RT_DEPENDENCIES = host-clang llvm
+COMPILER_RT_SUPPORTS_IN_SOURCE_BUILD = NO
 
 COMPILER_RT_INSTALL_STAGING = YES
 COMPILER_RT_INSTALL_TARGET = NO
Signature · 6 lines
2.34.1

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#6 lore raw

Re: [Buildroot] [PATCH v2, next] package/llvm-project/compiler-rt: fix circular dependency warning

Thomas, All,

On 2023-08-14 08:36 +0200, Thomas Devoogdt spake thusly:

Quoted 2 lines

make[4]: Circular include/sanitizer/allocator_interface.h <- include/sanitizer/allocator_interface.h dependency dropped. make[4]: Circular include/sanitizer/asan_interface.h <- include/sanitizer/asan_interface.h dependency dropped.

[--SNIP--]

Quoted 1 line

It's not clear why this cycle happens,

Well, it is rather easy to understand:

- include/CMakeLists.txt unconditionally declares dependency rules

    between header files in ${output_dir} and ${CMAKE_CURRENT_SOURCE_DIR}
    without provision for them to be the same directories;

- this triggers the above warnings from make, hich arbitrarily breaks circular dependencies, which in our case happens to be OK.

See how I extended the commit log for more details.

And this is actually a fix for an issue that already happens on master.
So: applied to master, thanks.

Regards,
Yann E. MORIN.

Quoted 38 lines

but compiler-rt also proposes to
create a separate build folder on their site: https://compiler-rt.llvm.org/.

---
v2:
- This patch was orignally submitted to fix the ninja build, which does a hard assert on circular dependencies. Make is less strict on it, and retries with arbitrary dropping dependencies. The oringal git message was wrong to mention that it fixes the ninja build since ninja is not enabled by default anyway. So in v2, I just changed the git message to only mention the circular dependency warning.

- Original Ninja error:

ninja: error: dependency cycle: include/sanitizer/allocator_interface.h -> include/sanitizer/allocator_interface.h

---
 package/llvm-project/compiler-rt/compiler-rt.mk | 1 +
 1 file changed, 1 insertion(+)
package/llvm-project/compiler-rt/compiler-rt.mk +1 -0
index 03dc76eaea..5d9c3d8fa8 100644
@@ -11,6 +11,7 @@ COMPILER_RT_LICENSE = NCSA MIT
 COMPILER_RT_LICENSE_FILES = LICENSE.TXT
 COMPILER_RT_CPE_ID_VENDOR = llvm
 COMPILER_RT_DEPENDENCIES = host-clang llvm
+COMPILER_RT_SUPPORTS_IN_SOURCE_BUILD = NO
 
 COMPILER_RT_INSTALL_STAGING = YES
 COMPILER_RT_INSTALL_TARGET = NO
Signature · 6 lines
2.34.1

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
Signature · 10 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#7 lore raw

Re: [Buildroot] [PATCH v2, next] package/llvm-project/compiler-rt: fix circular dependency warning

Quoted 15 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

Thomas, All,
On 2023-08-14 08:36 +0200, Thomas Devoogdt spake thusly:

Quoted 2 lines

make[4]: Circular include/sanitizer/allocator_interface.h <- include/sanitizer/allocator_interface.h dependency dropped. make[4]: Circular include/sanitizer/asan_interface.h <- include/sanitizer/asan_interface.h dependency dropped.

[--SNIP--]

Quoted 1 line

It's not clear why this cycle happens,

Well, it is rather easy to understand:

- include/CMakeLists.txt unconditionally declares dependency rules

    between header files in ${output_dir} and ${CMAKE_CURRENT_SOURCE_DIR}
    without provision for them to be the same directories;
Quoted 7 lines

- this triggers the above warnings from make, hich arbitrarily breaks circular dependencies, which in our case happens to be OK.

See how I extended the commit log for more details.

And this is actually a fix for an issue that already happens on master.
So: applied to master, thanks.

Committed to 2023.02.x and 2023.05.x, thanks. Luckily git is smart enough to notice that those don't have the llvm-project/ sub directory and do the right thing.

Signature · 5 lines
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot