* Re: [PATCH/gdbtk] Update stackwin gdb api usage
@ 2012-05-25 10:23 Roland Schwingel
0 siblings, 0 replies; 3+ messages in thread
From: Roland Schwingel @ 2012-05-25 10:23 UTC (permalink / raw)
To: insight
Hi...
Thanks for reviewing Keith!
I applied this now with all your comments taken into account.
Some of the formatting things had also been on my side not only
on your email clients side. Sorry for that!
ATTENTION: At present insight is not compiling!
This is not reasoned by my patch but due to changes inside of
gdb itself. I will formulate a (trivial) patch for that.
Roland
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH/gdbtk] Update stackwin gdb api usage
2012-04-02 15:21 Roland Schwingel
@ 2012-05-24 1:12 ` Keith Seitz
0 siblings, 0 replies; 3+ messages in thread
From: Keith Seitz @ 2012-05-24 1:12 UTC (permalink / raw)
To: Roland Schwingel; +Cc: insight
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH/gdbtk] Update stackwin gdb api usage
@ 2012-04-02 15:21 Roland Schwingel
2012-05-24 1:12 ` Keith Seitz
0 siblings, 1 reply; 3+ messages in thread
From: Roland Schwingel @ 2012-04-02 15:21 UTC (permalink / raw)
To: insight
[-- Attachment #1: Type: text/plain, Size: 2292 bytes --]
Hi...
Here is a new patch from my side for insight. It adresses 2 issues.
The small one first:
In the tcl code of the stackwindow there is an instance variable
maxwidth which is calculated but never used somewhere. My patch dares to
remove it.
The bigger one:
I am frequently seeing output in insight on windows 64 bit:
(Internal error: pc 0x0 in read in psymtab, but not in symtab.)
Especially when debugging objectiveC code but can also see in sometimes
when debugging plain c. This is triggered by stack unwinding (showing
stack window in insight). I tracked it down to 2 problems. One of it is
in insight itself and fixed with this patch.
The C code of insight dealing with the backtrace generation was made
somewhere in 2001 and remainded largely unchanged since then. It was
initially "inspired" by the code directly in gdb. GDB has changed since
then in that area. I updated insight's C code for generating the
backtrace for the tcl part to match again gdb's code in stack.c. I could
remove a bigger bunch of insight's private code in that area and replace
it directly with a call to find_frame_funname() of gdb itself. I also
removed some dead code here.
From what I could see now insight no longer genarates this error
message directly. I could still trigger it in some situations by just
issuing a "bt" from either insight's console or from within native gdb.
I have to dive into that. But with this patch insight itself should be
clean now in this regard.
I also updated copyright informations in the affected files.
Changelog:
2012-04-02 Roland Schwingel <roland.schwingel@onevision.com>
* generic/gdbtk-stack.c: Updated copyright informations.
include "stack.h" and "solib.h" (when needed).
(gdb_stack): Some reformatting.
(get_frame_name): Updated usage of gdb api for
backtrace generation. Some reformatting. Removed
dead code. Also handle frame type ARCH_FRAME now.
* library/stackwin.it[bh]: Remove dead instance
variable maxwidth. Updated copyright informations.
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?
Any comments? Is this ok?
Roland
[-- Attachment #2: stackwin.patch --]
[-- Type: text/plain, Size: 7022 bytes --]
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.
@@ -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"
@@ -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);
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;
}
}
}
@@ -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);
+
+ 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;
+ 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));
#endif
+ 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 {}
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-05-25 10:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-25 10:23 [PATCH/gdbtk] Update stackwin gdb api usage Roland Schwingel
-- strict thread matches above, loose matches on Subject: below --
2012-04-02 15:21 Roland Schwingel
2012-05-24 1:12 ` Keith Seitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).