From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15892 invoked by alias); 11 Jan 2012 16:45:38 -0000 Received: (qmail 15861 invoked by uid 22791); 11 Jan 2012 16:45:33 -0000 X-SWARE-Spam-Status: No, hits=-6.2 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 11 Jan 2012 16:45:18 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q0BGjIvO022602 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 11 Jan 2012 11:45:18 -0500 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q0BGjIiU008920; Wed, 11 Jan 2012 11:45:18 -0500 Received: from barimba (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id q0BGjGeA032418; Wed, 11 Jan 2012 11:45:17 -0500 From: Tom Tromey To: Jan Kratochvil Cc: gdb-patches@sourceware.org Subject: Re: RFC: enum pretty-printing support References: <20120110211713.GA1550@host2.jankratochvil.net> <20120110222424.GA4613@host2.jankratochvil.net> Date: Wed, 11 Jan 2012 16:46:00 -0000 In-Reply-To: <20120110222424.GA4613@host2.jankratochvil.net> (Jan Kratochvil's message of "Tue, 10 Jan 2012 23:24:24 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.92 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-01/txt/msg00356.txt.bz2 Tom> I think the C code has to be a bit more careful because it Tom> cannot readily be turned off. Tom> Tom> One idea is to have both. Jan> I agree. Here's a new patch. Let me know what you think. This adds basic flag enum printing to c-valprint.c. I looked at other languages, but I don't know their syntax enough to say whether this same approach is ok. Refactoring a bit to allow this later is trivial, if need be. Looking into this caught a bug in the original code. The "leftover value" code has to handle the case where the input value was 0. Otherwise, gdb will print nothing :) I only updated the DWARF reader since I know nothing about stabs; and, anyway, this is just a nicety. This version needs another doc review. I also added NEWS entries. Built and regtested on x86-64 F15. Tom b/gdb/ChangeLog: 2012-01-11 Tom Tromey PR python/13281: * gdbtypes.h (TYPE_FLAG_ENUM): New macro. (struct main_type) : New field. * dwarf2read.c (process_enumeration_scope): Detect "flag" enums. * NEWS: Add entries. * c-valprint.c (c_val_print) : Handle "flag" enums. * python/lib/gdb/printing.py (_EnumInstance): New class. (FlagEnumerationPrinter): Likewise. b/gdb/doc/ChangeLog: 2012-01-11 Tom Tromey * gdb.texinfo (gdb.printing): Document FlagEnumerationPrinter. b/gdb/testsuite/ChangeLog: 2012-01-11 Tom Tromey * gdb.base/printcmds.c (enum flag_enum): New. (three): New global. * gdb.base/printcmds.exp (test_print_enums): Add test for flag enum printing. * gdb.python/py-pp-maint.py (build_pretty_printer): Instantiate FlagEnumerationPrinter. * gdb.python/py-pp-maint.exp: Add tests for FlagEnumerationPrinter. * gdb.python/py-pp-maint.c (enum flag_enum): New. (fval): New global. diff --git a/gdb/NEWS b/gdb/NEWS index a9a7859..8119cf0 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -9,6 +9,18 @@ * The binary "gdbtui" can no longer be built or installed. Use "gdb -tui" instead. +* GDB will now print "flag" enums specially. A flag enum is one where + all the enumerator values have no bits in common when pairwise + "and"ed. When printing a value whose type is a flag enum, GDB will + show all the constants, e.g., for enum E { ONE = 1, TWO = 2}: + (gdb) print (enum E) 3 + $1 = [ONE | TWO] + +* Python scripting + + ** A new class, gdb.printing.FlagEnumerationPrinter, can be used to + apply "flag enum"-style pretty-printing to any enum. + *** Changes in GDB 7.4 * GDB now handles ambiguous linespecs more consistently; the existing diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c index 9949015..1d44e90 100644 --- a/gdb/c-valprint.c +++ b/gdb/c-valprint.c @@ -458,7 +458,38 @@ c_val_print (struct type *type, const gdb_byte *valaddr, } else { - print_longest (stream, 'd', 0, val); + if (TYPE_FLAG_ENUM (type)) + { + int first = 1; + + fputs_filtered ("[", stream); + for (i = 0; i < len; ++i) + { + QUIT; + + if ((val & TYPE_FIELD_BITPOS (type, i)) != 0) + { + if (!first) + fputs_filtered (" | ", stream); + first = 0; + + val &= ~ TYPE_FIELD_BITPOS (type, i); + fputs_filtered (TYPE_FIELD_NAME (type, i), stream); + } + } + + if (first || val != 0) + { + if (!first) + fputs_filtered (" | ", stream); + fputs_filtered ("unknown: ", stream); + print_longest (stream, 'd', 0, val); + } + + fputs_filtered ("]", stream); + } + else + print_longest (stream, 'd', 0, val); } break; diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 4a8ff7b..de2e390 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -24720,6 +24720,13 @@ Utility class for handling multiple printers, all recognized via regular expressions. @xref{Writing a Pretty-Printer}, for an example. +@item FlagEnumerationPrinter (@var{name}) +A pretty-printer which handles printing of @code{enum} values. Unlike +@value{GDBN}'s built-in @code{enum} printing, this printer attempts to +work properly when there is some overlap between the enumeration +constants. @var{name} is the name of the printer and also the name of +the @code{enum} type to look up. + @item register_pretty_printer (@var{obj}, @var{printer}, @var{replace}=False) Register @var{printer} with the pretty-printer list of @var{obj}. If @var{replace} is @code{True} then any existing copy of the printer diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 42dbac3..afb4337 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -7913,6 +7913,8 @@ process_enumeration_scope (struct die_info *die, struct dwarf2_cu *cu) int num_fields = 0; int unsigned_enum = 1; char *name; + int flag_enum = 1; + ULONGEST mask = 0; child_die = die->child; while (child_die && child_die->tag) @@ -7928,7 +7930,14 @@ process_enumeration_scope (struct die_info *die, struct dwarf2_cu *cu) { sym = new_symbol (child_die, this_type, cu); if (SYMBOL_VALUE (sym) < 0) - unsigned_enum = 0; + { + unsigned_enum = 0; + flag_enum = 0; + } + else if ((mask & SYMBOL_VALUE (sym)) != 0) + flag_enum = 0; + else + mask |= SYMBOL_VALUE (sym); if ((num_fields % DW_FIELD_ALLOC_CHUNK) == 0) { @@ -7961,6 +7970,8 @@ process_enumeration_scope (struct die_info *die, struct dwarf2_cu *cu) } if (unsigned_enum) TYPE_UNSIGNED (this_type) = 1; + if (flag_enum) + TYPE_FLAG_ENUM (this_type) = 1; } /* If we are reading an enum from a .debug_types unit, and the enum diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h index ddad0dc..f152945 100644 --- a/gdb/gdbtypes.h +++ b/gdb/gdbtypes.h @@ -290,6 +290,12 @@ enum type_instance_flag_value #define TYPE_DECLARED_CLASS(t) (TYPE_MAIN_TYPE (t)->flag_declared_class) +/* True if this type is a "flag" enum. A flag enum is one where all + the values are pairwise disjoint when "and"ed together. This + affects how enum values are printed. */ + +#define TYPE_FLAG_ENUM(t) (TYPE_MAIN_TYPE (t)->flag_flag_enum) + /* Constant type. If this is set, the corresponding type has a const modifier. */ @@ -399,6 +405,9 @@ struct main_type /* True if this type was declared with "class" rather than "struct". */ unsigned int flag_declared_class : 1; + /* True if this is an enum type with disjoint values. This affects + how the enum is printed. */ + unsigned int flag_flag_enum : 1; /* A discriminant telling us which field of the type_specific union is being used for this type, if any. */ diff --git a/gdb/python/lib/gdb/printing.py b/gdb/python/lib/gdb/printing.py index 98cfd27..0f399d0 100644 --- a/gdb/python/lib/gdb/printing.py +++ b/gdb/python/lib/gdb/printing.py @@ -206,3 +206,53 @@ class RegexpCollectionPrettyPrinter(PrettyPrinter): # Cannot find a pretty printer. Return None. return None + +# A helper class for printing enum types. This class is instantiated +# with a list of enumerators to print a particular Value. +class _EnumInstance: + def __init__(self, enumerators, val): + self.enumerators = enumerators + self.val = val + + def to_string(self): + flag_list = [] + v = long(self.val) + any_found = False + for (e_name, e_value) in self.enumerators: + if v & e_value != 0: + flag_list.append(e_name) + v = v & ~e_value + any_found = True + if not any_found or v != 0: + # Leftover value. + flag_list.append('' % v) + return "0x%x [%s]" % (self.val, " | ".join(flag_list)) + +class FlagEnumerationPrinter(PrettyPrinter): + """A pretty-printer which can be used to print a flag-style enumeration. + A flag-style enumeration is one where the enumerators are or'd + together to create values. The new printer will print these + symbolically using '|' notation. The printer must be registered + manually. This printer is most useful when an enum is flag-like, + but has some overlap. GDB's built-in printing will not handle + this case, but this printer will attempt to.""" + + def __init__(self, enum_type): + super(FlagEnumerationPrinter, self).__init__(enum_type) + self.initialized = False + + def __call__(self, val): + if not self.initialized: + self.initialized = True + flags = gdb.lookup_type(self.name) + self.enumerators = [] + for field in flags.fields(): + self.enumerators.append((field.name, field.bitpos)) + # Sorting the enumerators by value usually does the right + # thing. + self.enumerators.sort(key = lambda x: x.bitpos) + + if self.enabled: + return _EnumInstance(self.enumerators, val) + else: + return None diff --git a/gdb/testsuite/gdb.base/printcmds.c b/gdb/testsuite/gdb.base/printcmds.c index d37dfbd..743734b 100644 --- a/gdb/testsuite/gdb.base/printcmds.c +++ b/gdb/testsuite/gdb.base/printcmds.c @@ -96,6 +96,10 @@ enum some_volatile_enum { enumvolval1, enumvolval2 }; name. See PR11827. */ volatile enum some_volatile_enum some_volatile_enum = enumvolval1; +enum flag_enum { ONE = 1, TWO = 2 }; + +enum flag_enum three = ONE | TWO; + /* A structure with an embedded array at an offset > 0. The array has all elements with the same repeating value, which must not be the same as the value of the preceding fields in the structure for the diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp index d647ca8..36d3af6 100644 --- a/gdb/testsuite/gdb.base/printcmds.exp +++ b/gdb/testsuite/gdb.base/printcmds.exp @@ -701,6 +701,8 @@ proc test_print_array_constants {} { proc test_print_enums {} { # Regression test for PR11827. gdb_test "print some_volatile_enum" "enumvolval1" + + gdb_test "print three" " = \\\[ONE \\| TWO\\\]" } proc test_printf {} { diff --git a/gdb/testsuite/gdb.python/py-pp-maint.c b/gdb/testsuite/gdb.python/py-pp-maint.c index f65e5f7..e91193a 100644 --- a/gdb/testsuite/gdb.python/py-pp-maint.c +++ b/gdb/testsuite/gdb.python/py-pp-maint.c @@ -17,6 +17,16 @@ #include +enum flag_enum + { + FLAG_1 = 1, + FLAG_2 = 2, + FLAG_3 = 4, + ALL = FLAG_1 | FLAG_2 | FLAG_3 + }; + +enum flag_enum fval; + struct function_lookup_test { int x,y; diff --git a/gdb/testsuite/gdb.python/py-pp-maint.exp b/gdb/testsuite/gdb.python/py-pp-maint.exp index 1a2b213..5f591fb 100644 --- a/gdb/testsuite/gdb.python/py-pp-maint.exp +++ b/gdb/testsuite/gdb.python/py-pp-maint.exp @@ -74,23 +74,25 @@ gdb_test "print flt" " = x=<42> y=<43>" \ gdb_test "print ss" " = a= b=<$hex>> b= b=<$hex>>" \ "print ss enabled #1" +set num_pp 6 + gdb_test "disable pretty-printer" \ - "5 printers disabled.*0 of 5 printers enabled" + "$num_pp printers disabled.*0 of $num_pp printers enabled" gdb_test "enable pretty-printer" \ - "5 printers enabled.*5 of 5 printers enabled" + "$num_pp printers enabled.*$num_pp of $num_pp printers enabled" gdb_test "disable pretty-printer global" \ - "5 printers disabled.*0 of 5 printers enabled" + "$num_pp printers disabled.*0 of $num_pp printers enabled" gdb_test "enable pretty-printer" \ - "5 printers enabled.*5 of 5 printers enabled" + "$num_pp printers enabled.*$num_pp of $num_pp printers enabled" gdb_test "disable pretty-printer global lookup_function_lookup_test" \ - "1 printer disabled.*4 of 5 printers enabled" + "1 printer disabled.*[expr $num_pp - 1] of $num_pp printers enabled" gdb_test "disable pretty-printer global pp-test;.*" \ - "4 printers disabled.*0 of 5 printers enabled" + "[expr $num_pp - 1] printers disabled.*0 of $num_pp printers enabled" gdb_test "info pretty-printer global .*function" \ {.*function_lookup_test \[disabled\].*} @@ -105,19 +107,22 @@ gdb_test "print ss" " = {a = {a = 1, b = $hex}, b = {a = 2, b = $hex}}" \ "print ss disabled" gdb_test "enable pretty-printer global lookup_function_lookup_test" \ - "1 printer enabled.*1 of 5 printers enabled" + "1 printer enabled.*1 of $num_pp printers enabled" # This doesn't enable any printers because each subprinter in the collection # is still individually disabled. But this is still needed, to enable the # collection itself. gdb_test "enable pretty-printer global pp-test" \ - "0 printers enabled.*1 of 5 printers enabled" + "0 printers enabled.*1 of $num_pp printers enabled" gdb_test "enable pretty-printer global pp-test;.*ss.*" \ - "2 printers enabled.*3 of 5 printers enabled" + "2 printers enabled.*[expr $num_pp - 3] of $num_pp printers enabled" gdb_test "enable pretty-printer global pp-test;.*s.*" \ - "2 printers enabled.*5 of 5 printers enabled" + "2 printers enabled.*[expr $num_pp - 1] of $num_pp printers enabled" + +gdb_test "enable pretty-printer global pp-test;.*" \ + "1 printer enabled.*$num_pp of $num_pp printers enabled" gdb_test "info pretty-printer" \ {.*function_lookup_test.*pp-test.*struct ss.*} @@ -127,3 +132,18 @@ gdb_test "print flt" " = x=<42> y=<43>" \ gdb_test "print ss" " = a= b=<$hex>> b= b=<$hex>>" \ "print ss re-enabled" + +gdb_test "print (enum flag_enum) (FLAG_1)" \ + " = 0x1 .FLAG_1." \ + "print FLAG_1" + +gdb_test "print (enum flag_enum) (FLAG_1 | FLAG_3)" \ + " = 0x5 .FLAG_1 | FLAG_3." \ + "print FLAG_1 | FLAG_3" + +gdb_test "print (enum flag_enum) (4 + 8)" \ + " = 0xc .FLAG_1 | ." \ + "print FLAG_1 | 8" + + + diff --git a/gdb/testsuite/gdb.python/py-pp-maint.py b/gdb/testsuite/gdb.python/py-pp-maint.py index c988f88..1677371 100644 --- a/gdb/testsuite/gdb.python/py-pp-maint.py +++ b/gdb/testsuite/gdb.python/py-pp-maint.py @@ -67,6 +67,9 @@ def build_pretty_printer(): pp.add_printer('struct ss', '^struct ss$', lambda val: pp_ss(val)) pp.add_printer('ss', '^ss$', lambda val: pp_ss(val)) + pp.add_printer('enum flag_enum', '^flag_enum$', + gdb.printing.FlagEnumerationPrinter('enum flag_enum')) + return pp