Don't use kmalloc in get_fn_name()
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 13 Jul 2018 21:37:33 +0000 (17:37 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Fri, 13 Jul 2018 21:52:25 +0000 (17:52 -0400)
Back in commit 290962da6e27 ("Reflects symbol table into the kernel"), we
changed from using the old debuginfo to a autogenerated table of strings.
Those new strings are null-terminated.  The old debuginfo wasn't.
get_fn_name() needed to add the trailing \0, but we don't need to anymore.

The end result is that debug code that looks up a function name for a given
symbol, such as backtrace(), won't need to kmalloc.  This will help when we
are trying to backtrace failures in the allocator.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/include/kdebug.h
kern/src/alarm.c
kern/src/atomic.c
kern/src/kdebug.c
kern/src/monitor.c
kern/src/smp.c
kern/src/trap.c

index f8744c1..7b482aa 100644 (file)
@@ -7,7 +7,7 @@
 /* for includes */ struct proc;
 
 struct symtab_entry {
-       char *name;
+       const char *name;
        uintptr_t addr;
 };
 
@@ -49,8 +49,8 @@ void gen_backtrace(void (*pfunc)(void *, const char *), void *opaque);
 static inline uintptr_t get_caller_pc(void);
 
 /* Returns a null-terminated string with the function name for a given PC /
- * instruction pointer.  kfree() the result. */
-char *get_fn_name(uintptr_t pc);
+ * instruction pointer.  Returns NULL on failure. */
+const char *get_fn_name(uintptr_t pc);
 
 /* Returns the address of sym, or 0 if it does not exist */
 uintptr_t get_symbol_addr(char *sym);
index 52e21d3..b147095 100644 (file)
@@ -460,16 +460,13 @@ void print_chain(struct timer_chain *tchain)
               tchain->latest_time);
        TAILQ_FOREACH(i, &tchain->waiters, next) {
                uintptr_t f;
-               char *f_name;
 
                if (i->irq_ok)
                        f = (uintptr_t)i->func_irq;
                else
                        f = (uintptr_t)i->func;
-               f_name = get_fn_name(f);
                printk("\tWaiter %p, time %llu, func %p (%s)\n", i,
-                      i->wake_up_time, f, f_name);
-               kfree(f_name);
+                      i->wake_up_time, f, get_fn_name(f));
        }
        spin_unlock_irqsave(&tchain->lock);
 }
index 5669baa..7af4cfb 100644 (file)
@@ -102,17 +102,14 @@ void spin_unlock(spinlock_t *lock)
 void spinlock_debug(spinlock_t *lock)
 {
        uintptr_t pc = lock->call_site;
-       char *func_name;
 
        if (!pc) {
                printk("Lock %p: never locked\n", lock);
                return;
        }
-       func_name = get_fn_name(pc);
-       printk("Lock %p: currently %slocked.  Last locked at [<%p>] in %s on "
-              "core %d\n", lock, spin_locked(lock) ? "" : "un", pc, func_name,
+       printk("Lock %p: currently %slocked.  Last locked at [<%p>] in %s on core %d\n",
+              lock, spin_locked(lock) ? "" : "un", pc, get_fn_name(pc),
               lock->calling_core);
-       kfree(func_name);
 }
 
 #endif /* CONFIG_SPINLOCK_DEBUG */
index 787de72..6937d6d 100644 (file)
 
 struct symtab_entry gbl_symtab[1] __attribute__((weak)) = {{0, 0}};
 
-/* Returns a null-terminated string with the function name for a given PC /
- * instruction pointer.  kfree() the result. */
-char *get_fn_name(uintptr_t pc)
+/* Returns a null-terminated string from the reflected symbol table with the
+ * function name for a given PC / instruction pointer.  Returns NULL on
+ * failure. */
+const char *get_fn_name(uintptr_t pc)
 {
        struct symtab_entry *i, *prev = 0, *found = 0;
-       char *buf;
-       size_t name_len;
+
        /* Table is in ascending order.  As soon as we get to an entry greater than
         * us, we were in the previous one.  This is only true if we were given a
         * good PC.  Random addresses will just find the previous symbol. */
@@ -30,14 +30,9 @@ char *get_fn_name(uintptr_t pc)
                prev = i;
        }
        if (!found)
-               return 0;
+               return NULL;
        assert(found->name);
-       name_len = strlen(found->name) + 1;
-       buf = kmalloc(name_len, 0);
-       if (!buf)
-               return 0;
-       strlcpy(buf, found->name, name_len);
-       return buf;
+       return found->name;
 }
 
 uintptr_t get_symbol_addr(char *sym)
@@ -204,15 +199,12 @@ void debug_addr_pid(int pid, unsigned long addr)
 void print_backtrace_list(uintptr_t *pcs, size_t nr_pcs,
                                                  void (*pfunc)(void *, const char *), void *opaque)
 {
-       char *func_name;
        char bt_line[128];
 
        for (size_t i = 0; i < nr_pcs; i++) {
-               func_name = get_fn_name(pcs[i]);
                snprintf(bt_line, sizeof(bt_line), "#%02d [<%p>] in %s\n", i + 1,
-                                pcs[i], func_name);
+                                pcs[i], get_fn_name(pcs[i]));
                pfunc(opaque, bt_line);
-               kfree(func_name);
        }
 }
 
index 3465872..6a5fcaf 100644 (file)
@@ -680,16 +680,12 @@ static DEFINE_PERCPU(bool, mon_nmi_trace);
 
 static void emit_hwtf_backtrace(struct hw_trapframe *hw_tf)
 {
-       char *fn_name;
-
        if (mon_verbose_trace) {
                print_trapframe(hw_tf);
                backtrace_hwtf(hw_tf);
        }
-       fn_name = get_fn_name(get_hwtf_pc(hw_tf));
        printk("Core %d is at %p (%s)\n", core_id(), get_hwtf_pc(hw_tf),
-              fn_name);
-       kfree(fn_name);
+              get_fn_name(get_hwtf_pc(hw_tf)));
 }
 
 static void emit_vmtf_backtrace(struct vm_trapframe *vm_tf)
index d11083c..43ba748 100644 (file)
@@ -175,19 +175,18 @@ void reset_cpu_state_ticks(int coreid)
 static void pcpui_trace_kmsg_handler(void *event, void *data)
 {
        struct pcpu_trace_event *te = (struct pcpu_trace_event*)event;
-       char *func_name;
        uintptr_t addr;
+
        addr = te->arg1;
-       func_name = get_fn_name(addr);
-       printk("\tKMSG %p: %s\n", addr, func_name);
-       kfree(func_name);
+       printk("\tKMSG %p: %s\n", addr, get_fn_name(addr));
 }
 
 static void pcpui_trace_locks_handler(void *event, void *data)
 {
        struct pcpu_trace_event *te = (struct pcpu_trace_event*)event;
-       char *func_name;
+       const char *func_name;
        uintptr_t lock_addr = te->arg1;
+
        if (lock_addr > KERN_LOAD_ADDR)
                func_name = get_fn_name(lock_addr);
        else
@@ -195,8 +194,6 @@ static void pcpui_trace_locks_handler(void *event, void *data)
        printk("Time %uus, lock %p (%s)\n", te->arg0, lock_addr, func_name);
        printk("\t");
        spinlock_debug((spinlock_t*)lock_addr);
-       if (lock_addr > KERN_LOAD_ADDR)
-               kfree(func_name);
 }
 
 /* Add specific trace handlers here: */
index c29ad8e..a854866 100644 (file)
@@ -268,14 +268,13 @@ void print_kmsgs(uint32_t coreid)
        struct per_cpu_info *pcpui = &per_cpu_info[coreid];
        void __print_kmsgs(struct kernel_msg_list *list, char *type)
        {
-               char *fn_name;
                struct kernel_message *kmsg_i;
+
                STAILQ_FOREACH(kmsg_i, list, link) {
-                       fn_name = get_fn_name((long)kmsg_i->pc);
                        printk("%s KMSG on %d from %d to run %p(%s)(%p, %p, %p)\n", type,
-                              kmsg_i->dstid, kmsg_i->srcid, kmsg_i->pc, fn_name,
+                              kmsg_i->dstid, kmsg_i->srcid, kmsg_i->pc,
+                              get_fn_name((long)kmsg_i->pc),
                               kmsg_i->arg0, kmsg_i->arg1, kmsg_i->arg2);
-                       kfree(fn_name);
                }
        }
        __print_kmsgs(&pcpui->immed_amsgs, "Immedte");