gdb: don't treat empty enums as flag enums

Message ID 92a46a82cbc43033fc03942674b80979cc91e572.1676901184.git.aburgess@redhat.com
State Committed
Commit 85c7cb3c4b70cc484ecf3d72a116503876a28f0a
Headers
Series gdb: don't treat empty enums as flag enums |

Commit Message

Andrew Burgess Feb. 20, 2023, 1:53 p.m. UTC
  In C++ it is possible to use an empty enum as a strong typedef.  For
example, a user could write:

  enum class my_type : unsigned char {};

Now my_type can be used like 'unsigned char' except the compiler will
not allow implicit conversion too and from the native 'unsigned char'
type.

This is used in the standard library for things like std::byte.

Currently, when GDB prints a value of type my_type, it looks like
this:

  (gdb) print my_var
  $1 = (unknown: 0x4)

Which isn't great.  This gets worse when we consider something like:

  std::vector<my_type> vec;

When using a pretty-printer, this could look like this:

  std::vector of length 2, capacity 2 = {(unknown: 0x2), (unknown: 0x4)}

Clearly not great.  This is described in PR gdb/30148.

The problem here is in dwarf2/read.c, we assume all enums are flag
enums unless we find an enumerator with a non-flag like value.
Clearly an empty enum contains no non-flag values, so we assume the
enum is a flag enum.

I propose adding an extra check here; that is, an empty enum should
never be a flag enum.

With this the above cases look more like:

  (gdb) print my_var
  $1 = (unknown: 0x4)

and:

  std::vector of length 2, capacity 2 = {2, 4}

Which look much better.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30148
---
 gdb/dwarf2/read.c                   |  2 ++
 gdb/testsuite/gdb.cp/empty-enum.cc  | 31 +++++++++++++++++++
 gdb/testsuite/gdb.cp/empty-enum.exp | 48 +++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+)
 create mode 100644 gdb/testsuite/gdb.cp/empty-enum.cc
 create mode 100644 gdb/testsuite/gdb.cp/empty-enum.exp


base-commit: 733da2ced8f948b299bdd10b7ff40146095f43f9
  

Comments

Andreas Schwab Feb. 20, 2023, 1:58 p.m. UTC | #1
On Feb 20 2023, Andrew Burgess via Gdb-patches wrote:

> In C++ it is possible to use an empty enum as a strong typedef.  For
> example, a user could write:
>
>   enum class my_type : unsigned char {};
>
> Now my_type can be used like 'unsigned char' except the compiler will
> not allow implicit conversion too and from the native 'unsigned char'
> type.
>
> This is used in the standard library for things like std::byte.
>
> Currently, when GDB prints a value of type my_type, it looks like
> this:
>
>   (gdb) print my_var
>   $1 = (unknown: 0x4)
>
> Which isn't great.  This gets worse when we consider something like:
>
>   std::vector<my_type> vec;
>
> When using a pretty-printer, this could look like this:
>
>   std::vector of length 2, capacity 2 = {(unknown: 0x2), (unknown: 0x4)}
>
> Clearly not great.  This is described in PR gdb/30148.
>
> The problem here is in dwarf2/read.c, we assume all enums are flag
> enums unless we find an enumerator with a non-flag like value.
> Clearly an empty enum contains no non-flag values, so we assume the
> enum is a flag enum.
>
> I propose adding an extra check here; that is, an empty enum should
> never be a flag enum.
>
> With this the above cases look more like:
>
>   (gdb) print my_var
>   $1 = (unknown: 0x4)

I guess this should look different from the output above?
  
Andrew Burgess Feb. 20, 2023, 2:29 p.m. UTC | #2
Andreas Schwab <schwab@suse.de> writes:

> On Feb 20 2023, Andrew Burgess via Gdb-patches wrote:
>
>> In C++ it is possible to use an empty enum as a strong typedef.  For
>> example, a user could write:
>>
>>   enum class my_type : unsigned char {};
>>
>> Now my_type can be used like 'unsigned char' except the compiler will
>> not allow implicit conversion too and from the native 'unsigned char'
>> type.
>>
>> This is used in the standard library for things like std::byte.
>>
>> Currently, when GDB prints a value of type my_type, it looks like
>> this:
>>
>>   (gdb) print my_var
>>   $1 = (unknown: 0x4)
>>
>> Which isn't great.  This gets worse when we consider something like:
>>
>>   std::vector<my_type> vec;
>>
>> When using a pretty-printer, this could look like this:
>>
>>   std::vector of length 2, capacity 2 = {(unknown: 0x2), (unknown: 0x4)}
>>
>> Clearly not great.  This is described in PR gdb/30148.
>>
>> The problem here is in dwarf2/read.c, we assume all enums are flag
>> enums unless we find an enumerator with a non-flag like value.
>> Clearly an empty enum contains no non-flag values, so we assume the
>> enum is a flag enum.
>>
>> I propose adding an extra check here; that is, an empty enum should
>> never be a flag enum.
>>
>> With this the above cases look more like:
>>
>>   (gdb) print my_var
>>   $1 = (unknown: 0x4)
>
> I guess this should look different from the output above?

Oops.  Yeah, I updated the std::vector example below, but this one
should be:

  (gdb) print my_var
  $1 = 0x4

I've updated the commit message locally.

Thanks,
Andrew
  
Tom Tromey Feb. 21, 2023, 7:42 p.m. UTC | #3
>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> In C++ it is possible to use an empty enum as a strong typedef.  For
Andrew> example, a user could write:

Andrew>   enum class my_type : unsigned char {};

Andrew> Now my_type can be used like 'unsigned char' except the compiler will
Andrew> not allow implicit conversion too and from the native 'unsigned char'
Andrew> type.

...
Andrew> I propose adding an extra check here; that is, an empty enum should
Andrew> never be a flag enum.

Looks good to me, thank you.

Reviewed-By: Tom Tromey <tom@tromey.com>

Tom
  
Andrew Burgess Feb. 27, 2023, 2:15 p.m. UTC | #4
Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Andrew> In C++ it is possible to use an empty enum as a strong typedef.  For
> Andrew> example, a user could write:
>
> Andrew>   enum class my_type : unsigned char {};
>
> Andrew> Now my_type can be used like 'unsigned char' except the compiler will
> Andrew> not allow implicit conversion too and from the native 'unsigned char'
> Andrew> type.
>
> ...
> Andrew> I propose adding an extra check here; that is, an empty enum should
> Andrew> never be a flag enum.
>
> Looks good to me, thank you.
>
> Reviewed-By: Tom Tromey <tom@tromey.com>

Pushed.

Thanks,
Andrew
  

Patch

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index eb37c776989..df7f2da555c 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -13178,6 +13178,8 @@  update_enumeration_type_from_children (struct die_info *die,
       memcpy (type->fields (), fields.data (),
 	      sizeof (struct field) * fields.size ());
     }
+  else
+    flag_enum = 0;
 
   if (unsigned_enum)
     type->set_is_unsigned (true);
diff --git a/gdb/testsuite/gdb.cp/empty-enum.cc b/gdb/testsuite/gdb.cp/empty-enum.cc
new file mode 100644
index 00000000000..1a0eeced710
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/empty-enum.cc
@@ -0,0 +1,31 @@ 
+/* 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/>.  */
+
+enum enum1 {};
+
+enum class enum2 : unsigned char {};
+
+void
+breakpt (enum1 arg1, enum2 arg2)
+{
+  /* Nothing.  */
+}
+
+int
+main ()
+{
+  breakpt ((enum1) 8, (enum2) 4);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.cp/empty-enum.exp b/gdb/testsuite/gdb.cp/empty-enum.exp
new file mode 100644
index 00000000000..83cb8cb3973
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/empty-enum.exp
@@ -0,0 +1,48 @@ 
+# 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 how GDB displays empty enums.  At one point an enum with no
+# enumeration values would be considered a flag enum, and, as a
+# consequence any value with that type would display like:
+#
+#   (gdb) print enum_var
+#   $1 = (unknown: 0x8)
+#
+# Which resulted in a lot of noise.  Now GDB treats empty enums as a
+# non-flag enum, and should print them like this:
+#
+#   (gdb) print enum_var
+#   $1 = 8
+#
+# This test checks this behaviour.
+
+standard_testfile .cc
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile]} {
+    return -1
+}
+
+if {![runto_main]} {
+    return -1
+}
+
+gdb_breakpoint "breakpt"
+gdb_continue_to_breakpoint "stop in breakpt"
+
+
+gdb_test "print arg1" " = 8"
+gdb_test "print arg2" " = 4"
+
+gdb_test "ptype arg1" "type = enum enum1 : unsigned int \\{\\}"
+gdb_test "ptype arg2" "type = enum class enum2 : unsigned char \\{\\}"