From patchwork Mon Aug 15 19:27:59 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Evans X-Patchwork-Id: 14592 Received: (qmail 93649 invoked by alias); 15 Aug 2016 19:28:12 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 93624 invoked by uid 89); 15 Aug 2016 19:28:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.1 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=89, H*MI:sk:94eb2c0, H*M:sk:94eb2c0, sized X-HELO: mail-pa0-f74.google.com Received: from mail-pa0-f74.google.com (HELO mail-pa0-f74.google.com) (209.85.220.74) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 15 Aug 2016 19:28:01 +0000 Received: by mail-pa0-f74.google.com with SMTP id eq16so3596901pac.0 for ; Mon, 15 Aug 2016 12:28:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:message-id:date:subject:from:to:cc; bh=GSXhbH9SwWPs7Toa12C/GHTYXUubol67GyolHcORDW8=; b=k8x6W4TWft3Vyrnpvp9stmy+xASOx+e25NfkcbrOmMKdb46tj/EFPQHb3IVJC8mUiX rkYQwuMoIomYf5jDUKR4gDq3h+3SsFkdKr2n4gEngsctPbiPVP6YEqPDlDBDqAKLhsj/ MNhMUATGN4UkItfuSyRckAzuGtJFQgx0/SX/4zP1zfyvIzTjz3buuPLpGLsaIBEiSJaW A77TwYrt8lStPjq3+gsgRA/Lmk6mBWF4x+sX2Lo0c/oAyOXdIr75GSjFZHzJ9Wi+GsLH 4bvfrMCfbW+mtDePw5OTOdOU+aH+y5P4I7HuVswbUWFyoA+kOpbLYcqwxIKmEg+cuNmy mckQ== X-Gm-Message-State: AEkoouvIaT9tu5PAUKp32TON1C9YzoP5C1zymysIZaSAyHe5vX6ggHuP9CMtPpHMmD/DaCIOlw== MIME-Version: 1.0 X-Received: by 10.98.23.84 with SMTP id 81mr22135908pfx.7.1471289279692; Mon, 15 Aug 2016 12:27:59 -0700 (PDT) Message-ID: <94eb2c03bed8e14298053a213ae2@google.com> Date: Mon, 15 Aug 2016 19:27:59 +0000 Subject: Re: [PATCH 4/5]: Enhancements to "flags": i386 cleanup From: Doug Evans To: Pedro Alves Cc: gdb-patches , Wei-cheng Wang X-IsSubscribed: yes Pedro Alves writes: > On 08/08/2016 09:33 PM, Doug Evans wrote: > > > Hi. > > Sorry for the trouble. > > > > I think(!) these two patches fix things. > > Basically, I made "end" required again, and made single bit fields > > default to bool, > > and then tweaked a couple of xml files to minimize changes. > > > > https://sourceware.org/ml/gdb-patches/2016-08/msg00105.html > > https://sourceware.org/ml/gdb-patches/2016-08/msg00106.html > > > > It occurs to me I need to update the docs too. > > Additional patch to follow. > > Thanks! All looks good to me. Here is what I committed (to main and gdb-7.12-branch). Same as before except I needed to update a testcase. 2016-08-15 Doug Evans * features/aarch64-core.xml (cpsr_flags): Elide "type" and specify "end" in all fields. * features/aarch64.c: Regenerate. * features/i386/32bit-mpx.xml (_bndcfgu): Specify type of "preserved" and "enabled" fields. Correct size of "enabled" field. * features/i386/64bit-mpx.xml (_bndcfgu): Specify type of "preserved" and "enabled" fields. * features/i386/i386-avx-mpx-linux.c: Regenerate. * features/i386/i386-avx-mpx.c: Regenerate. * features/i386/i386-avx512-linux.c: Regenerate. * features/i386/i386-avx512.c: Regenerate. * features/i386/i386-mpx-linux.c: Regenerate. * features/i386/i386-mpx.c: Regenerate. * xml-tdesc.c (tdesc_start_field): Require "end" spec. Single bit fields default to "bool" type. Revert 2016-03-15 Doug Evans * features/i386/32bit-core.xml (i386_eflags): Remove "end" spec. * features/i386/32bit-sse.xml (i386_eflags): Ditto. * features/i386/64bit-core.xml (i386_eflags): Ditto. * features/i386/64bit-sse.xml (i386_eflags): Ditto. * features/i386/x32-core.xml (i386_eflags): Ditto. doc/ * gdb.texinfo (Target Description Format): Update docs on "end" field spec and field default type. testsuite/ * gdb.xml/extra-regs.xml: Update, end field now required, default type for single bitfields is bool. * gdb.xml/tdesc-regs.exp: Ditto. diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index f5dde61..0b5ec39 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -40681,16 +40681,13 @@ Bitfield values may be named with the empty string, @samp{""}, in which case the field is ``filler'' and its value is not printed. Not all bits need to be specified, so ``filler'' fields are optional. -The @var{start} value is required, and @var{end} and @var{type} -are optional. +The @var{start} and @var{end} values are required, and @var{type} +is optional. The field's @var{start} must be less than or equal to its @var{end}, and zero represents the least significant bit. -The default value of @var{end} is @var{start}, a single bit field. -The default value of @var{type} depends on whether the -@var{end} was specified. If @var{end} is specified then the default -value of @var{type} is an unsigned integer. If @var{end} is unspecified -then the default value of @var{type} is @code{bool}. +The default value of @var{type} is @code{bool} for single bit fields, +and an unsigned integer otherwise. Which to choose? Structures or flags? diff --git a/gdb/features/aarch64-core.xml b/gdb/features/aarch64-core.xml index 8f96296..7ff064d 100644 --- a/gdb/features/aarch64-core.xml +++ b/gdb/features/aarch64-core.xml @@ -44,23 +44,23 @@ - + - + - - - - + + + + - - + + - - - - + + + + diff --git a/gdb/features/aarch64.c b/gdb/features/aarch64.c index cec6956..e9eaed8 100644 --- a/gdb/features/aarch64.c +++ b/gdb/features/aarch64.c @@ -19,10 +19,10 @@ initialize_tdesc_aarch64 (void) feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.core"); type = tdesc_create_flags (feature, "cpsr_flags", 4); tdesc_add_flag (type, 0, "SP"); - tdesc_add_bitfield (type, "", 1, 1); + tdesc_add_flag (type, 1, ""); tdesc_add_bitfield (type, "EL", 2, 3); tdesc_add_flag (type, 4, "nRW"); - tdesc_add_bitfield (type, "", 5, 5); + tdesc_add_flag (type, 5, ""); tdesc_add_flag (type, 6, "F"); tdesc_add_flag (type, 7, "I"); tdesc_add_flag (type, 8, "A"); diff --git a/gdb/features/i386/32bit-core.xml b/gdb/features/i386/32bit-core.xml index b00d913..a27863f 100644 --- a/gdb/features/i386/32bit-core.xml +++ b/gdb/features/i386/32bit-core.xml @@ -8,23 +8,23 @@ - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + diff --git a/gdb/features/i386/32bit-mpx.xml b/gdb/features/i386/32bit-mpx.xml index b1c0615..8d319cf 100644 --- a/gdb/features/i386/32bit-mpx.xml +++ b/gdb/features/i386/32bit-mpx.xml @@ -25,8 +25,10 @@ - - + + + diff --git a/gdb/features/i386/32bit-sse.xml b/gdb/features/i386/32bit-sse.xml index 4448a7e..5a44d1e 100644 --- a/gdb/features/i386/32bit-sse.xml +++ b/gdb/features/i386/32bit-sse.xml @@ -23,20 +23,20 @@ - - - - - - - - - - - - - - + + + + + + + + + + + + + + diff --git a/gdb/features/i386/64bit-core.xml b/gdb/features/i386/64bit-core.xml index 6e847c1..92f4e87 100644 --- a/gdb/features/i386/64bit-core.xml +++ b/gdb/features/i386/64bit-core.xml @@ -8,23 +8,23 @@ - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + diff --git a/gdb/features/i386/64bit-mpx.xml b/gdb/features/i386/64bit-mpx.xml index 279b537..877cc0f 100644 --- a/gdb/features/i386/64bit-mpx.xml +++ b/gdb/features/i386/64bit-mpx.xml @@ -25,8 +25,9 @@ - - + + + diff --git a/gdb/features/i386/64bit-sse.xml b/gdb/features/i386/64bit-sse.xml index dd6a850..2a5271e 100644 --- a/gdb/features/i386/64bit-sse.xml +++ b/gdb/features/i386/64bit-sse.xml @@ -23,20 +23,20 @@ - - - - - - - - - - - - - - + + + + + + + + + + + + + + diff --git a/gdb/features/i386/i386-avx-mpx-linux.c b/gdb/features/i386/i386-avx-mpx-linux.c index 941f2b3..4b27bfc 100644 --- a/gdb/features/i386/i386-avx-mpx-linux.c +++ b/gdb/features/i386/i386-avx-mpx-linux.c @@ -168,7 +168,7 @@ initialize_tdesc_i386_avx_mpx_linux (void) tdesc_add_bitfield (type, "base", 12, 31); tdesc_add_bitfield (type, "reserved", 2, 11); tdesc_add_bitfield (type, "preserved", 1, 1); - tdesc_add_bitfield (type, "enabled", 0, 1); + tdesc_add_bitfield (type, "enabled", 0, 0); type = tdesc_create_union (feature, "cfgu"); field_type = tdesc_named_type (feature, "data_ptr"); diff --git a/gdb/features/i386/i386-avx-mpx.c b/gdb/features/i386/i386-avx-mpx.c index d822aac..b27b40a 100644 --- a/gdb/features/i386/i386-avx-mpx.c +++ b/gdb/features/i386/i386-avx-mpx.c @@ -163,7 +163,7 @@ initialize_tdesc_i386_avx_mpx (void) tdesc_add_bitfield (type, "base", 12, 31); tdesc_add_bitfield (type, "reserved", 2, 11); tdesc_add_bitfield (type, "preserved", 1, 1); - tdesc_add_bitfield (type, "enabled", 0, 1); + tdesc_add_bitfield (type, "enabled", 0, 0); type = tdesc_create_union (feature, "cfgu"); field_type = tdesc_named_type (feature, "data_ptr"); diff --git a/gdb/features/i386/i386-avx512-linux.c b/gdb/features/i386/i386-avx512-linux.c index 47a3319..0d3ab22 100644 --- a/gdb/features/i386/i386-avx512-linux.c +++ b/gdb/features/i386/i386-avx512-linux.c @@ -168,7 +168,7 @@ initialize_tdesc_i386_avx512_linux (void) tdesc_add_bitfield (type, "base", 12, 31); tdesc_add_bitfield (type, "reserved", 2, 11); tdesc_add_bitfield (type, "preserved", 1, 1); - tdesc_add_bitfield (type, "enabled", 0, 1); + tdesc_add_bitfield (type, "enabled", 0, 0); type = tdesc_create_union (feature, "cfgu"); field_type = tdesc_named_type (feature, "data_ptr"); diff --git a/gdb/features/i386/i386-avx512.c b/gdb/features/i386/i386-avx512.c index 6e8cb55..1cb68a1 100644 --- a/gdb/features/i386/i386-avx512.c +++ b/gdb/features/i386/i386-avx512.c @@ -163,7 +163,7 @@ initialize_tdesc_i386_avx512 (void) tdesc_add_bitfield (type, "base", 12, 31); tdesc_add_bitfield (type, "reserved", 2, 11); tdesc_add_bitfield (type, "preserved", 1, 1); - tdesc_add_bitfield (type, "enabled", 0, 1); + tdesc_add_bitfield (type, "enabled", 0, 0); type = tdesc_create_union (feature, "cfgu"); field_type = tdesc_named_type (feature, "data_ptr"); diff --git a/gdb/features/i386/i386-mpx-linux.c b/gdb/features/i386/i386-mpx-linux.c index 298b7ff..43ea192 100644 --- a/gdb/features/i386/i386-mpx-linux.c +++ b/gdb/features/i386/i386-mpx-linux.c @@ -158,7 +158,7 @@ initialize_tdesc_i386_mpx_linux (void) tdesc_add_bitfield (type, "base", 12, 31); tdesc_add_bitfield (type, "reserved", 2, 11); tdesc_add_bitfield (type, "preserved", 1, 1); - tdesc_add_bitfield (type, "enabled", 0, 1); + tdesc_add_bitfield (type, "enabled", 0, 0); type = tdesc_create_union (feature, "cfgu"); field_type = tdesc_named_type (feature, "data_ptr"); diff --git a/gdb/features/i386/i386-mpx.c b/gdb/features/i386/i386-mpx.c index c19af55..e832d2e 100644 --- a/gdb/features/i386/i386-mpx.c +++ b/gdb/features/i386/i386-mpx.c @@ -153,7 +153,7 @@ initialize_tdesc_i386_mpx (void) tdesc_add_bitfield (type, "base", 12, 31); tdesc_add_bitfield (type, "reserved", 2, 11); tdesc_add_bitfield (type, "preserved", 1, 1); - tdesc_add_bitfield (type, "enabled", 0, 1); + tdesc_add_bitfield (type, "enabled", 0, 0); type = tdesc_create_union (feature, "cfgu"); field_type = tdesc_named_type (feature, "data_ptr"); diff --git a/gdb/features/i386/x32-core.xml b/gdb/features/i386/x32-core.xml index c03cdea..ab51ffc 100644 --- a/gdb/features/i386/x32-core.xml +++ b/gdb/features/i386/x32-core.xml @@ -8,23 +8,23 @@ - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + diff --git a/gdb/testsuite/gdb.xml/extra-regs.xml b/gdb/testsuite/gdb.xml/extra-regs.xml index cbbaf76..997d659 100644 --- a/gdb/testsuite/gdb.xml/extra-regs.xml +++ b/gdb/testsuite/gdb.xml/extra-regs.xml @@ -15,12 +15,12 @@ - + - + @@ -31,18 +31,16 @@ - - - - - - - - + + + + + + + + - - - + diff --git a/gdb/testsuite/gdb.xml/tdesc-regs.exp b/gdb/testsuite/gdb.xml/tdesc-regs.exp index c197e28..e7d8c74 100644 --- a/gdb/testsuite/gdb.xml/tdesc-regs.exp +++ b/gdb/testsuite/gdb.xml/tdesc-regs.exp @@ -170,9 +170,9 @@ gdb_test "ptype \$structreg.v4" "type = int8_t __attribute__ \\(\\(vector_size\\ gdb_test "ptype \$bitfields" \ "type = struct struct2 {\r\n *uint64_t f1 : 35;\r\n *uint64_t f2 : 1;\r\n}" gdb_test "ptype \$flags" \ - "type = flag flags {\r\n *uint32_t X @0;\r\n *uint32_t Y @2;\r\n}" + "type = flag flags {\r\n *bool X @0;\r\n *uint32_t Y @2;\r\n}" gdb_test "ptype \$mixed_flags" \ - "type = flag mixed_flags {\r\n *bool A @0;\r\n *uint32_t B @1;\r\n *bool C @2;\r\n *uint32_t D @3;\r\n *uint32_t @4-5;\r\n *uint32_t E @6-7;\r\n *enum {yes = 1, no = 0, maybe = 2, so} Z @8-9;\r\n}" + "type = flag mixed_flags {\r\n *bool A @0;\r\n *uint32_t B @1-3;\r\n *bool C @4;\r\n *uint32_t D @5;\r\n *uint32_t @6-7;\r\n *enum {yes = 1, no = 0, maybe = 2, so} Z @8-9;\r\n}" load_description "core-only.xml" "" "test-regs.xml" # The extra register from the previous description should be gone. diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c index aa58385..eeaf79b 100644 --- a/gdb/xml-tdesc.c +++ b/gdb/xml-tdesc.c @@ -382,13 +382,19 @@ tdesc_start_field (struct gdb_xml_parser *parser, { struct tdesc_type *t = data->current_type; + /* Older versions of gdb can't handle elided end values. + Stick with that for now, to help ensure backward compatibility. + E.g., If a newer gdbserver is talking to an older gdb. */ + if (end == -1) + gdb_xml_error (parser, _("Missing end value")); + if (data->current_type_size == 0) gdb_xml_error (parser, _("Bitfields must live in explicitly sized types")); if (field_type_id != NULL && strcmp (field_type_id, "bool") == 0 - && !(start == end || end == -1)) + && start != end) { gdb_xml_error (parser, _("Boolean fields must be one bit in size")); @@ -400,29 +406,20 @@ tdesc_start_field (struct gdb_xml_parser *parser, "64 bits (unsupported)"), field_name); - if (end != -1) - { - /* Assume that the bit numbering in XML is "lsb-zero". Most - architectures other than PowerPC use this ordering. In the - future, we can add an XML tag to indicate "msb-zero" - numbering. */ - if (start > end) - gdb_xml_error (parser, _("Bitfield \"%s\" has start after end"), - field_name); - if (end >= data->current_type_size * TARGET_CHAR_BIT) - gdb_xml_error (parser, - _("Bitfield \"%s\" does not fit in struct")); - } + /* Assume that the bit numbering in XML is "lsb-zero". Most + architectures other than PowerPC use this ordering. In the + future, we can add an XML tag to indicate "msb-zero" numbering. */ + if (start > end) + gdb_xml_error (parser, _("Bitfield \"%s\" has start after end"), + field_name); + if (end >= data->current_type_size * TARGET_CHAR_BIT) + gdb_xml_error (parser, + _("Bitfield \"%s\" does not fit in struct")); - if (end == -1) - { - if (field_type != NULL) - tdesc_add_typed_bitfield (t, field_name, start, start, field_type); - else - tdesc_add_flag (t, start, field_name); - } - else if (field_type != NULL) + if (field_type != NULL) tdesc_add_typed_bitfield (t, field_name, start, end, field_type); + else if (start == end) + tdesc_add_flag (t, start, field_name); else tdesc_add_bitfield (t, field_name, start, end); }