[binutils-gdb] libctf: always name nameless types "", never NULL

Nick Alcock nix@sourceware.org
Thu Feb 4 16:02:56 GMT 2021


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ee87f50b8d2a0599675657a9fd2774c08261b29c

commit ee87f50b8d2a0599675657a9fd2774c08261b29c
Author: Nick Alcock <nick.alcock@oracle.com>
Date:   Fri Jan 29 13:33:11 2021 +0000

    libctf: always name nameless types "", never NULL
    
    The ctf_type_name_raw and ctf_type_aname_raw functions, which return the
    raw, unadorned name of CTF types, have one unfortunate wrinkle: they
    return NULL not only on error but when returning the name of types
    without a name in writable dicts.  This was unintended: it not only
    makes it impossible to reliably tell if a given call to
    ctf_type_name_raw failed (due to a bad string offset say), but also
    complicates all its callers, who now have to check for both NULL and "".
    
    The written-out form of CTF has no concept of a NULL pointer instead of
    a string: all null strings are strtab offset 0, "".  So the more we can
    do to remove this distinction from the writable form, the less complex
    the rest of our code needs to be.
    
    Armour against NULL in multiple places, arranging to return "" from
    ctf_type_name_raw if offset 0 is passed in, and removing a risky
    optimization from ctf_str_add* that avoided doing anything if a NULL was
    passed in: this added needless irregularity to the functions' API
    surface, since "" and NULL should be treated identically, and in the
    case of ctf_str_add_ref, we shouldn't skip adding the passed-in REF to
    the list of references to be updated no matter what the content of the
    string happens to be.
    
    This means we can simplify the deduplicator a tiny bit, also fixing a
    bug (latent when used by ld) where if the input dict was writable,
    we failed to realise when types were nameless and could end up creating
    deeply unhelpful synthetic forwards with no name, which we just banned
    a few commits ago, so the link failed.
    
    libctf/ChangeLog
    2021-01-27  Nick Alcock  <nick.alcock@oracle.com>
    
            * ctf-string.c (ctf_str_add): Treat adding a NULL as adding "".
            (ctf_str_add_ref): Likewise.
            (ctf_str_add_external): Likewise.
            * ctf-types.c (ctf_type_name_raw): Always return "" for offset 0.
            * ctf-dedup.c (ctf_dedup_multiple_input_dicts): Don't armour
            against NULL name.
            (ctf_dedup_maybe_synthesize_forward): Likewise.

Diff:
---
 libctf/ChangeLog    | 10 ++++++++++
 libctf/ctf-dedup.c  |  7 +++----
 libctf/ctf-string.c |  9 ++++++---
 libctf/ctf-types.c  |  8 +++++++-
 4 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/libctf/ChangeLog b/libctf/ChangeLog
index c700297abc6..35c22d9fa48 100644
--- a/libctf/ChangeLog
+++ b/libctf/ChangeLog
@@ -1,3 +1,13 @@
+2021-01-27  Nick Alcock  <nick.alcock@oracle.com>
+
+	* ctf-string.c (ctf_str_add): Treat adding a NULL as adding "".
+	(ctf_str_add_ref): Likewise.
+	(ctf_str_add_external): Likewise.
+	* ctf-types.c (ctf_type_name_raw): Always return "" for offset 0.
+	* ctf-dedup.c (ctf_dedup_multiple_input_dicts): Don't armour
+	against NULL name.
+	(ctf_dedup_maybe_synthesize_forward): Likewise.
+
 2021-01-27  Nick Alcock  <nick.alcock@oracle.com>
 
 	* ctf-create.c (ctf_serialize): Fix shadowing.
diff --git a/libctf/ctf-dedup.c b/libctf/ctf-dedup.c
index da88ae37147..001c2483a20 100644
--- a/libctf/ctf-dedup.c
+++ b/libctf/ctf-dedup.c
@@ -1776,7 +1776,7 @@ ctf_dedup_multiple_input_dicts (ctf_dict_t *output, ctf_dict_t **inputs,
   name = ctf_type_name_raw (input_fp, input_id);
 
   if ((fwdkind == CTF_K_STRUCT || fwdkind == CTF_K_UNION)
-      && name && name[0] != '\0')
+      && name[0] != '\0')
     {
       const void *origin;
 
@@ -2375,20 +2375,19 @@ ctf_dedup_maybe_synthesize_forward (ctf_dict_t *output, ctf_dict_t *target,
   ctf_dedup_t *td = &target->ctf_dedup;
   int kind;
   int fwdkind;
-  const char *name;
+  const char *name = ctf_type_name_raw (input, id);
   const char *decorated;
   void *v;
   ctf_id_t emitted_forward;
 
   if (!ctf_dynset_exists (od->cd_conflicting_types, hval, NULL)
       || target->ctf_flags & LCTF_CHILD
-      || !ctf_type_name_raw (input, id)
+      || name[0] == '\0'
       || (((kind = ctf_type_kind_unsliced (input, id)) != CTF_K_STRUCT
 	   && kind != CTF_K_UNION && kind != CTF_K_FORWARD)))
     return 0;
 
   fwdkind = ctf_type_kind_forwarded (input, id);
-  name = ctf_type_name_raw (input, id);
 
   ctf_dprintf ("Using synthetic forward for conflicted struct/union with "
 	       "hval %s\n", hval);
diff --git a/libctf/ctf-string.c b/libctf/ctf-string.c
index cb36f9166bc..91ad2e36db7 100644
--- a/libctf/ctf-string.c
+++ b/libctf/ctf-string.c
@@ -218,8 +218,9 @@ uint32_t
 ctf_str_add (ctf_dict_t *fp, const char *str)
 {
   ctf_str_atom_t *atom;
+
   if (!str)
-    return 0;
+    str = "";
 
   atom = ctf_str_add_ref_internal (fp, str, FALSE, TRUE, 0);
   if (!atom)
@@ -235,8 +236,9 @@ uint32_t
 ctf_str_add_ref (ctf_dict_t *fp, const char *str, uint32_t *ref)
 {
   ctf_str_atom_t *atom;
+
   if (!str)
-    return 0;
+    str = "";
 
   atom = ctf_str_add_ref_internal (fp, str, TRUE, TRUE, ref);
   if (!atom)
@@ -251,8 +253,9 @@ int
 ctf_str_add_external (ctf_dict_t *fp, const char *str, uint32_t offset)
 {
   ctf_str_atom_t *atom;
+
   if (!str)
-    return 0;
+    str = "";
 
   atom = ctf_str_add_ref_internal (fp, str, FALSE, FALSE, 0);
   if (!atom)
diff --git a/libctf/ctf-types.c b/libctf/ctf-types.c
index 4129fbc7b83..57a284d82e7 100644
--- a/libctf/ctf-types.c
+++ b/libctf/ctf-types.c
@@ -922,7 +922,10 @@ ctf_type_name (ctf_dict_t *fp, ctf_id_t type, char *buf, size_t len)
 }
 
 /* Lookup the given type ID and return its raw, unadorned, undecorated name.
-   The name will live as long as its ctf_dict_t does.  */
+   The name will live as long as its ctf_dict_t does.
+
+   The only decoration is that a NULL return always means an error: nameless
+   types return a null string.  */
 
 const char *
 ctf_type_name_raw (ctf_dict_t *fp, ctf_id_t type)
@@ -932,6 +935,9 @@ ctf_type_name_raw (ctf_dict_t *fp, ctf_id_t type)
   if ((tp = ctf_lookup_by_id (&fp, type)) == NULL)
     return NULL;		/* errno is set for us.  */
 
+  if (tp->ctt_name == 0)
+    return "";
+
   return ctf_strraw (fp, tp->ctt_name);
 }


More information about the Gdb-cvs mailing list