gdb: fix printing of flag enums with multi-bit enumerators

GDB has this feature where if an enum looks like it is meant to
represent binary flags, it will present the values of that type as a
bitwise OR of the flags that are set in the value.

The original motivation for this patch is to fix this behavior:

  enum hello { AAA = 0x1, BBB = 0xf0 };

  (gdb) p (enum hello) 0x11
  $1 = (AAA | BBB)

This is wrong because the bits set in BBB (0xf0) are not all set in the
value 0x11, but GDB presents it as if they all were.

I think that enumerations with enumerators that have more than one bit
set should simply not qualify as "flag enum", as far as this
heuristic is concerned.  I'm not sure what it means to have flags of
more than one bit.  So this is what this patch implements.

I have added an assert in generic_val_print_enum_1 to make sure the flag
enum types respect that, in case they are used by other debug info
readers, in the future.

I've enhanced the gdb.base/printcmds.exp test to cover this case.  I've
also added tests for printing flag enums with value 0, both when the
enumeration has and doesn't have an enumerator for value 0.

gdb/ChangeLog:

	* dwarf2/read.c: Include "count-one-bits.h".
	(update_enumeration_type_from_children): If an enumerator has
	multiple bits set, don't treat the enumeration as a "flag enum".
	* valprint.c (generic_val_print_enum_1): Assert that enumerators
	of flag enums have 0 or 1 bit set.

gdb/testsuite/ChangeLog:

	* gdb.base/printcmds.c (enum flag_enum): Prefix enumerators with
	FE_, add FE_NONE.
	(three): Update.
	(enum flag_enum_without_zero): New enum.
	(flag_enum_without_zero): New variable.
	(enum not_flag_enum): New enum.
	(three_not_flag): New variable.
	* gdb.base/printcmds.exp (test_artificial_arrays): Update.
	(test_print_enums): Add more tests for printing flag enums.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ee7298b68757d7ea153024dbb65509780ee26baf..ca9f36474535fe1cbe6d27d3304d64497b1bc121 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2020-02-18  Simon Marchi  <simon.marchi@efficios.com>
+
+       * dwarf2/read.c: Include "count-one-bits.h".
+       (update_enumeration_type_from_children): If an enumerator has
+       multiple bits set, don't treat the enumeration as a "flag enum".
+       * valprint.c (generic_val_print_enum_1): Assert that enumerators
+       of flag enums have 0 or 1 bit set.
+
 2020-02-18  Bernd Edlinger  <bernd.edlinger@hotmail.de>
 
        * aarch64-tdep.c (aarch64_displaced_step_copy_insn): Use an explicit

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index e74383e01dccbe34feb60da15f1e148b54703a29..5a77b62b5a053ee73357e6c43c896453f4fdc419 100644 (file)
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -82,6 +82,7 @@
 #include "gdbsupport/selftest.h"
 #include "rust-lang.h"
 #include "gdbsupport/pathstuff.h"
+#include "count-one-bits.h"
 
 /* When == 1, print basic high level tracing messages.
    When > 1, be more verbose.
@@ -15526,10 +15527,15 @@ update_enumeration_type_from_children (struct die_info *die,
          unsigned_enum = 0;
          flag_enum = 0;
        }
-      else if ((mask & value) != 0)
-       flag_enum = 0;
       else
-       mask |= value;
+       {
+         if (count_one_bits_ll (value) >= 2)
+           flag_enum = 0;
+         else if ((mask & value) != 0)
+           flag_enum = 0;
+         else
+           mask |= value;
+       }
 
       /* If we already know that the enum type is neither unsigned, nor
         a flag type, no need to look at the rest of the enumerates.  */

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index e8ef3d37a8e149e82ad8ba5fe891942fcd8bce8c..5aaf5feecdfee0e421bc3efafdb026d2c1c46d2e 100644 (file)
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,15 @@
+2020-02-18  Simon Marchi  <simon.marchi@efficios.com>
+
+       * gdb.base/printcmds.c (enum flag_enum): Prefix enumerators with
+       FE_, add FE_NONE.
+       (three): Update.
+       (enum flag_enum_without_zero): New enum.
+       (flag_enum_without_zero): New variable.
+       (enum not_flag_enum): New enum.
+       (three_not_flag): New variable.
+       * gdb.base/printcmds.exp (test_artificial_arrays): Update.
+       (test_print_enums): Add more tests for printing flag enums.
+
 2020-02-18  Tom de Vries  <tdevries@suse.de>
 
        * lib/ada.exp (gdb_compile_ada_1): Factor out of ...

diff --git a/gdb/testsuite/gdb.base/printcmds.c b/gdb/testsuite/gdb.base/printcmds.c
index 57e04e6c01f3765523588e1f21045561d9742222..acb3cb3ad25ea4802a5bdd2b4e2550ebbdcd2e49 100644 (file)
--- a/gdb/testsuite/gdb.base/printcmds.c
+++ b/gdb/testsuite/gdb.base/printcmds.c
@@ -96,9 +96,35 @@ 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 };
+/* An enum considered as a "flag enum".  */
+enum flag_enum
+{
+  FE_NONE = 0x00,
+  FE_ONE  = 0x01,
+  FE_TWO  = 0x02,
+};
+
+enum flag_enum three = FE_ONE | FE_TWO;
+
+/* Another enum considered as a "flag enum", but with no enumerator with value
+   0.  */
+enum flag_enum_without_zero
+{
+  FEWZ_ONE = 0x01,
+  FEWZ_TWO = 0x02,
+};
+
+enum flag_enum_without_zero flag_enum_without_zero = 0;
+
+/* Not a flag enum, an enumerator value has multiple bits sets.  */
+enum not_flag_enum
+{
+  NFE_ONE = 0x01,
+  NFE_TWO = 0x02,
+  NFE_F0  = 0xf0,
+};
 
-enum flag_enum three = ONE | TWO;
+enum not_flag_enum three_not_flag = NFE_ONE | NFE_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

diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
index 6e98b7943ba3924126e968d4a5086aa02a187caa..6afb965af0668fb4fb2c91757116068b25a80511 100644 (file)
--- a/gdb/testsuite/gdb.base/printcmds.exp
+++ b/gdb/testsuite/gdb.base/printcmds.exp
@@ -653,9 +653,9 @@ proc test_artificial_arrays {} {
     gdb_test_escape_braces "p int1dim\[0\]${ctrlv}@2${ctrlv}@3" \
        "({{0, 1}, {2, 3}, {4, 5}}|\[Cc\]annot.*)" \
        {p int1dim[0]@2@3}
-    gdb_test_escape_braces "p int1dim\[0\]${ctrlv}@TWO" " = {0, 1}" \
+    gdb_test_escape_braces "p int1dim\[0\]${ctrlv}@FE_TWO" " = {0, 1}" \
         {p int1dim[0]@TWO}
-    gdb_test_escape_braces "p int1dim\[0\]${ctrlv}@TWO${ctrlv}@three" \
+    gdb_test_escape_braces "p int1dim\[0\]${ctrlv}@FE_TWO${ctrlv}@three" \
        "({{0, 1}, {2, 3}, {4, 5}}|\[Cc\]annot.*)" \
        {p int1dim[0]@TWO@three}
     gdb_test_escape_braces {p/x (short [])0x12345678} \
@@ -736,7 +736,21 @@ proc test_print_enums {} {
     # Regression test for PR11827.
     gdb_test "print some_volatile_enum" "enumvolval1"
 
-    gdb_test "print three" " = \\\(ONE \\| TWO\\\)"
+    # Print a flag enum.
+    gdb_test "print three" [string_to_regexp " = (FE_ONE | FE_TWO)"]
+
+    # Print a flag enum with value 0, where an enumerator has value 0.
+    gdb_test "print (enum flag_enum) 0x0" [string_to_regexp " = FE_NONE"]
+
+    # Print a flag enum with value 0, where no enumerator has value 0.
+    gdb_test "print flag_enum_without_zero" [string_to_regexp " = (unknown: 0)"]
+
+    # Print a flag enum with unknown bits set.
+    gdb_test "print (enum flag_enum) 0xf1" [string_to_regexp " = (FE_ONE | unknown: 240)"]
+
+    # Test printing an enum not considered a "flag enum" (because one of its
+    # enumerators has multiple bits set).
+    gdb_test "print three_not_flag" [string_to_regexp " = 3"]
 }
 
 proc test_printf {} {

diff --git a/gdb/valprint.c b/gdb/valprint.c
index f26a87da3bd46f947cf2994e3b405bc58a93b786..77b9a4993d79306b5fa5ba1367f4ee24445fc3b7 100644 (file)
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -39,6 +39,7 @@
 #include "cli/cli-option.h"
 #include "gdbarch.h"
 #include "cli/cli-style.h"
+#include "count-one-bits.h"
 
 /* Maximum number of wchars returned from wchar_iterate.  */
 #define MAX_WCHARS 4
@@ -638,7 +639,12 @@ generic_val_print_enum_1 (struct type *type, LONGEST val,
        {
          QUIT;
 
-         if ((val & TYPE_FIELD_ENUMVAL (type, i)) != 0)
+         ULONGEST enumval = TYPE_FIELD_ENUMVAL (type, i);
+         int nbits = count_one_bits_ll (enumval);
+
+         gdb_assert (nbits == 0 || nbits == 1);
+
+         if ((val & enumval) != 0)
            {
              if (!first)
                fputs_filtered (" | ", stream);