Increase size of main_type::nfields
authorTom Tromey <tromey@adacore.com>
Wed, 11 Jan 2023 19:42:40 +0000 (12:42 -0700)
committerTom Tromey <tromey@adacore.com>
Thu, 9 Feb 2023 14:55:34 +0000 (07:55 -0700)
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
gdb/gdb-gdb.py.in
gdb/gdbtypes.h
gdb/testsuite/gdb.dwarf2/mega-enum.exp [new file with mode: 0644]

index 55661156b86bc1aca8547cda60638d0dfab47652..d48aa61b4bc4f791b084a0e4c4c703a248e17041 100644 (file)
@@ -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)
     {
index dbc4d773e0b60ba9b5b5204dd4a57cdead58241a..95b7d84966f055ed59a19a3e4100bd4ff4452bdc 100644 (file)
@@ -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())
index 18ea1bca19c9db8960a19f3c2548f2d26648f059..aaed12f97eec545b45ac5f05949e89f8f446cdf2 100644 (file)
@@ -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 (file)
index 0000000..9d47931
--- /dev/null
@@ -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"