[ASan][libc++] Initialize __r_ variable with lambda

[AdvenamTacet]

This commit is a refactor (increases readability) and optimization fix.

This is a fixed commit of #76200 First reverthed here: 1ea7a56

Please, check original PR for details.

The difference is a return type of the lambda.

Original description:

This commit addresses optimization and instrumentation challenges encountered within comma constructors.

1. _LIBCPP_STRING_INTERNAL_MEMORY_ACCESS does not work in comma constructors.
2. Code inside comma constructors is not always correctly optimized. Problematic code examples:
   • : __r_(((__str.__is_long() ? 0 : (__str.__annotate_delete(), 0)), std::move(__str.__r_))) {
   • : __r_(__r_([&](){ if(!__s.__is_long()) __s.__annotate_delete(); return std::move(__s.__r_);}())) {

However, lambda with argument seems to be correctly optimized. This patch uses that fact.

Use of lambda based on idea from @ldionne.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Tacet (AdvenamTacet)

Changes

This commit is a refactor (increases readability) and optimization fix.

This is a fixed commit of #76200 First reverthed here: 1ea7a56

Please, check original PR for details.

The difference is a return type of the lambda.

Original description:

This commit addresses optimization and instrumentation challenges encountered within comma constructors.

1. _LIBCPP_STRING_INTERNAL_MEMORY_ACCESS does not work in comma constructors.
2. Code inside comma constructors is not always correctly optimized. Problematic code examples:
   • : __r_(((__str.__is_long() ? 0 : (__str.__annotate_delete(), 0)), std::move(__str.__r_))) {
   • : __r_(__r_([&](){ if(!__s.__is_long()) __s.__annotate_delete(); return std::move(__s.__r_);}())) {

However, lambda with argument seems to be correctly optimized. This patch uses that fact.

Use of lambda based on idea from @ldionne.

---

Full diff: https://github.com/llvm/llvm-project/pull/77394.diff

1 Files Affected:

• (modified) libcxx/include/string (+1-1)

diff --git a/libcxx/include/string b/libcxx/include/string
index c676182fba8bac..c3cbb560efa911 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -922,7 +922,7 @@ public:
       // Turning off ASan instrumentation for variable initialization with _LIBCPP_STRING_INTERNAL_MEMORY_ACCESS
       // does not work consistently during initialization of __r_, so we instead unpoison __str's memory manually first.
       // __str's memory needs to be unpoisoned only in the case where it's a short string.
-      : __r_(((__str.__is_long() ? 0 : (__str.__annotate_delete(), 0)), std::move(__str.__r_))) {
+      : __r_([](basic_string &__s) -> auto&& { if(!__s.__is_long()) __s.__annotate_delete(); return std::move(__s.__r_); }(__str)) {
     __str.__r_.first() = __rep();
     __str.__annotate_new(0);
     if (!__is_long())

[AdvenamTacet]

I see failing buildbots (this time cuda buildbots), bo I see no connection with my changes, therefore I'm not reverting the PR.

• https://lab.llvm.org/buildbot/#/builders/46/builds/61725
• https://lab.llvm.org/buildbot/#/builders/55/builds/61849

Example error:

/buildbot/cuda-p4-0/work/clang-cuda-p4/clang/bin/../include/c++/v1/string:2025:33: error: use of undeclared identifier 'noinline'; did you mean 'inline'?
 2025 |   _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_NOINLINE void __erase_external_with_move(size_type __pos, size_type __n);
      |                                 ^
/buildbot/cuda-p4-0/work/clang-cuda-p4/clang/bin/../include/c++/v1/__config:1192:45: note: expanded from macro '_LIBCPP_NOINLINE'
 1192 | #    define _LIBCPP_NOINLINE __attribute__((__noinline__))
      |                                             ^
/buildbot/cuda-p4-0/work/clang-cuda-p4/external/cuda/cuda-11.8/include/crt/host_defines.h:83:24: note: expanded from macro '__noinline__'
   83 |         __attribute__((noinline))
      |

I don't even modify code close to _LIBCPP_NOINLINE.

[ldionne]

Yeah those have been failing for a while due to the addition of __noinline__ in <string>. We're still discussing what to do.

[AdvenamTacet]

Thank you for confirmation!