Skip to content

Commit 5d50e84

Browse files
committed
[bp/1.37] Don't use sysroot when building using non-hermetic toolchain (#43681)
Commit Message: Currently, even when using non-hermetic toolchain we in a few places use Envoy sysroot. Depending on the environment you use when building it can cause linking issues. The problem is that sysroot contains a libc library, while libc++ comes with the toolchain. If libc++ coming with the toolchain was built against a different version of libc than the one we have in Envoy sysroot, libc++ may contain references to the symbols that do not exist in the libc that we have in Envoy sysroot - that results in linking failures. In practice it affects systems that use libc version much newere than what we have in Envoy sysroot. The newer version of libc exposes more functions and libc++, when linked againt it, takes advantage of those newer symbols. As a result, when you combine libc++ linked against a newer version of libc and older version of libc you get linking issues due to a bunch of undefined symbols that libc++ uses. Envoy sysroot currently supports libc version 2.31 (the default) and 2.28 (which seem to only exist for Istio, because Istio intentionally wants to use as old version of libc as practical, because it needs to distribute Envoy binaries that will run outside of container environments and using a very old version of libc makes it easier). glibc 2.31 is 6 years old at this point, so many distribution (both popular like Ubuntu and more rare like Azure Linux) moved on to newer versions of glibc. There are a few ways we can handle this situation: 1. We can add more sysroot versions with newer versions of libc - given that we don't test non-hermetic toolchain builts and different versions of sysroot on CI, I think it's better to not place the burden of supporting different sysroots on the community. 2. We can add libc++ to the sysroot - I didn't test it, but I'd imagine it's possible to make it work, but because hermetic toolchain already comes with libc++ linked against an old enough version of libc to not cause problems, that will introduce some duplication (both sysroot and toolchain will have a version of libc++ libraries) 3. We can say that non-hermetic builds for Envoy just aren't supported at all - that's undesirable, because LLVM toolchains on GitHub are only available for a few operating systems (Ubuntu, RedHat) and if you're not using any of those, you are out of luck. 4. When not using hermetic toolchain, just don't provide sysroot and let the host toolchain figure out how and where to find the libraries in the host. The option 4 makes most sense to me, so this PR implements that, but we can discuss other options as well. Additional Description: It would be nice to cherry pick this to 1.37 release branch as well, that is if we aggree that this approach makes sense. Risk Level: low Testing: I did test builds both with hermetic toolchain and with host toolchain for both FIPS and non-FIPS builds to verify that the changes work. And CI run regular tests with hermetic toolchain. Docs Changes: n/a Release Notes: n/a Platform Specific Features: n/a --------- Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
1 parent 31896c5 commit 5d50e84

3 files changed

Lines changed: 94 additions & 25 deletions

File tree

bazel/external/fips_build.bzl

Lines changed: 50 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,23 @@ BUILD_COMMAND = """
55
set -eo pipefail
66
77
# c++
8-
SYSROOT="$$(realpath $$(dirname "$(location %s)"))"
8+
SYSROOT="%s"
9+
if [ -n "$${SYSROOT}" ]; then
10+
SYSROOT="$$(realpath $$(dirname "$${SYSROOT}"))"
11+
export SYSROOT_FLAG="--sysroot=$${SYSROOT}"
12+
else
13+
export SYSROOT_FLAG=""
14+
fi
915
export CC="$$(realpath $(CC))"
1016
# bazel doesnt expose CXX so we have to construct it (or use foreign_cc)
1117
if [[ "%s" == "libc++" ]]; then
12-
export CXXFLAGS="-stdlib=libc++ --sysroot=$${SYSROOT}"
13-
export LDFLAGS="-fuse-ld=lld -stdlib=libc++ -lc++ -lc++abi -lm -pthread --sysroot=$${SYSROOT}"
18+
export CXXFLAGS="-stdlib=libc++ $${SYSROOT_FLAG}"
19+
export LDFLAGS="-fuse-ld=lld -stdlib=libc++ -lc++ -lc++abi -lm -pthread $${SYSROOT_FLAG}"
1420
else
15-
export CXXFLAGS="--sysroot=$${SYSROOT}"
16-
export LDFLAGS="-fuse-ld=lld -lstdc++ -lm -pthread --sysroot=$${SYSROOT}"
21+
export CXXFLAGS="$${SYSROOT_FLAG}"
22+
export LDFLAGS="-fuse-ld=lld -lstdc++ -lm -pthread $${SYSROOT_FLAG}"
1723
fi
18-
export CGO_CFLAGS="--sysroot=$${SYSROOT}"
24+
export CGO_CFLAGS="$${SYSROOT_FLAG}"
1925
export CGO_CXXFLAGS="$${CXXFLAGS}"
2026
export CGO_LDFLAGS="$${LDFLAGS}"
2127
@@ -63,14 +69,20 @@ PYTHON_BIN=$$(realpath $(PYTHON3))
6369
export CC="$$(realpath $(CC))"
6470
export CXX="$$(realpath $(CC))"
6571
export AR="$$(realpath $(AR))"
66-
SYSROOT="$$(realpath $$(dirname "$(location %s)"))"
72+
SYSROOT="%s"
73+
if [ -n "$${SYSROOT}" ]; then
74+
SYSROOT="$$(realpath $$(dirname "$${SYSROOT}"))"
75+
export SYSROOT_FLAG="--sysroot=$${SYSROOT}"
76+
else
77+
export SYSROOT_FLAG=""
78+
fi
6779
# bazel doesnt expose CXX so we have to construct it (or use foreign_cc)
6880
if [[ "%s" == "libc++" ]]; then
69-
export CXXFLAGS="-stdlib=libc++ --sysroot=$${SYSROOT}"
70-
export LDFLAGS="-fuse-ld=lld -stdlib=libc++ -lc++ -lc++abi -lm -pthread --sysroot=$${SYSROOT}"
81+
export CXXFLAGS="-stdlib=libc++ $${SYSROOT_FLAG}"
82+
export LDFLAGS="-fuse-ld=lld -stdlib=libc++ -lc++ -lc++abi -lm -pthread $${SYSROOT_FLAG}"
7183
else
72-
export CXXFLAGS="--sysroot=$${SYSROOT}"
73-
export LDFLAGS="-fuse-ld=lld -lstdc++ -lm -pthread --sysroot=$${SYSROOT}"
84+
export CXXFLAGS="$${SYSROOT_FLAG}"
85+
export LDFLAGS="-fuse-ld=lld -lstdc++ -lm -pthread $${SYSROOT_FLAG}"
7486
fi
7587
cd $$SRC_DIR
7688
OUTPUT=$$(mktemp)
@@ -82,21 +94,34 @@ fi
8294
cp ninja $$OUT_FILE
8395
"""
8496

85-
def _create_build_config(prefix, lib, arch, arch_alias):
97+
def _config_name(prefix, arch, lib, hermetic_sysroot):
98+
return "%s_%s_%s%s" % (prefix, arch, lib, "_hermetic_sysroot" if hermetic_sysroot else "")
99+
100+
def _create_build_config(prefix, lib, arch, arch_alias, hermetic_sysroot):
86101
"""Create the config_setting_group combination."""
87102
conditions = ["@platforms//cpu:%s" % arch]
103+
104+
# We currently support only libc++ and libstdc++, so these variants are mutually exclusive
88105
if lib == "libc++":
89106
conditions.append("@envoy//bazel:libc++_enabled")
107+
else:
108+
conditions.append("@envoy//bazel:libstdc++_enabled")
109+
110+
if hermetic_sysroot:
111+
conditions.append("@envoy_repo//:use_hermetic_sysroot")
112+
else:
113+
conditions.append("@envoy_repo//:use_local_sysroot")
114+
90115
selects.config_setting_group(
91-
name = "%s_%s_%s" % (prefix, arch, lib),
116+
name = _config_name(prefix, arch, lib, hermetic_sysroot),
92117
match_all = conditions,
93118
)
94119

95-
def _create_boringssl_fips_build_command(lib, arch, arch_alias):
120+
def _create_boringssl_fips_build_command(lib, arch, arch_alias, hermetic_sysroot):
96121
"""Create the command."""
97-
_create_build_config("boringssl", lib, arch, arch_alias)
122+
_create_build_config("boringssl", lib, arch, arch_alias, hermetic_sysroot)
98123
return BUILD_COMMAND % (
99-
"@sysroot_linux_%s//:WORKSPACE" % arch_alias,
124+
("$(location @sysroot_linux_%s//:WORKSPACE)" % arch_alias) if hermetic_sysroot else "",
100125
lib,
101126
"@fips_cmake_linux_%s" % arch,
102127
"@fips_go_linux_%s" % arch_alias,
@@ -105,31 +130,35 @@ def _create_boringssl_fips_build_command(lib, arch, arch_alias):
105130
def boringssl_fips_build_command(arches, libs):
106131
"""Create conditional commands from the cartesian product of possible arches/stdlib."""
107132
return {
108-
":boringssl_%s_%s" % (arch, lib): _create_boringssl_fips_build_command(
133+
":%s" % _config_name("boringssl", arch, lib, hermetic_sysroot): _create_boringssl_fips_build_command(
109134
lib,
110135
arch,
111136
arch_alias,
137+
hermetic_sysroot,
112138
)
113139
for arch, arch_alias in arches.items()
114140
for lib in libs
141+
for hermetic_sysroot in [False, True]
115142
}
116143

117-
def _create_ninja_build_command(lib, arch, arch_alias):
144+
def _create_ninja_build_command(lib, arch, arch_alias, hermetic_sysroot):
118145
"""Create the command."""
119-
_create_build_config("ninja", lib, arch, arch_alias)
146+
_create_build_config("ninja", lib, arch, arch_alias, hermetic_sysroot)
120147
return NINJA_BUILD_COMMAND % (
121-
"@sysroot_linux_%s//:WORKSPACE" % arch_alias,
148+
("$(location @sysroot_linux_%s//:WORKSPACE)" % arch_alias) if hermetic_sysroot else "",
122149
lib,
123150
)
124151

125152
def ninja_build_command(arches, libs):
126153
"""Create the ninja command conditioned to correct stdlib."""
127154
return {
128-
":ninja_%s_%s" % (arch, lib): _create_ninja_build_command(
155+
":%s" % _config_name("ninja", arch, lib, hermetic_sysroot): _create_ninja_build_command(
129156
lib,
130157
arch,
131158
arch_alias,
159+
hermetic_sysroot,
132160
)
133161
for arch, arch_alias in arches.items()
134162
for lib in libs
163+
for hermetic_sysroot in [False, True]
135164
}

bazel/repo.bzl

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,19 @@ def _envoy_repo_impl(repository_ctx):
9393
llvm_path = repository_ctx.os.environ.get("BAZEL_LLVM_PATH", "")
9494
local_llvm = "True" if llvm_path else "False"
9595

96-
repository_ctx.file("compiler.bzl", "LLVM_PATH = '%s'" % (llvm_path))
96+
# By default, even when local toolchain is used, we still use the hermetic
97+
# sysroot, when it's undesirable, you can set this environment variable to True
98+
# to fallback to the host libc.
99+
#
100+
# NOTE: The cleanest way to provide this environment variable would be via Bazel's
101+
# repo_env flag. It's particularly important when using remote build execution (aka
102+
# RBE), as host environment variables are not directly passed to remote workers.
103+
local_sysroot = repository_ctx.os.environ.get("BAZEL_USE_HOST_SYSROOT", "False")
104+
105+
repository_ctx.file("compiler.bzl", """
106+
LLVM_PATH = '%s'
107+
USE_LOCAL_SYSROOT = %s
108+
""" % (llvm_path, local_sysroot))
97109
repository_ctx.file("version.bzl", "VERSION = '%s'\nAPI_VERSION = '%s'" % (version, api_version))
98110
repository_ctx.file("path.bzl", "PATH = '%s'" % repo_version_path.dirname)
99111
repository_ctx.file("envoy_repo.py", "PATH = '%s'\nVERSION = '%s'\nAPI_VERSION = '%s'" % (repo_version_path.dirname, version, api_version))
@@ -121,6 +133,34 @@ config_setting(
121133
visibility = ["//visibility:public"],
122134
)
123135
136+
bool_flag(
137+
name = "use_local_sysroot_flag",
138+
build_setting_default = %s,
139+
visibility = ["//visibility:public"],
140+
)
141+
142+
config_setting(
143+
name = "use_local_sysroot",
144+
flag_values = {
145+
":use_local_sysroot_flag": "True",
146+
},
147+
constraint_values = [
148+
"@platforms//os:linux",
149+
],
150+
visibility = ["//visibility:public"],
151+
)
152+
153+
config_setting(
154+
name = "use_hermetic_sysroot",
155+
flag_values = {
156+
":use_local_sysroot_flag": "False",
157+
},
158+
constraint_values = [
159+
"@platforms//os:linux",
160+
],
161+
visibility = ["//visibility:public"],
162+
)
163+
124164
py_library(
125165
name = "envoy_repo",
126166
srcs = ["envoy_repo.py"],
@@ -253,7 +293,7 @@ py_console_script_binary(
253293
data = [":envoy_repo.py"],
254294
)
255295
256-
''' % local_llvm)
296+
''' % (local_llvm, local_sysroot))
257297

258298
_envoy_repo = repository_rule(
259299
implementation = _envoy_repo_impl,

bazel/toolchains.bzl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
load("@envoy_repo//:compiler.bzl", "LLVM_PATH")
1+
load("@envoy_repo//:compiler.bzl", "LLVM_PATH", "USE_LOCAL_SYSROOT")
22
load("@envoy_toolshed//repository:utils.bzl", "arch_alias")
33
load("@toolchains_llvm//toolchain:rules.bzl", "llvm_toolchain")
44

@@ -15,7 +15,7 @@ def envoy_toolchains():
1515
name = "llvm_toolchain",
1616
llvm_version = "18.1.8",
1717
cxx_standard = {"": "c++20"},
18-
sysroot = {
18+
sysroot = {} if USE_LOCAL_SYSROOT else {
1919
"linux-x86_64": "@sysroot_linux_amd64//:sysroot",
2020
"linux-aarch64": "@sysroot_linux_arm64//:sysroot",
2121
},

0 commit comments

Comments
 (0)