Allow naming and configuring order of symbol finders

Currently, the bare-bones add_symbol_finder() interface only allows adding a symbol finder that is called before any existing finders. It'd be useful to be able to specify the order that symbol finders should be called in and to selectively enable and disable them. To do that, we also need finders to have a name to identify them by. So, replace add_symbol_finder() (which hasn't been in a release yet) with a set of interfaces providing this flexibility: register_symbol_finder(), set_enabled_symbol_finders(), registered_symbol_finders(), and enabled_symbol_finders(). In particular, this flexibility will be very useful for a plugin system: pre-installed plugins can register symbol finders that the user can choose to enable or disable. Type and object finders will also get this treatment, although add_type_finder() and add_object_finder() may need to stick around for backwards-compatibility. Signed-off-by: Omar Sandoval <[email protected]>
osandov · Apr 26, 2024 · 3dbf48a · 3dbf48a
1 parent aa49866
commit 3dbf48a
Show file tree

Hide file tree

Showing 9 changed files with 377 additions and 90 deletions.
diff --git a/_drgn.pyi b/_drgn.pyi
@@ -21,6 +21,7 @@ from typing import (
     Mapping,
     Optional,
     Sequence,
+    Set,
     Tuple,
     Union,
     overload,
@@ -460,33 +461,60 @@ class Program:
             return an :class:`Object` or ``None`` if not found.
         """
         ...
-    def add_symbol_finder(
-        self, fn: Callable[[Optional[str], Optional[int], bool], Sequence[Symbol]]
+    def register_symbol_finder(
+        self,
+        name: str,
+        fn: Callable[[Optional[str], Optional[int], bool], Sequence[Symbol]],
+        *,
+        enable_index: Optional[int] = None,
     ) -> None:
         """
         Register a callback for finding symbols in the program.
 
+        This does not enable the finder unless *enable_index* is given.
+
         The callback should take three arguments: a search name, a search
-        address, and a boolean flag 'one' indicating whether to return only
-        the single best match. When the 'one' flag is True, the callback should
+        address, and a boolean flag *one* indicating whether to return only the
+        single best match. When the *one* flag is ``True``, the callback should
         return a list containing at most one :class:`Symbol`. When the flag is
-        False, the callback should return a list of all matching
+        ``False``, the callback should return a list of all matching
         :class:`Symbol`\\ s. Both the name and address arguments are optional.
         If both are provided, then the result(s) should match both. If neither
         are provided, the finder should return all available symbols. If no
         result is found, the return should be an empty list.
 
-        Callbacks are called in reverse order of the order they were added
-        (i.e,, the most recently added callback is called first). When the
-        'one' flag is set, the search will short-circuit after the first
-        finder which returns a result, and subsequent finders will not be
-        called. Otherwise, all callbacks will be called, and all results will be
-        returned.
+        :param name: Finder name.
+        :param fn: Callable taking ``(name, address, one)`` and returning a
+            sequence of :class:`Symbol`\\ s.
+        :param enable_index: Insert the finder into the list of enabled finders
+            at the given index. If -1 or greater than the number of enabled
+            finders, insert it at the end. If ``None`` or not given, don't
+            enable the finder.
+        :raises ValueError: if there is already a finder with the given name
+        """
+        ...
+    def registered_symbol_finders(self) -> Set[str]:
+        """Return the names of all registered symbol finders."""
+        ...
+    def set_enabled_symbol_finders(self, names: Sequence[str]) -> None:
+        """
+        Set the list of enabled symbol finders.
 
-        :param fn: Callable taking name, address, and 'one' flag, and
-            returning a sequence of :class:`Symbol`\\ s.
+        Finders are called in the same order as the list. When the *one* flag
+        is set, the search will short-circuit after the first finder which
+        returns a result, and subsequent finders will not be called. Otherwise,
+        all callbacks will be called, and all results will be returned.
+
+        Finders that are not in the list are not called.
+
+        :param names: Names of finders to enable, in order.
+        :raises ValueError: if no finder has a given name or the same name is
+            given more than once
         """
         ...
+    def enabled_symbol_finders(self) -> List[str]:
+        """Return the names of enabled symbol finders, in order."""
+        ...
     def set_core_dump(self, path: Union[Path, int]) -> None:
         """
         Set the program to a core dump.

diff --git a/libdrgn/debug_info.c b/libdrgn/debug_info.c
@@ -2186,8 +2186,9 @@ void drgn_debug_info_init(struct drgn_debug_info *dbinfo,
 	drgn_program_add_object_finder_impl(prog, &dbinfo->object_finder,
 					    drgn_debug_info_find_object,
 					    dbinfo);
-	drgn_program_add_symbol_finder_impl(prog, &dbinfo->symbol_finder,
-					    elf_symbols_search, prog);
+	drgn_program_register_symbol_finder_impl(prog, &dbinfo->symbol_finder,
+						 "elf", elf_symbols_search,
+						 prog, 0);
 	drgn_module_table_init(&dbinfo->modules);
 	c_string_set_init(&dbinfo->module_names);
 	drgn_dwarf_info_init(dbinfo);

diff --git a/libdrgn/drgn.h b/libdrgn/drgn.h
@@ -580,6 +580,11 @@ drgn_program_add_memory_segment(struct drgn_program *prog, uint64_t address,
  */
 bool drgn_filename_matches(const char *haystack, const char *needle);
 
+enum {
+	DRGN_HANDLER_REGISTER_ENABLE_LAST = SIZE_MAX - 1,
+	DRGN_HANDLER_REGISTER_DONT_ENABLE = SIZE_MAX,
+};
+
 /**
  * Callback for finding a type.
  *
@@ -985,17 +990,57 @@ typedef struct drgn_error *
 /**
  * Register a symbol finding callback.
  *
- * Callbacks are called in reverse order that they were originally added. In
- * case of a search for multiple symbols, then the results of all callbacks are
- * concatenated. If the search is for a single symbol, then the first callback
- * which finds a symbol will short-circuit the search.
+ * @param[in] name Finder name. This is copied.
+ * @param[in] fn Callback.
+ * @param[in] arg Argument to pass to @p fn.
+ * @param[in] enable_index Insert the finder into the list of enabled finders at
+ * the given index. If @ref DRGN_HANDLER_REGISTER_ENABLE_LAST or greater than
+ * the number of enabled finders, insert it at the end. If @ref
+ * DRGN_HANDLER_REGISTER_DONT_ENABLE, don’t enable the finder.
+ */
+struct drgn_error *
+drgn_program_register_symbol_finder(struct drgn_program *prog, const char *name,
+				    drgn_symbol_find_fn fn, void *arg,
+				    size_t enable_index);
+
+/**
+ * Get the names of all registered symbol finders.
+ *
+ * The order of the names is arbitrary.
  *
- * @param[in] fn Symbol search function
- * @param[in] arg Argument to pass to the callback
+ * @param[out] names_ret Returned array of names.
+ * @param[out] count_ret Returned number of names in @p names_ret.
+ */
+struct drgn_error *
+drgn_program_registered_symbol_finders(struct drgn_program *prog,
+				       const char ***names_ret,
+				       size_t *count_ret);
+
+/**
+ * Set the list of enabled symbol finders.
+ *
+ * Finders are called in the same order as the list. In case of a search for
+ * multiple symbols, then the results of all callbacks are concatenated. If the
+ * search is for a single symbol, then the first callback which finds a symbol
+ * will short-circuit the search.
+ *
+ * @param[in] names Names of finders to enable, in order.
+ * @param[in] count Number of names in @p names.
+ */
+struct drgn_error *
+drgn_program_set_enabled_symbol_finders(struct drgn_program *prog,
+					const char * const *names,
+					size_t count);
+
+/**
+ * Get the names of enabled symbol finders, in order.
+ *
+ * @param[out] names_ret Returned array of names.
+ * @param[out] count_ret Returned number of names in @p names_ret.
  */
 struct drgn_error *
-drgn_program_add_symbol_finder(struct drgn_program *prog,
-			       drgn_symbol_find_fn fn, void *arg);
+drgn_program_enabled_symbol_finders(struct drgn_program *prog,
+				    const char ***names_ret, size_t *count_ret);
 
 /** Element type and size. */
 struct drgn_element_info {

diff --git a/libdrgn/program.c b/libdrgn/program.c
@@ -112,17 +112,6 @@ void drgn_program_init(struct drgn_program *prog,
 	drgn_object_init(&prog->vmemmap, prog);
 }
 
-static void drgn_program_deinit_symbol_finders(struct drgn_program *prog)
-{
-	struct drgn_symbol_finder *finder = prog->symbol_finders;
-	while (finder) {
-		struct drgn_symbol_finder *next = finder->next;
-		if (finder->free)
-			free(finder);
-		finder = next;
-	}
-}
-
 void drgn_program_deinit(struct drgn_program *prog)
 {
 	if (prog->core_dump_notes_cached) {
@@ -145,8 +134,8 @@ void drgn_program_deinit(struct drgn_program *prog)
 
 	drgn_object_deinit(&prog->vmemmap);
 
+	drgn_handler_list_deinit(&prog->symbol_finders);
 	drgn_object_index_deinit(&prog->oindex);
-	drgn_program_deinit_symbol_finders(prog);
 	drgn_program_deinit_types(prog);
 	drgn_memory_reader_deinit(&prog->reader);
 
@@ -1782,6 +1771,74 @@ drgn_program_find_object(struct drgn_program *prog, const char *name,
 				      ret);
 }
 
+struct drgn_error *
+drgn_program_register_symbol_finder_impl(struct drgn_program *prog,
+					 struct drgn_symbol_finder *finder,
+					 const char *name,
+					 drgn_symbol_find_fn fn, void *arg,
+					 size_t enable_index)
+{
+	struct drgn_error *err;
+	if (finder) {
+		finder->handler.name = name;
+		finder->handler.free = false;
+	} else {
+		finder = malloc(sizeof(*finder));
+		if (!finder)
+			return &drgn_enomem;
+		finder->handler.name = strdup(name);
+		if (!finder->handler.name) {
+			free(finder);
+			return &drgn_enomem;
+		}
+		finder->handler.free = true;
+	}
+	finder->fn = fn;
+	finder->arg = arg;
+	err = drgn_handler_list_register(&prog->symbol_finders,
+					 &finder->handler, enable_index,
+					 "symbol finder");
+	if (err && finder->handler.free) {
+		free((char *)finder->handler.name);
+		free(finder);
+	}
+	return err;
+}
+
+LIBDRGN_PUBLIC struct drgn_error *
+drgn_program_register_symbol_finder(struct drgn_program *prog, const char *name,
+				    drgn_symbol_find_fn fn, void *arg,
+				    size_t enable_index)
+{
+	return drgn_program_register_symbol_finder_impl(prog, NULL, name, fn,
+							arg, enable_index);
+}
+
+LIBDRGN_PUBLIC struct drgn_error *
+drgn_program_registered_symbol_finders(struct drgn_program *prog,
+				       const char ***names_ret,
+				       size_t *count_ret)
+{
+	return drgn_handler_list_registered(&prog->symbol_finders, names_ret,
+					    count_ret);
+}
+
+LIBDRGN_PUBLIC struct drgn_error *
+drgn_program_set_enabled_symbol_finders(struct drgn_program *prog,
+					const char * const *names, size_t count)
+{
+	return drgn_handler_list_set_enabled(&prog->symbol_finders, names,
+					     count, "symbol finder");
+}
+
+LIBDRGN_PUBLIC struct drgn_error *
+drgn_program_enabled_symbol_finders(struct drgn_program *prog,
+				    const char ***names_ret, size_t *count_ret)
+{
+	return drgn_handler_list_enabled(&prog->symbol_finders, names_ret,
+					 count_ret);
+}
+
 struct drgn_error *drgn_error_symbol_not_found(uint64_t address)
 {
 	return drgn_error_format(DRGN_ERROR_LOOKUP,
@@ -1795,46 +1852,17 @@ drgn_program_symbols_search(struct drgn_program *prog, const char *name,
 			    struct drgn_symbol_result_builder *builder)
 {
 	struct drgn_error *err = NULL;
-	struct drgn_symbol_finder *finder = prog->symbol_finders;
-	while (finder) {
+	drgn_handler_list_for_each_enabled(struct drgn_symbol_finder, finder,
+					   &prog->symbol_finders) {
 		err = finder->fn(name, addr, flags, finder->arg, builder);
 		if (err ||
 		    ((flags & DRGN_FIND_SYMBOL_ONE)
 		     && drgn_symbol_result_builder_count(builder) > 0))
 			break;
-		finder = finder->next;
 	}
 	return err;
 }
 
-struct drgn_error *
-drgn_program_add_symbol_finder_impl(struct drgn_program *prog,
-				    struct drgn_symbol_finder *finder,
-				    drgn_symbol_find_fn fn, void *arg)
-{
-	if (finder) {
-		finder->free = false;
-	} else {
-		finder = malloc(sizeof(*finder));
-		if (!finder)
-			return &drgn_enomem;
-		finder->free = true;
-	}
-	finder->fn = fn;
-	finder->arg = arg;
-	finder->next = prog->symbol_finders;
-	prog->symbol_finders = finder;
-	return NULL;
-}
-
-LIBDRGN_PUBLIC struct drgn_error *
-drgn_program_add_symbol_finder(struct drgn_program *prog,
-			       drgn_symbol_find_fn fn,
-			       void *arg)
-{
-	return drgn_program_add_symbol_finder_impl(prog, NULL, fn, arg);
-}
-
 LIBDRGN_PUBLIC struct drgn_error *
 drgn_program_find_symbols_by_name(struct drgn_program *prog, const char *name,
 				  struct drgn_symbol ***syms_ret,

diff --git a/libdrgn/program.h b/libdrgn/program.h
@@ -21,6 +21,7 @@
 
 #include "debug_info.h"
 #include "drgn.h"
+#include "handler.h"
 #include "hash_table.h"
 #include "language.h"
 #include "memory_reader.h"
@@ -110,7 +111,7 @@ struct drgn_program {
 	 */
 	struct drgn_object_index oindex;
 	struct drgn_debug_info dbinfo;
-	struct drgn_symbol_finder *symbol_finders;
+	struct drgn_handler_list symbol_finders;
 
 	/*
 	 * Program information.
@@ -379,9 +380,12 @@ drgn_program_find_symbol_by_address_internal(struct drgn_program *prog,
 					     struct drgn_symbol **ret);
 
 struct drgn_error *
-drgn_program_add_symbol_finder_impl(struct drgn_program *prog,
-				    struct drgn_symbol_finder *finder,
-				    drgn_symbol_find_fn fn, void *arg);
+drgn_program_register_symbol_finder_impl(struct drgn_program *prog,
+					 struct drgn_symbol_finder *finder,
+					 const char *name,
+					 drgn_symbol_find_fn fn, void *arg,
+					 size_t enable_index);
+
 /**
  * Call before a blocking (I/O or long-running) operation.
  *