Make StackFrame.name fall back to symbol/PC and add StackFrame.functi…

…on_name

Multiple people have lamented that StackFrame.name is None for functions
implemented in assembly or missing debug info for any other reason. With
DWARFless debugging, this will be way more common. My original hope was
that StackFrame.name would strictly be the function name from the
debugging information and that callers would fall back to getting the
symbol name themselves. However, the distinction isn't super meaningful
to users, so let's add the fallback directly to StackFrame.name and add
StackFrame.function_name with the old behavior of StackFrame.name.

Signed-off-by: Omar Sandoval <[email protected]>

_drgn.pyi

@@ -2774,23 +2774,28 @@ class StackFrame:
     (int)1
     """
 
-    name: Final[Optional[str]]
+    name: Final[str]
+    """
+    Name of the function or symbol at this frame.
+
+    This tries to get the best available name for this frame in the following
+    order:
+
+    1. The name of the function in the source code based on debugging
+       information (:attr:`frame.function_name <function_name>`).
+    2. The name of the symbol in the binary (:meth:`frame.symbol().name
+       <symbol>`).
+    3. The program counter in hexadecimal (:attr:`hex(frame.pc) <pc>`).
+    4. The string "???".
+    """
+
+    function_name: Final[Optional[str]]
     """
     Name of the function at this frame, or ``None`` if it could not be
     determined.
 
     The name cannot be determined if debugging information is not available for
-    the function, e.g., because it is implemented in assembly. It may be
-    desirable to use the symbol name or program counter as a fallback:
-
-    .. code-block:: python3
-
-        name = frame.name
-        if name is None:
-            try:
-                name = frame.symbol().name
-            except LookupError:
-                name = hex(frame.pc)
+    the function, e.g., because it is implemented in assembly.
     """
 
     is_inline: Final[bool]

libdrgn/drgn.h

@@ -3717,13 +3717,24 @@ bool drgn_stack_frame_interrupted(struct drgn_stack_trace *trace, size_t frame);
 struct drgn_error *drgn_format_stack_frame(struct drgn_stack_trace *trace,
 					   size_t frame, char **ret);
 
+/**
+ * Get the best available name for a stack frame.
+ *
+ * @param[out] ret Returned name. On success, it must be freed with @c free().
+ * On error, it is not modified.
+ * @return @c NULL on success, non-@c NULL on error.
+ */
+struct drgn_error *drgn_stack_frame_name(struct drgn_stack_trace *trace,
+					 size_t frame, char **ret);
+
 /**
  * Get the name of the function at a stack frame.
  *
  * @return Function name. This is valid until the stack trace is destroyed; it
  * should not be freed. @c NULL if the name could not be determined.
  */
-const char *drgn_stack_frame_name(struct drgn_stack_trace *trace, size_t frame);
+const char *drgn_stack_frame_function_name(struct drgn_stack_trace *trace,
+					   size_t frame);
 
 /** Return whether a stack frame is for an inlined call. */
 bool drgn_stack_frame_is_inline(struct drgn_stack_trace *trace, size_t frame);

libdrgn/python/stack_trace.c

@@ -272,9 +272,20 @@ static PyObject *StackFrame_registers(StackFrame *self)
 
 static PyObject *StackFrame_get_name(StackFrame *self, void *arg)
 {
-	const char *name = drgn_stack_frame_name(self->trace->trace, self->i);
-	if (name)
-		return PyUnicode_FromString(name);
+	_cleanup_free_ char *name = NULL;
+	struct drgn_error *err = drgn_stack_frame_name(self->trace->trace,
+						       self->i, &name);
+	if (err)
+		return set_drgn_error(err);
+	return PyUnicode_FromString(name);
+}
+
+static PyObject *StackFrame_get_function_name(StackFrame *self, void *arg)
+{
+	const char *function_name =
+		drgn_stack_frame_function_name(self->trace->trace, self->i);
+	if (function_name)
+		return PyUnicode_FromString(function_name);
 	else
 		Py_RETURN_NONE;
 }
@@ -336,6 +347,8 @@ static PyMethodDef StackFrame_methods[] = {
 
 static PyGetSetDef StackFrame_getset[] = {
 	{"name", (getter)StackFrame_get_name, NULL, drgn_StackFrame_name_DOC},
+	{"function_name", (getter)StackFrame_get_function_name, NULL,
+	 drgn_StackFrame_function_name_DOC},
 	{"is_inline", (getter)StackFrame_get_is_inline, NULL,
 	 drgn_StackFrame_is_inline_DOC},
 	{"interrupted", (getter)StackFrame_get_interrupted, NULL,

libdrgn/stack_trace.c

@@ -117,9 +117,10 @@ drgn_format_stack_trace(struct drgn_stack_trace *trace, char **ret)
 
 		struct drgn_register_state *regs = trace->frames[frame].regs;
 		struct optional_uint64 pc;
-		const char *name = drgn_stack_frame_name(trace, frame);
-		if (name) {
-			if (!string_builder_append(&str, name))
+		const char *function_name =
+			drgn_stack_frame_function_name(trace, frame);
+		if (function_name) {
+			if (!string_builder_append(&str, function_name))
 				return &drgn_enomem;
 		} else if ((pc = drgn_register_state_get_pc(regs)).has_value) {
 			_cleanup_symbol_ struct drgn_symbol *sym = NULL;
@@ -198,8 +199,9 @@ drgn_format_stack_frame(struct drgn_stack_trace *trace, size_t frame, char **ret
 			return &drgn_enomem;
 	}
 
-	const char *name = drgn_stack_frame_name(trace, frame);
-	if (name && !string_builder_appendf(&str, " in %s", name))
+	const char *function_name = drgn_stack_frame_function_name(trace, frame);
+	if (function_name
+	    && !string_builder_appendf(&str, " in %s", function_name))
 		return &drgn_enomem;
 
 	int line, column;
@@ -224,8 +226,42 @@ drgn_format_stack_frame(struct drgn_stack_trace *trace, size_t frame, char **ret
 	return NULL;
 }
 
-LIBDRGN_PUBLIC const char *drgn_stack_frame_name(struct drgn_stack_trace *trace,
-						 size_t frame)
+LIBDRGN_PUBLIC
+struct drgn_error *drgn_stack_frame_name(struct drgn_stack_trace *trace,
+					 size_t frame, char **ret)
+{
+	struct drgn_error *err;
+	char *name;
+	const char *function_name = drgn_stack_frame_function_name(trace, frame);
+	if (function_name) {
+		name = strdup(function_name);
+	} else {
+		struct drgn_register_state *regs = trace->frames[frame].regs;
+		struct optional_uint64 pc = drgn_register_state_get_pc(regs);
+		if (pc.has_value) {
+			_cleanup_symbol_ struct drgn_symbol *sym = NULL;
+			err = drgn_program_find_symbol_by_address_internal(trace->prog,
+									   pc.value - !regs->interrupted,
+									   &sym);
+			if (err)
+				return err;
+			if (sym)
+				name = strdup(sym->name);
+			else if (asprintf(&name, "0x%" PRIx64, pc.value) < 0)
+				name = NULL;
+		} else {
+			name = strdup("???");
+		}
+	}
+	if (!name)
+		return &drgn_enomem;
+	*ret = name;
+	return NULL;
+}
+
+LIBDRGN_PUBLIC
+const char *drgn_stack_frame_function_name(struct drgn_stack_trace *trace,
+					   size_t frame)
 {
 	Dwarf_Die *scopes = trace->frames[frame].scopes;
 	size_t num_scopes = trace->frames[frame].num_scopes;
@@ -463,11 +499,12 @@ drgn_stack_frame_find_object(struct drgn_stack_trace *trace, size_t frame_i,
 	}
 	if (!die.addr) {
 not_found:;
-		const char *frame_name = drgn_stack_frame_name(trace, frame_i);
-		if (frame_name) {
+		const char *function_name =
+			drgn_stack_frame_function_name(trace, frame_i);
+		if (function_name) {
 			return drgn_error_format(DRGN_ERROR_LOOKUP,
 						 "could not find '%s' in '%s'",
-						 name, frame_name);
+						 name, function_name);
 		} else {
 			return drgn_error_format(DRGN_ERROR_LOOKUP,
 						 "could not find '%s'", name);

tests/test_stack_trace.py

@@ -0,0 +1,28 @@
+# Copyright (c) Meta Platforms, Inc. and affiliates.
+# SPDX-License-Identifier: LGPL-2.1-or-later
+
+from drgn import Program
+from tests import TestCase
+from tests.resources import get_resource
+
+
+class TestLinuxUserspaceCoreDump(TestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.prog = Program()
+        cls.prog.set_enabled_debug_info_finders([])
+        cls.prog.set_core_dump(get_resource("crashme.core"))
+        cls.prog.load_debug_info([get_resource("crashme"), get_resource("crashme.so")])
+        cls.trace = cls.prog.crashed_thread().stack_trace()
+
+    def test_stack_frame_name(self):
+        self.assertEqual(self.trace[0].name, "c")
+        self.assertEqual(self.trace[5].name, "0x7f6112ad8088")
+        self.assertEqual(self.trace[7].name, "_start")
+        self.assertEqual(self.trace[8].name, "???")
+
+    def test_stack_frame_function_name(self):
+        self.assertEqual(self.trace[0].function_name, "c")
+        self.assertIsNone(self.trace[5].function_name)
+        self.assertIsNone(self.trace[7].function_name)
+        self.assertIsNone(self.trace[8].function_name)