Skip to content

Commit

Permalink
[Runner] Adjust clang flags to make it work slightly better on Linux (#…
Browse files Browse the repository at this point in the history
…405)

* [Runner] Adjust `clang` flags to make it work slightly better on Linux

* [Runner] Remove completely `${sysroot}/include` from headers search paths for clang

Doesn't seem to be useful (it's empty for all platforms as far as I could see)
and it's in the default search path anyway:

```console
sandbox:${WORKSPACE} # /opt/x86_64-linux-musl/bin/clang -target ${target} --sysroot=/opt/${target}/${target}/sys-root -v -E -x c - < /dev/null
clang version 18.1.7 (/home/tim/.cache/BinaryBuilder/downloads/clones/llvm-project.git-1df819a03ecf6890e3787b27bfd4f160aeeeeacd50a98d003be8b0893f11a9be 768118d1ad38bf13c545828f67bd6b474d61fc55)
Target: aarch64-unknown-linux-gnu
Thread model: posix
InstalledDir: /opt/x86_64-linux-musl/bin
 (in-process)
 "/opt/x86_64-linux-musl/bin/clang-18" -cc1 -triple aarch64-unknown-linux-gnu -E -disable-free -clear-ast-before-backend -disable-llvm-verifier -discard-value-names -main-file-name - -mrelocation-model pic -pic-level 2 -pic-is-pie -mframe-pointer=non-leaf -fmath-errno -ffp-contract=on -fno-rounding-math -mconstructor-aliases -funwind-tables=2 -target-cpu generic -target-feature +v8a -target-feature +fp-armv8 -target-feature +neon -target-abi aapcs -debugger-tuning=gdb -fdebug-compilation-dir=/workspace -v -fcoverage-compilation-dir=/workspace -resource-dir /opt/x86_64-linux-musl/lib/clang/18 -isysroot /opt/aarch64-linux-gnu/aarch64-linux-gnu/sys-root -internal-isystem /opt/x86_64-linux-musl/lib/clang/18/include -internal-isystem /opt/aarch64-linux-gnu/aarch64-linux-gnu/sys-root/usr/local/include -internal-externc-isystem /opt/aarch64-linux-gnu/aarch64-linux-gnu/sys-root/include -internal-externc-isystem /opt/aarch64-linux-gnu/aarch64-linux-gnu/sys-root/usr/include -source-date-epoch 0 -ferror-limit 19 -fno-signed-char -fgnuc-version=4.2.1 -fskip-odr-check-in-gmf -fcolor-diagnostics -target-feature +outline-atomics -faddrsig -D__GCC_HAVE_DWARF2_CFI_ASM=1 -o - -x c -
clang -cc1 version 18.1.7 based upon LLVM 18.1.7 default target x86_64-alpine-linux-musl
ignoring nonexistent directory "/opt/aarch64-linux-gnu/aarch64-linux-gnu/sys-root/usr/local/include"
ignoring nonexistent directory "/opt/aarch64-linux-gnu/aarch64-linux-gnu/sys-root/include"
 /opt/x86_64-linux-musl/lib/clang/18/include
 /opt/aarch64-linux-gnu/aarch64-linux-gnu/sys-root/usr/include
End of search list.
```

* [Runner] Shuffle some flags around to fix headers search paths order

* Bump version
giordano authored Jan 23, 2025
1 parent bcde7e8 commit 087e705
Showing 3 changed files with 46 additions and 26 deletions.
2 changes: 1 addition & 1 deletion Project.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name = "BinaryBuilderBase"
uuid = "7f725544-6523-48cd-82d1-3fa08ff4056e"
authors = ["Elliot Saba <[email protected]>"]
version = "1.35.0"
version = "1.35.1"

[deps]
Bzip2_jll = "6e34b625-4abd-537c-b88f-471c36dfa7a0"
52 changes: 34 additions & 18 deletions src/Runner.jl
Original file line number Diff line number Diff line change
@@ -394,7 +394,6 @@ function generate_compiler_wrappers!(platform::AbstractPlatform; bin_path::Abstr
end

function sanitize_compile_flags!(p::AbstractPlatform, flags::Vector{String})
san = sanitize(p)
if sanitize(p) !== nothing
if sanitize(p) == "memory"
append!(flags, ["-fsanitize=memory"])
@@ -427,35 +426,52 @@ function generate_compiler_wrappers!(platform::AbstractPlatform; bin_path::Abstr
if lock_microarchitecture
append!(flags, get_march_flags(arch(p), march(p), "clang"))
end

append!(flags, [
# We add `-Wno-unused-command-line-argument` so that if someone does something like
# `clang -Werror -o foo a.o b.o`, it doesn't complain due to the fact that that is using
# `clang` as a linker (and we have no real way to detect that in the wrapper), which
# will cause `clang` to complain about compiler flags being passed in.
"-Wno-unused-command-line-argument",
# We need to override the typical C++ include search paths, because it always includes
# the toolchain C++ headers first. Valentin tracked this down to:
# https://github.com/llvm/llvm-project/blob/0378f3a90341d990236c44f297b923a32b35fab1/clang/lib/Driver/ToolChains/Darwin.cpp#L1944-L1978
"-nostdinc++",
# For systems other than macOS this directory doesn't exist out-of-the-box in our
# toolchain, but you can put in there the headers of the C++ standard library for libc++
# from LLLVMLibcxx_jll. This must come before GCC header files (added below).
"-isystem /opt/$(aatriplet(p))/$(aatriplet(p))/sys-root/usr/include/c++/v1",
])

if Sys.isapple(p)
macos_version_flags = clang_use_lld ? (min_macos_version_flags()[1],) : min_macos_version_flags()
append!(flags, String[
# On MacOS, we need to override the typical C++ include search paths, because it always includes
# the toolchain C++ headers first. Valentin tracked this down to:
# https://github.com/llvm/llvm-project/blob/0378f3a90341d990236c44f297b923a32b35fab1/clang/lib/Driver/ToolChains/Darwin.cpp#L1944-L1978
"-nostdinc++",
"-isystem",
"/opt/$(aatriplet(p))/$(aatriplet(p))/sys-root/usr/include/c++/v1",
# We also add `-Wno-unused-command-line-argument` so that if someone does something like
# `clang -Werror -o foo a.o b.o`, it doesn't complain due to the fact that that is using
# `clang` as a linker (and we have no real way to detect that in the wrapper), which will
# cause `clang` to complain about compiler flags being passed in.
"-Wno-unused-command-line-argument",
macos_version_flags...,
])
end

sanitize_compile_flags!(p, flags)
if Sys.isfreebsd(p)
add_system_includedir(flags)
end

if !Sys.isbsd(p) && !isnothing(gcc_version)
append!(flags, String["-isystem /opt/$(aatriplet(p))/$(aatriplet(p))/include/c++/$(gcc_version)",
"-isystem /opt/$(aatriplet(p))/$(aatriplet(p))/include/c++/$(gcc_version)/$(aatriplet(p))",
"-isystem /opt/$(aatriplet(p))/$(aatriplet(p))/include/c++/$(gcc_version)/backward",
"-isystem /opt/$(aatriplet(p))/$(aatriplet(p))/include",
"-isystem /opt/$(aatriplet(p))/$(aatriplet(p))/sys-root/include"])
if !Sys.isbsd(p)
# GCC header files
if !isnothing(gcc_version)
append!(flags, [
"-isystem /opt/$(aatriplet(p))/$(aatriplet(p))/include/c++/$(gcc_version)",
"-isystem /opt/$(aatriplet(p))/$(aatriplet(p))/include/c++/$(gcc_version)/$(aatriplet(p))",
"-isystem /opt/$(aatriplet(p))/$(aatriplet(p))/include/c++/$(gcc_version)/backward",
])
end
# MinGW header files
if Sys.iswindows(p)
append!(flags, [
"-isystem /opt/$(aatriplet(p))/$(aatriplet(p))/include",
])
end
end

if Sys.iswindows(p) && nbits(p) == 32
push!(flags, "-fsjlj-exceptions")
end
18 changes: 11 additions & 7 deletions test/runners.jl
Original file line number Diff line number Diff line change
@@ -197,7 +197,9 @@ end
end

# This tests only that compilers for all platforms can build and link simple C++ code
@testset "Compilation - $(platform) - $(compiler)" for platform in platforms, compiler in ("c++", "g++", "clang++")
# Note: we test the slightly weird `clang -x c++` as compiler driver because that's used
# in some cases and we want to make sure it works correctly.
@testset "Compilation - $(platform) - $(compiler)" for platform in platforms, (compiler, linker) in (("c++", "c++"), ("g++", "g++"), ("clang -x c++", "clang++"))
mktempdir() do dir
ur = preferred_runner()(dir; platform=platform)
iobuff = IOBuffer()
@@ -221,13 +223,15 @@ end
echo '$(test_cpp)' > test.cpp
echo '$(main_cpp)' > main.cpp
# Build object file
$(compiler) -Werror -std=c++11 -c test.cpp -o test.o
# Build shared library
$(compiler) -Werror -std=c++11 $(needfpic) -shared test.cpp -o libtest.\${dlext}
$(compiler) $(needfpic) -Werror -std=c++11 -c test.cpp -o test.o
# Link shared library
$(linker) -shared test.o -o libtest.\${dlext}
# Build and link program with object file
$(compiler) -Werror -std=c++11 -o main main.cpp test.o
# Build and link program with shared library
$(compiler) -Werror -std=c++11 -o main main.cpp -L. -ltest
$(compiler) $(needfpic) -Werror -std=c++11 -c main.cpp -o main.o
# Link main program with test object file
$(linker) -o main main.o test.o
# Link main program with shared library
$(linker) -o main main.o -L. -ltest
"""
cmd = `/bin/bash -c "$(test_script)"`
@test run(ur, cmd, iobuff; tee_stream=devnull)

2 comments on commit 087e705

@giordano
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JuliaRegistrator
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Registration pull request created: JuliaRegistries/General/123589

Tip: Release Notes

Did you know you can add release notes too? Just add markdown formatted text underneath the comment after the text
"Release notes:" and it will be added to the registry PR, and if TagBot is installed it will also be added to the
release that TagBot creates. i.e.

@JuliaRegistrator register

Release notes:

## Breaking changes

- blah

To add them here just re-invoke and the PR will be updated.

Tagging

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a v1.35.1 -m "<description of version>" 087e70545de164cad9b92d459f88b7b275988dca
git push origin v1.35.1

Please sign in to comment.