From 2a65bf30afb6eec8e718de5bd74f707915759ced Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Wed, 14 Jun 2023 12:08:23 -0700 Subject: [PATCH] libsysprof-analyze: add symbol kind property for symbols Additionally, add the pid as the binary nick for processes so that we can show them in the callgraph with the process name. --- .../sysprof-bundled-symbolizer.c | 25 ++++++++--- .../sysprof-callgraph-frame.c | 6 +-- src/libsysprof-analyze/sysprof-callgraph.c | 19 ++++---- .../sysprof-document-symbols.c | 9 ++-- src/libsysprof-analyze/sysprof-document.c | 10 ++--- .../sysprof-elf-symbolizer.c | 6 ++- .../sysprof-jitmap-symbolizer.c | 3 +- .../sysprof-kallsyms-symbolizer.c | 6 ++- src/libsysprof-analyze/sysprof-process-info.c | 11 +++-- .../sysprof-symbol-private.h | 28 ++++++------ src/libsysprof-analyze/sysprof-symbol.c | 43 ++++++++++++++++--- src/libsysprof-analyze/sysprof-symbol.h | 27 +++++++++--- .../tests/test-symbol-cache.c | 2 +- 13 files changed, 134 insertions(+), 61 deletions(-) diff --git a/src/libsysprof-analyze/sysprof-bundled-symbolizer.c b/src/libsysprof-analyze/sysprof-bundled-symbolizer.c index b7052056..5893d32b 100644 --- a/src/libsysprof-analyze/sysprof-bundled-symbolizer.c +++ b/src/libsysprof-analyze/sysprof-bundled-symbolizer.c @@ -208,11 +208,26 @@ sysprof_bundled_symbolizer_symbolize (SysprofSymbolizer *symbolizer, } if (ret->offset < (self->endptr - self->beginptr)) - return _sysprof_symbol_new (sysprof_strings_get (strings, &self->beginptr[ret->offset]), - NULL, - g_steal_pointer (&tag), - ret->addr_begin, - ret->addr_end); + { + const char *name = &self->beginptr[ret->offset]; + SysprofSymbolKind kind; + + if (g_str_has_prefix (name, "- -")) + kind = SYSPROF_SYMBOL_KIND_CONTEXT_SWITCH; + else if (context == SYSPROF_ADDRESS_CONTEXT_KERNEL) + kind = SYSPROF_SYMBOL_KIND_KERNEL; + else if (name[0] == '[') + kind = SYSPROF_SYMBOL_KIND_PROCESS; + else + kind = SYSPROF_SYMBOL_KIND_USER; + + return _sysprof_symbol_new (sysprof_strings_get (strings, name), + NULL, + g_steal_pointer (&tag), + ret->addr_begin, + ret->addr_end, + kind); + } return NULL; } diff --git a/src/libsysprof-analyze/sysprof-callgraph-frame.c b/src/libsysprof-analyze/sysprof-callgraph-frame.c index ffb0572e..ed37dde3 100644 --- a/src/libsysprof-analyze/sysprof-callgraph-frame.c +++ b/src/libsysprof-analyze/sysprof-callgraph-frame.c @@ -402,10 +402,8 @@ sysprof_callgraph_frame_list_traceables_async (SysprofCallgraphFrame *self, SysprofCallgraphSummary *summary = node->summary; SysprofSymbol *symbol = summary->symbol; - if (symbol->is_context_switch || - symbol->is_everything || - symbol->is_untraceable || - symbol->is_process) + if (symbol->kind != SYSPROF_SYMBOL_KIND_USER && + symbol->kind != SYSPROF_SYMBOL_KIND_KERNEL) continue; if (bitset == NULL) diff --git a/src/libsysprof-analyze/sysprof-callgraph.c b/src/libsysprof-analyze/sysprof-callgraph.c index caf0f08f..114362a4 100644 --- a/src/libsysprof-analyze/sysprof-callgraph.c +++ b/src/libsysprof-analyze/sysprof-callgraph.c @@ -168,11 +168,12 @@ sysprof_callgraph_class_init (SysprofCallgraphClass *klass) object_class->dispose = sysprof_callgraph_dispose; object_class->finalize = sysprof_callgraph_finalize; - everything = _sysprof_symbol_new (g_ref_string_new_intern ("[Everything]"), NULL, NULL, 0, 0); - everything->is_everything = TRUE; - - untraceable = _sysprof_symbol_new (g_ref_string_new_intern ("[Unwindable]"), NULL, NULL, 0, 0); - everything->is_untraceable = TRUE; + everything = _sysprof_symbol_new (g_ref_string_new_intern ("All Processes"), + NULL, NULL, 0, 0, + SYSPROF_SYMBOL_KIND_ROOT); + untraceable = _sysprof_symbol_new (g_ref_string_new_intern ("Unwindable"), + NULL, NULL, 0, 0, + SYSPROF_SYMBOL_KIND_UNWINDABLE); } static void @@ -197,9 +198,11 @@ sysprof_callgraph_populate_callers (SysprofCallgraph *self, if (iter->parent != NULL) { SysprofSymbol *parent_symbol = iter->parent->summary->symbol; + SysprofSymbolKind parent_kind = sysprof_symbol_get_kind (parent_symbol); guint pos; - if (!(parent_symbol->is_process || parent_symbol->is_everything) && + if (parent_kind != SYSPROF_SYMBOL_KIND_PROCESS && + parent_kind != SYSPROF_SYMBOL_KIND_ROOT && !g_ptr_array_find (iter->summary->callers, parent_symbol, &pos)) g_ptr_array_add (iter->summary->callers, parent_symbol); } @@ -308,12 +311,12 @@ sysprof_callgraph_add_traceable (SysprofCallgraph *self, * that it was a corrupted unwind when recording. */ if (n_symbols == 1 && - symbols[0]->is_context_switch && + _sysprof_symbol_is_context_switch (symbols[0]) && final_context == SYSPROF_ADDRESS_CONTEXT_USER) symbols[0] = untraceable; /* We saved 3 extra spaces for these above so that we can - * tack on the "Process" symbol and the "Everything" symbol. + * tack on the "Process" symbol and the "All Processes" symbol. * If the final address context places us in Kernel, we want * to add a "- - Kernel - -" symbol to ensure that we are * accounting cost to the kernel for the process. diff --git a/src/libsysprof-analyze/sysprof-document-symbols.c b/src/libsysprof-analyze/sysprof-document-symbols.c index b18d58e5..057dc789 100644 --- a/src/libsysprof-analyze/sysprof-document-symbols.c +++ b/src/libsysprof-analyze/sysprof-document-symbols.c @@ -148,6 +148,7 @@ sysprof_document_symbols_worker (GTask *task, { "- - Guest Kernel - -", SYSPROF_ADDRESS_CONTEXT_GUEST_KERNEL }, { "- - Guest User - -", SYSPROF_ADDRESS_CONTEXT_GUEST_USER }, }; + g_autoptr(GRefString) context_switch = g_ref_string_new_intern ("Context Switch"); Symbolize *state = task_data; EggBitsetIter iter; EggBitset *bitset; @@ -168,9 +169,11 @@ sysprof_document_symbols_worker (GTask *task, /* Create static symbols for context switch use */ for (guint cs = 0; cs < G_N_ELEMENTS (context_switches); cs++) { - g_autoptr(SysprofSymbol) symbol = _sysprof_symbol_new (g_ref_string_new_intern (context_switches[cs].name), NULL, NULL, 0, 0); - - symbol->is_context_switch = TRUE; + g_autoptr(SysprofSymbol) symbol = _sysprof_symbol_new (g_ref_string_new_intern (context_switches[cs].name), + NULL, + g_ref_string_acquire (context_switch), + 0, 0, + SYSPROF_SYMBOL_KIND_CONTEXT_SWITCH); /* TODO: It would be nice if we had enough insight from the capture header * as to the host system, so we can show "vmlinuz" and "Linux" respectively diff --git a/src/libsysprof-analyze/sysprof-document.c b/src/libsysprof-analyze/sysprof-document.c index 6903d337..4075e4a2 100644 --- a/src/libsysprof-analyze/sysprof-document.c +++ b/src/libsysprof-analyze/sysprof-document.c @@ -486,13 +486,13 @@ sysprof_document_load_processes (SysprofDocument *self) if ((parts = g_strsplit (cmdline , " ", 2))) { - g_autofree char *wrapped = g_strdup_printf ("[%s]", parts[0]); + GRefString *nick = g_ref_string_acquire (process_info->fallback_symbol->binary_nick); g_clear_object (&process_info->symbol); process_info->symbol = - _sysprof_symbol_new (sysprof_strings_get (self->strings, wrapped), - NULL, NULL, 0, 0); - process_info->symbol->is_process = TRUE; + _sysprof_symbol_new (sysprof_strings_get (self->strings, parts[0]), + NULL, g_steal_pointer (&nick), 0, 0, + SYSPROF_SYMBOL_KIND_PROCESS); } } } @@ -1334,7 +1334,7 @@ sysprof_document_list_symbols_in_traceable (SysprofDocument *self, symbols = g_alloca (sizeof (SysprofSymbol *) * stack_depth); n_symbols = sysprof_document_symbolize_traceable (self, traceable, symbols, stack_depth, &final_context); - if (n_symbols > 0 && symbols[0]->is_context_switch) + if (n_symbols > 0 && _sysprof_symbol_is_context_switch (symbols[0])) { symbols++; n_symbols--; diff --git a/src/libsysprof-analyze/sysprof-elf-symbolizer.c b/src/libsysprof-analyze/sysprof-elf-symbolizer.c index dfd69e72..9fe85171 100644 --- a/src/libsysprof-analyze/sysprof-elf-symbolizer.c +++ b/src/libsysprof-analyze/sysprof-elf-symbolizer.c @@ -134,7 +134,8 @@ sysprof_elf_symbolizer_symbolize (SysprofSymbolizer *symbolizer, sysprof_strings_get (strings, path), sysprof_strings_get (strings, sysprof_elf_get_nick (elf)), map_begin + (begin_address - file_offset), - map_begin + (end_address - file_offset)); + map_begin + (end_address - file_offset), + SYSPROF_SYMBOL_KIND_USER); fallback: /* Fallback, we failed to locate the symbol within a file we can @@ -148,7 +149,8 @@ fallback: end_address = address + 1; return _sysprof_symbol_new (sysprof_strings_get (strings, name), - NULL, NULL, begin_address, end_address); + NULL, NULL, begin_address, end_address, + SYSPROF_SYMBOL_KIND_USER); } static void diff --git a/src/libsysprof-analyze/sysprof-jitmap-symbolizer.c b/src/libsysprof-analyze/sysprof-jitmap-symbolizer.c index 3e08d870..ff866274 100644 --- a/src/libsysprof-analyze/sysprof-jitmap-symbolizer.c +++ b/src/libsysprof-analyze/sysprof-jitmap-symbolizer.c @@ -210,7 +210,8 @@ create_symbol: NULL, NULL, match->address, - match->address + 1); + match->address + 1, + SYSPROF_SYMBOL_KIND_USER); } static void diff --git a/src/libsysprof-analyze/sysprof-kallsyms-symbolizer.c b/src/libsysprof-analyze/sysprof-kallsyms-symbolizer.c index 0995ea97..a3c873a9 100644 --- a/src/libsysprof-analyze/sysprof-kallsyms-symbolizer.c +++ b/src/libsysprof-analyze/sysprof-kallsyms-symbolizer.c @@ -279,7 +279,8 @@ sysprof_kallsyms_symbolizer_symbolize (SysprofSymbolizer *symbolizer, NULL, g_ref_string_acquire (linux_string), ksym->address, - next->address ? next->address : ksym->address + LAST_SYMBOL_LEN); + next->address ? next->address : ksym->address + LAST_SYMBOL_LEN, + SYSPROF_SYMBOL_KIND_KERNEL); if (address < ksym->address) right = mid; @@ -298,7 +299,8 @@ failure: NULL, g_ref_string_acquire (linux_string), address, - address + 1); + address + 1, + SYSPROF_SYMBOL_KIND_KERNEL); } } diff --git a/src/libsysprof-analyze/sysprof-process-info.c b/src/libsysprof-analyze/sysprof-process-info.c index 7e0de75f..19dc9429 100644 --- a/src/libsysprof-analyze/sysprof-process-info.c +++ b/src/libsysprof-analyze/sysprof-process-info.c @@ -33,17 +33,22 @@ sysprof_process_info_new (SysprofMountNamespace *mount_namespace, int pid) { SysprofProcessInfo *self; + char pidstr[32]; char symname[32]; - g_snprintf (symname, sizeof symname, "[Process %d]", pid); + g_snprintf (symname, sizeof symname, "Process %d", pid); + g_snprintf (pidstr, sizeof pidstr, "(%d)", pid); self = g_atomic_rc_box_new0 (SysprofProcessInfo); self->pid = pid; self->address_layout = sysprof_address_layout_new (); self->symbol_cache = sysprof_symbol_cache_new (); self->mount_namespace = mount_namespace; - self->fallback_symbol = _sysprof_symbol_new (g_ref_string_new (symname), NULL, NULL, 0, 0); - self->fallback_symbol->is_process = TRUE; + self->fallback_symbol = _sysprof_symbol_new (g_ref_string_new (symname), + NULL, + g_ref_string_new (pidstr), + 0, 0, + SYSPROF_SYMBOL_KIND_PROCESS); return self; } diff --git a/src/libsysprof-analyze/sysprof-symbol-private.h b/src/libsysprof-analyze/sysprof-symbol-private.h index bedbf047..3f641ac4 100644 --- a/src/libsysprof-analyze/sysprof-symbol-private.h +++ b/src/libsysprof-analyze/sysprof-symbol-private.h @@ -38,17 +38,15 @@ struct _SysprofSymbol SysprofAddress begin_address; SysprofAddress end_address; - guint is_context_switch : 1; - guint is_everything : 1; - guint is_untraceable : 1; - guint is_process : 1; + guint kind : 3; }; -SysprofSymbol *_sysprof_symbol_new (GRefString *name, - GRefString *binary_path, - GRefString *binary_nick, - SysprofAddress begin_address, - SysprofAddress end_address); +SysprofSymbol *_sysprof_symbol_new (GRefString *name, + GRefString *binary_path, + GRefString *binary_nick, + SysprofAddress begin_address, + SysprofAddress end_address, + SysprofSymbolKind kind); static inline SysprofSymbol * _sysprof_symbol_copy (SysprofSymbol *self) @@ -59,11 +57,8 @@ _sysprof_symbol_copy (SysprofSymbol *self) self->binary_path ? g_ref_string_acquire (self->binary_path) : NULL, self->binary_nick ? g_ref_string_acquire (self->binary_nick) : NULL, self->begin_address, - self->end_address); - copy->is_context_switch = self->is_context_switch; - copy->is_everything = self->is_everything; - copy->is_untraceable = self->is_untraceable; - copy->is_process = self->is_process; + self->end_address, + self->kind); return copy; } @@ -78,13 +73,16 @@ _sysprof_symbol_equal (const SysprofSymbol *a, if (a->hash != b->hash) return FALSE; + if (a->kind != b->kind) + return FALSE; + return strcmp (a->name, b->name) == 0; } static inline gboolean _sysprof_symbol_is_context_switch (SysprofSymbol *symbol) { - return symbol->is_context_switch; + return symbol->kind == SYSPROF_SYMBOL_KIND_CONTEXT_SWITCH; } G_END_DECLS diff --git a/src/libsysprof-analyze/sysprof-symbol.c b/src/libsysprof-analyze/sysprof-symbol.c index e367916e..7435cc5c 100644 --- a/src/libsysprof-analyze/sysprof-symbol.c +++ b/src/libsysprof-analyze/sysprof-symbol.c @@ -27,9 +27,10 @@ G_DEFINE_FINAL_TYPE (SysprofSymbol, sysprof_symbol, G_TYPE_OBJECT) enum { PROP_0, - PROP_NAME, PROP_BINARY_NICK, PROP_BINARY_PATH, + PROP_KIND, + PROP_NAME, N_PROPS }; @@ -69,6 +70,10 @@ sysprof_symbol_get_property (GObject *object, g_value_set_string (value, sysprof_symbol_get_binary_path (self)); break; + case PROP_KIND: + g_value_set_enum (value, sysprof_symbol_get_kind (self)); + break; + default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); } @@ -97,12 +102,19 @@ sysprof_symbol_class_init (SysprofSymbolClass *klass) NULL, (G_PARAM_READABLE | G_PARAM_STATIC_STRINGS)); + properties [PROP_KIND] = + g_param_spec_enum ("kind", NULL, NULL, + SYSPROF_TYPE_SYMBOL_KIND, + SYSPROF_SYMBOL_KIND_USER, + (G_PARAM_READABLE | G_PARAM_STATIC_STRINGS)); + g_object_class_install_properties (object_class, N_PROPS, properties); } static void sysprof_symbol_init (SysprofSymbol *self) { + self->kind = SYSPROF_SYMBOL_KIND_USER; } const char * @@ -130,11 +142,12 @@ sysprof_symbol_get_binary_path (SysprofSymbol *self) } SysprofSymbol * -_sysprof_symbol_new (GRefString *name, - GRefString *binary_path, - GRefString *binary_nick, - SysprofAddress begin_address, - SysprofAddress end_address) +_sysprof_symbol_new (GRefString *name, + GRefString *binary_path, + GRefString *binary_nick, + SysprofAddress begin_address, + SysprofAddress end_address, + SysprofSymbolKind kind) { SysprofSymbol *self; @@ -145,6 +158,7 @@ _sysprof_symbol_new (GRefString *name, self->begin_address = begin_address; self->end_address = end_address; self->hash = g_str_hash (name); + self->kind = kind; /* If we got a path for the symbol, add that to the hash so that we * can be sure that we're working with a symbol in the same file when @@ -161,6 +175,9 @@ _sysprof_symbol_new (GRefString *name, self->hash ^= g_str_hash (base); } + if (binary_nick != NULL) + self->hash ^= g_str_hash (binary_nick); + return self; } @@ -176,3 +193,17 @@ sysprof_symbol_hash (const SysprofSymbol *self) { return self->hash; } + +SysprofSymbolKind +sysprof_symbol_get_kind (SysprofSymbol *self) +{ + return self ? self->kind : 0; +} + +G_DEFINE_ENUM_TYPE (SysprofSymbolKind, sysprof_symbol_kind, + G_DEFINE_ENUM_VALUE (SYSPROF_SYMBOL_KIND_ROOT, "root"), + G_DEFINE_ENUM_VALUE (SYSPROF_SYMBOL_KIND_PROCESS, "process"), + G_DEFINE_ENUM_VALUE (SYSPROF_SYMBOL_KIND_CONTEXT_SWITCH, "context-switch"), + G_DEFINE_ENUM_VALUE (SYSPROF_SYMBOL_KIND_USER, "user"), + G_DEFINE_ENUM_VALUE (SYSPROF_SYMBOL_KIND_KERNEL, "kernel"), + G_DEFINE_ENUM_VALUE (SYSPROF_SYMBOL_KIND_UNWINDABLE, "unwindable")) diff --git a/src/libsysprof-analyze/sysprof-symbol.h b/src/libsysprof-analyze/sysprof-symbol.h index 6a51603b..059c87ff 100644 --- a/src/libsysprof-analyze/sysprof-symbol.h +++ b/src/libsysprof-analyze/sysprof-symbol.h @@ -27,21 +27,36 @@ G_BEGIN_DECLS +typedef enum _SysprofSymbolKind +{ + SYSPROF_SYMBOL_KIND_ROOT = 1, + SYSPROF_SYMBOL_KIND_PROCESS, + SYSPROF_SYMBOL_KIND_CONTEXT_SWITCH, + SYSPROF_SYMBOL_KIND_USER, + SYSPROF_SYMBOL_KIND_KERNEL, + SYSPROF_SYMBOL_KIND_UNWINDABLE, +} SysprofSymbolKind; + #define SYSPROF_TYPE_SYMBOL (sysprof_symbol_get_type()) +#define SYSPROF_TYPE_SYMBOL_KIND (sysprof_symbol_kind_get_type()) SYSPROF_AVAILABLE_IN_ALL G_DECLARE_FINAL_TYPE (SysprofSymbol, sysprof_symbol, SYSPROF, SYMBOL, GObject) SYSPROF_AVAILABLE_IN_ALL -const char *sysprof_symbol_get_name (SysprofSymbol *self); +GType sysprof_symbol_kind_get_type (void) G_GNUC_CONST; SYSPROF_AVAILABLE_IN_ALL -const char *sysprof_symbol_get_binary_nick (SysprofSymbol *self); +const char *sysprof_symbol_get_name (SysprofSymbol *self); SYSPROF_AVAILABLE_IN_ALL -const char *sysprof_symbol_get_binary_path (SysprofSymbol *self); +const char *sysprof_symbol_get_binary_nick (SysprofSymbol *self); SYSPROF_AVAILABLE_IN_ALL -guint sysprof_symbol_hash (const SysprofSymbol *self); +const char *sysprof_symbol_get_binary_path (SysprofSymbol *self); SYSPROF_AVAILABLE_IN_ALL -gboolean sysprof_symbol_equal (const SysprofSymbol *a, - const SysprofSymbol *b); +SysprofSymbolKind sysprof_symbol_get_kind (SysprofSymbol *self); +SYSPROF_AVAILABLE_IN_ALL +guint sysprof_symbol_hash (const SysprofSymbol *self); +SYSPROF_AVAILABLE_IN_ALL +gboolean sysprof_symbol_equal (const SysprofSymbol *a, + const SysprofSymbol *b); G_END_DECLS diff --git a/src/libsysprof-analyze/tests/test-symbol-cache.c b/src/libsysprof-analyze/tests/test-symbol-cache.c index 2b4ef463..52610d81 100644 --- a/src/libsysprof-analyze/tests/test-symbol-cache.c +++ b/src/libsysprof-analyze/tests/test-symbol-cache.c @@ -42,7 +42,7 @@ create_symbol (const char *name, { g_assert (begin < end); - return _sysprof_symbol_new (g_ref_string_new (name), NULL, NULL, begin, end); + return _sysprof_symbol_new (g_ref_string_new (name), NULL, NULL, begin, end, SYSPROF_SYMBOL_KIND_USER); } static int