[libcxx-commits] [libcxx] [ASan][libc++] String annotations optimizations fix with lambda (PR #76200)

[via libcxx-commits]

https://github.com/AdvenamTacet updated https://github.com/llvm/llvm-project/pull/76200

>From f52466e162d1ae56b9982829b9aa87e0be37b99f Mon Sep 17 00:00:00 2001
From: Advenam Tacet <advenam.tacet at trailofbits.com>
Date: Fri, 22 Dec 2023 02:25:39 +0100
Subject: [PATCH 1/2] [ASan][libc++] String annotations optimizations fix with
 lambda

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.
---
 libcxx/include/string | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libcxx/include/string b/libcxx/include/string
index c676182fba8bac..03f6655bb1e76e 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -922,7 +922,10 @@ 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_))) {
+      // Lambda is used because of optimization challenges encountered within comma constructors.
+      // Lambda with argument is correctly optimized, but it does not solve the problem with internal memory
+      // access macro.
+      : __r_([](basic_string &__s){ if(!__s.__is_long()) __s.__annotate_delete(); return std::move(__s.__r_);}(__str)) {
     __str.__r_.first() = __rep();
     __str.__annotate_new(0);
     if (!__is_long())

>From ab9bff2bad4a3ac48930aea7452fcdc8786c12d2 Mon Sep 17 00:00:00 2001
From: Advenam Tacet <advenam.tacet at trailofbits.com>
Date: Wed, 27 Dec 2023 09:36:56 +0100
Subject: [PATCH 2/2] Remove a commnet

---
 libcxx/include/string | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/libcxx/include/string b/libcxx/include/string
index 03f6655bb1e76e..a67693fd28c649 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -922,9 +922,6 @@ 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.
-      // Lambda is used because of optimization challenges encountered within comma constructors.
-      // Lambda with argument is correctly optimized, but it does not solve the problem with internal memory
-      // access macro.
       : __r_([](basic_string &__s){ if(!__s.__is_long()) __s.__annotate_delete(); return std::move(__s.__r_);}(__str)) {
     __str.__r_.first() = __rep();
     __str.__annotate_new(0);