public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2] Increase size of main_type::nfields
@ 2023-01-27 15:11 Tom Tromey
  2023-02-09 15:01 ` Tom Tromey
  0 siblings, 1 reply; 2+ messages in thread
From: Tom Tromey @ 2023-01-27 15:11 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

main_type::nfields is a 'short', and has been for many years.  PR
c++/29985 points out that 'short' is too narrow for an enum that
contains more than 2^15 constants.

This patch bumps the size of 'nfields'.  To verify that the field
isn't directly used, it is also renamed.  Note that this does not
affect the size of main_type on x86-64 Fedora 36.  And, if it does
have a negative effect somewhere, it's worth considering that types
could be shrunk more drastically by using subclasses for the different
codes.

This is v2 of this patch, which has these changes:

* I changed nfields to 'unsigned', per Simon's request.  I looked at
  changing all the uses, but this quickly fans out into a very large
  patch.  (One additional tweak was needed, though.)

* I wrote a test case.  I discovered that GCC cannot compile a large
  enough C test case, so I resorted to using the DWARF assembler.
  This test doesn't reproduce the crash, but it does fail without the
  patch.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29985
---
 gdb/compile/compile-cplus-types.c      |  2 +-
 gdb/gdb-gdb.py.in                      |  4 +-
 gdb/gdbtypes.h                         | 10 ++--
 gdb/testsuite/gdb.dwarf2/mega-enum.exp | 67 ++++++++++++++++++++++++++
 4 files changed, 75 insertions(+), 8 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/mega-enum.exp

diff --git a/gdb/compile/compile-cplus-types.c b/gdb/compile/compile-cplus-types.c
index 55661156b86..d48aa61b4bc 100644
--- a/gdb/compile/compile-cplus-types.c
+++ b/gdb/compile/compile-cplus-types.c
@@ -981,7 +981,7 @@ compile_cplus_convert_func (compile_cplus_instance *instance,
   gcc_type return_type = instance->convert_type (target_type);
 
   std::vector<gcc_type> elements (type->num_fields ());
-  struct gcc_type_array array = { type->num_fields (), elements.data () };
+  struct gcc_type_array array = { (int) type->num_fields (), elements.data () };
   int artificials = 0;
   for (int i = 0; i < type->num_fields (); ++i)
     {
diff --git a/gdb/gdb-gdb.py.in b/gdb/gdb-gdb.py.in
index dbc4d773e0b..95b7d84966f 100644
--- a/gdb/gdb-gdb.py.in
+++ b/gdb/gdb-gdb.py.in
@@ -261,8 +261,8 @@ class StructMainTypePrettyPrinter:
         fields.append("flags = [%s]" % self.flags_to_string())
         fields.append("owner = %s" % self.owner_to_string())
         fields.append("target_type = %s" % self.val["m_target_type"])
-        if self.val["nfields"] > 0:
-            for fieldno in range(self.val["nfields"]):
+        if self.val["m_nfields"] > 0:
+            for fieldno in range(self.val["m_nfields"]):
                 fields.append(self.struct_field_img(fieldno))
         if self.val["code"] == gdb.TYPE_CODE_RANGE:
             fields.append(self.bounds_img())
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 18ea1bca19c..aaed12f97ee 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -818,7 +818,7 @@ struct main_type
   /* * Number of fields described for this type.  This field appears
      at this location because it packs nicely here.  */
 
-  short nfields;
+  unsigned int m_nfields;
 
   /* * Name of this type, or NULL if none.
 
@@ -944,15 +944,15 @@ struct type
   }
 
   /* Get the number of fields of this type.  */
-  int num_fields () const
+  unsigned int num_fields () const
   {
-    return this->main_type->nfields;
+    return this->main_type->m_nfields;
   }
 
   /* Set the number of fields of this type.  */
-  void set_num_fields (int num_fields)
+  void set_num_fields (unsigned int num_fields)
   {
-    this->main_type->nfields = num_fields;
+    this->main_type->m_nfields = num_fields;
   }
 
   /* Get the fields array of this type.  */
diff --git a/gdb/testsuite/gdb.dwarf2/mega-enum.exp b/gdb/testsuite/gdb.dwarf2/mega-enum.exp
new file mode 100644
index 00000000000..9d47931eb76
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/mega-enum.exp
@@ -0,0 +1,67 @@
+# Copyright 2023 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/>.
+
+# Test a huge enum.  A colossal enum.  If you will, a mega enum.
+
+load_lib dwarf.exp
+require dwarf2_support
+
+standard_testfile main.c mega-enum.S
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    global srcfile
+
+    cu {} {
+	DW_TAG_compile_unit {
+		{DW_AT_language @DW_LANG_C}
+		{DW_AT_name	$srcfile}
+		{DW_AT_comp_dir /tmp}
+	} {
+	    declare_labels integer_label
+
+	    integer_label: DW_TAG_base_type {
+		{DW_AT_byte_size 4 DW_FORM_sdata}
+		{DW_AT_encoding	 @DW_ATE_signed}
+		{DW_AT_name	 int}
+	    }
+
+	    DW_TAG_enumeration_type {
+		{DW_AT_name mega_enum}
+		{DW_AT_type :$integer_label}
+	    } {
+		# In the past gdb used a 'short' for the field count.
+		# But this fails if there are too many fields.	If the
+		# field count here is too high, gas takes a long time;
+		# so use a number that's just big enough.
+		for {set i 0} {$i < 65538} {incr i} {
+		    DW_TAG_enumerator {
+			{DW_AT_name DEI_$i}
+			{DW_AT_const_value $i DW_FORM_sdata}
+		    }
+		}
+	    }
+	}
+    }
+}
+
+if {[prepare_for_testing "failed to prepare" ${testfile} \
+	 [list $srcfile $asm_file] {nodebug}]} {
+    return -1
+}
+
+# The largest known Fermat prime.
+gdb_test "print/d DEI_65537" " = 65537"
+gdb_test "print (enum mega_enum) 65537" " = DEI_65537"
-- 
2.38.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] Increase size of main_type::nfields
  2023-01-27 15:11 [PATCH v2] Increase size of main_type::nfields Tom Tromey
@ 2023-02-09 15:01 ` Tom Tromey
  0 siblings, 0 replies; 2+ messages in thread
From: Tom Tromey @ 2023-02-09 15:01 UTC (permalink / raw)
  To: Tom Tromey via Gdb-patches; +Cc: Tom Tromey

>>>>> "Tom" == Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> main_type::nfields is a 'short', and has been for many years.  PR
Tom> c++/29985 points out that 'short' is too narrow for an enum that
Tom> contains more than 2^15 constants.

Tom> This patch bumps the size of 'nfields'.

I'm checking this in now.

Tom

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-02-09 15:01 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-27 15:11 [PATCH v2] Increase size of main_type::nfields Tom Tromey
2023-02-09 15:01 ` Tom Tromey

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).