Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: simplify cc_configure and static link for libc++ #7329

Merged
merged 10 commits into from
Jul 11, 2019
Merged
52 changes: 1 addition & 51 deletions bazel/cc_configure.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2,57 +2,8 @@ load("@bazel_tools//tools/cpp:cc_configure.bzl", _upstream_cc_autoconf_impl = "c
load("@bazel_tools//tools/cpp:lib_cc_configure.bzl", "get_cpu_value")
load("@bazel_tools//tools/cpp:unix_cc_configure.bzl", "find_cc")

# Stub for `repository_ctx.which()` that always succeeds. See comments in
# `_find_cxx` for details.
def _quiet_fake_which(program):
return struct(_envoy_fake_which = program)

# Stub for `repository_ctx.which()` that always fails. See comments in
# `_find_cxx` for details.
def _noisy_fake_which(program):
return None

# Find a good path for the C++ compiler, by hooking into Bazel's C compiler
# detection. Uses `$CXX` if found, otherwise defaults to `g++` because Bazel
# defaults to `gcc`.
def _find_cxx(repository_ctx):
# Bazel's `find_cc` helper uses the repository context to inspect `$CC`.
# Replace this value with `$CXX` if set.
environ_cxx = repository_ctx.os.environ.get("CXX", "g++")
fake_os = struct(
environ = {"CC": environ_cxx},
)

# We can't directly assign `repository_ctx.which` to a struct attribute
# because Skylark doesn't support bound method references. Instead, stub
# out `which()` using a two-pass approach:
#
# * The first pass uses a stub that always succeeds, passing back a special
# value containing the original parameter.
# * If we detect the special value, we know that `find_cc` found a compiler
# name but don't know if that name could be resolved to an executable path.
# So do the `which()` call ourselves.
# * If our `which()` failed, call `find_cc` again with a dummy which that
# always fails. The error raised by `find_cc` will be identical to what Bazel
# would generate for a missing C compiler.
#
# See https://github.com/bazelbuild/bazel/issues/4644 for more context.
real_cxx = find_cc(struct(
which = _quiet_fake_which,
os = fake_os,
), {})
if hasattr(real_cxx, "_envoy_fake_which"):
real_cxx = repository_ctx.which(real_cxx._envoy_fake_which)
if real_cxx == None:
find_cc(struct(
which = _noisy_fake_which,
os = fake_os,
), {})
return real_cxx

def _build_envoy_cc_wrapper(repository_ctx):
real_cc = find_cc(repository_ctx, {})
real_cxx = _find_cxx(repository_ctx)

# Copy our CC wrapper script into @local_config_cc, with the true paths
# to the C and C++ compiler injected in. The wrapper will use these paths
Expand All @@ -64,7 +15,6 @@ def _build_envoy_cc_wrapper(repository_ctx):
repository_ctx.template("extra_tools/envoy_cc_wrapper", repository_ctx.attr._envoy_cc_wrapper, {
"{ENVOY_REAL_CC}": repr(str(real_cc)),
"{ENVOY_CFLAGS}": repr(str(repository_ctx.os.environ.get("CFLAGS", ""))),
"{ENVOY_REAL_CXX}": repr(str(real_cxx)),
"{ENVOY_CXXFLAGS}": repr(str(repository_ctx.os.environ.get("CXXFLAGS", ""))),
})
return repository_ctx.path("extra_tools/envoy_cc_wrapper")
Expand Down Expand Up @@ -93,6 +43,7 @@ cc_autoconf = repository_rule(
"ABI_VERSION",
"BAZEL_COMPILER",
"BAZEL_HOST_SYSTEM",
"BAZEL_CXXOPTS",
lizan marked this conversation as resolved.
Show resolved Hide resolved
"BAZEL_LINKOPTS",
"BAZEL_PYTHON",
"BAZEL_SH",
Expand All @@ -108,7 +59,6 @@ cc_autoconf = repository_rule(
"USE_CLANG_CL",
"CC",
"CFLAGS",
"CXX",
Copy link
Contributor

Choose a reason for hiding this comment

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

When building with GCC, does the final link happen with gcc or g++?

Copy link
Member Author

Choose a reason for hiding this comment

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

gcc

"CXXFLAGS",
"CC_CONFIGURE_DEBUG",
"CC_TOOLCHAIN_NAME",
Expand Down
26 changes: 15 additions & 11 deletions bazel/cc_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
import sys
import tempfile

envoy_real_cc = {ENVOY_REAL_CC}
envoy_real_cxx = {ENVOY_REAL_CXX}
compiler = {ENVOY_REAL_CC}
envoy_cflags = {ENVOY_CFLAGS}
envoy_cxxflags = {ENVOY_CXXFLAGS}

Expand Down Expand Up @@ -37,11 +36,9 @@ def is_cpp_flag(arg):
def modify_driver_args(input_driver_flags):
# Detect if we're building for C++ or vanilla C.
if any(map(is_cpp_flag, input_driver_flags)):
compiler = envoy_real_cxx
# Append CXXFLAGS to all C++ targets (this is mostly for dependencies).
argv = shlex.split(envoy_cxxflags)
else:
compiler = envoy_real_cc
# Append CFLAGS to all C targets (this is mostly for dependencies).
argv = shlex.split(envoy_cflags)

Expand All @@ -50,9 +47,8 @@ def modify_driver_args(input_driver_flags):
# b) replace all occurrences of -lstdc++ with -lc++ (when linking against libc++).
if "-static-libstdc++" in input_driver_flags or "-stdlib=libc++" in envoy_cxxflags:
for arg in input_driver_flags:
if arg == "-lstdc++":
if "-stdlib=libc++" in envoy_cxxflags:
argv.append("-lc++")
if arg in ("-lstdc++", "-static-libstdc++"):
pass
elif arg.startswith("-Wl,@"):
# tempfile.mkstemp will write to the out-of-sandbox tempdir
# unless the user has explicitly set environment variables
Expand All @@ -67,6 +63,14 @@ def modify_driver_args(input_driver_flags):
else:
argv += input_driver_flags

# This flags should after all libraries
if "-lstdc++" in input_driver_flags:
if "-stdlib=libc++" in envoy_cxxflags:
argv.append("-l:libc++.a")
argv.append("-l:libc++abi.a")
else:
argv.append("-l:libstdc++.a")

# Bazel will add -fuse-ld=gold in some cases, gcc/clang will take the last -fuse-ld argument,
# so whenever we see lld once, add it to the end.
if "-fuse-ld=lld" in argv:
Expand All @@ -87,13 +91,13 @@ def modify_driver_args(input_driver_flags):
# See https://github.com/envoyproxy/envoy/issues/2987
argv.append("-Wno-maybe-uninitialized")

return compiler, argv
return argv


def main():
# Append CXXFLAGS to correctly detect include paths for either libstdc++ or libc++.
if sys.argv[1:5] == ["-E", "-xc++", "-", "-v"]:
os.execv(envoy_real_cxx, [envoy_real_cxx] + sys.argv[1:] + shlex.split(envoy_cxxflags))
os.execv(compiler, [compiler] + sys.argv[1:] + shlex.split(envoy_cxxflags))

if sys.argv[1].startswith("@"):
# Read flags from file
Expand All @@ -102,7 +106,7 @@ def main():
input_driver_flags = fd.read().splitlines()

# Compute new args
compiler, new_driver_args = modify_driver_args(input_driver_flags)
new_driver_args = modify_driver_args(input_driver_flags)

# Write args to temp file
(new_flagfile_fd, new_flagfile_path) = tempfile.mkstemp(dir="./", suffix=".linker-params")
Expand All @@ -116,7 +120,7 @@ def main():
else:
# TODO(https://github.com/bazelbuild/bazel/issues/7687): Remove this branch
# when Bazel 0.27 is released.
compiler, new_args = modify_driver_args(sys.argv[1:])
new_args = modify_driver_args(sys.argv[1:])
lizan marked this conversation as resolved.
Show resolved Hide resolved

os.execv(compiler, [compiler] + new_args)

Expand Down
6 changes: 2 additions & 4 deletions bazel/envoy_internal.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,8 @@ def envoy_select_force_libcpp(if_libcpp, default = None):

def envoy_static_link_libstdcpp_linkopts():
return envoy_select_force_libcpp(
# TODO(PiotrSikora): statically link libc++ once that's possible.
# See: https://reviews.llvm.org/D53238
["-stdlib=libc++"],
["-static-libstdc++", "-static-libgcc"],
["-stdlib=libc++", "-l:libc++.a", "-l:libc++abi.a", "-static-libgcc"],
["-l:libstdc++.a", "-static-libgcc"],
)

# Dependencies on tcmalloc_and_profiler should be wrapped with this function.
Expand Down
4 changes: 1 addition & 3 deletions test/exe/envoy_static_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@ fi
if [[ ${DYNLIBS} =~ "libc++" ]]; then
echo "libc++ is dynamically linked:"
echo "${DYNLIBS}"
# TODO(PiotrSikora): enforce this once there is a way to statically link libc++.
# See: https://reviews.llvm.org/D53238
exit 0
exit 1
lizan marked this conversation as resolved.
Show resolved Hide resolved
fi

if [[ ${DYNLIBS} =~ "libstdc++" || ${DYNLIBS} =~ "libgcc" ]]; then
Expand Down