Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 3 additions & 3 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ common --experimental_allow_tags_propagation
build:linux --copt=-fdebug-types-section
build:linux --copt=-fPIC
build:linux --copt=-Wno-deprecated-declarations
build:linux --cxxopt=-std=c++17 --host_cxxopt=-std=c++17
build:linux --cxxopt=-std=c++20 --host_cxxopt=-std=c++20
build:linux --cxxopt=-fsized-deallocation --host_cxxopt=-fsized-deallocation
build:linux --conlyopt=-fexceptions
build:linux --fission=dbg,opt
Expand Down Expand Up @@ -133,7 +133,7 @@ build:clang-asan --linkopt --rtlib=compiler-rt
build:clang-asan --linkopt --unwindlib=libgcc

# macOS
build:macos --cxxopt=-std=c++17 --host_cxxopt=-std=c++17
build:macos --cxxopt=-std=c++20 --host_cxxopt=-std=c++20
build:macos --action_env=PATH=/opt/homebrew/bin:/opt/local/bin:/usr/local/bin:/usr/bin:/bin
build:macos --host_action_env=PATH=/opt/homebrew/bin:/opt/local/bin:/usr/local/bin:/usr/bin:/bin
build:macos --define tcmalloc=disabled
Expand Down Expand Up @@ -194,7 +194,7 @@ build:libc++ --define force_libcpp=enabled

build:libc++20 --config=libc++
# gRPC has a lot of deprecated-enum-enum-conversion warning. Remove once it is addressed
build:libc++20 --cxxopt=-std=c++20 --copt=-Wno-error=deprecated-enum-enum-conversion
build:libc++20 --copt=-Wno-error=deprecated-enum-enum-conversion

# Optimize build for binary size reduction.
build:sizeopt -c opt --copt -Os
Expand Down
15 changes: 15 additions & 0 deletions bazel/cel-cpp.patch
Original file line number Diff line number Diff line change
@@ -1,3 +1,18 @@
diff --git a/base/memory.h b/base/memory.h
index 3552e19..0fbe618 100644
--- a/base/memory.h
+++ b/base/memory.h
@@ -166,8 +166,8 @@ std::enable_if_t<IsDerivedHeapDataV<F>, Handle<T>> HandleFactory<T>::Make(
#if defined(__cpp_lib_is_pointer_interconvertible) && \
__cpp_lib_is_pointer_interconvertible >= 201907L
// Only available in C++20.
- static_assert(std::is_pointer_interconvertible_base_of_v<Data, F>,
- "F must be pointer interconvertible to Data");
+// static_assert(std::is_pointer_interconvertible_base_of_v<Data, F>,
+// "F must be pointer interconvertible to Data");
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we remove the "//"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I plan to update cel-cpp immediately and get rid of this patch or fix it upstream. I can't tell if this is a real issue or not with GCC.

#endif
if (memory_manager.memory_management() == MemoryManagement::kPooling) {
void* addr;
diff --git a/eval/internal/interop.cc b/eval/internal/interop.cc
index 3acde6c..20f8ea3 100644
--- a/eval/internal/interop.cc
Expand Down
40 changes: 40 additions & 0 deletions bazel/proxy_wasm_cpp_sdk.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
diff --git a/proxy_wasm_api.h b/proxy_wasm_api.h
index 166b49c..b44637c 100644
--- a/proxy_wasm_api.h
+++ b/proxy_wasm_api.h
@@ -1207,8 +1207,9 @@ struct SimpleHistogram {
template <typename... Tags> struct Counter : public MetricBase {
static Counter<Tags...> *New(std::string_view name, MetricTagDescriptor<Tags>... fieldnames);

- Counter<Tags...>(std::string_view name, MetricTagDescriptor<Tags>... descriptors)
- : Counter<Tags...>(std::string(name), std::vector<MetricTag>({toMetricTag(descriptors)...})) {
+ template <typename... T>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is proxy_wasm_api incompatible with C++20?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this is just some helper AFAICT which is only used in one place in a test.

+ Counter(std::string_view name, MetricTagDescriptor<T>... descriptors)
+ : Counter<T...>(std::string(name), std::vector<MetricTag>({toMetricTag(descriptors)...})) {
}

SimpleCounter resolve(Tags... f) {
@@ -1256,8 +1257,9 @@ inline Counter<Tags...> *Counter<Tags...>::New(std::string_view name,
template <typename... Tags> struct Gauge : public MetricBase {
static Gauge<Tags...> *New(std::string_view name, MetricTagDescriptor<Tags>... fieldnames);

- Gauge<Tags...>(std::string_view name, MetricTagDescriptor<Tags>... descriptors)
- : Gauge<Tags...>(std::string(name), std::vector<MetricTag>({toMetricTag(descriptors)...})) {}
+ template <typename... T>
+ Gauge(std::string_view name, MetricTagDescriptor<T>... descriptors)
+ : Gauge<T...>(std::string(name), std::vector<MetricTag>({toMetricTag(descriptors)...})) {}

SimpleGauge resolve(Tags... f) {
std::vector<std::string> fields{toString(f)...};
@@ -1302,8 +1304,9 @@ inline Gauge<Tags...> *Gauge<Tags...>::New(std::string_view name,
template <typename... Tags> struct Histogram : public MetricBase {
static Histogram<Tags...> *New(std::string_view name, MetricTagDescriptor<Tags>... fieldnames);

- Histogram<Tags...>(std::string_view name, MetricTagDescriptor<Tags>... descriptors)
- : Histogram<Tags...>(std::string(name),
+ template <typename... T>
+ Histogram(std::string_view name, MetricTagDescriptor<T>... descriptors)
+ : Histogram<T...>(std::string(name),
std::vector<MetricTag>({toMetricTag(descriptors)...})) {}

SimpleHistogram resolve(Tags... f) {
8 changes: 7 additions & 1 deletion bazel/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -1278,7 +1278,13 @@ def _upb():
)

def _proxy_wasm_cpp_sdk():
external_http_archive(name = "proxy_wasm_cpp_sdk")
external_http_archive(
name = "proxy_wasm_cpp_sdk",
patch_args = ["-p1"],
patches = [
"@envoy//bazel:proxy_wasm_cpp_sdk.patch",
],
)

def _proxy_wasm_cpp_host():
external_http_archive(
Expand Down
27 changes: 12 additions & 15 deletions bazel/v8.patch
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
# 1. Use already imported python dependencies
# 2. Disable pointer compression (limits the maximum number of WasmVMs).
# 3. Add support for --define=no_debug_info=1.
# 3. Add support for --define=no_debug_info=1.
# 4. Allow compiling v8 on macOS 10.15 to 13.0. TODO(dio): Will remove this patch when https://bugs.chromium.org/p/v8/issues/detail?id=13428 is fixed.
# 5. Don't expose Wasm C API (only Wasm C++ API).

diff --git a/BUILD.bazel b/BUILD.bazel
index 4e89f90e7e..ced403d5aa 100644
index 4e89f90..0df4f67 100644
--- a/BUILD.bazel
+++ b/BUILD.bazel
@@ -4,7 +4,7 @@
Expand All @@ -17,10 +17,6 @@ index 4e89f90e7e..ced403d5aa 100644
load(
"@v8//:bazel/defs.bzl",
"v8_binary",
diff --git a/BUILD.bazel b/BUILD.bazel
index 4e89f90e7e..3fcb38b3f3 100644
--- a/BUILD.bazel
+++ b/BUILD.bazel
@@ -157,7 +157,7 @@ v8_int(
# If no explicit value for v8_enable_pointer_compression, we set it to 'none'.
v8_string(
Expand All @@ -31,7 +27,7 @@ index 4e89f90e7e..3fcb38b3f3 100644

# Default setting for v8_enable_pointer_compression.
diff --git a/bazel/defs.bzl b/bazel/defs.bzl
index e957c0fad3..a6de50e6ab 100644
index e957c0f..0327669 100644
--- a/bazel/defs.bzl
+++ b/bazel/defs.bzl
@@ -116,6 +116,7 @@ def _default_args():
Expand All @@ -42,15 +38,16 @@ index e957c0fad3..a6de50e6ab 100644
"-std=c++17",
],
"@v8//bazel/config:is_gcc": [
@@ -131,6 +132,7 @@ def _default_args():
@@ -131,6 +132,8 @@ def _default_args():
"-Wno-redundant-move",
"-Wno-return-type",
"-Wno-stringop-overflow",
+ "-Wno-nonnull",
+ "-Wno-pessimizing-move",
# Use GNU dialect, because GCC doesn't allow using
# ##__VA_ARGS__ when in standards-conforming mode.
"-std=gnu++17",
@@ -151,6 +153,23 @@ def _default_args():
@@ -151,6 +154,23 @@ def _default_args():
"-fno-integrated-as",
],
"//conditions:default": [],
Expand All @@ -75,10 +72,10 @@ index e957c0fad3..a6de50e6ab 100644
includes = ["include"],
linkopts = select({
diff --git a/src/wasm/c-api.cc b/src/wasm/c-api.cc
index ce3f569fd5..dc8a4c4f6a 100644
index 4473e20..65a6ec7 100644
--- a/src/wasm/c-api.cc
+++ b/src/wasm/c-api.cc
@@ -2238,6 +2238,8 @@ auto Instance::exports() const -> ownvec<Extern> {
@@ -2247,6 +2247,8 @@ auto Instance::exports() const -> ownvec<Extern> {

} // namespace wasm

Expand All @@ -87,22 +84,22 @@ index ce3f569fd5..dc8a4c4f6a 100644
// BEGIN FILE wasm-c.cc

extern "C" {
@@ -3257,3 +3259,5 @@ wasm_instance_t* wasm_frame_instance(const wasm_frame_t* frame) {
@@ -3274,3 +3276,5 @@ wasm_instance_t* wasm_frame_instance(const wasm_frame_t* frame) {
#undef WASM_DEFINE_SHARABLE_REF

} // extern "C"
+
+#endif
diff --git a/third_party/inspector_protocol/code_generator.py b/third_party/inspector_protocol/code_generator.py
index c3768b8..d4a1dda 100644
index c3768b8..d4a1dda 100755
--- a/third_party/inspector_protocol/code_generator.py
+++ b/third_party/inspector_protocol/code_generator.py
@@ -16,6 +16,8 @@ try:
except ImportError:
import simplejson as json

+sys.path += [os.path.dirname(__file__)]
+
import pdl

try:
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ name: envoy.filters.http.language
default_language: {}
supported_languages: {}
)EOF";
config_helper_.prependFilter(fmt::format(yaml, default_language, supported_languages));
config_helper_.prependFilter(
fmt::format(fmt::runtime(yaml), default_language, supported_languages));
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ class MySQLIntegrationTest : public testing::TestWithParam<Network::Address::IpV
public MySQLTestUtils,
public BaseIntegrationTest {
std::string mysqlConfig() {
return fmt::format(TestEnvironment::readFileToStringForTest(TestEnvironment::runfilesPath(
"contrib/mysql_proxy/filters/network/test/mysql_test_config.yaml")),
Platform::null_device_path,
Network::Test::getLoopbackAddressString(GetParam()),
Network::Test::getLoopbackAddressString(GetParam()),
Network::Test::getAnyAddressString(GetParam()));
return fmt::format(
fmt::runtime(TestEnvironment::readFileToStringForTest(TestEnvironment::runfilesPath(
"contrib/mysql_proxy/filters/network/test/mysql_test_config.yaml"))),
Platform::null_device_path, Network::Test::getLoopbackAddressString(GetParam()),
Network::Test::getLoopbackAddressString(GetParam()),
Network::Test::getAnyAddressString(GetParam()));
}

public:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ class PostgresBaseIntegrationTest : public testing::TestWithParam<Network::Addre
std::string postgresConfig(SSLConfig downstream_ssl_config, SSLConfig upstream_ssl_config,
std::string additional_filters) {
std::string main_config = fmt::format(
TestEnvironment::readFileToStringForTest(TestEnvironment::runfilesPath(
"contrib/postgres_proxy/filters/network/test/postgres_test_config.yaml-template")),
fmt::runtime(TestEnvironment::readFileToStringForTest(TestEnvironment::runfilesPath(
"contrib/postgres_proxy/filters/network/test/postgres_test_config.yaml-template"))),
Platform::null_device_path, Network::Test::getLoopbackAddressString(GetParam()),
Network::Test::getLoopbackAddressString(GetParam()),
std::get<1>(upstream_ssl_config), // upstream SSL transport socket
Expand Down
2 changes: 2 additions & 0 deletions mobile/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ build:mobile-remote-ci-linux --config=mobile-remote-ci-common
build:mobile-remote-ci-linux-clang --action_env=CC=/opt/llvm/bin/clang
build:mobile-remote-ci-linux-clang --action_env=CXX=/opt/llvm/bin/clang++
build:mobile-remote-ci-linux-clang --config=mobile-remote-ci-linux
# Temporary revert to C++17 for mobile NDK builds.
build:mobile-remote-ci-linux-clang --cxxopt=-std=c++17 --host_cxxopt=-std=c++17

#############################################################################
# mobile-remote-ci-linux-asan: These options are Linux-only using Clang and AddressSanitizer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class PlatformBridgeFilterTest : public testing::Test {
)EOF";

std::string expected_state = fmt::format(
expected_state_template, name, error_response, request.iteration_state,
fmt::runtime(expected_state_template), name, error_response, request.iteration_state,
request.on_headers_called, request.headers_forwarded, request.on_data_called,
request.data_forwarded, request.on_trailers_called, request.trailers_forwarded,
request.on_resume_called, request.pending_headers, request.buffer, request.pending_trailers,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,14 @@ void Utility::extractCommonAccessLogProperties(
}

if (stream_info.upstreamInfo().has_value()) {
#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdangling-reference"
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we have quite a few of these. Is -Wdangling-reference finding false positives? Should we consider just disabling this warning in our build globally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, gcc implementation is buggy, you can find reports in other projects. The pattern here is unusual but the same, so maybe better keep it since we know when the false positive happens.

#endif
const auto& upstream_info = stream_info.upstreamInfo().value().get();
#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic pop
#endif
if (upstream_info.upstreamHost() != nullptr) {
Network::Utility::addressToProtobufAddress(
*upstream_info.upstreamHost()->address(),
Expand Down
8 changes: 8 additions & 0 deletions source/extensions/common/dubbo/hessian2_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,15 @@
#include "envoy/buffer/buffer.h"

#include "absl/strings/string_view.h"

#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdangling-reference"
#endif
#include "hessian2/basic_codec/object_codec.hpp"
#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic pop
#endif
#include "hessian2/codec.hpp"
#include "hessian2/object.hpp"
#include "hessian2/reader.hpp"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,16 @@
#include "envoy/buffer/buffer.h"

#include "absl/strings/string_view.h"

#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdangling-reference"
#endif
#include "hessian2/basic_codec/object_codec.hpp"
#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic pop
#endif

#include "hessian2/codec.hpp"
#include "hessian2/object.hpp"
#include "hessian2/reader.hpp"
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/filters/udp/dns_filter/dns_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ DnsFilter::DnsFilter(Network::UdpReadFilterCallbacks& callbacks,
for (const auto& ip : iplist) {
incrementExternalQueryTypeAnswerCount(query->type_);
const std::chrono::seconds ttl = getDomainTTL(query->name_);
message_parser_.storeDnsAnswerRecord(context, *query, ttl, std::move(ip));
message_parser_.storeDnsAnswerRecord(context, *query, ttl, ip);
}
sendDnsResponse(std::move(context));
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,15 @@ void GrpcHealthCheckerImpl::GrpcActiveHealthCheckSession::onInterval() {
request_encoder_ = &client_->newStream(*this);
request_encoder_->getStream().addCallbacks(*this);

#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdangling-reference"
#endif
const std::string& authority =
getHostname(host_, parent_.authority_value_, parent_.cluster_.info());
#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic pop
#endif
auto headers_message =
Grpc::Common::prepareHeaders(authority, parent_.service_method_.service()->full_name(),
parent_.service_method_.name(), absl::nullopt);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,6 @@ HttpHealthCheckerImpl::HttpActiveHealthCheckSession::HttpActiveHealthCheckSessio
response_body_(std::make_unique<Buffer::OwnedImpl>()),
hostname_(
HealthCheckerFactory::getHostname(host, parent_.host_value_, parent_.cluster_.info())),

local_connection_info_provider_(std::make_shared<Network::ConnectionInfoSetterImpl>(
Network::Utility::getCanonicalIpv4LoopbackAddress(),
Network::Utility::getCanonicalIpv4LoopbackAddress())),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,14 @@ class HttpHealthCheckerImpl : public HealthCheckerImplBase {
Http::CodecClientPtr client_;
Http::ResponseHeaderMapPtr response_headers_;
Buffer::InstancePtr response_body_;
#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdangling-reference"
#endif
const std::string& hostname_;
#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic pop
#endif
Network::ConnectionInfoProviderSharedPtr local_connection_info_provider_;
// Keep small members (bools and enums) at the end of class, to reduce alignment overhead.
const Http::Protocol protocol_;
Expand Down
7 changes: 7 additions & 0 deletions source/extensions/health_checkers/thrift/thrift.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,14 @@ class ThriftHealthChecker : public Upstream::HealthCheckerImplBase {

private:
ThriftHealthChecker& parent_;
#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdangling-reference"
#endif
const std::string& hostname_;
#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic pop
#endif
ClientPtr client_;
bool expect_close_{};
};
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/stat_sinks/common/statsd/statsd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ TcpStatsdSink::TcpStatsdSink(const LocalInfo::LocalInfo& local_info,
const auto cluster_or_error =
Config::Utility::checkCluster("tcp statsd", cluster_name, cluster_manager);
THROW_IF_STATUS_NOT_OK(cluster_or_error, throw);
const auto cluster = std::move(cluster_or_error.value());
const auto cluster = cluster_or_error.value();
cluster_info_ = cluster->get().info();
tls_->set([this](Event::Dispatcher& dispatcher) -> ThreadLocal::ThreadLocalObjectSharedPtr {
return std::make_shared<TlsSink>(*this, dispatcher);
Expand Down
2 changes: 1 addition & 1 deletion test/common/quic/envoy_quic_proof_verifier_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class EnvoyQuicProofVerifierTest : public testing::Test {
public:
EnvoyQuicProofVerifierTest()
: root_ca_cert_(cert_chain_.substr(cert_chain_.rfind("-----BEGIN CERTIFICATE-----"))),
leaf_cert_([=]() {
leaf_cert_([this]() {
std::stringstream pem_stream(cert_chain_);
std::vector<std::string> chain = quic::CertificateView::LoadPemFromStream(&pem_stream);
return chain[0];
Expand Down
2 changes: 1 addition & 1 deletion test/common/tcp_proxy/tcp_proxy_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class TcpProxyTest : public TcpProxyTestBase {
.RetiresOnSaturation();
EXPECT_CALL(conn_pool_, newConnection(_))
.WillOnce(Invoke(
[=](Tcp::ConnectionPool::Callbacks& cb) -> Tcp::ConnectionPool::Cancellable* {
[=, this](Tcp::ConnectionPool::Callbacks& cb) -> Tcp::ConnectionPool::Cancellable* {
conn_pool_callbacks_.push_back(&cb);
return onNewConnection(conn_pool_handles_.at(i).get());
}))
Expand Down
2 changes: 1 addition & 1 deletion test/common/tcp_proxy/tcp_proxy_test_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class TcpProxyTestBase : public testing::Test {
void raiseEventUpstreamConnected(uint32_t conn_index) {
EXPECT_CALL(filter_callbacks_.connection_, readDisable(false));
EXPECT_CALL(*upstream_connection_data_.at(conn_index), addUpstreamCallbacks(_))
.WillOnce(Invoke([=](Tcp::ConnectionPool::UpstreamCallbacks& cb) -> void {
.WillOnce(Invoke([=, this](Tcp::ConnectionPool::UpstreamCallbacks& cb) -> void {
upstream_callbacks_ = &cb;

// Simulate TCP conn pool upstream callbacks. This is safe because the TCP proxy never
Expand Down
Loading