[PR,python/17364] Cleanup registration of __gdb_builtin_type_bound128 pretty-printer

Message ID AC542571535E904D8E8ADAE745D60B192F92CA48@IRSMSX104.ger.corp.intel.com
State New, archived
Headers

Commit Message

Walfred Tedeschi Oct. 6, 2014, 7:33 a.m. UTC
  Hi Doug,

I have also submitted a patch for this last week. Though I was not creating a new directory for pretty printers.
https://sourceware.org/ml/gdb-patches/2014-09/msg00867.html

I would like though to understand a bit more your comment on having x86 on the name of the register. What is the purpose of that?
(I could change it in case)

About registration of the pretty printer. We could do the following:
1. Register an event listener for new obj_files.
2. Query at every event the architecture of the  inferior. (In case we had an event new_inferior loaded it would be cheaper.)
3. In case of x86 or amd64 we can then register the pretty-printer for the bound registers.

Regards,
-Fred




-----Original Message-----
From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Doug Evans
Sent: Saturday, October 04, 2014 8:47 PM
To: gdb-patches@sourceware.org
Subject: [PATCH][PR python/17364] Cleanup registration of __gdb_builtin_type_bound128 pretty-printer

Hi.

The registration of this pretty-printer is all wrong.
This patch creates a new global "builtin" collection of pretty-printers, adds the bound pretty-printer to it, and provides an API call to let one register more builtin pretty-printers.

I'd really like to get this into 7.8.1,
I don't want the way it is currently registered to get built on.

One thing that still bothers me is that the bound128 type is x86-specific and yet the printer is arch-independent.
That should require changing the name __gdb_builtin_type_bound128 in i386-tdep.c to include x86 in the name (or some such), but maybe that's too big a change for now.
But as gdb becomes more multi-arch, attention to collisions and confusion in global namespaces (e.g., the space of builtin pretty-printers) is going to be more important.
[One way to solve this would be to record such printers with the arch, but that feels like overkill.]

2014-10-03  Doug Evans  <xdje42@gmail.com>

	PR python/17364
	* python/lib/gdb/__init__.py (packages): Add "printer".
	* python/lib/gdb/command/bound_registers.py: Moved to ...
	* python/lib/gdb/printer/bound_registers.py: ... here.
	Add printer to global set of builtin printers.  Rename printer from
	"bound" to "bound128".
	* python/lib/gdb/printing.py (_builtin_pretty_printers): New global,
	registered as global "builtin" printer.
	(add_builtin_pretty_printer): New function.
	* data-directory/Makefile.in (PYTHON_FILE_LIST): Update, and add
	gdb/printer/__init__.py.

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052
  

Comments

Doug Evans Oct. 9, 2014, 4:23 p.m. UTC | #1
On Mon, Oct 6, 2014 at 12:33 AM, Tedeschi, Walfred
<walfred.tedeschi@intel.com> wrote:
> Hi Doug,
>
> I have also submitted a patch for this last week. Though I was not creating a new directory for pretty printers.
> https://sourceware.org/ml/gdb-patches/2014-09/msg00867.html
>
> I would like though to understand a bit more your comment on having x86 on the name of the register. What is the purpose of that?
> (I could change it in case)

Imagine a multi-arch gdb with several arches having bounds registers.
How do we distinguish them in the pretty-printer namespace?

> About registration of the pretty printer. We could do the following:
> 1. Register an event listener for new obj_files.
> 2. Query at every event the architecture of the  inferior. (In case we had an event new_inferior loaded it would be cheaper.)
> 3. In case of x86 or amd64 we can then register the pretty-printer for the bound registers.

We could do something like that for 7.9 alright.
At the moment it feels like a bit of overkill just for one pretty-printer.
And for 7.8.1 I want to do something simple.
  
Walfred Tedeschi Oct. 13, 2014, 6:56 a.m. UTC | #2
Hello Doug,

I will prepare a patch as soon as I have some cycles.

Thanks and regards,
-Fred

-----Original Message-----
From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Doug Evans

Sent: Thursday, October 09, 2014 6:23 PM
To: Tedeschi, Walfred
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH][PR python/17364] Cleanup registration of __gdb_builtin_type_bound128 pretty-printer

On Mon, Oct 6, 2014 at 12:33 AM, Tedeschi, Walfred <walfred.tedeschi@intel.com> wrote:
> Hi Doug,

>

> I have also submitted a patch for this last week. Though I was not creating a new directory for pretty printers.

> https://sourceware.org/ml/gdb-patches/2014-09/msg00867.html

>

> I would like though to understand a bit more your comment on having x86 on the name of the register. What is the purpose of that?

> (I could change it in case)


Imagine a multi-arch gdb with several arches having bounds registers.
How do we distinguish them in the pretty-printer namespace?

> About registration of the pretty printer. We could do the following:

> 1. Register an event listener for new obj_files.

> 2. Query at every event the architecture of the  inferior. (In case we 

> had an event new_inferior loaded it would be cheaper.) 3. In case of x86 or amd64 we can then register the pretty-printer for the bound registers.


We could do something like that for 7.9 alright.
At the moment it feels like a bit of overkill just for one pretty-printer.
And for 7.8.1 I want to do something simple.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052
  
Doug Evans Oct. 13, 2014, 6:09 p.m. UTC | #3
Hi.
The 7.8.1 release is blocked on this.
If you don't have cycles, I can do it today.
Have we reached an agreement on what to do?

On Sun, Oct 12, 2014 at 11:56 PM, Tedeschi, Walfred
<walfred.tedeschi@intel.com> wrote:
> Hello Doug,
>
> I will prepare a patch as soon as I have some cycles.
>
> Thanks and regards,
> -Fred
>
> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Doug Evans
> Sent: Thursday, October 09, 2014 6:23 PM
> To: Tedeschi, Walfred
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH][PR python/17364] Cleanup registration of __gdb_builtin_type_bound128 pretty-printer
>
> On Mon, Oct 6, 2014 at 12:33 AM, Tedeschi, Walfred <walfred.tedeschi@intel.com> wrote:
>> Hi Doug,
>>
>> I have also submitted a patch for this last week. Though I was not creating a new directory for pretty printers.
>> https://sourceware.org/ml/gdb-patches/2014-09/msg00867.html
>>
>> I would like though to understand a bit more your comment on having x86 on the name of the register. What is the purpose of that?
>> (I could change it in case)
>
> Imagine a multi-arch gdb with several arches having bounds registers.
> How do we distinguish them in the pretty-printer namespace?
>
>> About registration of the pretty printer. We could do the following:
>> 1. Register an event listener for new obj_files.
>> 2. Query at every event the architecture of the  inferior. (In case we
>> had an event new_inferior loaded it would be cheaper.) 3. In case of x86 or amd64 we can then register the pretty-printer for the bound registers.
>
> We could do something like that for 7.9 alright.
> At the moment it feels like a bit of overkill just for one pretty-printer.
> And for 7.8.1 I want to do something simple.
> Intel GmbH
> Dornacher Strasse 1
> 85622 Feldkirchen/Muenchen, Deutschland
> Sitz der Gesellschaft: Feldkirchen bei Muenchen
> Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
> Registergericht: Muenchen HRB 47456
> Ust.-IdNr./VAT Registration No.: DE129385895
> Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052
  

Patch

diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in index 1e8cd4b..00c70bb 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -64,7 +64,6 @@  PYTHON_FILE_LIST = \
 	gdb/printing.py \
 	gdb/prompt.py \
 	gdb/xmethod.py \
-	gdb/command/bound_registers.py \
 	gdb/command/__init__.py \
 	gdb/command/xmethods.py \
 	gdb/command/frame_filters.py \
@@ -74,7 +73,9 @@  PYTHON_FILE_LIST = \
 	gdb/command/explore.py \
 	gdb/function/__init__.py \
 	gdb/function/caller_is.py \
-	gdb/function/strfns.py
+	gdb/function/strfns.py \
+	gdb/printer/__init__.py \
+	gdb/printer/bound_registers.py
 
 @HAVE_PYTHON_TRUE@PYTHON_FILES = $(PYTHON_FILE_LIST)  @HAVE_PYTHON_FALSE@PYTHON_FILES = diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py index 557e168..8c6eee2 100644
--- a/gdb/python/lib/gdb/__init__.py
+++ b/gdb/python/lib/gdb/__init__.py
@@ -81,7 +81,8 @@  PYTHONDIR = os.path.dirname(os.path.dirname(__file__))
 
 packages = [
     'function',
-    'command'
+    'command',
+    'printer'
 ]
 
 # pkgutil.iter_modules is not available prior to Python 2.6.  Instead, diff --git a/gdb/python/lib/gdb/command/bound_registers.py b/gdb/python/lib/gdb/command/bound_registers.py
deleted file mode 100644
index 24d4c45..0000000
--- a/gdb/python/lib/gdb/command/bound_registers.py
+++ /dev/null
@@ -1,45 +0,0 @@ 
-# Pretty-printer utilities.
-# Copyright (C) 2013-2014 Free Software Foundation, Inc.
-
-# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 3 of the License, or -# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the -# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License -# along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-import gdb.printing
-
-class BoundPrinter:
-    """Adds size field to a _rawbound128 type."""
-
-    def __init__ (self, val):
-        self.val = val
-
-    def to_string (self):
-        upper = self.val["ubound"]
-        lower = self.val["lbound"]
-        size  = (long) ((upper) - (lower))
-        if size > -1:
-            size = size + 1
-        result = '{lbound = %s, ubound = %s} : size %s' % (lower, upper, size)
-        return result
-
-# There are two pattern matching used: first one is related to a library -# second is related to the type. Since we are displaying a register all -# libraries are accepted. Type to be processed is the same present -# in the xml file.
-
-def build_pretty_printer ():
-    pp = gdb.printing.RegexpCollectionPrettyPrinter (".*")
-    pp.add_printer ('bound', '^__gdb_builtin_type_bound128', BoundPrinter)
-    return pp
-
-gdb.printing.register_pretty_printer (gdb.current_objfile (),
-                                      build_pretty_printer ())
diff --git a/gdb/python/lib/gdb/printer/__init__.py b/gdb/python/lib/gdb/printer/__init__.py
new file mode 100644
index 0000000..ca768c89
--- /dev/null
+++ b/gdb/python/lib/gdb/printer/__init__.py
@@ -0,0 +1,16 @@ 
+# Copyright (C) 2010-2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify 
+# it under the terms of the GNU General Public License as published by 
+# the Free Software Foundation; either version 3 of the License, or # 
+(at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful, # but 
+WITHOUT ANY WARRANTY; without even the implied warranty of # 
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the # GNU 
+General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License # 
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+
diff --git a/gdb/python/lib/gdb/printer/bound_registers.py b/gdb/python/lib/gdb/printer/bound_registers.py
new file mode 100644
index 0000000..e585dcb
--- /dev/null
+++ b/gdb/python/lib/gdb/printer/bound_registers.py
@@ -0,0 +1,36 @@ 
+# Pretty-printer utilities.
+# Copyright (C) 2013-2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify 
+# it under the terms of the GNU General Public License as published by 
+# the Free Software Foundation; either version 3 of the License, or # 
+(at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful, # but 
+WITHOUT ANY WARRANTY; without even the implied warranty of # 
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the # GNU 
+General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License # 
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import gdb.printing
+
+class Bound128Printer:
+    """Adds size field to a _rawbound128 type."""
+
+    def __init__ (self, val):
+        self.val = val
+
+    def to_string (self):
+        upper = self.val["ubound"]
+        lower = self.val["lbound"]
+        size  = (long) ((upper) - (lower))
+        if size > -1:
+            size = size + 1
+        result = '{lbound = %s, ubound = %s} : size %s' % (lower, upper, size)
+        return result
+
+gdb.printing.add_builtin_pretty_printer ('bound128',
+                                         '^__gdb_builtin_type_bound128',
+                                         Bound128Printer)
diff --git a/gdb/python/lib/gdb/printing.py b/gdb/python/lib/gdb/printing.py index 2940b93..ff5250a 100644
--- a/gdb/python/lib/gdb/printing.py
+++ b/gdb/python/lib/gdb/printing.py
@@ -263,3 +263,17 @@  class FlagEnumerationPrinter(PrettyPrinter):
             return _EnumInstance(self.enumerators, val)
         else:
             return None
+
+
+# Builtin pretty-printers.
+# The set is defined as empty, and files in printing/*.py add their 
+printers # to this with add_builtin_pretty_printer.
+
+_builtin_pretty_printers = RegexpCollectionPrettyPrinter("builtin")
+
+register_pretty_printer(None, _builtin_pretty_printers)
+
+# Add a builtin pretty-printer.
+
+def add_builtin_pretty_printer(name, regexp, printer):
+    _builtin_pretty_printers.add_printer(name, regexp, printer)