Make pointers in ValueStore stable across insertions (#5576)

This avoids reallocating the backing buffer in ValueStore so that
references into the ValueStore are never invalidated when adding new
values. This works especially well since we never delete values from a
ValueStore.

The strategy used is to allocate chunks of a fixed size, and inserting
into each chunk until it is full before allocating the next. The
ValueStore starts with an initial allocated chunk in all cases, so that
there is only a single indirection for adding and accessing values from
this chunk. After it's full, additional chunks are allocated in a
vector, so two indirections are required to add or access values in
these chunks.

This obviates the need for
#5529 as we no longer
need to worry about holding pointers into a ValueStore.

We introduce a Flatten operation for ranges. It flattens a "range over
ranges over Ts" down to a "range over Ts". This allows us to make an
range over the values in the ValueStore from a range over the chunks in
the ValueStore. See
https://doc.rust-lang.org/stable/std/iter/trait.Iterator.html#method.flatten
for inspiration for this name choice. Flatten is used in one other case
where we were writing two levels of for loops to do the same thing.

The `array_ref()` accessor is changed to `values()` and its now a range
(typed as a `ValueStoreRange`) over all values as references (like
ArrayRef was, but without random access).

As pointers to a ValueStore can no longer be invalidated, we remove the
ASAN poisoning feature and support from ValueStore.

This may cause a regression in our compile benchmark of up to 5%, though
that is close to or within the noise of the benchmark. We can look at
ways to optimize things further in the future. Perhaps by tuning the
chunk size further, or by making later chunks larger than earlier
chunks, or other strategies.

Author: danakj

bazel/cc_toolchains/clang_cc_toolchain_config.bzl

@@ -643,22 +643,6 @@ def _impl(ctx):
         ],
     )
 
-    # A feature that enables poisoning of value stores to detect use after
-    # potential reallocation bugs.
-    #
-    # TODO: Remove this and leave poisoning always on once these bugs are
-    # fixed.
-    poison_value_stores = feature(
-        name = "poison_value_stores",
-        requires = [feature_set(["asan"])],
-        flag_sets = [flag_set(
-            actions = all_compile_actions,
-            flag_groups = [flag_group(flags = [
-                "-DCARBON_POISON_VALUE_STORES=1",
-            ])],
-        )],
-    )
-
     fuzzer = feature(
         name = "fuzzer",
         flag_sets = [flag_set(
@@ -1153,7 +1137,6 @@ def _impl(ctx):
         asan,
         asan_min_size,
         enable_in_fastbuild,
-        poison_value_stores,
         fuzzer,
         layering_check,
         module_maps,

toolchain/base/BUILD

@@ -86,7 +86,10 @@ cc_library(
 
 cc_library(
     name = "value_store",
-    hdrs = ["value_store.h"],
+    hdrs = [
+        "value_store.h",
+        "value_store_chunk.h",
+    ],
     deps = [
         ":mem_usage",
         ":yaml",

toolchain/base/int.h

@@ -346,8 +346,8 @@ class IntStore {
   // into the ID space.
   auto OutputYaml() const -> Yaml::OutputMapping;
 
-  auto array_ref() const -> llvm::ArrayRef<llvm::APInt> {
-    return values_.array_ref();
+  auto values() const [[clang::lifetimebound]] -> auto {
+    return values_.values();
   }
   auto size() const -> size_t { return values_.size(); }