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: switch to libc++ by default #8859

Merged
merged 19 commits into from
Nov 6, 2019
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .azure-pipelines/linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ jobs:
env:
ENVOY_DOCKER_BUILD_DIR: $(Build.StagingDirectory)
ENVOY_RBE: "true"
BAZEL_BUILD_EXTRA_OPTIONS: "--config=remote-ci --config=remote --jobs=$(RbeJobs) --curses=no"
BAZEL_BUILD_EXTRA_OPTIONS: "--config=remote-ci --jobs=$(RbeJobs) --curses=no"
BAZEL_REMOTE_CACHE: grpcs://remotebuildexecution.googleapis.com
BAZEL_REMOTE_INSTANCE: projects/envoy-ci/instances/default_instance
GCP_SERVICE_ACCOUNT_KEY: $(GcpServiceAccountKey)
Expand Down
4 changes: 1 addition & 3 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,6 @@ build:clang-msan --linkopt -fsanitize=memory
build:clang-msan --copt -fsanitize-memory-track-origins=2

# Clang with libc++
# TODO(cmluciano) fix and re-enable _LIBCPP_VERSION testing for TCMALLOC in Envoy::Stats::TestUtil::hasDeterministicMallocStats
# and update stats_integration_test with appropriate m_per_cluster value
build:libc++ --config=clang
build:libc++ --action_env=CXXFLAGS=-stdlib=libc++
build:libc++ --action_env=LDFLAGS=-stdlib=libc++
Expand Down Expand Up @@ -173,7 +171,7 @@ build:remote-ci --remote_cache=grpcs://remotebuildexecution.googleapis.com
build:remote-ci --remote_executor=grpcs://remotebuildexecution.googleapis.com

# Fuzz builds
build:asan-fuzzer --config=asan
build:asan-fuzzer --config=clang-asan
build:asan-fuzzer --define=FUZZING_ENGINE=libfuzzer
build:asan-fuzzer --copt=-DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
build:asan-fuzzer --copt=-fsanitize=fuzzer-no-link
Expand Down
15 changes: 4 additions & 11 deletions bazel/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,20 +149,13 @@ set different options. See below to configure test IP versions.

## Linking against libc++ on Linux

To link Envoy against libc++, use the following commands:
To link Envoy against libc++, follow the [quick start](#quick-start-bazel-build-for-developers) to setup Clang+LLVM and run:
```
export CC=clang
export CXX=clang++
bazel build --config=libc++ //source/exe:envoy-static
Copy link
Member

Choose a reason for hiding this comment

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

I think what I was wondering before is whether it's possible to just have --config-libc++ be the default?

The issue that I have found in the past is that I don't think there is a way to have a default config and then actual have a local config disable it and replace it with something else?

/wait-any

Copy link
Member Author

Choose a reason for hiding this comment

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

I will recommend having --config=libc++ in user.bazelrc file to make it default. It is possible to override but not easy, partially because toolchains are also defaulted to libstdc++.

Copy link
Member

Choose a reason for hiding this comment

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

OK, should we document that somewhere?

```
Note: this assumes that both: clang compiler and libc++ library are installed in the system,
and that `clang` and `clang++` are available in `$PATH`. On some systems, you might need to
include them in the search path, e.g. `export PATH=/usr/lib/llvm-9/bin:$PATH`.

You might also need to ensure libc++ is installed correctly on your system, e.g. on Ubuntu this
might look like `sudo apt-get install libc++abi-9-dev libc++-9-dev`.

Note: this configuration currently doesn't work with Remote Execution or Docker sandbox.
Or use our configuration with Remote Execution or Docker sandbox, pass `--config=remote-clang-libc++` or
`--config=docker-clang-libc++` respectively.

## Using a compiler toolchain in a non-standard location

Expand All @@ -178,7 +171,7 @@ for more details.
## Supported compiler versions

We now require Clang >= 5.0 due to known issues with std::string thread safety and C++14 support. GCC >= 7 is also
known to work. Currently the CI is running with Clang 8.
known to work. Currently the CI is running with Clang 9.

## Clang STL debug symbols

Expand Down
3 changes: 1 addition & 2 deletions bazel/envoy_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,8 @@ def envoy_cc_fuzz_test(
test_lib_name = name + "_lib"
envoy_cc_test_library(
name = test_lib_name,
deps = deps + [
deps = deps + envoy_stdlib_deps() + [
repository + "//test/fuzz:fuzz_runner_lib",
repository + "//bazel:dynamic_stdlib",
],
repository = repository,
tags = tags,
Expand Down
4 changes: 2 additions & 2 deletions bazel/fuzzit_wrapper.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ if [[ ! -z "${FUZZIT_API_KEY}" ]]; then
"${FUZZIT}" create target --skip-if-exists --public-corpus envoyproxy/"${FUZZIT_TARGET_NAME}"

# Run fuzzing first so this is not affected by local-regression timeout
"${FUZZIT}" create job --skip-if-not-exists --type fuzzing envoyproxy/"${FUZZIT_TARGET_NAME}" "${FUZZER_BINARY}"
"${FUZZIT}" create job --skip-if-not-exists --host "${ENVOY_BUILD_IMAGE}" --type fuzzing envoyproxy/"${FUZZIT_TARGET_NAME}" "${FUZZER_BINARY}"
fi

"${FUZZIT}" create job --skip-if-not-exists --type local-regression envoyproxy/"${FUZZIT_TARGET_NAME}" "${FUZZER_BINARY}"
"${FUZZIT}" create job --skip-if-not-exists --host "${ENVOY_BUILD_IMAGE}" --type local-regression envoyproxy/"${FUZZIT_TARGET_NAME}" "${FUZZER_BINARY}"
6 changes: 6 additions & 0 deletions ci/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ repository.

We use the Clang compiler for all CI runs with tests. We have an additional CI run with GCC which builds binary only.

# C++ standard library

As of November 2019 after [#8859](https://github.com/envoyproxy/envoy/pull/8859) the official released binary is
[linked against libc++ on Linux](https://github.com/envoyproxy/envoy/blob/master/bazel/README.md#linking-against-libc-on-linux).
To override the C++ standard library in your build, set environment variable `ENVOY_STDLIB` to `libstdc++` or `libc++`.
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for being dense but if we have bazelrc configs, why do we also need env variables?

Copy link
Member Author

Choose a reason for hiding this comment

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

ah this is for ci/do_ci.sh to inject bazelrc configs, let me make this clear.


# Building and running tests as a developer

An example basic invocation to build a developer version of the Envoy static binary (using the Bazel `fastbuild` type) is:
Expand Down
30 changes: 17 additions & 13 deletions ci/build_setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,32 +11,36 @@ export PPROF_PATH=/thirdparty_build/bin/pprof
echo "ENVOY_SRCDIR=${ENVOY_SRCDIR}"

function setup_gcc_toolchain() {
if [[ ! -z "${ENVOY_STDLIB}" && "${ENVOY_STDLIB}" != "libstdc++" ]]; then
echo "gcc toolchain doesn't support ${ENVOY_STDLIB}."
exit 1
fi
if [[ -z "${ENVOY_RBE}" ]]; then
export CC=gcc
export CXX=g++
export BAZEL_COMPILER=gcc
echo "$CC/$CXX toolchain configured"
else
export BAZEL_BUILD_OPTIONS="--config=rbe-toolchain-gcc ${BAZEL_BUILD_OPTIONS}"
export BAZEL_BUILD_OPTIONS="--config=remote-gcc ${BAZEL_BUILD_OPTIONS}"
fi
}

function setup_clang_toolchain() {
Copy link
Member

Choose a reason for hiding this comment

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

Q: Is there any way to make this the default in the core .bazelrc options such that someone could disable it and go back to libstdcxx if they want? This would provide consistency across all builds in general, not just CI builds. I'm not sure if there is a good way to do this though?

/wait-any

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I will revise the script.

ENVOY_STDLIB="${ENVOY_STDLIB:-libc++}"
if [[ -z "${ENVOY_RBE}" ]]; then
export BAZEL_BUILD_OPTIONS="--config=clang ${BAZEL_BUILD_OPTIONS}"
else
export BAZEL_BUILD_OPTIONS="--config=rbe-toolchain-clang ${BAZEL_BUILD_OPTIONS}"
fi
echo "clang toolchain configured"
}

function setup_clang_libcxx_toolchain() {
if [[ -z "${ENVOY_RBE}" ]]; then
export BAZEL_BUILD_OPTIONS="--config=libc++ ${BAZEL_BUILD_OPTIONS}"
if [[ "${ENVOY_STDLIB}" == "libc++" ]]; then
export BAZEL_BUILD_OPTIONS="--config=libc++ ${BAZEL_BUILD_OPTIONS}"
else
export BAZEL_BUILD_OPTIONS="--config=clang ${BAZEL_BUILD_OPTIONS}"
fi
else
export BAZEL_BUILD_OPTIONS="--config=rbe-toolchain-clang-libc++ ${BAZEL_BUILD_OPTIONS}"
if [[ "${ENVOY_STDLIB}" == "libc++" ]]; then
export BAZEL_BUILD_OPTIONS="--config=remote-clang-libc++ ${BAZEL_BUILD_OPTIONS}"
else
export BAZEL_BUILD_OPTIONS="--config=remote-clang ${BAZEL_BUILD_OPTIONS}"
fi
fi
echo "clang toolchain with libc++ configured"
echo "clang toolchain with ${ENVOY_STDLIB} configured"
}

# Create a fake home. Python site libs tries to do getpwuid(3) if we don't and the CI
Expand Down
5 changes: 3 additions & 2 deletions ci/do_ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,8 @@ elif [[ "$CI_TARGET" == "bazel.compile_time_options" ]]; then
--define path_normalization_by_default=true \
--define deprecated_features=disabled \
"
setup_clang_libcxx_toolchain
ENVOY_STDLIB="${ENVOY_STDLIB:-libstdc++}"
setup_clang_toolchain
# This doesn't go into CI but is available for developer convenience.
echo "bazel with different compiletime options build with tests..."

Expand Down Expand Up @@ -277,7 +278,7 @@ elif [[ "$CI_TARGET" == "bazel.fuzzit" ]]; then
echo "bazel ASAN libFuzzer build with fuzz tests ${FUZZ_TEST_TARGETS}"
echo "Building fuzzers and run under Fuzzit"
bazel_with_collection test ${BAZEL_BUILD_OPTIONS} --config=asan-fuzzer ${FUZZ_TEST_TARGETS} \
--test_env=FUZZIT_API_KEY --test_timeout=1200 --run_under=//bazel:fuzzit_wrapper
--test_env=FUZZIT_API_KEY --test_env=ENVOY_BUILD_IMAGE --test_timeout=1200 --run_under=//bazel:fuzzit_wrapper
exit 0
elif [[ "$CI_TARGET" == "fix_format" ]]; then
# proto_format.sh needs to build protobuf.
Expand Down
8 changes: 5 additions & 3 deletions ci/run_envoy_docker.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ USER_GROUP=root

[[ -t 1 ]] && DOCKER_TTY_OPTION=-it

export ENVOY_BUILD_IMAGE="${IMAGE_NAME}:${IMAGE_ID}"

mkdir -p "${ENVOY_DOCKER_BUILD_DIR}"
# Since we specify an explicit hash, docker-run will pull from the remote repo if missing.
docker run --rm ${DOCKER_TTY_OPTION} -e HTTP_PROXY=${http_proxy} -e HTTPS_PROXY=${https_proxy} \
-u "${USER}":"${USER_GROUP}" -v "${ENVOY_DOCKER_BUILD_DIR}":/build -v /var/run/docker.sock:/var/run/docker.sock ${GIT_VOLUME_OPTION} \
-e BAZEL_BUILD_EXTRA_OPTIONS -e BAZEL_EXTRA_TEST_OPTIONS -e BAZEL_REMOTE_CACHE \
-e BAZEL_REMOTE_INSTANCE -e GCP_SERVICE_ACCOUNT_KEY -e NUM_CPUS -e ENVOY_RBE -e FUZZIT_API_KEY \
-v "$PWD":/source --cap-add SYS_PTRACE --cap-add NET_RAW --cap-add NET_ADMIN "${IMAGE_NAME}":"${IMAGE_ID}" \
-e BAZEL_BUILD_EXTRA_OPTIONS -e BAZEL_EXTRA_TEST_OPTIONS -e BAZEL_REMOTE_CACHE -e ENVOY_STDLIB \
-e BAZEL_REMOTE_INSTANCE -e GCP_SERVICE_ACCOUNT_KEY -e NUM_CPUS -e ENVOY_RBE -e FUZZIT_API_KEY -e ENVOY_BUILD_IMAGE \
-v "$PWD":/source --cap-add SYS_PTRACE --cap-add NET_RAW --cap-add NET_ADMIN "${ENVOY_BUILD_IMAGE}" \
/bin/bash -lc "groupadd --gid $(id -g) -f envoygroup && useradd -o --uid $(id -u) --gid $(id -g) --no-create-home \
--home-dir /source envoybuild && usermod -a -G pcap envoybuild && su envoybuild -c \"cd source && $*\""
1 change: 1 addition & 0 deletions docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ Version history

1.13.0 (pending)
================
* build: official released binary is now built against libc++.
Copy link
Member

Choose a reason for hiding this comment

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

alpha order

* access log: added FILTER_STATE :ref:`access log formatters <config_access_log_format>` and gRPC access logger.
* api: remove all support for v1
* redis: performance improvement for larger split commands by avoiding string copies.
Expand Down
3 changes: 2 additions & 1 deletion test/common/stats/stat_test_utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ MemoryTest::Mode MemoryTest::mode() {
// on some platforms, so try to force-allocate some heap memory
// and determine whether we can measure it.
const size_t start_mem = Memory::Stats::totalCurrentlyAllocated();
volatile std::string long_string("more than 22 chars to exceed libc++ short-string optimization");
volatile std::unique_ptr<std::string> long_string = std::make_unique<std::string>(
"more than 22 chars to exceed libc++ short-string optimization");
const size_t end_mem = Memory::Stats::totalCurrentlyAllocated();
bool can_measure_memory = end_mem > start_mem;

Expand Down
9 changes: 6 additions & 3 deletions test/integration/stats_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) {
// 2019/09/30 8354 43310 44000 Implement transport socket match.
// 2019/10/17 8537 43308 44000 add new enum value HTTP3
// 2019/10/17 8484 43340 44000 stats: add unit support to histogram
// 2019/11/01 8859 43563 44000 build: switch to libc++ by default

// Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI
// 'release' builds, where we control the platform and tool-chain. So you
Expand All @@ -270,7 +271,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) {
// If you encounter a failure here, please see
// https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests
// for details on how to fix.
EXPECT_MEMORY_EQ(m_per_cluster, 43340); // 104 bytes higher than a debug build.
EXPECT_MEMORY_EQ(m_per_cluster, 43563); // 104 bytes higher than a debug build.
EXPECT_MEMORY_LE(m_per_cluster, 44000);
}

Expand Down Expand Up @@ -301,6 +302,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) {
// 2019/09/30 8354 34969 35000 Implement transport socket match.
// 2019/10/17 8537 34966 35000 add new enum value HTTP3
// 2019/10/17 8484 34998 35000 stats: add unit support to histogram
// 2019/11/01 8859 35221 36000 build: switch to libc++ by default

// Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI
// 'release' builds, where we control the platform and tool-chain. So you
Expand All @@ -314,7 +316,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) {
// If you encounter a failure here, please see
// https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests
// for details on how to fix.
EXPECT_MEMORY_EQ(m_per_cluster, 34998); // 104 bytes higher than a debug build.
EXPECT_MEMORY_EQ(m_per_cluster, 35221); // 104 bytes higher than a debug build.
EXPECT_MEMORY_LE(m_per_cluster, 36000);
}

Expand All @@ -340,6 +342,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeHostSizeWithStats) {
// ---------- ----- ----------------- -----
// 2019/09/09 8189 2739 3100 Initial per-host memory snapshot
// 2019/09/10 8216 1283 1315 Use primitive counters for host stats
// 2019/10/01 8850 1299 1315 build: switch to libc++ by default

// Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI
// 'release' builds, where we control the platform and tool-chain. So you
Expand All @@ -349,7 +352,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeHostSizeWithStats) {
// If you encounter a failure here, please see
// https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests
// for details on how to fix.
EXPECT_MEMORY_EQ(m_per_host, 1283);
EXPECT_MEMORY_EQ(m_per_host, 1299);
EXPECT_MEMORY_LE(m_per_host, 1315);
}

Expand Down