libctf: do not corrupt strings across ctf_serialize

The preceding change revealed a new bug: the string table is sorted for
better compression, so repeated serialization with type (or member)
additions in the middle can move strings around.  But every
serialization flushes the set of refs (the memory locations that are
automatically updated with a final string offset when the strtab is
updated), so if we are not to have string offsets go stale, we must do
all ref additions within the serialization code (which walks the
complete set of types and symbols anyway). Unfortunately, we were adding
one ref in another place: the type name in the dynamic type definitions,
which has a ref added to it by ctf_add_generic.

So adding a type, serializing (via, say, one of the ctf_write
functions), adding another type with a name that sorts earlier, and
serializing again will corrupt the name of the first type because it no
longer had a ref pointing to its dtd entry's name when its string offset
was shifted later in the strtab to mae way for the other type.

To ensure that we don't miss strings, we also maintain a set of *pending
refs* that will be added later (during serialization), and remove
entries from that set when the ref is finally added.  We always use
ctf_str_add_pending outside ctf-serialize.c, ensure that ctf_serialize
adds all strtab offsets as refs (even those in the dtds) on every
serialization, and mandate that no refs are live on entry to
ctf_serialize and that all pending refs are gone before strtab
finalization.  (Of necessity ctf_serialize has to traverse all strtab
offsets in the dtds in order to serialize them, so adding them as refs
at the same time is easy.)

(Note that we still can't erase unused atoms when we roll back, though
we can erase unused refs: members and enums are still not removed by
rollbacks and might reference strings added after the snapshot.)

libctf/ChangeLog
2021-03-18  Nick Alcock  <nick.alcock@oracle.com>

	* ctf-hash.c (ctf_dynset_elements): New.
	* ctf-impl.h (ctf_dynset_elements): Declare it.
	(ctf_str_add_pending): Likewise.
	(ctf_dict_t) <ctf_str_pending_ref>: New, set of refs that must be
	added during serialization.
	* ctf-string.c (ctf_str_create_atoms): Initialize it.
	(CTF_STR_ADD_REF): New flag.
	(CTF_STR_MAKE_PROVISIONAL): Likewise.
	(CTF_STR_PENDING_REF): Likewise.
	(ctf_str_add_ref_internal): Take a flags word rather than int
	params.  Populate, and clear out, ctf_str_pending_ref.
	(ctf_str_add): Adjust accordingly.
	(ctf_str_add_external): Likewise.
	(ctf_str_add_pending): New.
	(ctf_str_remove_ref): Also remove the potential ref if it is a
	pending ref.
	* ctf-serialize.c (ctf_serialize): Prohibit addition of strings
	with ctf_str_add_ref before serialization.  Ensure that the
	ctf_str_pending_ref set is empty before strtab finalization.
	(ctf_emit_type_sect): Add a ref to the ctt_name.
	* ctf-create.c (ctf_add_generic): Add the ctt_name as a pending
	ref.
	* testsuite/libctf-writable/reserialize-strtab-corruption.*: New test.

1 files changed, 52 insertions, 9 deletions

diff --git a/libctf/ctf-string.c b/libctf/ctf-string.c
index 91ad2e3..9f0e540 100644
--- a/libctf/ctf-string.c
+++ b/libctf/ctf-string.c
@@ -103,7 +103,7 @@ ctf_str_create_atoms (ctf_dict_t *fp)
 {
   fp->ctf_str_atoms = ctf_dynhash_create (ctf_hash_string, ctf_hash_eq_string,
 					  free, ctf_str_free_atom);
-  if (fp->ctf_str_atoms == NULL)
+  if (!fp->ctf_str_atoms)
     return -ENOMEM;
 
   if (!fp->ctf_prov_strtab)
@@ -113,6 +113,13 @@ ctf_str_create_atoms (ctf_dict_t *fp)
   if (!fp->ctf_prov_strtab)
     goto oom_prov_strtab;
 
+  if (!fp->ctf_str_pending_ref)
+    fp->ctf_str_pending_ref = ctf_dynset_create (htab_hash_pointer,
+						 htab_eq_pointer,
+						 NULL);
+  if (!fp->ctf_str_pending_ref)
+    goto oom_str_pending_ref;
+
   errno = 0;
   ctf_str_add (fp, "");
   if (errno == ENOMEM)
@@ -123,6 +130,9 @@ ctf_str_create_atoms (ctf_dict_t *fp)
  oom_str_add:
   ctf_dynhash_destroy (fp->ctf_prov_strtab);
   fp->ctf_prov_strtab = NULL;
+ oom_str_pending_ref:
+  ctf_dynset_destroy (fp->ctf_str_pending_ref);
+  fp->ctf_str_pending_ref = NULL;
  oom_prov_strtab:
   ctf_dynhash_destroy (fp->ctf_str_atoms);
   fp->ctf_str_atoms = NULL;
@@ -135,8 +145,13 @@ ctf_str_free_atoms (ctf_dict_t *fp)
 {
   ctf_dynhash_destroy (fp->ctf_prov_strtab);
   ctf_dynhash_destroy (fp->ctf_str_atoms);
+  ctf_dynset_destroy (fp->ctf_str_pending_ref);
 }
 
+#define CTF_STR_ADD_REF 0x1
+#define CTF_STR_MAKE_PROVISIONAL 0x2
+#define CTF_STR_PENDING_REF 0x4
+
 /* Add a string to the atoms table, copying the passed-in string.  Return the
    atom added. Return NULL only when out of memory (and do not touch the
    passed-in string in that case).  Possibly augment the ref list with the
@@ -144,7 +159,7 @@ ctf_str_free_atoms (ctf_dict_t *fp)
    provisional strtab.   */
 static ctf_str_atom_t *
 ctf_str_add_ref_internal (ctf_dict_t *fp, const char *str,
-			  int add_ref, int make_provisional, uint32_t *ref)
+			  int flags, uint32_t *ref)
 {
   char *newstr = NULL;
   ctf_str_atom_t *atom = NULL;
@@ -152,7 +167,7 @@ ctf_str_add_ref_internal (ctf_dict_t *fp, const char *str,
 
   atom = ctf_dynhash_lookup (fp->ctf_str_atoms, str);
 
-  if (add_ref)
+  if (flags & CTF_STR_ADD_REF)
     {
       if ((aref = malloc (sizeof (struct ctf_str_atom_ref))) == NULL)
 	return NULL;
@@ -161,8 +176,9 @@ ctf_str_add_ref_internal (ctf_dict_t *fp, const char *str,
 
   if (atom)
     {
-      if (add_ref)
+      if (flags & CTF_STR_ADD_REF)
 	{
+	  ctf_dynset_remove (fp->ctf_str_pending_ref, (void *) ref);
 	  ctf_list_append (&atom->csa_refs, aref);
 	  fp->ctf_str_num_refs++;
 	}
@@ -182,7 +198,7 @@ ctf_str_add_ref_internal (ctf_dict_t *fp, const char *str,
   atom->csa_str = newstr;
   atom->csa_snapshot_id = fp->ctf_snapshots;
 
-  if (make_provisional)
+  if (flags & CTF_STR_MAKE_PROVISIONAL)
     {
       atom->csa_offset = fp->ctf_str_prov_offset;
 
@@ -193,8 +209,14 @@ ctf_str_add_ref_internal (ctf_dict_t *fp, const char *str,
       fp->ctf_str_prov_offset += strlen (atom->csa_str) + 1;
     }
 
-  if (add_ref)
+  if (flags & CTF_STR_PENDING_REF)
+    {
+      if (ctf_dynset_insert (fp->ctf_str_pending_ref, (void *) ref) < 0)
+	goto oom;
+    }
+  else if (flags & CTF_STR_ADD_REF)
     {
+      ctf_dynset_remove (fp->ctf_str_pending_ref, (void *) ref);
       ctf_list_append (&atom->csa_refs, aref);
       fp->ctf_str_num_refs++;
     }
@@ -222,7 +244,7 @@ ctf_str_add (ctf_dict_t *fp, const char *str)
   if (!str)
     str = "";
 
-  atom = ctf_str_add_ref_internal (fp, str, FALSE, TRUE, 0);
+  atom = ctf_str_add_ref_internal (fp, str, CTF_STR_MAKE_PROVISIONAL, 0);
   if (!atom)
     return 0;
 
@@ -240,7 +262,26 @@ ctf_str_add_ref (ctf_dict_t *fp, const char *str, uint32_t *ref)
   if (!str)
     str = "";
 
-  atom = ctf_str_add_ref_internal (fp, str, TRUE, TRUE, ref);
+  atom = ctf_str_add_ref_internal (fp, str, CTF_STR_ADD_REF
+				   | CTF_STR_MAKE_PROVISIONAL, ref);
+  if (!atom)
+    return 0;
+
+  return atom->csa_offset;
+}
+
+/* Like ctf_str_add_ref(), but notes that this memory location must be added as
+   a ref by a later serialization phase, rather than adding it itself.  */
+uint32_t
+ctf_str_add_pending (ctf_dict_t *fp, const char *str, uint32_t *ref)
+{
+  ctf_str_atom_t *atom;
+
+  if (!str)
+    str = "";
+
+  atom = ctf_str_add_ref_internal (fp, str, CTF_STR_PENDING_REF
+				   | CTF_STR_MAKE_PROVISIONAL, ref);
   if (!atom)
     return 0;
 
@@ -257,7 +298,7 @@ ctf_str_add_external (ctf_dict_t *fp, const char *str, uint32_t offset)
   if (!str)
     str = "";
 
-  atom = ctf_str_add_ref_internal (fp, str, FALSE, FALSE, 0);
+  atom = ctf_str_add_ref_internal (fp, str, 0, 0);
   if (!atom)
     return 0;
 
@@ -307,6 +348,8 @@ ctf_str_remove_ref (ctf_dict_t *fp, const char *str, uint32_t *ref)
 	  free (aref);
 	}
     }
+
+  ctf_dynset_remove (fp->ctf_str_pending_ref, (void *) ref);
 }
 
 /* A ctf_dynhash_iter_remove() callback that removes atoms later than a given