We now use known prototypes for all aliased symbols (same address)
authorDima Kogan <dima@secretsauce.net>
Mon, 2 Jun 2014 09:01:57 +0000 (02:01 -0700)
committerChanho Park <chanho61.park@samsung.com>
Fri, 22 Aug 2014 11:38:26 +0000 (20:38 +0900)
Some libraries have multiple names for the same function. Prior to this patch,
it was possible to define a prototype for a symbol, and not have ltrace use it
because it saw a different symbol be called. libc is a common source of this.
For instance (on my amd64 Debian box) it defines the nanosleep symbol as both
'nanosleep' and '__GI___nanosleep', at the same address. If a calling library
calls '__GI___nanosleep', then an ltrace prototype for 'nanosleep' would not be
used, even though it should apply to this call

dwarf_prototypes.c
library.c
library.h
ltrace-elf.c
output.c
proc.c

index 6094658..b50c82e 100644 (file)
@@ -910,12 +910,7 @@ static bool import_subprogram(struct protolib *plib, struct library *lib,
 {
        // I use the linkage function name if there is one, otherwise the
        // plain name
-       const char *function_name = NULL;
-       Dwarf_Attribute attr;
-       if (dwarf_attr(die, DW_AT_linkage_name, &attr) != NULL)
-               function_name = dwarf_formstring(&attr);
-       if (function_name == NULL)
-               function_name = dwarf_diename(die);
+       const char *function_name = dwarf_diename(die);
        if (function_name == NULL) {
                complain(die, "Function has no name. Not importing");
                return true;
index 3a22519..7ccd7da 100644 (file)
--- a/library.c
+++ b/library.c
 #include "callback.h"
 #include "debug.h"
 #include "dict.h"
+#include "vect.h"
 #include "backend.h" // for arch_library_symbol_init, arch_library_init
 
+static void
+library_exported_names_init(struct library_exported_names *names);
+
 #ifndef OS_HAVE_LIBRARY_DATA
 int
 os_library_init(struct library *lib)
@@ -292,7 +296,7 @@ private_library_init(struct library *lib, enum library_type type)
        lib->own_pathname = 0;
 
        lib->symbols = NULL;
-       lib->exported_names = NULL;
+       library_exported_names_init(&lib->exported_names);
        lib->type = type;
 
 #if defined(HAVE_LIBDW)
@@ -316,18 +320,213 @@ library_init(struct library *lib, enum library_type type)
        return 0;
 }
 
-static int
-library_exported_name_clone(struct library_exported_name *retp,
-                           struct library_exported_name *exnm)
+
+
+
+
+static void _dtor_string(const char **tgt, void *data)
+{
+       free((char*)*tgt);
+}
+static int _clone_vect(struct vect **to, const struct vect **from, void *data)
+{
+       *to = malloc(sizeof(struct vect));
+       if(*to == NULL)
+               return -1;
+
+       return
+               VECT_CLONE(*to, *from, const char*,
+                          dict_clone_string,
+                          _dtor_string,
+                          NULL);
+}
+static void _dtor_vect(const struct vect **tgt, void *data)
+{
+       VECT_DESTROY(*tgt, const char*, _dtor_string, NULL);
+       free(*tgt);
+}
+
+static void
+library_exported_names_init(struct library_exported_names *names)
+{
+       DICT_INIT(&names->names,
+                 const char*, uint64_t,
+                 dict_hash_string, dict_eq_string, NULL);
+       DICT_INIT(&names->addrs,
+                 uint64_t, struct vect*,
+                 dict_hash_uint64, dict_eq_uint64, NULL);
+}
+
+static void
+library_exported_names_destroy(struct library_exported_names *names)
 {
-       char *name = exnm->own_name ? strdup(exnm->name) : (char *)exnm->name;
-       if (name == NULL)
+       DICT_DESTROY(&names->names,
+                    const char*, uint64_t,
+                    _dtor_string, NULL, NULL);
+       DICT_DESTROY(&names->addrs,
+                    uint64_t, struct vect*,
+                    NULL, _dtor_vect, NULL);
+}
+
+static int
+library_exported_names_clone(struct library_exported_names *retp,
+                            const struct library_exported_names *names)
+{
+       return
+               DICT_CLONE(&retp->names, &names->names,
+                          const char*, uint64_t,
+                          dict_clone_string, _dtor_string,
+                          NULL, NULL,
+                          NULL) ||
+               DICT_CLONE(&retp->addrs, &names->addrs,
+                          uint64_t, struct vect*,
+                          NULL, NULL,
+                          _clone_vect, _dtor_vect,
+                          NULL);
+}
+
+int library_exported_names_push(struct library_exported_names *names,
+                               uint64_t addr, const char *name,
+                               int own_name )
+{
+       // first, take ownership of the name, if it's not yet ours
+       if(!own_name)
+               name = strdup(name);
+       if(name == NULL)
                return -1;
-       retp->name = name;
-       retp->own_name = exnm->own_name;
+
+       // push to the name->addr map
+       int result = DICT_INSERT(&names->names, &name, &addr);
+       if(result == 1) {
+               // This symbol is already present in the table. This library has
+               // multiple copies of this symbol (probably corresponding to
+               // different symbol versions). I should handle this gracefully
+               // at some point, but for now I simply ignore later instances of
+               // any particular symbol
+               free(name);
+               return 0;
+       }
+
+       if(result != 0)
+               return result;
+
+       // push to the addr->names map
+       // I get the alias vector. If it doesn't yet exist, I make it
+       struct vect *aliases;
+       struct vect **paliases = DICT_FIND_REF(&names->addrs,
+                                              &addr, struct vect*);
+
+       if (paliases == NULL) {
+               aliases = malloc(sizeof(struct vect));
+               if(aliases == NULL)
+                       return -1;
+               VECT_INIT(aliases, const char*);
+               result = DICT_INSERT(&names->addrs, &addr, &aliases);
+               if(result != 0)
+                       return result;
+       }
+       else
+               aliases = *paliases;
+
+       const char *namedup = strdup(name);
+       if(namedup == NULL)
+               return -1;
+
+       result = vect_pushback(aliases, &namedup);
+       if(result != 0)
+               return result;
+
        return 0;
 }
 
+struct library_exported_names_each_context
+{
+       enum callback_status (*inner_cb)(const char *, void *);
+       void *data;
+       bool failure : 1;
+};
+static enum callback_status
+library_exported_names_each_cb(const char **key, uint64_t *value, void *data)
+{
+       struct library_exported_names_each_context *context =
+               (struct library_exported_names_each_context*)data;
+       enum callback_status status = context->inner_cb(*key, context->data);
+       if(status == CBS_FAIL)
+               context->failure = true;
+       return status;
+}
+bool library_exported_names_each(const struct library_exported_names *names,
+                                enum callback_status (*cb)(const char *,
+                                                           void *),
+                                void *data)
+{
+       struct library_exported_names_each_context context =
+               {.inner_cb = cb,
+                .data = data,
+                .failure = false};
+       DICT_EACH(&names->names,
+                 const char*, uint64_t,
+                 NULL, library_exported_names_each_cb, &context);
+       return !context.failure;
+}
+
+struct library_exported_names_each_alias_context
+{
+       enum callback_status (*inner_cb)(const char *, void *);
+       const char *origname;
+       void *data;
+       bool failure : 1;
+};
+static enum callback_status
+library_exported_names_each_alias_cb(const char **name, void *data)
+{
+       struct library_exported_names_each_alias_context *context =
+               (struct library_exported_names_each_alias_context*)data;
+
+       // I do not report the original name we were asked about. Otherwise, any
+       // time the caller asks for aliases of symbol "sym", I'll always report
+       // "sym" along with any actual aliases
+       if(strcmp(*name, context->origname) == 0)
+               return CBS_CONT;
+
+       enum callback_status status = context->inner_cb(*name, context->data);
+       if(status == CBS_FAIL)
+               context->failure = true;
+       return status;
+}
+
+bool library_exported_names_each_alias(
+       const struct library_exported_names *names,
+       const char *aliasname,
+       enum callback_status (*cb)(const char *,
+                                  void *),
+       void *data)
+{
+       // I have a symbol name. I look up its address, then get the list of
+       // aliased names
+       uint64_t *addr = DICT_FIND_REF(&names->names,
+                                      &aliasname, uint64_t);
+       if(addr == NULL)
+               return false;
+
+       // OK. I have an address. Get the list of symbols at this address
+       struct vect **aliases = DICT_FIND_REF(&names->addrs,
+                                            addr, struct vect*);
+       if(aliases == NULL)
+               return false;
+
+       struct library_exported_names_each_alias_context context =
+               {.inner_cb = cb,
+                .origname = aliasname,
+                .data = data,
+                .failure = false};
+       VECT_EACH(*aliases, const char*, NULL,
+                 library_exported_names_each_alias_cb, &context);
+}
+
+
+
+
 int
 library_clone(struct library *retp, struct library *lib)
 {
@@ -372,20 +571,9 @@ library_clone(struct library *retp, struct library *lib)
        }
 
        /* Clone exported names.  */
-       {
-               struct library_exported_name *it;
-               struct library_exported_name **nnamep = &retp->exported_names;
-               for (it = lib->exported_names; it != NULL; it = it->next) {
-                       *nnamep = malloc(sizeof(**nnamep));
-                       if (*nnamep == NULL
-                           || library_exported_name_clone(*nnamep, it) < 0) {
-                               free(*nnamep);
-                               goto fail;
-                       }
-                       nnamep = &(*nnamep)->next;
-               }
-               *nnamep = NULL;
-       }
+       if (library_exported_names_clone(&retp->exported_names,
+                                        &lib->exported_names) != 0)
+               goto fail;
 
        if (os_library_clone(retp, lib) < 0)
                goto fail;
@@ -419,14 +607,7 @@ library_destroy(struct library *lib)
        }
 
        /* Release exported names.  */
-       struct library_exported_name *it;
-       for (it = lib->exported_names; it != NULL; ) {
-               struct library_exported_name *next = it->next;
-               if (it->own_name)
-                       free((char *)it->name);
-               free(it);
-               it = next;
-       }
+       library_exported_names_destroy(&lib->exported_names);
 }
 
 void
index 4d649e0..f8a5cf8 100644 (file)
--- a/library.h
+++ b/library.h
 #define _LIBRARY_H_
 
 #include <stdint.h>
+#include <stdbool.h>
 
 #if defined(HAVE_LIBDW)
 # include <elfutils/libdwfl.h>
 #endif
 
+#include "dict.h"
 #include "callback.h"
 #include "forward.h"
 #include "sysdep.h"
@@ -41,11 +43,21 @@ enum toplt {
 size_t arch_addr_hash(const arch_addr_t *addr);
 int arch_addr_eq(const arch_addr_t *addr1, const arch_addr_t *addr2);
 
-/* For handling -l.  */
-struct library_exported_name {
-       struct library_exported_name *next;
-       const char *name;
-       int own_name : 1;
+
+/* For handling -l and for handling library export aliases (different symbol
+ * name, same address)
+ *
+ * This structure needs to
+ * - store (addr, name) tuples
+ * - be searchable by addr (when populating)
+ * - be searchable by name (when looking for aliases)
+ * - be enumeratable (by activate_latent_in())
+ */
+struct library_exported_names {
+       // I store the data in several structures to facilitate different types
+       // of access
+       struct dict names; // maps a name to an address
+       struct dict addrs; // maps an address to a vect of names
 };
 
 struct library_symbol {
@@ -158,8 +170,10 @@ struct library {
        /* List of names that this library implements, and that match
         * -l filter.  Each time a new library is mapped, its list of
         * exports is examined, and corresponding PLT slots are
-        * enabled.  */
-       struct library_exported_name *exported_names;
+        * enabled. This data structure also keeps track of export
+        * addresses to find symbols with different names, but same
+        * addresses */
+       struct library_exported_names exported_names;
 
        /* Prototype library associated with this library.  */
        struct protolib *protolib;
@@ -241,4 +255,34 @@ int arch_translate_address(struct ltelf *lte,
 int arch_translate_address_dyn(struct process *proc,
                               arch_addr_t addr, arch_addr_t *ret);
 
+
+/* Pushes a name/address tuple to the list of a library's exports. Returns 0 on
+ * success
+ */
+int library_exported_names_push(struct library_exported_names *names,
+                               uint64_t addr, const char *name,
+                               int own_name );
+
+/* Iterates through the a library's export list. The callback is called for
+ * every symbol a library exports. Symbol aliases do not apply here. If multiple
+ * symbols are defined at the same address, each is reported here. Returns true
+ * on success. If the callback fails at any point, this returns false
+ */
+bool library_exported_names_each(const struct library_exported_names *names,
+                                enum callback_status (*cb)(const char *,
+                                                           void *),
+                                void *data);
+
+/* Iterates through the a library's export list, reporting each symbol that is
+ * an alias of the given 'aliasname' symbol. This 'aliasname' symbol itself is
+ * NOT reported, so if this symbol is unique, the callback is not called at all.
+ * Returns true on success
+ */
+bool library_exported_names_each_alias(
+       const struct library_exported_names *names,
+       const char *aliasname,
+       enum callback_status (*cb)(const char *,
+                                  void *),
+       void *data);
+
 #endif /* _LIBRARY_H_ */
index f638342..a6a5531 100644 (file)
@@ -901,13 +901,8 @@ static int
 populate_this_symtab(struct process *proc, const char *filename,
                     struct ltelf *lte, struct library *lib,
                     Elf_Data *symtab, const char *strtab, size_t count,
-                    struct library_exported_name **names)
+                    struct library_exported_name*names)
 {
-       /* If a valid NAMES is passed, we pass in *NAMES a list of
-        * symbol names that this library exports.  */
-       if (names != NULL)
-               *names = NULL;
-
        /* Using sorted array would be arguably better, but this
         * should be well enough for the number of symbols that we
         * typically deal with.  */
@@ -957,20 +952,14 @@ populate_this_symtab(struct process *proc, const char *filename,
 
                /* If we are interested in exports, store this name.  */
                if (names != NULL) {
-                       struct library_exported_name *export
-                               = malloc(sizeof *export);
                        char *name_copy = strdup(name);
-
-                       if (name_copy == NULL || export == NULL) {
-                               free(name_copy);
-                               free(export);
+                       if (name_copy == NULL ||
+                           library_exported_names_push(names,
+                                                       sym.st_value,
+                                                       name_copy, 1) != 0)
+                       {
                                fprintf(stderr, "Couldn't store symbol %s.  "
                                        "Tracing may be incomplete.\n", name);
-                       } else {
-                               export->name = name_copy;
-                               export->own_name = 1;
-                               export->next = *names;
-                               *names = export;
                        }
                }
 
@@ -1115,7 +1104,7 @@ populate_symtab(struct process *proc, const char *filename,
 
        /* Check whether we want to trace symbols implemented by this
         * library (-l).  */
-       struct library_exported_name **names = NULL;
+       struct library_exported_name*names = NULL;
        if (exports) {
                debug(DEBUG_FUNCTION, "-l matches %s", lib->soname);
                names = &lib->exported_names;
index 10faee2..2128816 100644 (file)
--- a/output.c
+++ b/output.c
@@ -197,6 +197,28 @@ snip_period(char *buf)
        }
 }
 
+struct lookup_prototype_alias_context
+{
+       struct library *lib;
+       struct prototype *result; // output
+};
+static enum callback_status
+lookup_prototype_alias_cb(const char *name, void *data)
+{
+       struct lookup_prototype_alias_context *context =
+               (struct lookup_prototype_alias_context*)data;
+
+       struct library *lib = context->lib;
+
+       context->result =
+               protolib_lookup_prototype(lib->protolib, name,
+                                         lib->type != LT_LIBTYPE_SYSCALL);
+       if (context->result != NULL)
+               return CBS_STOP;
+
+       return CBS_CONT;
+}
+
 static struct prototype *
 library_get_prototype(struct library *lib, const char *name)
 {
@@ -235,8 +257,21 @@ library_get_prototype(struct library *lib, const char *name)
        if (lib->protolib == NULL)
                return NULL;
 
-       return protolib_lookup_prototype(lib->protolib, name,
-                                        lib->type != LT_LIBTYPE_SYSCALL);
+       struct prototype *result =
+               protolib_lookup_prototype(lib->protolib, name,
+                                         lib->type != LT_LIBTYPE_SYSCALL);
+       if (result != NULL)
+               return result;
+
+       // prototype not found. Is it aliased?
+       struct lookup_prototype_alias_context context = {.lib = lib,
+                                                        .result = NULL};
+       library_exported_names_each_alias(&lib->exported_names, name,
+                                         lookup_prototype_alias_cb,
+                                         &context);
+
+       // if found, the prototype is stored here, otherwise it's NULL
+       return context.result;
 }
 
 struct find_proto_data {
diff --git a/proc.c b/proc.c
index 5385510..6d6562f 100644 (file)
--- a/proc.c
+++ b/proc.c
@@ -867,23 +867,49 @@ proc_activate_delayed_symbol(struct process *proc,
        return breakpoint_for_symbol(libsym, proc);
 }
 
+
+struct activate_latent_in_context
+{
+       struct process *proc;
+       struct library *lib;
+};
 static enum callback_status
-activate_latent_in(struct process *proc, struct library *lib, void *data)
+activate_latent_in_cb(const char *name, void *data)
 {
-       struct library_exported_name *exported;
-       for (exported = data; exported != NULL; exported = exported->next) {
-               struct library_symbol *libsym = NULL;
-               while ((libsym = library_each_symbol(lib, libsym,
-                                                    library_symbol_named_cb,
-                                                    (void *)exported->name))
-                      != NULL)
-                       if (libsym->latent
-                           && proc_activate_latent_symbol(proc, libsym) < 0)
-                               return CBS_FAIL;
-       }
+       const struct activate_latent_in_context *ctx =
+               (const struct activate_latent_in_context*)data;
+
+       struct process *proc = ctx->proc;
+       struct library *lib  = ctx->lib;
+
+       struct library_symbol *libsym = NULL;
+       while ((libsym = library_each_symbol(lib, libsym,
+                                            library_symbol_named_cb,
+                                            (void *)name))
+              != NULL)
+               if (libsym->latent
+                   && proc_activate_latent_symbol(proc, libsym) < 0)
+                       return CBS_FAIL;
+
        return CBS_CONT;
 }
 
+static enum callback_status
+activate_latent_in(struct process *proc, struct library *lib, void *data)
+{
+       struct library_exported_names *exported_names =
+               (struct library_exported_names*)data;
+
+       struct activate_latent_in_context context = {.proc = proc,
+                                                    .lib = lib};
+       if (library_exported_names_each(exported_names,
+                                       activate_latent_in_cb,
+                                       &context))
+               return CBS_CONT;
+       else
+               return CBS_FAIL;
+}
+
 void
 proc_add_library(struct process *proc, struct library *lib)
 {
@@ -969,7 +995,7 @@ proc_add_library(struct process *proc, struct library *lib)
         * library itself).  */
        struct library *lib2 = NULL;
        while ((lib2 = proc_each_library(proc, lib2, activate_latent_in,
-                                        lib->exported_names)) != NULL)
+                                        &lib->exported_names)) != NULL)
                fprintf(stderr,
                        "Couldn't activate latent symbols for %s in %d: %s.\n",
                        lib2->soname, proc->pid, strerror(errno));