[libcxx-dev] How do _LIBCPP_DEBUG=1 "debug iterators" ever work, when linking against libc++.a?

[Arthur O'Dwyer via libcxx-dev]

Gentle bump.

Quoting myself for emphasis:
>
> The problem seems to be that
> (1) libc++.a needs to copy strings sometimes, so it contains codegen for
the copy constructor
> (2) libc++.a is compiled with -D_LIBCPP_DEBUG=0, i.e., assertions but no
debug iterators
> (3) Therefore any strings created by the copy constructor inside
libc++.a, don't get registered with the debug-iterators library
> (4) Therefore pretty much everything is affected by unpredictable
assert-fails??
>
> The real question is, how was this ever *supposed* to work?


On Sat, May 1, 2021 at 8:50 PM Arthur O'Dwyer <arthur.j.odwyer at gmail.com>
wrote:

> Hi folks,
>
> I recently added a buildkite CI step that runs libc++'s test suite with
> -D_LIBCPP_DEBUG=1. This turns on "debug iterators," which is a thing in the
> library that maintains a collection of all known container instances and
> iterator instances, and updates the collection on every
> container-construction, container-destruction, container-modification,
> iterator-construction, iterator-destruction, or iterator-modification. This
> enables it to print an error message on any iterator-modification,
> iterator-comparison, or iterator-dereference that violates the
> iterator-invalidation guarantees of the Standard.
> https://reviews.llvm.org/D100866 was the review that added that CI step.
> I marked a bunch of tests as "LIBCXX-DEBUG-FIXME", with the intent of
> fixing them all quickly. The patches for all-but-one of those FIXMEs are:
> https://reviews.llvm.org/D101674
> https://reviews.llvm.org/D101675
> https://reviews.llvm.org/D101676
> https://reviews.llvm.org/D101677
> https://reviews.llvm.org/D101678
> https://reviews.llvm.org/D101679
>
> I've just tracked down the very final LIBCXX-DEBUG-FIXME. Unfortunately,
> it's a doozy.
> The symptom is a segfault (null pointer dereference) in
>
> libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.proximate/proximate.pass.cpp
>
> It's a segfault instead of a _LIBCPP_ASSERT-failure, because the
> compiled-to-binary filesystem library was not built with _LIBCPP_ASSERT
> enabled. Okay, no problem, I should enable debug assertions in the library
> for this particular CI builder. I believe this is the patch for that:
>
> ```
> --- a/libcxx/cmake/caches/Generic-debug-iterators.cmake
> +++ b/libcxx/cmake/caches/Generic-debug-iterators.cmake
> @@ -1,2 +1,3 @@
>  set(LIBCXX_TEST_PARAMS "debug_level=1"
> "additional_features=LIBCXX-DEBUG-FIXME" CACHE STRING "")
>  set(LIBCXXABI_TEST_PARAMS "${LIBCXX_TEST_PARAMS}" CACHE STRING "")
> +set(LIBCXX_ENABLE_ASSERTIONS ON CACHE BOOL "")
> ```
>
> Now the symptom is a _LIBCPP_ASSERT-failure. The problem is that we're
> move-constructing a string, and the source string was never registered with
> the debug-iterators library.
>
> Reduced test case:
> ```
> cat > test.cpp <<EOF
> #include <string>
> void test_debug_function(std::__libcpp_debug_info const&) {
>     std::abort();
> }
> int main(int, char**) {
>     std::__libcpp_set_debug_function(&test_debug_function);
>     std::string x = "1234567890123456789012345678901234";
>     std::string y = x;  // calls into the compiled library
>     std::string z = std::move(y);
> }
> EOF
> bin/clang++ test.cpp -D_LIBCPP_DEBUG=1 lib/libc++.a lib/libc++abi.a
> ./a.out
> Abort trap: 6
> ```
>
> The problem seems to be that
> (1) libc++.a needs to copy strings sometimes, so it contains codegen for
> the copy constructor
> (2) libc++.a is compiled with -D_LIBCPP_DEBUG=0, i.e., assertions but no
> debug iterators
> (3) Therefore any strings created by the copy constructor inside libc++.a,
> don't get registered with the debug-iterators library
> (4) Therefore pretty much everything is affected by unpredictable
> assert-fails??
> I'm amazed that only one test out of our entire test suite seems to be
> affected by this issue. I would think that pretty much everything in the
> debug-iterators CI should be failing because of this; I'm not sure how it's
> working right now.
>
> The real question is, how was this ever *supposed* to work? It seems like
> the person-who-compiled-libc++.a and the
> person-compiling-their-user-code-against-libc++-headers need to agree on
> the setting of _LIBCPP_DEBUG:— If libc++.a has _LIBCPP_DEBUG=1 and the user
> code doesn't, everything explodes; and also vice versa. But this doesn't
> seem realistic.
>
> None of the offending basic_string constructors are currently marked with
> _LIBCPP_INLINE_VISIBILITY. I suspect that adding _LIBCPP_INLINE_VISIBILITY
> could help reduce the surface area of the problem even further... but based
> on what I said above, I don't see how the problem *ever* gets to zero,
> unless we can somehow prevent libc++.a from ever constructing a string (or
> at least never letting strings-it-constructs leak out into the user's code
> where _LIBCPP_DEBUG=1 might be in effect).
>
> https://reviews.llvm.org/D48616 may be related — it added some non-inline
> constructors — but the review comments don't talk about inline or
> visibility at all.
>
> What am I missing here? Thoughts?
>
> –Arthur
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/libcxx-dev/attachments/20210508/5ef59d18/attachment.html>