This is the mail archive of the
insight@sourceware.org
mailing list for the Insight project.
Re: [PATCH/gdbtk] Update stackwin gdb api usage
- From: Keith Seitz <keiths at redhat dot com>
- To: Roland Schwingel <roland at onevision dot com>
- Cc: insight at sourceware dot org
- Date: Wed, 23 May 2012 18:12:16 -0700
- Subject: Re: [PATCH/gdbtk] Update stackwin gdb api usage
- References: <4F79C3F2.3020908@onevision.com>
On 04/02/2012 08:21 AM, Roland Schwingel wrote:
A general question:
The direction of stack dump in insight is opposite to when issueing a
backtrace inside gdb itself. Is this on purpose? Wouldn't it be nicer
when insight's stack window would show the stack in the same order?
Yes, that was a design decision made many, many years ago. I don't have
a preference to sort the stack "properly." If you find the time to write
up a patch, I would consider/approve it.
diff -ruNp gdbtk_orig/generic/gdbtk-stack.c gdbtk/generic/gdbtk-stack.c
--- gdbtk_orig/generic/gdbtk-stack.c 2012-03-30 09:29:15.000000000 +0200
+++ gdbtk/generic/gdbtk-stack.c 2012-04-02 16:13:11.351540500 +0200
@@ -1,5 +1,6 @@
/* Tcl/Tk command definitions for Insight - Stack.
- Copyright (C) 2001-2012 Free Software Foundation, Inc.
+ Copyright (C) 2001, 2002, 2003, 2008, 2011
+ Free Software Foundation, Inc.
This file is part of GDB.
You already did this. :-)
@@ -26,6 +27,10 @@
#include "dictionary.h"
#include "varobj.h"
#include "arch-utils.h"
+#include "stack.h"
+#ifndef PC_SOLIB
+#include "solib.h"
+#endif
#include<tcl.h>
#include "gdbtk.h"
I believe the proper etiquette in gdb is to simply include solib.h
unconditionally. It should not hurt anything. I tested this on mingw32
and x86_64-linux with no troubles.
@@ -475,24 +480,24 @@ gdb_stack (ClientData clientData, Tcl_In
/* Find the outermost frame */
r = GDB_get_current_frame (&fi);
if (r != GDB_OK)
- return TCL_ERROR;
+ return TCL_ERROR;
while (fi != NULL)
{
top = fi;
- r = GDB_get_prev_frame (fi,&fi);
- if (r != GDB_OK)
- fi = NULL;
+ r = GDB_get_prev_frame (fi,&fi);
+ if (r != GDB_OK)
+ fi = NULL;
}
+ result_ptr->obj_ptr = Tcl_NewListObj (0, NULL);
+
/* top now points to the top (outermost frame) of the
stack, so point it to the requested start */
start = -start;
- r = GDB_find_relative_frame (top,&start,&top);
-
- result_ptr->obj_ptr = Tcl_NewListObj (0, NULL);
+ r = GDB_find_relative_frame (top,&start,&top);
Weird. My mail agent has removed the spaces after the commas. In the
patch, all looks well. There does appear to be some whitespace trailing
the "r = GDB_find_relative_frame (...);" line, though. Please
double-check and remove if it is actually there. [and not another trick
of my mail agent]
if (r != GDB_OK)
- return TCL_OK;
+ return TCL_OK;
/* If start != 0, then we have asked to start outputting
frames beyond the innermost stack frame */
@@ -503,8 +508,8 @@ gdb_stack (ClientData clientData, Tcl_In
{
get_frame_name (interp, result_ptr->obj_ptr, fi);
r = GDB_get_next_frame (fi,&fi);
- if (r != GDB_OK)
- break;
+ if (r != GDB_OK)
+ break;
"break;" doesn't appear indented properly, though. Please double-check.
}
}
}
This rest of the whitespace change is okay, but please submit these
sorts of patches separate from more substantial patches. It is a lot
easier for me to review without that cluttering up the patch.
@@ -515,14 +520,14 @@ gdb_stack (ClientData clientData, Tcl_In
/* A helper function for get_stack which adds information about
* the stack frame FI to the caller's LIST.
*
- * This is stolen from print_frame_info in stack.c.
+ * This is stolen from print_frame_info/print_frame in stack.c.
*/
+
static void
get_frame_name (Tcl_Interp *interp, Tcl_Obj *list, struct frame_info *fi)
{
- struct symtab_and_line sal;
struct symbol *func = NULL;
- const char *funname = 0;
+ const char *funname = NULL;
enum language funlang = language_unknown;
Tcl_Obj *objv[1];
@@ -532,75 +537,39 @@ get_frame_name (Tcl_Interp *interp, Tcl_
Tcl_ListObjAppendElement (interp, list, objv[0]);
return;
}
- if ((get_frame_type (fi) == SIGTRAMP_FRAME))
+ if (get_frame_type (fi) == SIGTRAMP_FRAME)
{
objv[0] = Tcl_NewStringObj ("<signal handler called>", -1);
Tcl_ListObjAppendElement (interp, list, objv[0]);
return;
}
-
- sal =
- find_pc_line (get_frame_pc (fi),
- get_next_frame (fi) != NULL
- && !(get_frame_type (fi) == SIGTRAMP_FRAME)
- && !(get_frame_type (fi) == DUMMY_FRAME));
-
- func = find_pc_function (get_frame_pc (fi));
- if (func)
- {
- struct minimal_symbol *msymbol = lookup_minimal_symbol_by_pc (get_frame_pc (fi));
- if (msymbol != NULL
- && (SYMBOL_VALUE_ADDRESS (msymbol)
- > BLOCK_START (SYMBOL_BLOCK_VALUE (func))))
- {
- func = 0;
- funname = GDBTK_SYMBOL_SOURCE_NAME (msymbol);
- funlang = SYMBOL_LANGUAGE (msymbol);
- }
- else
- {
- funname = GDBTK_SYMBOL_SOURCE_NAME (func);
- funlang = SYMBOL_LANGUAGE (func);
- }
- }
- else
+ if (get_frame_type (fi) == ARCH_FRAME)
{
- struct minimal_symbol *msymbol = lookup_minimal_symbol_by_pc (get_frame_pc (fi));
- if (msymbol != NULL)
- {
- funname = GDBTK_SYMBOL_SOURCE_NAME (msymbol);
- funlang = SYMBOL_LANGUAGE (msymbol);
- }
+ objv[0] = Tcl_NewStringObj ("<cross-architecture call>", -1);
+ Tcl_ListObjAppendElement (interp, list, objv[0]);
+ return;
}
- if (sal.symtab)
+ find_frame_funname (fi,&funname,&funlang,&func);
Argh! Darn mail agent! I apologize ahead of time if I ask you to watch
out for this, and it turns out to be my mail agent and not you.
+
+ if (funname)
{
objv[0] = Tcl_NewStringObj (funname, -1);
Tcl_ListObjAppendElement (interp, list, objv[0]);
}
else
{
-#if 0
- /* we have no convenient way to deal with this yet... */
- if (fi->pc != sal.pc || !sal.symtab)
- {
- deprecated_print_address_numeric (fi->pc, 1, gdb_stdout);
- printf_filtered (" in ");
- }
- printf_symbol_filtered (gdb_stdout, funname ? funname : "??", funlang,
- DMGL_ANSI);
-#endif
- objv[0] = Tcl_NewStringObj (funname != NULL ? funname : "??", -1);
+ char *lib = NULL;
Can you double-check the indent level on the above line? It doesn't look
right. [Apologies again if my mail agent maligned it.]
+ objv[0] = Tcl_NewStringObj (funname ? funname : "??", -1);
#ifdef PC_SOLIB
- if (!funname)
- {
- char *lib = PC_SOLIB (get_frame_pc (fi));
- if (lib)
- {
- Tcl_AppendStringsToObj (objv[0], " from ", lib, (char *) NULL);
- }
- }
+ lib = PC_SOLIB (get_frame_pc (fi));
+#else
+ lib = solib_name_from_address (get_frame_program_space (fi),
+ get_frame_pc (fi));
GNU coding standards prefer that "get_frame_pc (fi));" be aligned
underneath "get_frame_program_space (fi),".
#endif
This has nothing to do with your patch other than the fact that you are
touching this, but could you check the indentation on "#endif" while
you're touching this code? It looks a little off.
+ if (lib)
+ Tcl_AppendStringsToObj (objv[0], " from ", lib, (char *) NULL);
+
Tcl_ListObjAppendElement (interp, list, objv[0]);
}
}
diff -ruNp gdbtk_orig/library/stackwin.itb gdbtk/library/stackwin.itb
--- gdbtk_orig/library/stackwin.itb 2008-02-09 02:23:42.000000000 +0100
+++ gdbtk/library/stackwin.itb 2012-04-02 09:40:19.507691900 +0200
@@ -1,5 +1,5 @@
# Stack window for Insight.
-# Copyright (C) 1997, 1998, 1999, 2002, 2003, 2008 Red Hat
+# Copyright (C) 1997-2012 Red Hat, Inc.
#
# This program is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License (GPL) as published by
@@ -68,20 +68,14 @@ itcl::body StackWin::update {event} {
set frames {}
}
+ $itk_component(slb) delete 0 end
if {[llength $frames] == 0} {
- $itk_component(slb) delete 0 end
$itk_component(slb) insert end {NO STACK}
return
}
- $itk_component(slb) delete 0 end
set levels 0
foreach frame $frames {
- set len [string length $frame]
-
- if {$len> $maxwidth} {
- set maxwidth $len
- }
$itk_component(slb) insert end $frame
incr levels
}
diff -ruNp gdbtk_orig/library/stackwin.ith gdbtk/library/stackwin.ith
--- gdbtk_orig/library/stackwin.ith 2005-12-23 19:26:50.000000000 +0100
+++ gdbtk/library/stackwin.ith 2012-04-02 09:40:23.623824000 +0200
@@ -1,5 +1,5 @@
# Stack window class definition for GDBtk.
-# Copyright (C) 1997, 1998, 1999 Cygnus Solutions
+# Copyright (C) 1997-2012 Red Hat, Inc.
#
# This program is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License (GPL) as published by
@@ -20,7 +20,6 @@ itcl::class StackWin {
inherit EmbeddedWin GDBWin
private {
- variable maxwidth 40
variable Running 0
variable protect_me 0
method build_win {}
Ok with those minor changes.
Thanks!
Keith