(Re)-enable libcxx debug mode

- We previously compiled libcxx with _LIBCPP_DEBUG=0. In old libcxx
  versions, this (surprisingly) enabled the basic debug mode [1].

- In libcxx 15 (the version we are currently using), _LIBCPP_DEBUG=0
  does nothing [2], the replacement is _LIBCPP_ENABLE_DEBUG_MODE=1.

- The debug mode is only enabled in Debug builds. Their docs say the
  extra check change complexity guarantees + the asserts crash which we
  don't want in Release builds. The debug mode detects issues like in
  [3]. Crashes look like this (for example)

  ```
  /data/ch/contrib/llvm-project/libcxx/include/__iterator/wrap_iter.h:99: assertion ::std::__libcpp_is_constant_evaluated() || (__get_const_db()->__dereferenceable(this)) failed: Attempted to increment a non-incrementable iteratorAborted (core dumped)
  ```

- I had to mute some new clang-tidy warnings in places that deal with
  container iterators. They got heavier and copy-by-value now yields a
  warning, e.g.

     for (auto it : iterators) /// <-- warning
         [...]

[1] https://releases.llvm.org/12.0.0/projects/libcxx/docs/DesignDocs/DebugMode.html
[2] https://releases.llvm.org/15.0.0/projects/libcxx/docs/DesignDocs/DebugMode.html
[3] https://github.com/llvm/llvm-project/blob/main/libcxx/test/support/container_debug_tests.h
This commit is contained in:
Robert Schulze 2024-09-30 16:58:33 +00:00
parent 4f3c9c3ee8
commit b9b7a1091b
No known key found for this signature in database
GPG Key ID: 26703B55FB13728A
7 changed files with 28 additions and 16 deletions

View File

@ -1,4 +1,21 @@
set (CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -D_LIBCPP_DEBUG=0") # More checks in debug build.
if (CMAKE_BUILD_TYPE_UC STREQUAL "DEBUG")
# Enable libcxx debug mode:
# --> https://releases.llvm.org/15.0.0/projects/libcxx/docs/DesignDocs/DebugMode.html
# The docs say the debug mode violates complexity guarantees, so do this only for Debug builds.
set (CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -D_LIBCPP_ENABLE_DEBUG_MODE=1")
# Also, enable randomization of unspecified behavior, e.g. the order of equal elements in std::sort.
# "Randomization" means that Debug and Release builds use different seeds.
set (CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -D_LIBCPP_DEBUG_RANDOMIZE_UNSPECIFIED_STABILITY_SEED=12345678")
# Note: Libcxx also has provides extra assertions:
# --> https://releases.llvm.org/15.0.0/projects/libcxx/docs/UsingLibcxx.html#assertions-mode
# These look orthogonal to the debug mode but the debug mode imlicitly enables them:
# --> https://github.com/llvm/llvm-project/blob/release/15.x/libcxx/include/__assert#L29
# TODO Once we upgrade to LLVM 18+, reconsider all of the above as they introduced "hardening modes":
# https://libcxx.llvm.org/Hardening.html
endif ()
add_subdirectory(contrib/libcxxabi-cmake)
add_subdirectory(contrib/libcxx-cmake)

View File

@ -428,7 +428,7 @@ bool DNSResolver::updateCacheImpl(
{
updated = true;
String deleted_elements;
for (auto it : elements_to_drop)
for (auto & it : elements_to_drop)
{
if (!deleted_elements.empty())
deleted_elements += ", ";

View File

@ -25,15 +25,11 @@ namespace
* `curl` strips leading dot and accepts url gitlab.com as a match for no_proxy .gitlab.com,
* while `wget` does an exact match.
* */
std::string buildPocoRegexpEntryWithoutLeadingDot(const std::string & host)
std::string buildPocoRegexpEntryWithoutLeadingDot(std::string_view host)
{
std::string_view view_without_leading_dot = host;
if (host[0] == '.')
{
view_without_leading_dot = std::string_view {host.begin() + 1u, host.end()};
}
return RE2::QuoteMeta(view_without_leading_dot);
if (host.starts_with('.'))
host.remove_prefix(1);
return RE2::QuoteMeta(host);
}
}

View File

@ -36,8 +36,8 @@ public:
auto findByValue(const T & value) const
{
const auto it = value_to_name_map.find(value);
if (it == std::end(value_to_name_map))
auto it = value_to_name_map.find(value);
if (it == value_to_name_map.end())
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Unexpected value {} in enum", toString(value));
return it;
@ -58,7 +58,7 @@ public:
bool getNameForValue(const T & value, StringRef & result) const
{
const auto it = value_to_name_map.find(value);
if (it == std::end(value_to_name_map))
if (it == value_to_name_map.end())
return false;
result = it->second;

View File

@ -23,7 +23,7 @@ RefreshSet::Handle & RefreshSet::Handle::operator=(Handle && other) noexcept
id = std::move(other.id);
inner_table_id = std::move(other.inner_table_id);
dependencies = std::move(other.dependencies);
iter = std::move(other.iter);
iter = std::move(other.iter); /// NOLINT(performance-move-const-arg)
inner_table_iter = std::move(other.inner_table_iter);
metric_increment = std::move(other.metric_increment);
return *this;

View File

@ -604,7 +604,7 @@ MergeTask::StageRuntimeContextPtr MergeTask::ExecuteAndFinalizeHorizontalPart::g
new_ctx->rows_sources_temporary_file = std::move(ctx->rows_sources_temporary_file);
new_ctx->column_sizes = std::move(ctx->column_sizes);
new_ctx->compression_codec = std::move(ctx->compression_codec);
new_ctx->it_name_and_type = std::move(ctx->it_name_and_type);
new_ctx->it_name_and_type = std::move(ctx->it_name_and_type); /// NOLINT(performance-move-const-arg)
new_ctx->read_with_direct_io = std::move(ctx->read_with_direct_io);
new_ctx->need_sync = std::move(ctx->need_sync);

View File

@ -149,7 +149,6 @@ void traverse(const ParserImpl::Element & e, std::shared_ptr<JSONNode> node)
std::shared_ptr<JSONNode> parseJSON(const String & json)
{
std::string_view view{json.begin(), json.end()};
ParserImpl::Element document;
ParserImpl p;