[libcxxabi] copy back std::string_view patches from LLVM

[nickdesaulniers]

Summary

I made a series of changes to LLVM's demangle in:

• D148348
• D148353
• D148363
• D148375

and so did Fangrui in 3ece37b3fa2c and Ashay in D149061.

I didn't notice the banner about there being two copies of this in tree
and was modifying the downstream versions. Copy these changes back to
the upstream version. Oops!

[nickdesaulniers]

libcxxabi/src/demangle/ItaniumDemangle.h

20

From the build bots:

/home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-libcxx-precommit-ci-group-sh1r-1/llvm-project/libcxx-ci/libcxxabi/src/demangle/ItaniumDemangle.h:20:10: fatal error: 'StringViewExtras.h' file not found
#include "StringViewExtras.h"
         ^~~~~~~~~~~~~~~~~~~~

I need to copy back this file, too.

[nickdesaulniers]

libcxxabi/src/demangle/ItaniumDemangle.h

20

I did that in the parent commit https://reviews.llvm.org/D148556. Perhaps the build bot didn't test both patches together in series?

FWICT, I marked the parent revision at 1:43pm, and harbormaster marked the build tests complete at 2:16pm, but I can't tell when harbormaster started the test. Perhaps I just need a rebase in order to rekick harbormaster now that the relationships between patches in the series is properly expressed in phab?

[phosek]

LGTM

[nickdesaulniers]

TODO(ndesaulniers): D148546 has more changes to appease msvc's impl of std::string_view, those need to be recopied in this patch.

[nickdesaulniers]

• rebase on final version of D148546, copy back formatting and msvc changes

[nickdesaulniers]

libcxxabi/src/demangle/ItaniumDemangle.h

2489

wrong namespace for this file

[nickdesaulniers]

libcxxabi/src/demangle/ItaniumDemangle.h

1851

also, std::string_view::substr may throw std::out_of_range which can't/shouldn't be used in libcxxabi (results in incredibly spooky linkage failures that took me longer than I'd like to admit to track down). That should be fixed first downstream, then this change rebased on that (with those changes recopied).

[nickdesaulniers]

libcxxabi/src/demangle/ItaniumDemangle.h

1851

https://reviews.llvm.org/D148959

[nickdesaulniers]

Also needs a rebase on D149061.

[ldionne]

LGTM once the blocker is resolved, assuming this makes the libcxxabi copy equivalent to the llvm copy. Also, please make sure you rebase onto main and re-upload the patch to get a green CI run before you merge this. Thanks!

[nickdesaulniers]

• rebase

[MaskRay]

Are the bots green now?

[nickdesaulniers]

• fixup cxa_demangle.cpp. I had this later in my local patch series, but as presubmit pointed out, this needs to in atomically now with the rest of these changes, not later.

[nickdesaulniers]

In D148566#4354662, @MaskRay wrote:

Are the bots green now?

No. :(
https://reviews.llvm.org/harbormaster/unit/view/7358113/
They spotted a change I had locally in a child patch (to libcxxabi/src/cxa_demangle.cpp) that needs to go in this commit. Should be fixed up now, but will wait again for presubmit bots. Change should be uncontroversial and limited to that file:
https://reviews.llvm.org/D148566?vs=523556&id=523921

[nickdesaulniers]

Hmm...some unittests are failing to link...

/usr/bin/ld: /tmp/lit-tmp-xl26yrf1/unittest_demangle-74952e.o: in function `std::__1::basic_string_view<char, std::__1::char_traits<char> >::basic_string_view[abi:v170000](char const*, unsigned long)':
unittest_demangle.pass.cpp:(.text._ZNSt3__117basic_string_viewIcNS_11char_traitsIcEEEC2B7v170000EPKcm[_ZNSt3__117basic_string_viewIcNS_11char_traitsIcEEEC2B7v170000EPKcm]+0x6d): undefined reference to `abort_message'
/usr/bin/ld: unittest_demangle.pass.cpp:(.text._ZNSt3__117basic_string_viewIcNS_11char_traitsIcEEEC2B7v170000EPKcm[_ZNSt3__117basic_string_viewIcNS_11char_traitsIcEEEC2B7v170000EPKcm]+0xca): undefined reference to `abort_message'
/usr/bin/ld: /tmp/lit-tmp-xl26yrf1/unittest_demangle-74952e.o: in function `unsigned long std::__1::__char_traits_length_checked[abi:v170000]<std::__1::char_traits<char> >(std::__1::char_traits<char>::char_type const*)':
unittest_demangle.pass.cpp:(.text._ZNSt3__128__char_traits_length_checkedB7v170000INS_11char_traitsIcEEEEmPKNT_9char_typeE[_ZNSt3__128__char_traits_length_checkedB7v170000INS_11char_traitsIcEEEEmPKNT_9char_typeE]+0x40): undefined reference to `abort_message'
/usr/bin/ld: /tmp/lit-tmp-xl26yrf1/unittest_demangle-74952e.o: in function `std::__1::basic_string_view<char, std::__1::char_traits<char> >::remove_prefix[abi:v170000](unsigned long)':
unittest_demangle.pass.cpp:(.text._ZNSt3__117basic_string_viewIcNS_11char_traitsIcEEE13remove_prefixB7v170000Em[_ZNSt3__117basic_string_viewIcNS_11char_traitsIcEEE13remove_prefixB7v170000Em]+0x5e): undefined reference to `abort_message'
/usr/bin/ld: /tmp/lit-tmp-xl26yrf1/unittest_demangle-74952e.o: in function `std::__1::basic_string_view<char, std::__1::char_traits<char> >::front[abi:v170000]() const':
unittest_demangle.pass.cpp:(.text._ZNKSt3__117basic_string_viewIcNS_11char_traitsIcEEE5frontB7v170000Ev[_ZNKSt3__117basic_string_viewIcNS_11char_traitsIcEEE5frontB7v170000Ev]+0x51): undefined reference to `abort_message'
/usr/bin/ld: /tmp/lit-tmp-xl26yrf1/unittest_demangle-74952e.o:unittest_demangle.pass.cpp:(.text._ZNKSt3__117basic_string_viewIcNS_11char_traitsIcEEEixB7v170000Em[_ZNKSt3__117basic_string_viewIcNS_11char_traitsIcEEEixB7v170000Em]+0x5e): more undefined references to `abort_message' follow
/usr/bin/ld: /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-3bf078479ae8-1/llvm-project/libcxx-ci/build/generic-cxx2b/test/Output/unittest_demangle.pass.cpp.dir/t.tmp.exe: hidden symbol `abort_message' isn't defined
/usr/bin/ld: final link failed: bad value

related to https://reviews.llvm.org/D149092 (or https://reviews.llvm.org/D150825)

[nickdesaulniers]

In D148566#4361245, @nickdesaulniers wrote:

Hmm...some unittests are failing to link...

/usr/bin/ld: /tmp/lit-tmp-xl26yrf1/unittest_demangle-74952e.o: in function `std::__1::basic_string_view<char, std::__1::char_traits<char> >::basic_string_view[abi:v170000](char const*, unsigned long)':
unittest_demangle.pass.cpp:(.text._ZNSt3__117basic_string_viewIcNS_11char_traitsIcEEEC2B7v170000EPKcm[_ZNSt3__117basic_string_viewIcNS_11char_traitsIcEEEC2B7v170000EPKcm]+0x6d): undefined reference to `abort_message'
/usr/bin/ld: unittest_demangle.pass.cpp:(.text._ZNSt3__117basic_string_viewIcNS_11char_traitsIcEEEC2B7v170000EPKcm[_ZNSt3__117basic_string_viewIcNS_11char_traitsIcEEEC2B7v170000EPKcm]+0xca): undefined reference to `abort_message'
/usr/bin/ld: /tmp/lit-tmp-xl26yrf1/unittest_demangle-74952e.o: in function `unsigned long std::__1::__char_traits_length_checked[abi:v170000]<std::__1::char_traits<char> >(std::__1::char_traits<char>::char_type const*)':
unittest_demangle.pass.cpp:(.text._ZNSt3__128__char_traits_length_checkedB7v170000INS_11char_traitsIcEEEEmPKNT_9char_typeE[_ZNSt3__128__char_traits_length_checkedB7v170000INS_11char_traitsIcEEEEmPKNT_9char_typeE]+0x40): undefined reference to `abort_message'
/usr/bin/ld: /tmp/lit-tmp-xl26yrf1/unittest_demangle-74952e.o: in function `std::__1::basic_string_view<char, std::__1::char_traits<char> >::remove_prefix[abi:v170000](unsigned long)':
unittest_demangle.pass.cpp:(.text._ZNSt3__117basic_string_viewIcNS_11char_traitsIcEEE13remove_prefixB7v170000Em[_ZNSt3__117basic_string_viewIcNS_11char_traitsIcEEE13remove_prefixB7v170000Em]+0x5e): undefined reference to `abort_message'
/usr/bin/ld: /tmp/lit-tmp-xl26yrf1/unittest_demangle-74952e.o: in function `std::__1::basic_string_view<char, std::__1::char_traits<char> >::front[abi:v170000]() const':
unittest_demangle.pass.cpp:(.text._ZNKSt3__117basic_string_viewIcNS_11char_traitsIcEEE5frontB7v170000Ev[_ZNKSt3__117basic_string_viewIcNS_11char_traitsIcEEE5frontB7v170000Ev]+0x51): undefined reference to `abort_message'
/usr/bin/ld: /tmp/lit-tmp-xl26yrf1/unittest_demangle-74952e.o:unittest_demangle.pass.cpp:(.text._ZNKSt3__117basic_string_viewIcNS_11char_traitsIcEEEixB7v170000Em[_ZNKSt3__117basic_string_viewIcNS_11char_traitsIcEEEixB7v170000Em]+0x5e): more undefined references to `abort_message' follow
/usr/bin/ld: /home/libcxx-builder/.buildkite-agent/builds/google-libcxx-builder-3bf078479ae8-1/llvm-project/libcxx-ci/build/generic-cxx2b/test/Output/unittest_demangle.pass.cpp.dir/t.tmp.exe: hidden symbol `abort_message' isn't defined
/usr/bin/ld: final link failed: bad value

related to https://reviews.llvm.org/D149092 (or https://reviews.llvm.org/D150825)

I can build DemangleTests just fine, though I do set -DLLVM_ENABLE_LLD=ON to link with LLD rather than BFD. Let me try BFD.

Strange, setting -DLLVM_USE_LINKER=ld (and hacking out LINKER_SUPPORTS_COLOR_DIAGNOSTICS from the cmake rules):

$ cd llvm/build; ninja DemangleTests; cd -
$ llvm-readelf -p .comment llvm/build/unittests/Demangle/DemangleTests
String dump of section '.comment':
[     0] GCC: (Debian 12.2.0-14) 12.2.0
[    1f] Android (10006735, +pgo, +bolt, +lto, -mlgo, based on r487747b) clang version 17.0.1 (https://android.googlesource.com/toolchain/llvm-project d9f89f4d16663d5012e5c09495f3b30ece3d2362)

(Shows that I was able to link via ld.bfd successfully) I wonder what else could be going wrong for this particular presubmit? _LIBCPP_VERBOSE_ABORT is not set AFAICT.

[nickdesaulniers]

In D148566#4362390, @nickdesaulniers wrote:

I can build DemangleTests just fine, though I do set -DLLVM_ENABLE_LLD=ON to link with LLD rather than BFD. Let me try BFD.

(Shows that I was able to link via ld.bfd successfully) I wonder what else could be going wrong for this particular presubmit? _LIBCPP_VERBOSE_ABORT is not set AFAICT.

Ah, it's check-cxxabi ninja target that's failing to build (w/ gcc and bfd). I can repro now. Digging (this seems related to _LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS).

[nickdesaulniers]

In D148566#4362478, @nickdesaulniers wrote:

In D148566#4362390, @nickdesaulniers wrote:

I can build DemangleTests just fine, though I do set -DLLVM_ENABLE_LLD=ON to link with LLD rather than BFD. Let me try BFD.

(Shows that I was able to link via ld.bfd successfully) I wonder what else could be going wrong for this particular presubmit? _LIBCPP_VERBOSE_ABORT is not set AFAICT.

Ah, it's check-cxxabi ninja target that's failing to build (w/ gcc and bfd). I can repro now. Digging (this seems related to _LIBCXXABI_DISABLE_VISIBILITY_ANNOTATIONS).

https://reviews.llvm.org/D151160

[nickdesaulniers]

• rebase onto D151160

[nickdesaulniers]

• rebase onto D151260

[nickdesaulniers]

• one more rebase to rekick presubmit tests

[ldionne]

Please land when the tests are green. Rebasing should take care of that.

[nickdesaulniers]

• rebase again for presubmit CI

[nickdesaulniers]

libcxxabi/src/demangle/ItaniumDemangle.h

3723

egads...libcxx/utils/ci/run-buildbot generic-debug-mode is failing because:

llvm::StringView::end and std::string_view::end have different semantics! llvm::StringView::end returns the last element, std::string_view::end returns 1 past the end element!

Let me fix that first downstream, then rebase this on top again.

[nickdesaulniers]

• rebase onto D151760, copy back Ashay's changes from D149061