[libcxx-commits] [libcxx] [ASan][libc++] Correct (explicit) annotation size (PR #79292)

via libcxx-commits libcxx-commits at lists.llvm.org
Thu Jan 25 08:34:31 PST 2024


================
@@ -44,6 +45,14 @@ TEST_CONSTEXPR_CXX20 void test_string() {
   test(S("12345678901234567890"), "", 0, S("12345678901234567890"));
   test(S("12345678901234567890"), "12345", 5, S("1234567890123456789012345"));
   test(S("12345678901234567890"), "12345678901234567890", 20, S("1234567890123456789012345678901234567890"));
+
+  // Starting from long string (no SSO)
+  test(S("1234567890123456789012345678901234567890"), "", 0, S("1234567890123456789012345678901234567890"));
----------------
AdvenamTacet wrote:

No. I think old code is correct (at leas, has correct results) with how the `__grow_by_and_replace` function is used right now.

However, analogical tests helped me detect a problem some time ago in a different place, so I just added those test cases here for future, as I was already looking at that file.

Trying to find an example of incorrect behavior, I ran locally test below and both, old and new version passed. (I tested with and without short string annotations.)

```cpp
template <class S>
void test_asan(size_t const a, size_t const b, size_t const c) {
  S sa(a, 'a'), sb(b, 'b'), sc(c, 'c');
  LIBCPP_ASSERT(is_string_asan_correct(sa));
  sa.append(sb.c_str(), sb.size());
  LIBCPP_ASSERT(is_string_asan_correct(sa));
  LIBCPP_ASSERT(is_string_asan_correct(sb));
  LIBCPP_ASSERT(sa.size() == a + b);
  sa.append(sc.c_str(), sc.size());
  LIBCPP_ASSERT(is_string_asan_correct(sa));
  LIBCPP_ASSERT(is_string_asan_correct(sc));
  LIBCPP_ASSERT(sa.size() == a + b + c);
}

void test_loop() {
  for(size_t i = 0; i < 120; ++i)
    for(size_t j = 0; j < 120; ++j)
      for(size_t k = 0; k < 120; ++k) {
        test_asan<std::string>(i, j, k);
      }
}
```

I briefly review other functions using `__grow_by_and_replace` and I think there is no possible test case causing incorrect annotation with old code.

But I'm focused on understanding why https://github.com/llvm/llvm-project/pull/79049 got reverted now, so here I was mostly checking that the new version works as expected.

https://github.com/llvm/llvm-project/pull/79292


More information about the libcxx-commits mailing list