[Bug]: absl::Cord: Append/Prepend of a large std::string crashes after SetExpectedChecksum() on an empty cord

Describe the issue

If an empty cord has an expected checksum set, appending or prepending an rvalue std::string longer than kMaxBytesToCopy (511 bytes) passes a null CordRep to CordRepBtree::Create(). With assertions enabled this fails edge != nullptr in IsDataEdge(); with NDEBUG it segfaults. I expected the append to work, as it does with 511 bytes.

It goes through AppendTreeToTree() (cord.cc:186), ForceBtree() (cord.cc:169) and CordRepBtree::Create(), which ends up calling IsDataEdge(nullptr). Prepend() fails the same way through PrependTreeToTree().

On an empty cord, SetCrcCordState() stores the checksum as a CordRepCrc with a null child. Most mutating paths call MaybeRemoveEmptyCrcNode() first, including the string_view overloads used for 511 bytes or less. The large-string branches of Cord::Append(T&&) and Cord::Prepend(T&&) go straight to AppendTree()/PrependTree() instead. (The header says SetExpectedChecksum() has no effect on an empty cord, but ExpectedChecksum() does return a value afterwards, so the checksum is kept.)

These two branches look like they were missed in 1db72eb03e ("Support empty Cords with an expected checksum"), which added MaybeRemoveEmptyCrcNode() to the other append and prepend paths. Before that commit SetExpectedChecksum() returned early for an empty cord, which is what the header comment still describes.

Adding the same call to both branches fixes it:

--- a/absl/strings/cord.cc
+++ b/absl/strings/cord.cc
@@ -558,6 +558,7 @@ void Cord::Append(T&& src) {
   if (src.size() <= kMaxBytesToCopy) {
     Append(absl::string_view(src));
   } else {
+    contents_.MaybeRemoveEmptyCrcNode();
     CordRep* rep = CordRepFromString(std::forward<T>(src));
     contents_.AppendTree(rep, CordzUpdateTracker::kAppendString);
   }
@@ -637,6 +638,7 @@ inline void Cord::Prepend(T&& src) {
   if (src.size() <= kMaxBytesToCopy) {
     Prepend(absl::string_view(src));
   } else {
+    contents_.MaybeRemoveEmptyCrcNode();
     CordRep* rep = CordRepFromString(std::forward<T>(src));
     contents_.PrependTree(rep, CordzUpdateTracker::kPrependString);
   }

With this change the example (both Append and Prepend) prints size=512 in ASan+UBSan, assert-enabled and NDEBUG builds, and absl_cord_test passes under ASan.

Steps to reproduce the problem

repro.cc:

#include <cstdio>
#include <string>

#include "absl/strings/cord.h"

int main() {
  absl::Cord cord;
  cord.SetExpectedChecksum(1);
  cord.Append(std::string(512, 'x'));
  std::printf("size=%zu\n", cord.size());
}

CMakeLists.txt, next to an abseil-cpp checkout:

cmake_minimum_required(VERSION 3.16)
project(repro CXX)
set(CMAKE_CXX_STANDARD 17)
add_subdirectory(abseil-cpp)
add_executable(repro repro.cc)
target_link_libraries(repro absl::cord)

cmake -S . -B build -DCMAKE_CXX_COMPILER=clang++
cmake --build build --target repro
./build/repro

repro: .../abseil-cpp/absl/strings/internal/cord_data_edge.h:33: bool absl::cord_internal::IsDataEdge(const absl::cord_internal::CordRep *): Assertion `edge != nullptr' failed.

With -DCMAKE_BUILD_TYPE=Release the same program dies with a segmentation fault. cord.Prepend(...) instead of cord.Append(...) fails the same way, and std::string(511, 'x') works for both.

What version of Abseil are you using?

3a80a7794c7405b95dfa1f1afa26b37adf817636 (master)

What operating system and version are you using?

Ubuntu 22.04.5 LTS, x86_64 (kernel 6.8.0)

What compiler and version are you using?

Ubuntu clang version 15.0.7
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/lib/llvm-15/bin
Selected GCC installation: /usr/lib/gcc/x86_64-linux-gnu/12

What build system are you using?

cmake version 3.22.1

Additional context

Also reproduced with GCC 11.4 (assert-enabled and NDEBUG builds). On an empty cord with a checksum, the other ways of adding data that I tried (string_view, const Cord&, Cord&& and CordBuffer appends, Prepend(const Cord&), GetAppendBuffer(), operator=(std::string&&)) work; only the two std::string&& overloads crash.