* [binutils-gdb] Revert "libctf: do not corrupt strings across ctf_serialize"
@ 2024-04-19 15:51 Nick Alcock
0 siblings, 0 replies; only message in thread
From: Nick Alcock @ 2024-04-19 15:51 UTC (permalink / raw)
To: binutils-cvs
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3301ddba1badc50a06f5d21c78790d508969081d
commit 3301ddba1badc50a06f5d21c78790d508969081d
Author: Nick Alcock <nick.alcock@oracle.com>
Date: Tue Jan 30 13:40:56 2024 +0000
Revert "libctf: do not corrupt strings across ctf_serialize"
This reverts commit 986e9e3aa03f854bedacef7fac38fe8f009a416c.
(We do not revert the testcase -- it remains valid -- but we are
taking a different, less complex and more robust approach.)
This also deletes the pending refs abstraction without (yet)
replacing it, so some tests will fail for a commit or two.
Diff:
---
libctf/ctf-create.c | 27 ++----------------
libctf/ctf-hash.c | 6 ----
libctf/ctf-impl.h | 6 +---
libctf/ctf-serialize.c | 24 +---------------
libctf/ctf-string.c | 76 ++++++--------------------------------------------
5 files changed, 14 insertions(+), 125 deletions(-)
diff --git a/libctf/ctf-create.c b/libctf/ctf-create.c
index 23bbf92ff1a..9d86b961132 100644
--- a/libctf/ctf-create.c
+++ b/libctf/ctf-create.c
@@ -464,8 +464,7 @@ ctf_add_generic (ctf_dict_t *fp, uint32_t flag, const char *name, int kind,
type = ++fp->ctf_typemax;
type = LCTF_INDEX_TO_TYPE (fp, type, (fp->ctf_flags & LCTF_CHILD));
- dtd->dtd_data.ctt_name = ctf_str_add_pending (fp, name,
- &dtd->dtd_data.ctt_name);
+ dtd->dtd_data.ctt_name = ctf_str_add_ref (fp, name, &dtd->dtd_data.ctt_name);
dtd->dtd_type = type;
if (dtd->dtd_data.ctt_name == 0 && name != NULL && name[0] != '\0')
@@ -1080,21 +1079,11 @@ ctf_add_enumerator (ctf_dict_t *fp, ctf_id_t enid, const char *name,
return -1; /* errno is set for us. */
en = (ctf_enum_t *) dtd->dtd_vlen;
- if (dtd->dtd_vlen != old_vlen)
- {
- ptrdiff_t move = (signed char *) dtd->dtd_vlen - (signed char *) old_vlen;
-
- /* Remove pending refs in the old vlen region and reapply them. */
-
- for (i = 0; i < vlen; i++)
- ctf_str_move_pending (fp, &en[i].cte_name, move);
- }
-
for (i = 0; i < vlen; i++)
if (strcmp (ctf_strptr (fp, en[i].cte_name), name) == 0)
return (ctf_set_errno (ofp, ECTF_DUPLICATE));
- en[i].cte_name = ctf_str_add_pending (fp, name, &en[i].cte_name);
+ en[i].cte_name = ctf_str_add_ref (fp, name, &en[i].cte_name);
en[i].cte_value = value;
if (en[i].cte_name == 0 && name != NULL && name[0] != '\0')
@@ -1154,16 +1143,6 @@ ctf_add_member_offset (ctf_dict_t *fp, ctf_id_t souid, const char *name,
return (ctf_set_errno (ofp, ctf_errno (fp)));
memb = (ctf_lmember_t *) dtd->dtd_vlen;
- if (dtd->dtd_vlen != old_vlen)
- {
- ptrdiff_t move = (signed char *) dtd->dtd_vlen - (signed char *) old_vlen;
-
- /* Remove pending refs in the old vlen region and reapply them. */
-
- for (i = 0; i < vlen; i++)
- ctf_str_move_pending (fp, &memb[i].ctlm_name, move);
- }
-
if (name != NULL)
{
for (i = 0; i < vlen; i++)
@@ -1193,7 +1172,7 @@ ctf_add_member_offset (ctf_dict_t *fp, ctf_id_t souid, const char *name,
return -1; /* errno is set for us. */
}
- memb[vlen].ctlm_name = ctf_str_add_pending (fp, name, &memb[vlen].ctlm_name);
+ memb[vlen].ctlm_name = ctf_str_add_ref (fp, name, &memb[vlen].ctlm_name);
memb[vlen].ctlm_type = type;
if (memb[vlen].ctlm_name == 0 && name != NULL && name[0] != '\0')
return -1; /* errno is set for us. */
diff --git a/libctf/ctf-hash.c b/libctf/ctf-hash.c
index f8032ae4d86..77b8478479e 100644
--- a/libctf/ctf-hash.c
+++ b/libctf/ctf-hash.c
@@ -669,12 +669,6 @@ ctf_dynset_lookup (ctf_dynset_t *hp, const void *key)
return NULL;
}
-size_t
-ctf_dynset_elements (ctf_dynset_t *hp)
-{
- return htab_elements ((struct htab *) hp);
-}
-
/* TRUE/FALSE return. */
int
ctf_dynset_exists (ctf_dynset_t *hp, const void *key, const void **orig_key)
diff --git a/libctf/ctf-impl.h b/libctf/ctf-impl.h
index 8ce489a55ba..c16ef185fdc 100644
--- a/libctf/ctf-impl.h
+++ b/libctf/ctf-impl.h
@@ -383,8 +383,7 @@ struct ctf_dict
ctf_dynhash_t *ctf_names; /* Hash table of remaining type names. */
ctf_lookup_t ctf_lookups[5]; /* Pointers to nametabs for name lookup. */
ctf_strs_t ctf_str[2]; /* Array of string table base and bounds. */
- ctf_dynhash_t *ctf_str_atoms; /* Hash table of ctf_str_atoms_t. */
- ctf_dynset_t *ctf_str_pending_ref; /* Locations awaiting ref addition. */
+ ctf_dynhash_t *ctf_str_atoms; /* Hash table of ctf_str_atoms_t. */
uint64_t ctf_str_num_refs; /* Number of refs to cts_str_atoms. */
uint32_t ctf_str_prov_offset; /* Latest provisional offset assigned so far. */
unsigned char *ctf_base; /* CTF file pointer. */
@@ -664,7 +663,6 @@ extern int ctf_dynset_insert (ctf_dynset_t *, void *);
extern void ctf_dynset_remove (ctf_dynset_t *, const void *);
extern void ctf_dynset_destroy (ctf_dynset_t *);
extern void *ctf_dynset_lookup (ctf_dynset_t *, const void *);
-extern size_t ctf_dynset_elements (ctf_dynset_t *);
extern int ctf_dynset_exists (ctf_dynset_t *, const void *key,
const void **orig_key);
extern int ctf_dynset_next (ctf_dynset_t *, ctf_next_t **, void **key);
@@ -727,8 +725,6 @@ extern int ctf_str_create_atoms (ctf_dict_t *);
extern void ctf_str_free_atoms (ctf_dict_t *);
extern uint32_t ctf_str_add (ctf_dict_t *, const char *);
extern uint32_t ctf_str_add_ref (ctf_dict_t *, const char *, uint32_t *ref);
-extern uint32_t ctf_str_add_pending (ctf_dict_t *, const char *, uint32_t *);
-extern int ctf_str_move_pending (ctf_dict_t *, uint32_t *, ptrdiff_t);
extern int ctf_str_add_external (ctf_dict_t *, const char *, uint32_t offset);
extern void ctf_str_remove_ref (ctf_dict_t *, const char *, uint32_t *ref);
extern void ctf_str_rollback (ctf_dict_t *, ctf_snapshot_id_t);
diff --git a/libctf/ctf-serialize.c b/libctf/ctf-serialize.c
index 9dd7fbda285..2afc7be7c48 100644
--- a/libctf/ctf-serialize.c
+++ b/libctf/ctf-serialize.c
@@ -822,10 +822,7 @@ ctf_emit_type_sect (ctf_dict_t *fp, unsigned char **tptr)
copied = (ctf_stype_t *) t; /* name is at the start: constant offset. */
if (copied->ctt_name
&& (name = ctf_strraw (fp, copied->ctt_name)) != NULL)
- {
- ctf_str_add_ref (fp, name, &copied->ctt_name);
- ctf_str_add_ref (fp, name, &dtd->dtd_data.ctt_name);
- }
+ ctf_str_add_ref (fp, name, &copied->ctt_name);
copied->ctt_size = type_ctt_size;
t += len;
@@ -960,7 +957,6 @@ ctf_serialize (ctf_dict_t *fp)
ctf_varent_t *dvarents;
ctf_strs_writable_t strtab;
int err;
- int num_missed_str_refs;
int sym_functions = 0;
unsigned char *t;
@@ -980,16 +976,6 @@ ctf_serialize (ctf_dict_t *fp)
if (fp->ctf_stypes > 0)
return (ctf_set_errno (fp, ECTF_RDONLY));
- /* The strtab refs table must be empty at this stage. Any refs already added
- will be corrupted by any modifications, including reserialization, after
- strtab finalization is complete. Only this function, and functions it
- calls, may add refs, and all memory locations (including in the dtds)
- containing strtab offsets must be traversed as part of serialization, and
- refs added. */
-
- if (!ctf_assert (fp, fp->ctf_str_num_refs == 0))
- return -1; /* errno is set for us. */
-
/* Fill in an initial CTF header. We will leave the label, object,
and function sections empty and only output a header, type section,
and string table. The type section begins at a 4-byte aligned
@@ -1103,12 +1089,6 @@ ctf_serialize (ctf_dict_t *fp)
assert (t == (unsigned char *) buf + sizeof (ctf_header_t) + hdr.cth_stroff);
- /* Every string added outside serialization by ctf_str_add_pending should
- now have been added by ctf_add_ref. */
- num_missed_str_refs = ctf_dynset_elements (fp->ctf_str_pending_ref);
- if (!ctf_assert (fp, num_missed_str_refs == 0))
- goto err; /* errno is set for us. */
-
/* Construct the final string table and fill out all the string refs with the
final offsets. Then purge the refs list, because we're about to move this
strtab onto the end of the buf, invalidating all the offsets. */
@@ -1211,10 +1191,8 @@ ctf_serialize (ctf_dict_t *fp)
ctf_str_free_atoms (nfp);
nfp->ctf_str_atoms = fp->ctf_str_atoms;
nfp->ctf_prov_strtab = fp->ctf_prov_strtab;
- nfp->ctf_str_pending_ref = fp->ctf_str_pending_ref;
fp->ctf_str_atoms = NULL;
fp->ctf_prov_strtab = NULL;
- fp->ctf_str_pending_ref = NULL;
memset (&fp->ctf_dtdefs, 0, sizeof (ctf_list_t));
memset (&fp->ctf_errs_warnings, 0, sizeof (ctf_list_t));
fp->ctf_add_processing = NULL;
diff --git a/libctf/ctf-string.c b/libctf/ctf-string.c
index a16cd3398b7..3ce2b254001 100644
--- a/libctf/ctf-string.c
+++ b/libctf/ctf-string.c
@@ -128,7 +128,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)
+ if (fp->ctf_str_atoms == NULL)
return -ENOMEM;
if (!fp->ctf_prov_strtab)
@@ -138,13 +138,6 @@ 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)
@@ -155,9 +148,6 @@ 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;
@@ -170,13 +160,8 @@ 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
@@ -184,7 +169,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 flags, uint32_t *ref)
+ int add_ref, int make_provisional, uint32_t *ref)
{
char *newstr = NULL;
ctf_str_atom_t *atom = NULL;
@@ -192,7 +177,7 @@ ctf_str_add_ref_internal (ctf_dict_t *fp, const char *str,
atom = ctf_dynhash_lookup (fp->ctf_str_atoms, str);
- if (flags & CTF_STR_ADD_REF)
+ if (add_ref)
{
if ((aref = malloc (sizeof (struct ctf_str_atom_ref))) == NULL) {
ctf_set_errno (fp, ENOMEM);
@@ -203,9 +188,8 @@ ctf_str_add_ref_internal (ctf_dict_t *fp, const char *str,
if (atom)
{
- if (flags & CTF_STR_ADD_REF)
+ if (add_ref)
{
- ctf_dynset_remove (fp->ctf_str_pending_ref, (void *) ref);
ctf_list_append (&atom->csa_refs, aref);
fp->ctf_str_num_refs++;
}
@@ -225,7 +209,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 (flags & CTF_STR_MAKE_PROVISIONAL)
+ if (make_provisional)
{
atom->csa_offset = fp->ctf_str_prov_offset;
@@ -236,14 +220,8 @@ ctf_str_add_ref_internal (ctf_dict_t *fp, const char *str,
fp->ctf_str_prov_offset += strlen (atom->csa_str) + 1;
}
- if (flags & CTF_STR_PENDING_REF)
+ if (add_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++;
}
@@ -272,7 +250,7 @@ ctf_str_add (ctf_dict_t *fp, const char *str)
if (!str)
str = "";
- atom = ctf_str_add_ref_internal (fp, str, CTF_STR_MAKE_PROVISIONAL, 0);
+ atom = ctf_str_add_ref_internal (fp, str, FALSE, TRUE, 0);
if (!atom)
return 0;
@@ -290,47 +268,13 @@ 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, CTF_STR_ADD_REF
- | CTF_STR_MAKE_PROVISIONAL, ref);
+ atom = ctf_str_add_ref_internal (fp, str, TRUE, TRUE, 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;
-
- return atom->csa_offset;
-}
-
-/* Note that a pending ref now located at NEW_REF has moved by BYTES bytes. */
-int
-ctf_str_move_pending (ctf_dict_t *fp, uint32_t *new_ref, ptrdiff_t bytes)
-{
- if (bytes == 0)
- return 0;
-
- if (ctf_dynset_insert (fp->ctf_str_pending_ref, (void *) new_ref) < 0)
- return (ctf_set_errno (fp, ENOMEM));
-
- ctf_dynset_remove (fp->ctf_str_pending_ref,
- (void *) ((signed char *) new_ref - bytes));
- return 0;
-}
-
/* Add an external strtab reference at OFFSET. Returns zero if the addition
failed, nonzero otherwise. */
int
@@ -341,7 +285,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, 0, 0);
+ atom = ctf_str_add_ref_internal (fp, str, FALSE, FALSE, 0);
if (!atom)
return 0;
@@ -391,8 +335,6 @@ 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
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2024-04-19 15:51 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-19 15:51 [binutils-gdb] Revert "libctf: do not corrupt strings across ctf_serialize" Nick Alcock
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).